Skip to content

Commit

Permalink
[BUG] Fix permission check failed with empty workspace for find method (
Browse files Browse the repository at this point in the history
#6527)

* Fix permission check failed with empty workspace for find method

Signed-off-by: Hailong Cui <[email protected]>

* Changeset file for PR #6527 created/updated

* Add unit test case

Signed-off-by: Hailong Cui <[email protected]>

* address review comments

Signed-off-by: Hailong Cui <[email protected]>

---------

Signed-off-by: Hailong Cui <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
1 parent bbde563 commit 432afc7
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 1 deletion.
2 changes: 2 additions & 0 deletions changelogs/fragments/6527.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Permission check failed with empty workspace for find method ([#6527](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6527))
Original file line number Diff line number Diff line change
Expand Up @@ -176,5 +176,24 @@ describe('WorkspaceIdConsumerWrapper', () => {
workspacesSearchOperator: 'AND',
});
});

it(`Should not pass a empty workspace array`, async () => {
const workspaceIdConsumerWrapper = new WorkspaceIdConsumerWrapper(true);
const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
updateWorkspaceState(mockRequest, {});
const mockedWrapperClient = workspaceIdConsumerWrapper.wrapperFactory({
client: mockedClient,
typeRegistry: requestHandlerContext.savedObjects.typeRegistry,
request: mockRequest,
});
await mockedWrapperClient.find({
type: ['dashboard', 'visualization'],
});
// empty workspace array will get deleted
expect(mockedClient.find).toBeCalledWith({
type: ['dashboard', 'visualization'],
workspacesSearchOperator: 'OR',
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ export class WorkspaceIdConsumerWrapper {
findOptions.workspaces.splice(index, 1);
}
}
if (findOptions.workspaces && findOptions.workspaces.length === 0) {
delete findOptions.workspaces;
}
return wrapperOptions.client.find(findOptions);
},
bulkGet: wrapperOptions.client.bulkGet,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,55 @@ describe('WorkspaceSavedObjectsClientWrapper', () => {
},
});
});
it('should call client.find with ACLSearchParams if no workspaces is provided', async () => {
const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper();
// no workspaces
await wrapper.find({
type: 'dashboards',
});
expect(clientMock.find).toHaveBeenCalledWith({
type: 'dashboards',
ACLSearchParams: {
principals: {
users: ['user-1'],
},
permissionModes: ['read', 'write'],
},
});

// workspaces parameter is undefined
clientMock.find.mockReset();
await wrapper.find({
type: 'dashboards',
workspaces: undefined,
});
expect(clientMock.find).toHaveBeenLastCalledWith({
type: 'dashboards',
ACLSearchParams: {
principals: {
users: ['user-1'],
},
permissionModes: ['read', 'write'],
},
});

// empty workspaces array
clientMock.find.mockReset();
await wrapper.find({
type: 'dashboards',
workspaces: [],
});
expect(clientMock.find).toHaveBeenLastCalledWith({
type: 'dashboards',
workspaces: [],
ACLSearchParams: {
principals: {
users: ['user-1'],
},
permissionModes: ['read', 'write'],
},
});
});
it('should call client.find with only read permission if find workspace and permissionModes provided', async () => {
const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper();
await wrapper.find({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ export class WorkspaceSavedObjectsClientWrapper {
})
).saved_objects.map((item) => item.id);

if (options.workspaces) {
if (options.workspaces && options.workspaces.length > 0) {
const permittedWorkspaces = options.workspaces.filter((item) =>
permittedWorkspaceIds.includes(item)
);
Expand Down

0 comments on commit 432afc7

Please sign in to comment.