Skip to content

Commit

Permalink
[Workspace]Restrict saved objects finding when workspace enabled (ope…
Browse files Browse the repository at this point in the history 
…nsearch-project#7125)

* Limit data source saved objects finding and access when workspace enabled

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Remove options.workspaces drop behavior in conflict client wrapper

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Changeset file for PR opensearch-project#7125 created/updated

* Update src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>

* Remove formatFindParams

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix code format

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix ACLSearchParams and empty workspaces

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Refactor workspace client wrapper find

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Remove global saved objects validation logic

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update uts and its

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Changeset file for PR opensearch-project#7125 created/updated

* Remove no need changes and refactor

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Fix global saved objects not found

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Change saved object get to assert

Signed-off-by: Lin Wang <wonglam@amazon.com>

---------

Signed-off-by: Lin Wang <wonglam@amazon.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Co-authored-by: Yulong Ruan <ruanyl@amazon.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>
  • Loading branch information
@wanglam @opensearch-changeset-bot
@ruanyl @SuZhou-Joe
5 people authored Jul 5, 2024
1 parent e0945af commit 1cb5956
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 291 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/7125.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- [Workspace]Restrict saved objects finding when workspace enabled ([#7125](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7125))
64 changes: 31 additions & 33 deletions src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ describe('#getQueryParams', () => {
expect(result.query.bool.filter[1]).toEqual(undefined);
});

it('workspacesSearchOperator prvided as "OR"', () => {
it('workspacesSearchOperator provided as "OR"', () => {
const result: Result = getQueryParams({
registry,
workspaces: ['foo'],
Expand All @@ -665,22 +665,6 @@ describe('#getQueryParams', () => {
expect(result.query.bool.filter[1]).toEqual({
bool: {
should: [
{
bool: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
{
bool: {
minimum_should_match: 1,
Expand Down Expand Up @@ -718,22 +702,6 @@ describe('#getQueryParams', () => {
expect(result.query.bool.filter[1]).toEqual({
bool: {
should: [
{
bool: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
{
bool: {
filter: [
Expand Down Expand Up @@ -771,6 +739,36 @@ describe('#getQueryParams', () => {
});
});
});

describe('when using empty workspace search', () => {
it('workspacesSearchOperator provided as "OR"', () => {
const result: Result = getQueryParams({
registry,
workspaces: [],
workspacesSearchOperator: 'OR',
});
expect(result.query.bool.filter[1]).toEqual({
bool: {
should: [
{
match_none: {},
},
],
},
});
});

it('workspacesSearchOperator provided as "AND"', () => {
const result: Result = getQueryParams({
registry,
workspaces: [],
workspacesSearchOperator: 'AND',
});
expect(result.query.bool.filter[1]).toEqual({
match_none: {},
});
});
});
});

describe('namespaces property', () => {
Expand Down
61 changes: 20 additions & 41 deletions src/core/server/saved_objects/service/lib/search_dsl/query_params.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,53 +286,32 @@ export function getQueryParams({
}
}

if (workspaces?.length) {
if (workspacesSearchOperator === 'OR') {
ACLSearchParamsShouldClause.push({
bool: {
should: workspaces.map((workspace) => {
return getClauseForWorkspace(workspace);
}),
minimum_should_match: 1,
},
});
} else {
bool.filter.push({
bool: {
should: workspaces.map((workspace) => {
return getClauseForWorkspace(workspace);
}),
minimum_should_match: 1,
},
});
if (workspaces) {
const conditions =
workspacesSearchOperator === 'OR' ? ACLSearchParamsShouldClause : bool.filter;

switch (workspaces.length) {
case 0:
conditions.push({
match_none: {},
});
break;
default:
conditions.push({
bool: {
should: workspaces.map((workspace) => {
return getClauseForWorkspace(workspace);
}),
minimum_should_match: 1,
},
});
}
}

if (ACLSearchParamsShouldClause.length) {
bool.filter.push({
bool: {
should: [
/**
* Return those objects without workspaces field and permissions field to keep find API backward compatible
*/
{
bool: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
...ACLSearchParamsShouldClause,
],
should: ACLSearchParamsShouldClause,
},
});
}
Expand Down
6 changes: 3 additions & 3 deletions ...aved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,10 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test',
],
});

const findAdvancedSettings = await osdTestServer.request
.get(root, `/api/saved_objects/_find?type=${advancedSettings.type}`)
const getAdvancedSettingsResult = await osdTestServer.request
.get(root, `/api/saved_objects/${advancedSettings.type}/${packageInfo.version}`)
.expect(200);
expect(findAdvancedSettings.body.total).toEqual(1);
expect(getAdvancedSettingsResult.body.id).toBe(packageInfo.version);
});

it('checkConflicts when importing ndjson', async () => {
Expand Down
56 changes: 43 additions & 13 deletions ...ace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,20 +241,20 @@ describe('WorkspaceSavedObjectsClientWrapper', () => {
});

describe('find', () => {
it('should throw not authorized error when user not permitted', async () => {
let error;
try {
await notPermittedSavedObjectedClient.find({
type: 'dashboard',
workspaces: ['workspace-1'],
perPage: 999,
page: 1,
});
} catch (e) {
error = e;
}
it('should return empty result if user not permitted', async () => {
const result = await notPermittedSavedObjectedClient.find({
type: 'dashboard',
workspaces: ['workspace-1'],
perPage: 999,
page: 1,
});

expect(SavedObjectsErrorHelpers.isNotAuthorizedError(error)).toBe(true);
expect(result).toEqual({
saved_objects: [],
total: 0,
page: 1,
per_page: 999,
});
});

it('should return consistent inner workspace data when user permitted', async () => {
Expand All @@ -269,6 +269,36 @@ describe('WorkspaceSavedObjectsClientWrapper', () => {
true
);
});

it('should return consistent result when workspaces and ACLSearchParams not provided', async () => {
const result = await permittedSavedObjectedClient.find({
type: 'dashboard',
perPage: 999,
page: 1,
});

expect(result.saved_objects).toEqual(
expect.arrayContaining([
expect.objectContaining({ id: 'inner-workspace-dashboard-1' }),
expect.objectContaining({ id: 'acl-controlled-dashboard-2' }),
])
);
});

it('should return acl controled dashboards when only ACLSearchParams provided', async () => {
const result = await permittedSavedObjectedClient.find({
type: 'dashboard',
perPage: 999,
page: 1,
ACLSearchParams: {
permissionModes: ['read', 'write'],
},
});

expect(result.saved_objects).toEqual(
expect.arrayContaining([expect.objectContaining({ id: 'acl-controlled-dashboard-2' })])
);
});
});

describe('create', () => {
Expand Down
Loading

0 comments on commit 1cb5956

Please sign in to comment.