From 72fc93886e148dd1a82504ef854cb1855ca06bc6 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 20 Mar 2024 10:21:58 +0800 Subject: [PATCH 01/21] dashboard admin(groups/users) implementation and add unit/integration test This reverts commit 47e10e4d81ca193028297868beb9eaaced1c00dc. --- config/opensearch_dashboards.yml | 5 + src/plugins/workspace/config.ts | 8 + src/plugins/workspace/server/plugin.ts | 3 +- ...space_saved_objects_client_wrapper.test.ts | 191 +++++++++++++++++- ...space_saved_objects_client_wrapper.test.ts | 166 ++++++++++++++- .../workspace_saved_objects_client_wrapper.ts | 43 +++- 6 files changed, 410 insertions(+), 6 deletions(-) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 11e772087749..0c2fca9ea340 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -290,3 +290,8 @@ # Set the value to true to enable workspace feature # workspace.enabled: false + +# Set the backend roles in groups or users, whoever has the backend roles defined in this config will be regard as dashboard admin. +# Dashboard admin will have the access to all the workspaces and objects inside OpenSearch Dashboards. +# workspace.dashboardAdmin.groups: ["dashboard_admin"] +# workspace.dashboardAdmin.users: ["dashboard_admin"] \ No newline at end of file diff --git a/src/plugins/workspace/config.ts b/src/plugins/workspace/config.ts index 70c87ac00cfc..c5cf89f97249 100644 --- a/src/plugins/workspace/config.ts +++ b/src/plugins/workspace/config.ts @@ -10,6 +10,14 @@ export const configSchema = schema.object({ permission: schema.object({ enabled: schema.boolean({ defaultValue: true }), }), + dashboardAdmin: schema.object({ + groups: schema.arrayOf(schema.string(), { + defaultValue: [], + }), + users: schema.arrayOf(schema.string(), { + defaultValue: [], + }), + }), }); export type WorkspacePluginConfigType = TypeOf; diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index d425f17d08eb..b0aaa4a4255e 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -81,7 +81,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.permissionControl = new SavedObjectsPermissionControl(this.logger); this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( - this.permissionControl + this.permissionControl, + { config$: this.config$ } ); core.savedObjects.addClientWrapper( diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index 2a7fb0e440b5..d2a8b9addade 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -51,6 +51,7 @@ const repositoryKit = (() => { const permittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); const notPermittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); +const dashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); describe('WorkspaceSavedObjectsClientWrapper', () => { let internalSavedObjectsRepository: ISavedObjectsRepository; @@ -59,6 +60,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { let osd: TestOpenSearchDashboardsUtils; let permittedSavedObjectedClient: SavedObjectsClientContract; let notPermittedSavedObjectedClient: SavedObjectsClientContract; + let dashboardAdminSavedObjectedClient: SavedObjectsClientContract; beforeAll(async function () { servers = createTestServers({ @@ -69,6 +71,10 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { osd: { workspace: { enabled: true, + dashboardAdmin: { + groups: ['dashboard_admin'], + users: ['dashboard_admin'], + }, }, savedObjects: { permission: { @@ -125,14 +131,19 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation((request) => { if (request === notPermittedRequest) { return { users: ['bar'] }; + } else if (request === permittedRequest) { + return { users: ['foo'] }; } - return { users: ['foo'] }; + return { groups: ['dashboard_admin'] }; }); permittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient(permittedRequest); notPermittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( notPermittedRequest ); + dashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( + dashboardAdminRequest + ); }); afterAll(async () => { @@ -172,6 +183,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { (await permittedSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2')).error ).toBeUndefined(); }); + + it('should return consistent dashboard when user is dashboard admin', async () => { + expect( + (await dashboardAdminSavedObjectedClient.get('dashboard', 'inner-workspace-dashboard-1')) + .error + ).toBeUndefined(); + expect( + (await dashboardAdminSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2')) + .error + ).toBeUndefined(); + }); }); describe('bulkGet', () => { @@ -215,6 +237,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).saved_objects.length ).toEqual(1); }); + + it('should return consistent dashboard when user is dashboard admin', async () => { + expect( + ( + await dashboardAdminSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, + ]) + ).saved_objects.length + ).toEqual(1); + expect( + ( + await dashboardAdminSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, + ]) + ).saved_objects.length + ).toEqual(1); + }); }); describe('find', () => { @@ -246,6 +285,19 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { true ); }); + + it('should return consistent inner workspace data when user is dashboard admin', async () => { + const result = await dashboardAdminSavedObjectedClient.find({ + type: 'dashboard', + workspaces: ['workspace-1'], + perPage: 999, + page: 1, + }); + + expect(result.saved_objects.some((item) => item.id === 'inner-workspace-dashboard-1')).toBe( + true + ); + }); }); describe('create', () => { @@ -278,6 +330,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', createResult.id); }); + it('should able to create saved objects into any workspaces after create called when user is dashboard admin', async () => { + const createResult = await dashboardAdminSavedObjectedClient.create( + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + expect(createResult.error).toBeUndefined(); + await dashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); + }); + it('should throw forbidden error when create with override', async () => { let error; try { @@ -309,6 +373,20 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.error).toBeUndefined(); }); + + it('should able to create with override when user is dashboard admin', async () => { + const createResult = await dashboardAdminSavedObjectedClient.create( + 'dashboard', + {}, + { + id: 'inner-workspace-dashboard-1', + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.error).toBeUndefined(); + }); }); describe('bulkCreate', () => { @@ -337,6 +415,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', objectId); }); + it('should able to create saved objects into any workspaces after bulkCreate called when user is dashboard damin', async () => { + const objectId = new Date().getTime().toString(16).toUpperCase(); + const result = await dashboardAdminSavedObjectedClient.bulkCreate( + [{ type: 'dashboard', attributes: {}, id: objectId }], + { + workspaces: ['workspace-1'], + } + ); + expect(result.saved_objects.length).toEqual(1); + await dashboardAdminSavedObjectedClient.delete('dashboard', objectId); + }); + it('should throw forbidden error when create with override', async () => { let error; try { @@ -377,6 +467,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.saved_objects).toHaveLength(1); }); + + it('should able to bulk create with override when user is dashboard admin', async () => { + const createResult = await dashboardAdminSavedObjectedClient.bulkCreate( + [ + { + id: 'inner-workspace-dashboard-1', + type: 'dashboard', + attributes: {}, + }, + ], + { + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.saved_objects).toHaveLength(1); + }); }); describe('update', () => { @@ -414,6 +522,27 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { .error ).toBeUndefined(); }); + + it('should update saved objects for any workspaces when user is dashboard admin', async () => { + expect( + ( + await dashboardAdminSavedObjectedClient.update( + 'dashboard', + 'inner-workspace-dashboard-1', + {} + ) + ).error + ).toBeUndefined(); + expect( + ( + await dashboardAdminSavedObjectedClient.update( + 'dashboard', + 'acl-controlled-dashboard-2', + {} + ) + ).error + ).toBeUndefined(); + }); }); describe('bulkUpdate', () => { @@ -459,6 +588,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).saved_objects.length ).toEqual(1); }); + + it('should bulk update saved objects for any workspaces when user is dashboard admin', async () => { + expect( + ( + await dashboardAdminSavedObjectedClient.bulkUpdate([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, + ]) + ).saved_objects.length + ).toEqual(1); + expect( + ( + await dashboardAdminSavedObjectedClient.bulkUpdate([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, + ]) + ).saved_objects.length + ).toEqual(1); + }); }); describe('delete', () => { @@ -526,5 +672,48 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); + + it('should be able to delete any data when user is dashboard admin', async () => { + const createResultOne = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + permissions: { + read: { users: ['foo'] }, + write: { users: ['foo'] }, + }, + } + ); + + await dashboardAdminSavedObjectedClient.delete('dashboard', createResultOne.id); + + let errorOne; + try { + errorOne = await dashboardAdminSavedObjectedClient.get('dashboard', createResultOne.id); + } catch (e) { + errorOne = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(errorOne)).toBe(true); + + const createResultTwo = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + + await dashboardAdminSavedObjectedClient.delete('dashboard', createResultTwo.id); + + let errorTwo; + try { + errorTwo = await dashboardAdminSavedObjectedClient.get('dashboard', createResultTwo.id); + } catch (e) { + errorTwo = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(errorTwo)).toBe(true); + }); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 6b40f6e60fa0..955b535764d2 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -3,10 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { of } from 'rxjs'; import { SavedObjectsErrorHelpers } from '../../../../core/server'; import { WorkspaceSavedObjectsClientWrapper } from './workspace_saved_objects_client_wrapper'; -const generateWorkspaceSavedObjectsClientWrapper = () => { +const generateWorkspaceSavedObjectsClientWrapper = (isDashboardAdmin = false) => { const savedObjectsStore = [ { type: 'dashboard', @@ -79,9 +80,24 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }), validateSavedObjectsACL: jest.fn(), batchValidate: jest.fn(), - getPrincipalsFromRequest: jest.fn().mockImplementation(() => ({ users: ['user-1'] })), + getPrincipalsFromRequest: jest.fn().mockImplementation(() => { + return isDashboardAdmin ? { groups: ['dashboard_admin'] } : { users: ['user-1'] }; + }), + }; + const configMock = { + enabled: true, + permission: { + enabled: true, + }, + dashboardAdmin: { + groups: ['dashboard_admin'], + users: ['dashboard_admin'], + }, }; - const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); + const optionsMock = { + config$: of(configMock), + }; + const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock, optionsMock); wrapper.setScopedClient(() => ({ find: jest.fn().mockImplementation(async () => ({ saved_objects: [{ id: 'workspace-1', type: 'workspace' }], @@ -94,6 +110,7 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { requestMock, }; }; +const isDashboardAdmin = true; describe('WorkspaceSavedObjectsClientWrapper', () => { describe('wrapperFactory', () => { @@ -128,6 +145,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.delete(...deleteArgs); expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); }); + it('should call client.delete if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + const deleteArgs = ['dashboard', 'not-permitted-dashboard'] as const; + await wrapper.delete(...deleteArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); + }); }); describe('update', () => { @@ -170,6 +198,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.update(...updateArgs); expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); }); + it('should call client.update if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + const updateArgs = [ + 'dashboard', + 'not-permitted-dashboard', + { + bar: 'for', + }, + ] as const; + await wrapper.update(...updateArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); + }); }); describe('bulk update', () => { @@ -205,6 +250,19 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.bulkUpdate(objectsToUpdate, {}); expect(clientMock.bulkUpdate).toHaveBeenCalledWith(objectsToUpdate, {}); }); + it('should call client.bulkUpdate if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + const bulkUpdateArgs = [ + { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, + ]; + await wrapper.bulkUpdate(bulkUpdateArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.bulkUpdate).toHaveBeenCalledWith(bulkUpdateArgs); + }); }); describe('bulk create', () => { @@ -289,6 +347,25 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); + it('should call client.bulkCreate if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + const objectsToBulkCreate = [ + { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, + ]; + await wrapper.bulkCreate(objectsToBulkCreate, { + overwrite: true, + workspaces: ['not-permitted-workspace'], + }); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { + overwrite: true, + workspaces: ['not-permitted-workspace'], + }); + }); }); describe('create', () => { @@ -363,6 +440,30 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); }); + it('should call client.create if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + await wrapper.create( + 'dashboard', + { foo: 'bar' }, + { + id: 'not-permitted-dashboard', + overwrite: true, + } + ); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.create).toHaveBeenCalledWith( + 'dashboard', + { foo: 'bar' }, + { + id: 'not-permitted-dashboard', + overwrite: true, + } + ); + }); }); describe('get', () => { it('should return saved object if no need to validate permission', async () => { @@ -424,6 +525,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(clientMock.get).toHaveBeenCalledWith(...getArgs); expect(result).toMatchInlineSnapshot(`[Error: Not Found]`); }); + it('should call client.get and return result with arguments backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + const getArgs = ['dashboard', 'not-permitted-dashboard'] as const; + const result = await wrapper.get(...getArgs); + expect(clientMock.get).toHaveBeenCalledWith(...getArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(result.id).toBe('not-permitted-dashboard'); + }); }); describe('bulk get', () => { it("should call permission validate with object's workspace and throw permission error", async () => { @@ -489,6 +602,27 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { {} ); }); + it('should call client.bulkGet and return result with arguments if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + const bulkGetArgs = [ + { + type: 'dashboard', + id: 'foo', + }, + { + type: 'dashboard', + id: 'not-permitted-dashboard', + }, + ]; + const result = await wrapper.bulkGet(bulkGetArgs); + expect(clientMock.bulkGet).toHaveBeenCalledWith(bulkGetArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(result.saved_objects.length).toBe(2); + }); }); describe('find', () => { it('should call client.find with ACLSearchParams for workspace type', async () => { @@ -564,6 +698,22 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }, }); }); + it('should call client.find with arguments if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + await wrapper.find({ + type: 'dashboard', + workspaces: ['workspace-1', 'not-permitted-workspace'], + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'dashboard', + workspaces: ['workspace-1', 'not-permitted-workspace'], + }); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); }); describe('deleteByWorkspace', () => { it('should call permission validate with workspace and throw workspace permission error if not permitted', async () => { @@ -592,6 +742,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.deleteByWorkspace('workspace-1', {}); expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-1', {}); }); + it('should call client.deleteByWorkspace if backend role is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + await wrapper.deleteByWorkspace('not-permitted-workspace'); + expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('not-permitted-workspace'); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); }); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index c515f555fa4b..9e378558bece 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -4,6 +4,8 @@ */ import { i18n } from '@osd/i18n'; +import { Observable } from 'rxjs'; +import { first } from 'rxjs/operators'; import { OpenSearchDashboardsRequest, @@ -32,6 +34,7 @@ import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WorkspacePermissionMode, } from '../../common/constants'; +import { WorkspacePluginConfigType } from '../../config'; // Can't throw unauthorized for now, the page will be refreshed if unauthorized const generateWorkspacePermissionError = () => @@ -68,6 +71,7 @@ const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[] export class WorkspaceSavedObjectsClientWrapper { private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; + private config?: WorkspacePluginConfigType; private formatWorkspacePermissionModeToStringArray( permission: WorkspacePermissionMode | WorkspacePermissionMode[] ): string[] { @@ -140,6 +144,22 @@ export class WorkspaceSavedObjectsClientWrapper { return false; }; + private isDashboardAdmin(request: OpenSearchDashboardsRequest): boolean { + const config = this.config || ({} as WorkspacePluginConfigType); + + // Before the user login OSD, calling the getPrincipalsFromRequest method will throw 'NOT_AUTHORIZED' exception. + try { + const { groups = [], users = [] } = this.permissionControl.getPrincipalsFromRequest(request); + const adminGroups = config?.dashboardAdmin?.groups || []; + const adminUsers = config?.dashboardAdmin?.users || []; + const groupMatchAny = groups?.some((group) => adminGroups.includes(group)) || false; + const userMatchAny = users?.some((user) => adminUsers.includes(user)) || false; + return groupMatchAny || userMatchAny; + } catch (e) { + return false; + } + } + /** * check if the type include workspace * Workspace permission check is totally different from object permission check. @@ -527,6 +547,12 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.deleteByWorkspace(workspace, options); }; + const isDashboardAdmin = this.isDashboardAdmin(wrapperOptions.request); + + if (isDashboardAdmin) { + return wrapperOptions.client; + } + return { ...wrapperOptions.client, get: getWithWorkspacePermissionControl, @@ -545,5 +571,20 @@ export class WorkspaceSavedObjectsClientWrapper { }; }; - constructor(private readonly permissionControl: SavedObjectsPermissionControlContract) {} + constructor( + private readonly permissionControl: SavedObjectsPermissionControlContract, + private readonly options: { + config$: Observable; + } + ) { + this.options?.config$.subscribe((config) => { + this.config = config; + }); + this.options?.config$ + .pipe(first()) + .toPromise() + .then((config) => { + this.config = config; + }); + } } From 29236e3134935197d9049776a1faee663fe51b3a Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 20 Mar 2024 15:04:36 +0800 Subject: [PATCH 02/21] Add test cases of user Id matching dashboard admin --- config/opensearch_dashboards.yml | 2 +- ...space_saved_objects_client_wrapper.test.ts | 307 +++++++++++++++--- ...space_saved_objects_client_wrapper.test.ts | 190 +++++++++-- .../workspace_saved_objects_client_wrapper.ts | 16 +- 4 files changed, 437 insertions(+), 78 deletions(-) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 0c2fca9ea340..e99f7b77690a 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -291,7 +291,7 @@ # Set the value to true to enable workspace feature # workspace.enabled: false -# Set the backend roles in groups or users, whoever has the backend roles defined in this config will be regard as dashboard admin. +# Set the backend roles in groups or users, whoever has the backend roles or exactly match the user ids defined in this config will be regard as dashboard admin. # Dashboard admin will have the access to all the workspaces and objects inside OpenSearch Dashboards. # workspace.dashboardAdmin.groups: ["dashboard_admin"] # workspace.dashboardAdmin.users: ["dashboard_admin"] \ No newline at end of file diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index d2a8b9addade..fd240e064dea 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -51,7 +51,8 @@ const repositoryKit = (() => { const permittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); const notPermittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); -const dashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); +const groupIsDashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); +const UserIdIsDashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); describe('WorkspaceSavedObjectsClientWrapper', () => { let internalSavedObjectsRepository: ISavedObjectsRepository; @@ -60,7 +61,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { let osd: TestOpenSearchDashboardsUtils; let permittedSavedObjectedClient: SavedObjectsClientContract; let notPermittedSavedObjectedClient: SavedObjectsClientContract; - let dashboardAdminSavedObjectedClient: SavedObjectsClientContract; + let groupIsDashboardAdminSavedObjectedClient: SavedObjectsClientContract; + let UserIdIsdashboardAdminSavedObjectedClient: SavedObjectsClientContract; beforeAll(async function () { servers = createTestServers({ @@ -129,20 +131,21 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation((request) => { - if (request === notPermittedRequest) { - return { users: ['bar'] }; - } else if (request === permittedRequest) { - return { users: ['foo'] }; - } - return { groups: ['dashboard_admin'] }; + if (request === notPermittedRequest) return { users: ['bar'] }; + else if (request === permittedRequest) return { users: ['foo'] }; + else if (request === groupIsDashboardAdminRequest) return { groups: ['dashboard_admin'] }; + return { users: ['dashboard_admin'] }; }); permittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient(permittedRequest); notPermittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( notPermittedRequest ); - dashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( - dashboardAdminRequest + groupIsDashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( + groupIsDashboardAdminRequest + ); + UserIdIsdashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( + UserIdIsDashboardAdminRequest ); }); @@ -184,14 +187,41 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toBeUndefined(); }); - it('should return consistent dashboard when user is dashboard admin', async () => { + it('should return consistent dashboard when groups match dashboard admin', async () => { expect( - (await dashboardAdminSavedObjectedClient.get('dashboard', 'inner-workspace-dashboard-1')) - .error + ( + await groupIsDashboardAdminSavedObjectedClient.get( + 'dashboard', + 'inner-workspace-dashboard-1' + ) + ).error ).toBeUndefined(); expect( - (await dashboardAdminSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2')) - .error + ( + await groupIsDashboardAdminSavedObjectedClient.get( + 'dashboard', + 'acl-controlled-dashboard-2' + ) + ).error + ).toBeUndefined(); + }); + + it('should return consistent dashboard when user ids match dashboard admin', async () => { + expect( + ( + await UserIdIsdashboardAdminSavedObjectedClient.get( + 'dashboard', + 'inner-workspace-dashboard-1' + ) + ).error + ).toBeUndefined(); + expect( + ( + await UserIdIsdashboardAdminSavedObjectedClient.get( + 'dashboard', + 'acl-controlled-dashboard-2' + ) + ).error ).toBeUndefined(); }); }); @@ -238,17 +268,34 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toEqual(1); }); - it('should return consistent dashboard when user is dashboard admin', async () => { + it('should return consistent dashboard when groups match dashboard admin', async () => { expect( ( - await dashboardAdminSavedObjectedClient.bulkGet([ + await groupIsDashboardAdminSavedObjectedClient.bulkGet([ { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, ]) ).saved_objects.length ).toEqual(1); expect( ( - await dashboardAdminSavedObjectedClient.bulkGet([ + await groupIsDashboardAdminSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, + ]) + ).saved_objects.length + ).toEqual(1); + }); + + it('should return consistent dashboard when user ids match dashboard admin', async () => { + expect( + ( + await UserIdIsdashboardAdminSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, + ]) + ).saved_objects.length + ).toEqual(1); + expect( + ( + await UserIdIsdashboardAdminSavedObjectedClient.bulkGet([ { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, ]) ).saved_objects.length @@ -286,8 +333,21 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); }); - it('should return consistent inner workspace data when user is dashboard admin', async () => { - const result = await dashboardAdminSavedObjectedClient.find({ + it('should return consistent inner workspace data when groups match dashboard admin', async () => { + const result = await groupIsDashboardAdminSavedObjectedClient.find({ + type: 'dashboard', + workspaces: ['workspace-1'], + perPage: 999, + page: 1, + }); + + expect(result.saved_objects.some((item) => item.id === 'inner-workspace-dashboard-1')).toBe( + true + ); + }); + + it('should return consistent inner workspace data when user ids match dashboard admin', async () => { + const result = await UserIdIsdashboardAdminSavedObjectedClient.find({ type: 'dashboard', workspaces: ['workspace-1'], perPage: 999, @@ -330,8 +390,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', createResult.id); }); - it('should able to create saved objects into any workspaces after create called when user is dashboard admin', async () => { - const createResult = await dashboardAdminSavedObjectedClient.create( + it('should able to create saved objects into any workspaces after create called when groups match dashboard admin', async () => { + const createResult = await groupIsDashboardAdminSavedObjectedClient.create( 'dashboard', {}, { @@ -339,7 +399,19 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); expect(createResult.error).toBeUndefined(); - await dashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); + await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); + }); + + it('should able to create saved objects into any workspaces after create called when user ids match dashboard admin', async () => { + const createResult = await UserIdIsdashboardAdminSavedObjectedClient.create( + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + expect(createResult.error).toBeUndefined(); + await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); }); it('should throw forbidden error when create with override', async () => { @@ -374,8 +446,22 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.error).toBeUndefined(); }); - it('should able to create with override when user is dashboard admin', async () => { - const createResult = await dashboardAdminSavedObjectedClient.create( + it('should able to create with override when groups match dashboard admin', async () => { + const createResult = await groupIsDashboardAdminSavedObjectedClient.create( + 'dashboard', + {}, + { + id: 'inner-workspace-dashboard-1', + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.error).toBeUndefined(); + }); + + it('should able to create with override when uesr ids match dashboard admin', async () => { + const createResult = await UserIdIsdashboardAdminSavedObjectedClient.create( 'dashboard', {}, { @@ -415,16 +501,28 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', objectId); }); - it('should able to create saved objects into any workspaces after bulkCreate called when user is dashboard damin', async () => { + it('should able to create saved objects into any workspaces after bulkCreate called when groups match dashboard damin', async () => { const objectId = new Date().getTime().toString(16).toUpperCase(); - const result = await dashboardAdminSavedObjectedClient.bulkCreate( + const result = await groupIsDashboardAdminSavedObjectedClient.bulkCreate( [{ type: 'dashboard', attributes: {}, id: objectId }], { workspaces: ['workspace-1'], } ); expect(result.saved_objects.length).toEqual(1); - await dashboardAdminSavedObjectedClient.delete('dashboard', objectId); + await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', objectId); + }); + + it('should able to create saved objects into any workspaces after bulkCreate called when user ids match dashboard damin', async () => { + const objectId = new Date().getTime().toString(16).toUpperCase(); + const result = await UserIdIsdashboardAdminSavedObjectedClient.bulkCreate( + [{ type: 'dashboard', attributes: {}, id: objectId }], + { + workspaces: ['workspace-1'], + } + ); + expect(result.saved_objects.length).toEqual(1); + await UserIdIsdashboardAdminSavedObjectedClient.delete('dashboard', objectId); }); it('should throw forbidden error when create with override', async () => { @@ -468,8 +566,26 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.saved_objects).toHaveLength(1); }); - it('should able to bulk create with override when user is dashboard admin', async () => { - const createResult = await dashboardAdminSavedObjectedClient.bulkCreate( + it('should able to bulk create with override when groups match dashboard admin', async () => { + const createResult = await groupIsDashboardAdminSavedObjectedClient.bulkCreate( + [ + { + id: 'inner-workspace-dashboard-1', + type: 'dashboard', + attributes: {}, + }, + ], + { + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.saved_objects).toHaveLength(1); + }); + + it('should able to bulk create with override when user ids match dashboard admin', async () => { + const createResult = await UserIdIsdashboardAdminSavedObjectedClient.bulkCreate( [ { id: 'inner-workspace-dashboard-1', @@ -523,10 +639,31 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toBeUndefined(); }); - it('should update saved objects for any workspaces when user is dashboard admin', async () => { + it('should update saved objects for any workspaces when groups match dashboard admin', async () => { + expect( + ( + await groupIsDashboardAdminSavedObjectedClient.update( + 'dashboard', + 'inner-workspace-dashboard-1', + {} + ) + ).error + ).toBeUndefined(); + expect( + ( + await groupIsDashboardAdminSavedObjectedClient.update( + 'dashboard', + 'acl-controlled-dashboard-2', + {} + ) + ).error + ).toBeUndefined(); + }); + + it('should update saved objects for any workspaces when user ids match dashboard admin', async () => { expect( ( - await dashboardAdminSavedObjectedClient.update( + await UserIdIsdashboardAdminSavedObjectedClient.update( 'dashboard', 'inner-workspace-dashboard-1', {} @@ -535,7 +672,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toBeUndefined(); expect( ( - await dashboardAdminSavedObjectedClient.update( + await UserIdIsdashboardAdminSavedObjectedClient.update( 'dashboard', 'acl-controlled-dashboard-2', {} @@ -589,17 +726,34 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toEqual(1); }); - it('should bulk update saved objects for any workspaces when user is dashboard admin', async () => { + it('should bulk update saved objects for any workspaces when groups match dashboard admin', async () => { expect( ( - await dashboardAdminSavedObjectedClient.bulkUpdate([ + await groupIsDashboardAdminSavedObjectedClient.bulkUpdate([ { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, ]) ).saved_objects.length ).toEqual(1); expect( ( - await dashboardAdminSavedObjectedClient.bulkUpdate([ + await groupIsDashboardAdminSavedObjectedClient.bulkUpdate([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, + ]) + ).saved_objects.length + ).toEqual(1); + }); + + it('should bulk update saved objects for any workspaces when user ids match dashboard admin', async () => { + expect( + ( + await UserIdIsdashboardAdminSavedObjectedClient.bulkUpdate([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, + ]) + ).saved_objects.length + ).toEqual(1); + expect( + ( + await UserIdIsdashboardAdminSavedObjectedClient.bulkUpdate([ { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, ]) ).saved_objects.length @@ -673,8 +827,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); - it('should be able to delete any data when user is dashboard admin', async () => { - const createResultOne = await repositoryKit.create( + it('should be able to delete any data when groups match dashboard admin', async () => { + const createPermittedResult = await repositoryKit.create( internalSavedObjectsRepository, 'dashboard', {}, @@ -686,17 +840,20 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); - await dashboardAdminSavedObjectedClient.delete('dashboard', createResultOne.id); + await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createPermittedResult.id); - let errorOne; + let permittedError; try { - errorOne = await dashboardAdminSavedObjectedClient.get('dashboard', createResultOne.id); + permittedError = await groupIsDashboardAdminSavedObjectedClient.get( + 'dashboard', + createPermittedResult.id + ); } catch (e) { - errorOne = e; + permittedError = e; } - expect(SavedObjectsErrorHelpers.isNotFoundError(errorOne)).toBe(true); + expect(SavedObjectsErrorHelpers.isNotFoundError(permittedError)).toBe(true); - const createResultTwo = await repositoryKit.create( + const createACLResult = await repositoryKit.create( internalSavedObjectsRepository, 'dashboard', {}, @@ -705,15 +862,67 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); - await dashboardAdminSavedObjectedClient.delete('dashboard', createResultTwo.id); + await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createACLResult.id); - let errorTwo; + let ACLError; try { - errorTwo = await dashboardAdminSavedObjectedClient.get('dashboard', createResultTwo.id); + ACLError = await groupIsDashboardAdminSavedObjectedClient.get( + 'dashboard', + createACLResult.id + ); + } catch (e) { + ACLError = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(ACLError)).toBe(true); + }); + + it('should be able to delete any data when user ids match dashboard admin', async () => { + const createPermittedResult = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + permissions: { + read: { users: ['foo'] }, + write: { users: ['foo'] }, + }, + } + ); + + await UserIdIsdashboardAdminSavedObjectedClient.delete('dashboard', createPermittedResult.id); + + let permittedError; + try { + permittedError = await UserIdIsdashboardAdminSavedObjectedClient.get( + 'dashboard', + createPermittedResult.id + ); + } catch (e) { + permittedError = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(permittedError)).toBe(true); + + const createACLResult = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + + await UserIdIsdashboardAdminSavedObjectedClient.delete('dashboard', createACLResult.id); + + let ACLError; + try { + ACLError = await UserIdIsdashboardAdminSavedObjectedClient.get( + 'dashboard', + createACLResult.id + ); } catch (e) { - errorTwo = e; + ACLError = e; } - expect(SavedObjectsErrorHelpers.isNotFoundError(errorTwo)).toBe(true); + expect(SavedObjectsErrorHelpers.isNotFoundError(ACLError)).toBe(true); }); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 955b535764d2..a8d0e1f9d05c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -7,7 +7,11 @@ import { of } from 'rxjs'; import { SavedObjectsErrorHelpers } from '../../../../core/server'; import { WorkspaceSavedObjectsClientWrapper } from './workspace_saved_objects_client_wrapper'; -const generateWorkspaceSavedObjectsClientWrapper = (isDashboardAdmin = false) => { +const GROUP_MATCH_DASHBOARD_ADMIN = 'match_group'; +const USER_MATCH_DASHBOARD_ADMIN = 'match_user_id'; +const NO_DASHBOARD_ADMIN = 'math_none'; + +const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) => { const savedObjectsStore = [ { type: 'dashboard', @@ -81,7 +85,9 @@ const generateWorkspaceSavedObjectsClientWrapper = (isDashboardAdmin = false) => validateSavedObjectsACL: jest.fn(), batchValidate: jest.fn(), getPrincipalsFromRequest: jest.fn().mockImplementation(() => { - return isDashboardAdmin ? { groups: ['dashboard_admin'] } : { users: ['user-1'] }; + if (role === GROUP_MATCH_DASHBOARD_ADMIN) return { groups: ['dashboard_admin'] }; + else if (role === USER_MATCH_DASHBOARD_ADMIN) return { users: ['dashboard_admin'] }; + return { users: ['user-1'] }; }), }; const configMock = { @@ -110,7 +116,6 @@ const generateWorkspaceSavedObjectsClientWrapper = (isDashboardAdmin = false) => requestMock, }; }; -const isDashboardAdmin = true; describe('WorkspaceSavedObjectsClientWrapper', () => { describe('wrapperFactory', () => { @@ -145,12 +150,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.delete(...deleteArgs); expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); }); - it('should call client.delete if backend role is dashboard admin', async () => { + it('should call client.delete if groups match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + const deleteArgs = ['dashboard', 'not-permitted-dashboard'] as const; + await wrapper.delete(...deleteArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); + }); + it('should call client.delete if user ids match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); const deleteArgs = ['dashboard', 'not-permitted-dashboard'] as const; await wrapper.delete(...deleteArgs); expect(permissionControlMock.validate).not.toHaveBeenCalled(); @@ -198,12 +214,29 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.update(...updateArgs); expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); }); - it('should call client.update if backend role is dashboard admin', async () => { + it('should call client.update if groups match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + const updateArgs = [ + 'dashboard', + 'not-permitted-dashboard', + { + bar: 'for', + }, + ] as const; + await wrapper.update(...updateArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); + }); + it('should call client.update if user ids match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); const updateArgs = [ 'dashboard', 'not-permitted-dashboard', @@ -250,12 +283,25 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.bulkUpdate(objectsToUpdate, {}); expect(clientMock.bulkUpdate).toHaveBeenCalledWith(objectsToUpdate, {}); }); - it('should call client.bulkUpdate if backend role is dashboard admin', async () => { + it('should call client.bulkUpdate if group match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + const bulkUpdateArgs = [ + { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, + ]; + await wrapper.bulkUpdate(bulkUpdateArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.bulkUpdate).toHaveBeenCalledWith(bulkUpdateArgs); + }); + it('should call client.bulkUpdate if user ids match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); const bulkUpdateArgs = [ { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, ]; @@ -347,12 +393,31 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); - it('should call client.bulkCreate if backend role is dashboard admin', async () => { + it('should call client.bulkCreate if groups match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + const objectsToBulkCreate = [ + { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, + ]; + await wrapper.bulkCreate(objectsToBulkCreate, { + overwrite: true, + workspaces: ['not-permitted-workspace'], + }); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { + overwrite: true, + workspaces: ['not-permitted-workspace'], + }); + }); + it('should call client.bulkCreate if user ids match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); const objectsToBulkCreate = [ { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, ]; @@ -440,12 +505,36 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); }); - it('should call client.create if backend role is dashboard admin', async () => { + it('should call client.create if groups match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + await wrapper.create( + 'dashboard', + { foo: 'bar' }, + { + id: 'not-permitted-dashboard', + overwrite: true, + } + ); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(clientMock.create).toHaveBeenCalledWith( + 'dashboard', + { foo: 'bar' }, + { + id: 'not-permitted-dashboard', + overwrite: true, + } + ); + }); + it('should call client.create if user ids match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); await wrapper.create( 'dashboard', { foo: 'bar' }, @@ -525,12 +614,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(clientMock.get).toHaveBeenCalledWith(...getArgs); expect(result).toMatchInlineSnapshot(`[Error: Not Found]`); }); - it('should call client.get and return result with arguments backend role is dashboard admin', async () => { + it('should call client.get and return result with arguments if groups match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + const getArgs = ['dashboard', 'not-permitted-dashboard'] as const; + const result = await wrapper.get(...getArgs); + expect(clientMock.get).toHaveBeenCalledWith(...getArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(result.id).toBe('not-permitted-dashboard'); + }); + it('should call client.get and return result with arguments if user ids match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); const getArgs = ['dashboard', 'not-permitted-dashboard'] as const; const result = await wrapper.get(...getArgs); expect(clientMock.get).toHaveBeenCalledWith(...getArgs); @@ -602,12 +703,33 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { {} ); }); - it('should call client.bulkGet and return result with arguments if backend role is dashboard admin', async () => { + it('should call client.bulkGet and return result with arguments if groups match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + const bulkGetArgs = [ + { + type: 'dashboard', + id: 'foo', + }, + { + type: 'dashboard', + id: 'not-permitted-dashboard', + }, + ]; + const result = await wrapper.bulkGet(bulkGetArgs); + expect(clientMock.bulkGet).toHaveBeenCalledWith(bulkGetArgs); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + expect(result.saved_objects.length).toBe(2); + }); + it('should call client.bulkGet and return result with arguments if user ids match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); const bulkGetArgs = [ { type: 'dashboard', @@ -698,12 +820,28 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }, }); }); - it('should call client.find with arguments if backend role is dashboard admin', async () => { + it('should call client.find with arguments if groups match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + await wrapper.find({ + type: 'dashboard', + workspaces: ['workspace-1', 'not-permitted-workspace'], + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'dashboard', + workspaces: ['workspace-1', 'not-permitted-workspace'], + }); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); + it('should call client.find with arguments if user ids match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); await wrapper.find({ type: 'dashboard', workspaces: ['workspace-1', 'not-permitted-workspace'], @@ -742,12 +880,22 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.deleteByWorkspace('workspace-1', {}); expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-1', {}); }); - it('should call client.deleteByWorkspace if backend role is dashboard admin', async () => { + it('should call client.deleteByWorkspace if groups match dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); + await wrapper.deleteByWorkspace('not-permitted-workspace'); + expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('not-permitted-workspace'); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); + it('should call client.deleteByWorkspace if user ids match dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(isDashboardAdmin); + } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); await wrapper.deleteByWorkspace('not-permitted-workspace'); expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('not-permitted-workspace'); expect(permissionControlMock.validate).not.toHaveBeenCalled(); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 9e378558bece..34e1fef0859f 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -146,18 +146,20 @@ export class WorkspaceSavedObjectsClientWrapper { private isDashboardAdmin(request: OpenSearchDashboardsRequest): boolean { const config = this.config || ({} as WorkspacePluginConfigType); + let groups: string[]; + let users: string[]; - // Before the user login OSD, calling the getPrincipalsFromRequest method will throw 'NOT_AUTHORIZED' exception. + // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. try { - const { groups = [], users = [] } = this.permissionControl.getPrincipalsFromRequest(request); - const adminGroups = config?.dashboardAdmin?.groups || []; - const adminUsers = config?.dashboardAdmin?.users || []; - const groupMatchAny = groups?.some((group) => adminGroups.includes(group)) || false; - const userMatchAny = users?.some((user) => adminUsers.includes(user)) || false; - return groupMatchAny || userMatchAny; + ({ groups = [], users = [] } = this.permissionControl.getPrincipalsFromRequest(request)); } catch (e) { return false; } + const adminGroups = config?.dashboardAdmin?.groups || []; + const adminUsers = config?.dashboardAdmin?.users || []; + const groupMatchAny = groups?.some((group) => adminGroups.includes(group)) || false; + const userMatchAny = users?.some((user) => adminUsers.includes(user)) || false; + return groupMatchAny || userMatchAny; } /** From 22323c10aa1ad2cbc09a99359383020c5c1fba1c Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 20 Mar 2024 16:01:01 +0800 Subject: [PATCH 03/21] delete useless code --- .../workspace_saved_objects_client_wrapper.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 34e1fef0859f..a6b113322609 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -155,10 +155,10 @@ export class WorkspaceSavedObjectsClientWrapper { } catch (e) { return false; } - const adminGroups = config?.dashboardAdmin?.groups || []; - const adminUsers = config?.dashboardAdmin?.users || []; - const groupMatchAny = groups?.some((group) => adminGroups.includes(group)) || false; - const userMatchAny = users?.some((user) => adminUsers.includes(user)) || false; + const adminGroups = config.dashboardAdmin?.groups || []; + const adminUsers = config.dashboardAdmin?.users || []; + const groupMatchAny = groups.some((group) => adminGroups.includes(group)) || false; + const userMatchAny = users.some((user) => adminUsers.includes(user)) || false; return groupMatchAny || userMatchAny; } From f9666a00bff286a6dba2eb726af911d0f0f8eed4 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Thu, 21 Mar 2024 13:57:22 +0800 Subject: [PATCH 04/21] change isDashboard function name to isRequestByDashboardAdmin --- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index a6b113322609..a74df5295b62 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -144,7 +144,7 @@ export class WorkspaceSavedObjectsClientWrapper { return false; }; - private isDashboardAdmin(request: OpenSearchDashboardsRequest): boolean { + private isRequestByDashboardAdmin(request: OpenSearchDashboardsRequest): boolean { const config = this.config || ({} as WorkspacePluginConfigType); let groups: string[]; let users: string[]; @@ -549,7 +549,7 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.deleteByWorkspace(workspace, options); }; - const isDashboardAdmin = this.isDashboardAdmin(wrapperOptions.request); + const isDashboardAdmin = this.isRequestByDashboardAdmin(wrapperOptions.request); if (isDashboardAdmin) { return wrapperOptions.client; From 166451a0f5b713912b32c47cf05713b711c65cf6 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 2 Apr 2024 13:19:32 +0800 Subject: [PATCH 05/21] dashboard admin config integrating with dynamic application config Signed-off-by: yubonluo --- src/legacy/server/osd_server.d.ts | 1 + .../workspace/opensearch_dashboards.json | 2 +- src/plugins/workspace/server/plugin.ts | 43 +++++++++++-- .../workspace_saved_objects_client_wrapper.ts | 61 +++++++++++++------ src/plugins/workspace/server/types.ts | 5 ++ 5 files changed, 87 insertions(+), 25 deletions(-) diff --git a/src/legacy/server/osd_server.d.ts b/src/legacy/server/osd_server.d.ts index 9d94349cf1c0..b2a14abc3bf2 100644 --- a/src/legacy/server/osd_server.d.ts +++ b/src/legacy/server/osd_server.d.ts @@ -64,6 +64,7 @@ declare module '@hapi/hapi' { interface PluginsStates { workspace?: { id?: string; + isDashboardAdmin?: boolean; }; } } diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 7d94a7491a00..039486a22cbf 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -6,6 +6,6 @@ "requiredPlugins": [ "savedObjects" ], - "optionalPlugins": ["savedObjectsManagement"], + "optionalPlugins": ["savedObjectsManagement", "applicationConfig"], "requiredBundles": ["opensearchDashboardsReact"] } diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index ce4efda93d51..e6f76de69e53 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -16,17 +16,22 @@ import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, } from '../common/constants'; -import { IWorkspaceClientImpl } from './types'; +import { AppPluginSetupDependencies, IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; -import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { + cleanWorkspaceId, + getWorkspaceIdFromUrl, + updateWorkspaceState, +} from '../../../core/server/utils'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; +import { isRequestByDashboardAdmin } from './saved_objects/workspace_saved_objects_client_wrapper'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -60,7 +65,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.config$ = initializerContext.config.create(); } - public async setup(core: CoreSetup) { + public async setup(core: CoreSetup, { applicationConfig }: AppPluginSetupDependencies) { this.logger.debug('Setting up Workspaces service'); const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise(); const isPermissionControlEnabled = @@ -83,9 +88,39 @@ export class WorkspacePlugin implements Plugin<{}, {}> { if (isPermissionControlEnabled) { this.permissionControl = new SavedObjectsPermissionControl(this.logger); + this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig); + if (!!applicationConfig) { + core.http.registerOnPostAuth(async (request, response, toolkit) => { + const [coreStart] = await core.getStartServices(); + const scopeClient = coreStart.opensearch.client.asScoped(request); + const configClient = applicationConfig.getConfigurationClient(scopeClient); + + const [adminGroups, adminUsers] = await Promise.all([ + configClient.getEntityConfig('workspace.dashboardAdmin.groups').catch(() => { + return undefined; + }), + configClient.getEntityConfig('workspace.dashboardAdmin.users').catch(() => { + return undefined; + }), + ]); + + const isDashboardAdmin = isRequestByDashboardAdmin( + request, + adminGroups ? [adminGroups] : [], + adminUsers ? [adminUsers] : [], + this.permissionControl! + ); + updateWorkspaceState(request, { + isDashboardAdmin, + }); + return toolkit.next(); + }); + } + this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( this.permissionControl, - { config$: this.config$ } + { config$: this.config$ }, + !!applicationConfig ); core.savedObjects.addClientWrapper( diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index a74df5295b62..84b3d14341db 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -7,6 +7,7 @@ import { i18n } from '@osd/i18n'; import { Observable } from 'rxjs'; import { first } from 'rxjs/operators'; +import { getWorkspaceState } from '../../../../core/server/utils'; import { OpenSearchDashboardsRequest, SavedObject, @@ -69,6 +70,29 @@ const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[] return !values || values.length === 0 ? defaultValues : values; }; +export const isRequestByDashboardAdmin = ( + request: OpenSearchDashboardsRequest, + adminGroups: string[], + adminUsers: string[], + permissionControl: SavedObjectsPermissionControlContract +): boolean => { + if (adminGroups.length === 0 && adminUsers.length === 0) return false; + + let groups: string[]; + let users: string[]; + + // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. + try { + ({ groups = [], users = [] } = permissionControl.getPrincipalsFromRequest(request)); + } catch (e) { + return false; + } + + const groupMatchAny = groups.some((group) => adminGroups.includes(group)) || false; + const userMatchAny = users.some((user) => adminUsers.includes(user)) || false; + return groupMatchAny || userMatchAny; +}; + export class WorkspaceSavedObjectsClientWrapper { private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; private config?: WorkspacePluginConfigType; @@ -144,24 +168,6 @@ export class WorkspaceSavedObjectsClientWrapper { return false; }; - private isRequestByDashboardAdmin(request: OpenSearchDashboardsRequest): boolean { - const config = this.config || ({} as WorkspacePluginConfigType); - let groups: string[]; - let users: string[]; - - // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. - try { - ({ groups = [], users = [] } = this.permissionControl.getPrincipalsFromRequest(request)); - } catch (e) { - return false; - } - const adminGroups = config.dashboardAdmin?.groups || []; - const adminUsers = config.dashboardAdmin?.users || []; - const groupMatchAny = groups.some((group) => adminGroups.includes(group)) || false; - const userMatchAny = users.some((user) => adminUsers.includes(user)) || false; - return groupMatchAny || userMatchAny; - } - /** * check if the type include workspace * Workspace permission check is totally different from object permission check. @@ -549,7 +555,21 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.deleteByWorkspace(workspace, options); }; - const isDashboardAdmin = this.isRequestByDashboardAdmin(wrapperOptions.request); + let isDashboardAdmin: boolean = false; + if (this.applicationConfig) { + const workspaceState = getWorkspaceState(wrapperOptions.request); + isDashboardAdmin = workspaceState?.isDashboardAdmin || false; + } else { + const config = this.config || ({} as WorkspacePluginConfigType); + const adminGroups = config.dashboardAdmin?.groups || []; + const adminUsers = config.dashboardAdmin?.users || []; + isDashboardAdmin = isRequestByDashboardAdmin( + wrapperOptions.request, + adminGroups, + adminUsers, + this.permissionControl + ); + } if (isDashboardAdmin) { return wrapperOptions.client; @@ -577,7 +597,8 @@ export class WorkspaceSavedObjectsClientWrapper { private readonly permissionControl: SavedObjectsPermissionControlContract, private readonly options: { config$: Observable; - } + }, + private readonly applicationConfig: boolean ) { this.options?.config$.subscribe((config) => { this.config = config; diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index ba1ff8a9cd47..26215cd43bb0 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { ApplicationConfigPluginSetup } from 'src/plugins/application_config/server'; import { Logger, OpenSearchDashboardsRequest, @@ -132,3 +133,7 @@ export type WorkspacePermissionItem = { | WorkspacePermissionMode.Write >; } & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); + +export interface AppPluginSetupDependencies { + applicationConfig: ApplicationConfigPluginSetup; +} From 363aeaa55d0e6beaf8814efabc27c5fb224b2215 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 2 Apr 2024 13:53:52 +0800 Subject: [PATCH 06/21] Optimize the code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index e6f76de69e53..69c61dee79a4 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -96,12 +96,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { const configClient = applicationConfig.getConfigurationClient(scopeClient); const [adminGroups, adminUsers] = await Promise.all([ - configClient.getEntityConfig('workspace.dashboardAdmin.groups').catch(() => { - return undefined; - }), - configClient.getEntityConfig('workspace.dashboardAdmin.users').catch(() => { - return undefined; - }), + configClient.getEntityConfig('workspace.dashboardAdmin.groups').catch(() => undefined), + configClient.getEntityConfig('workspace.dashboardAdmin.users').catch(() => undefined), ]); const isDashboardAdmin = isRequestByDashboardAdmin( From c8bb0b51202a88a2cc3cab354bc5e927861a3c32 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 2 Apr 2024 15:16:25 +0800 Subject: [PATCH 07/21] fix test error Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index c448fcf209f9..5e919d9c58a0 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -5,6 +5,7 @@ import { coreMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; +import { AppPluginSetupDependencies } from './types'; describe('Workspace server plugin', () => { it('#setup', async () => { @@ -16,9 +17,16 @@ describe('Workspace server plugin', () => { enabled: true, }, }); + const mockApplicationConfig = { + getConfigurationClient: jest.fn().mockResolvedValue({}), + registerConfigurationClient: jest.fn().mockResolvedValue({}), + }; + const mockDependencies: AppPluginSetupDependencies = { + applicationConfig: mockApplicationConfig, + }; setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn())); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); - await workspacePlugin.setup(setupMock); + await workspacePlugin.setup(setupMock, mockDependencies); expect(value).toMatchInlineSnapshot(` Object { "workspaces": Object { From 74d9a269b5a047ac27f57a4c0bb78ae8ef4cd8ae Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 3 Apr 2024 09:52:10 +0800 Subject: [PATCH 08/21] delete useless code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 83 +++-- ...space_saved_objects_client_wrapper.test.ts | 284 +++--------------- ...space_saved_objects_client_wrapper.test.ts | 212 ++----------- .../workspace_saved_objects_client_wrapper.ts | 62 +--- 4 files changed, 132 insertions(+), 509 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 69c61dee79a4..5c351eec1e47 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,6 +11,7 @@ import { Plugin, Logger, CoreStart, + OpenSearchDashboardsRequest, } from '../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, @@ -31,7 +32,6 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; -import { isRequestByDashboardAdmin } from './saved_objects/workspace_saved_objects_client_wrapper'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -60,6 +60,26 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }); } + private isRequestByDashboardAdmin( + request: OpenSearchDashboardsRequest, + groups: string[], + users: string[], + configGroups: string[], + configUsers: string[] + ) { + if (configGroups.length === 0 && configUsers.length === 0) { + updateWorkspaceState(request, { + isDashboardAdmin: false, + }); + return; + } + const groupMatchAny = groups.some((group) => configGroups.includes(group)) || false; + const userMatchAny = users.some((user) => configUsers.includes(user)) || false; + updateWorkspaceState(request, { + isDashboardAdmin: groupMatchAny || userMatchAny, + }); + } + constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); this.config$ = initializerContext.config.create(); @@ -88,35 +108,56 @@ export class WorkspacePlugin implements Plugin<{}, {}> { if (isPermissionControlEnabled) { this.permissionControl = new SavedObjectsPermissionControl(this.logger); - this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig); - if (!!applicationConfig) { - core.http.registerOnPostAuth(async (request, response, toolkit) => { + core.http.registerOnPostAuth(async (request, response, toolkit) => { + let groups: string[]; + let users: string[]; + + // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. + try { + ({ groups = [], users = [] } = this.permissionControl!.getPrincipalsFromRequest(request)); + } catch (e) { + return toolkit.next(); + } + if (groups.length === 0 && users.length === 0) { + updateWorkspaceState(request, { + isDashboardAdmin: false, + }); + return toolkit.next(); + } + + if (!!applicationConfig) { + this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig); const [coreStart] = await core.getStartServices(); const scopeClient = coreStart.opensearch.client.asScoped(request); - const configClient = applicationConfig.getConfigurationClient(scopeClient); - - const [adminGroups, adminUsers] = await Promise.all([ - configClient.getEntityConfig('workspace.dashboardAdmin.groups').catch(() => undefined), - configClient.getEntityConfig('workspace.dashboardAdmin.users').catch(() => undefined), + const applicationConfigClient = applicationConfig.getConfigurationClient(scopeClient); + + const [configGroups, configUsers] = await Promise.all([ + applicationConfigClient + .getEntityConfig('workspace.dashboardAdmin.groups') + .catch(() => undefined), + applicationConfigClient + .getEntityConfig('workspace.dashboardAdmin.users') + .catch(() => undefined), ]); - const isDashboardAdmin = isRequestByDashboardAdmin( + this.isRequestByDashboardAdmin( request, - adminGroups ? [adminGroups] : [], - adminUsers ? [adminUsers] : [], - this.permissionControl! + groups, + users, + configGroups ? [configGroups] : [], + configUsers ? [configUsers] : [] ); - updateWorkspaceState(request, { - isDashboardAdmin, - }); return toolkit.next(); - }); - } + } + + const configGroups = config.dashboardAdmin.groups || []; + const configUsers = config.dashboardAdmin.users || []; + this.isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers); + return toolkit.next(); + }); this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( - this.permissionControl, - { config$: this.config$ }, - !!applicationConfig + this.permissionControl ); core.savedObjects.addClientWrapper( diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index fd240e064dea..6ce411b743c8 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -17,6 +17,7 @@ import { } from '../../../../../core/server'; import { httpServerMock } from '../../../../../../src/core/server/mocks'; import * as utilsExports from '../../utils'; +import { updateWorkspaceState } from '../../../../../core/server/utils'; const repositoryKit = (() => { const savedObjects: Array<{ type: string; id: string }> = []; @@ -51,8 +52,7 @@ const repositoryKit = (() => { const permittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); const notPermittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); -const groupIsDashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); -const UserIdIsDashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); +const dashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); describe('WorkspaceSavedObjectsClientWrapper', () => { let internalSavedObjectsRepository: ISavedObjectsRepository; @@ -61,8 +61,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { let osd: TestOpenSearchDashboardsUtils; let permittedSavedObjectedClient: SavedObjectsClientContract; let notPermittedSavedObjectedClient: SavedObjectsClientContract; - let groupIsDashboardAdminSavedObjectedClient: SavedObjectsClientContract; - let UserIdIsdashboardAdminSavedObjectedClient: SavedObjectsClientContract; + let dashboardAdminSavedObjectedClient: SavedObjectsClientContract; beforeAll(async function () { servers = createTestServers({ @@ -132,20 +131,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation((request) => { if (request === notPermittedRequest) return { users: ['bar'] }; - else if (request === permittedRequest) return { users: ['foo'] }; - else if (request === groupIsDashboardAdminRequest) return { groups: ['dashboard_admin'] }; - return { users: ['dashboard_admin'] }; + else return { users: ['foo'] }; }); permittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient(permittedRequest); notPermittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( notPermittedRequest ); - groupIsDashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( - groupIsDashboardAdminRequest - ); - UserIdIsdashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( - UserIdIsDashboardAdminRequest + updateWorkspaceState(dashboardAdminRequest, { isDashboardAdmin: true }); + dashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( + dashboardAdminRequest ); }); @@ -187,41 +182,14 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toBeUndefined(); }); - it('should return consistent dashboard when groups match dashboard admin', async () => { - expect( - ( - await groupIsDashboardAdminSavedObjectedClient.get( - 'dashboard', - 'inner-workspace-dashboard-1' - ) - ).error - ).toBeUndefined(); - expect( - ( - await groupIsDashboardAdminSavedObjectedClient.get( - 'dashboard', - 'acl-controlled-dashboard-2' - ) - ).error - ).toBeUndefined(); - }); - - it('should return consistent dashboard when user ids match dashboard admin', async () => { + it('should return consistent dashboard when groups/users is dashboard admin', async () => { expect( - ( - await UserIdIsdashboardAdminSavedObjectedClient.get( - 'dashboard', - 'inner-workspace-dashboard-1' - ) - ).error + (await dashboardAdminSavedObjectedClient.get('dashboard', 'inner-workspace-dashboard-1')) + .error ).toBeUndefined(); expect( - ( - await UserIdIsdashboardAdminSavedObjectedClient.get( - 'dashboard', - 'acl-controlled-dashboard-2' - ) - ).error + (await dashboardAdminSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2')) + .error ).toBeUndefined(); }); }); @@ -268,34 +236,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toEqual(1); }); - it('should return consistent dashboard when groups match dashboard admin', async () => { - expect( - ( - await groupIsDashboardAdminSavedObjectedClient.bulkGet([ - { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, - ]) - ).saved_objects.length - ).toEqual(1); - expect( - ( - await groupIsDashboardAdminSavedObjectedClient.bulkGet([ - { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, - ]) - ).saved_objects.length - ).toEqual(1); - }); - - it('should return consistent dashboard when user ids match dashboard admin', async () => { + it('should return consistent dashboard when groups/users is dashboard admin', async () => { expect( ( - await UserIdIsdashboardAdminSavedObjectedClient.bulkGet([ + await dashboardAdminSavedObjectedClient.bulkGet([ { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, ]) ).saved_objects.length ).toEqual(1); expect( ( - await UserIdIsdashboardAdminSavedObjectedClient.bulkGet([ + await dashboardAdminSavedObjectedClient.bulkGet([ { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, ]) ).saved_objects.length @@ -333,21 +284,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); }); - it('should return consistent inner workspace data when groups match dashboard admin', async () => { - const result = await groupIsDashboardAdminSavedObjectedClient.find({ - type: 'dashboard', - workspaces: ['workspace-1'], - perPage: 999, - page: 1, - }); - - expect(result.saved_objects.some((item) => item.id === 'inner-workspace-dashboard-1')).toBe( - true - ); - }); - - it('should return consistent inner workspace data when user ids match dashboard admin', async () => { - const result = await UserIdIsdashboardAdminSavedObjectedClient.find({ + it('should return consistent inner workspace data when groups/users is dashboard admin', async () => { + const result = await dashboardAdminSavedObjectedClient.find({ type: 'dashboard', workspaces: ['workspace-1'], perPage: 999, @@ -390,20 +328,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', createResult.id); }); - it('should able to create saved objects into any workspaces after create called when groups match dashboard admin', async () => { - const createResult = await groupIsDashboardAdminSavedObjectedClient.create( - 'dashboard', - {}, - { - workspaces: ['workspace-1'], - } - ); - expect(createResult.error).toBeUndefined(); - await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); - }); - - it('should able to create saved objects into any workspaces after create called when user ids match dashboard admin', async () => { - const createResult = await UserIdIsdashboardAdminSavedObjectedClient.create( + it('should able to create saved objects into any workspaces after create called when groups/users is dashboard admin', async () => { + const createResult = await dashboardAdminSavedObjectedClient.create( 'dashboard', {}, { @@ -411,7 +337,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); expect(createResult.error).toBeUndefined(); - await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); + await dashboardAdminSavedObjectedClient.delete('dashboard', createResult.id); }); it('should throw forbidden error when create with override', async () => { @@ -446,22 +372,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.error).toBeUndefined(); }); - it('should able to create with override when groups match dashboard admin', async () => { - const createResult = await groupIsDashboardAdminSavedObjectedClient.create( - 'dashboard', - {}, - { - id: 'inner-workspace-dashboard-1', - overwrite: true, - workspaces: ['workspace-1'], - } - ); - - expect(createResult.error).toBeUndefined(); - }); - - it('should able to create with override when uesr ids match dashboard admin', async () => { - const createResult = await UserIdIsdashboardAdminSavedObjectedClient.create( + it('should able to create with override when groups/users is dashboard admin', async () => { + const createResult = await dashboardAdminSavedObjectedClient.create( 'dashboard', {}, { @@ -501,28 +413,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', objectId); }); - it('should able to create saved objects into any workspaces after bulkCreate called when groups match dashboard damin', async () => { + it('should able to create saved objects into any workspaces after bulkCreate called when groups/users is dashboard damin', async () => { const objectId = new Date().getTime().toString(16).toUpperCase(); - const result = await groupIsDashboardAdminSavedObjectedClient.bulkCreate( + const result = await dashboardAdminSavedObjectedClient.bulkCreate( [{ type: 'dashboard', attributes: {}, id: objectId }], { workspaces: ['workspace-1'], } ); expect(result.saved_objects.length).toEqual(1); - await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', objectId); - }); - - it('should able to create saved objects into any workspaces after bulkCreate called when user ids match dashboard damin', async () => { - const objectId = new Date().getTime().toString(16).toUpperCase(); - const result = await UserIdIsdashboardAdminSavedObjectedClient.bulkCreate( - [{ type: 'dashboard', attributes: {}, id: objectId }], - { - workspaces: ['workspace-1'], - } - ); - expect(result.saved_objects.length).toEqual(1); - await UserIdIsdashboardAdminSavedObjectedClient.delete('dashboard', objectId); + await dashboardAdminSavedObjectedClient.delete('dashboard', objectId); }); it('should throw forbidden error when create with override', async () => { @@ -566,26 +466,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.saved_objects).toHaveLength(1); }); - it('should able to bulk create with override when groups match dashboard admin', async () => { - const createResult = await groupIsDashboardAdminSavedObjectedClient.bulkCreate( - [ - { - id: 'inner-workspace-dashboard-1', - type: 'dashboard', - attributes: {}, - }, - ], - { - overwrite: true, - workspaces: ['workspace-1'], - } - ); - - expect(createResult.saved_objects).toHaveLength(1); - }); - - it('should able to bulk create with override when user ids match dashboard admin', async () => { - const createResult = await UserIdIsdashboardAdminSavedObjectedClient.bulkCreate( + it('should able to bulk create with override when groups/users is dashboard admin', async () => { + const createResult = await dashboardAdminSavedObjectedClient.bulkCreate( [ { id: 'inner-workspace-dashboard-1', @@ -639,10 +521,10 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toBeUndefined(); }); - it('should update saved objects for any workspaces when groups match dashboard admin', async () => { + it('should update saved objects for any workspaces when groups/users is dashboard admin', async () => { expect( ( - await groupIsDashboardAdminSavedObjectedClient.update( + await dashboardAdminSavedObjectedClient.update( 'dashboard', 'inner-workspace-dashboard-1', {} @@ -651,28 +533,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toBeUndefined(); expect( ( - await groupIsDashboardAdminSavedObjectedClient.update( - 'dashboard', - 'acl-controlled-dashboard-2', - {} - ) - ).error - ).toBeUndefined(); - }); - - it('should update saved objects for any workspaces when user ids match dashboard admin', async () => { - expect( - ( - await UserIdIsdashboardAdminSavedObjectedClient.update( - 'dashboard', - 'inner-workspace-dashboard-1', - {} - ) - ).error - ).toBeUndefined(); - expect( - ( - await UserIdIsdashboardAdminSavedObjectedClient.update( + await dashboardAdminSavedObjectedClient.update( 'dashboard', 'acl-controlled-dashboard-2', {} @@ -726,34 +587,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).toEqual(1); }); - it('should bulk update saved objects for any workspaces when groups match dashboard admin', async () => { + it('should bulk update saved objects for any workspaces when groups/users is dashboard admin', async () => { expect( ( - await groupIsDashboardAdminSavedObjectedClient.bulkUpdate([ + await dashboardAdminSavedObjectedClient.bulkUpdate([ { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, ]) ).saved_objects.length ).toEqual(1); expect( ( - await groupIsDashboardAdminSavedObjectedClient.bulkUpdate([ - { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, - ]) - ).saved_objects.length - ).toEqual(1); - }); - - it('should bulk update saved objects for any workspaces when user ids match dashboard admin', async () => { - expect( - ( - await UserIdIsdashboardAdminSavedObjectedClient.bulkUpdate([ - { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, - ]) - ).saved_objects.length - ).toEqual(1); - expect( - ( - await UserIdIsdashboardAdminSavedObjectedClient.bulkUpdate([ + await dashboardAdminSavedObjectedClient.bulkUpdate([ { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, ]) ).saved_objects.length @@ -827,56 +671,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); - it('should be able to delete any data when groups match dashboard admin', async () => { - const createPermittedResult = await repositoryKit.create( - internalSavedObjectsRepository, - 'dashboard', - {}, - { - permissions: { - read: { users: ['foo'] }, - write: { users: ['foo'] }, - }, - } - ); - - await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createPermittedResult.id); - - let permittedError; - try { - permittedError = await groupIsDashboardAdminSavedObjectedClient.get( - 'dashboard', - createPermittedResult.id - ); - } catch (e) { - permittedError = e; - } - expect(SavedObjectsErrorHelpers.isNotFoundError(permittedError)).toBe(true); - - const createACLResult = await repositoryKit.create( - internalSavedObjectsRepository, - 'dashboard', - {}, - { - workspaces: ['workspace-1'], - } - ); - - await groupIsDashboardAdminSavedObjectedClient.delete('dashboard', createACLResult.id); - - let ACLError; - try { - ACLError = await groupIsDashboardAdminSavedObjectedClient.get( - 'dashboard', - createACLResult.id - ); - } catch (e) { - ACLError = e; - } - expect(SavedObjectsErrorHelpers.isNotFoundError(ACLError)).toBe(true); - }); - - it('should be able to delete any data when user ids match dashboard admin', async () => { + it('should be able to delete any data when groups/users is dashboard admin', async () => { const createPermittedResult = await repositoryKit.create( internalSavedObjectsRepository, 'dashboard', @@ -889,11 +684,11 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); - await UserIdIsdashboardAdminSavedObjectedClient.delete('dashboard', createPermittedResult.id); + await dashboardAdminSavedObjectedClient.delete('dashboard', createPermittedResult.id); let permittedError; try { - permittedError = await UserIdIsdashboardAdminSavedObjectedClient.get( + permittedError = await dashboardAdminSavedObjectedClient.get( 'dashboard', createPermittedResult.id ); @@ -911,14 +706,11 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); - await UserIdIsdashboardAdminSavedObjectedClient.delete('dashboard', createACLResult.id); + await dashboardAdminSavedObjectedClient.delete('dashboard', createACLResult.id); let ACLError; try { - ACLError = await UserIdIsdashboardAdminSavedObjectedClient.get( - 'dashboard', - createACLResult.id - ); + ACLError = await dashboardAdminSavedObjectedClient.get('dashboard', createACLResult.id); } catch (e) { ACLError = e; } diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index a8d0e1f9d05c..eccc70f6f991 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -3,13 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { of } from 'rxjs'; +import { getWorkspaceState, updateWorkspaceState } from '../../../../core/server/utils'; import { SavedObjectsErrorHelpers } from '../../../../core/server'; import { WorkspaceSavedObjectsClientWrapper } from './workspace_saved_objects_client_wrapper'; +import { httpServerMock } from '../../../../core/server/mocks'; -const GROUP_MATCH_DASHBOARD_ADMIN = 'match_group'; -const USER_MATCH_DASHBOARD_ADMIN = 'match_user_id'; -const NO_DASHBOARD_ADMIN = 'math_none'; +const DASHBOARD_ADMIN = 'dashnoard_admin'; +const NO_DASHBOARD_ADMIN = 'no_dashnoard_admin'; const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) => { const savedObjectsStore = [ @@ -68,7 +68,8 @@ const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) = find: jest.fn(), deleteByWorkspace: jest.fn(), }; - const requestMock = {}; + const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); + if (role === DASHBOARD_ADMIN) updateWorkspaceState(requestMock, { isDashboardAdmin: true }); const wrapperOptions = { client: clientMock, request: requestMock, @@ -85,25 +86,11 @@ const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) = validateSavedObjectsACL: jest.fn(), batchValidate: jest.fn(), getPrincipalsFromRequest: jest.fn().mockImplementation(() => { - if (role === GROUP_MATCH_DASHBOARD_ADMIN) return { groups: ['dashboard_admin'] }; - else if (role === USER_MATCH_DASHBOARD_ADMIN) return { users: ['dashboard_admin'] }; return { users: ['user-1'] }; }), }; - const configMock = { - enabled: true, - permission: { - enabled: true, - }, - dashboardAdmin: { - groups: ['dashboard_admin'], - users: ['dashboard_admin'], - }, - }; - const optionsMock = { - config$: of(configMock), - }; - const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock, optionsMock); + + const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); wrapper.setScopedClient(() => ({ find: jest.fn().mockImplementation(async () => ({ saved_objects: [{ id: 'workspace-1', type: 'workspace' }], @@ -150,23 +137,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.delete(...deleteArgs); expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); }); - it('should call client.delete if groups match dashboard admin', async () => { + it('should call client.delete if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - const deleteArgs = ['dashboard', 'not-permitted-dashboard'] as const; - await wrapper.delete(...deleteArgs); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); - }); - it('should call client.delete if user ids match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); + expect(getWorkspaceState(requestMock)).toEqual({ + isDashboardAdmin: true, + }); const deleteArgs = ['dashboard', 'not-permitted-dashboard'] as const; await wrapper.delete(...deleteArgs); expect(permissionControlMock.validate).not.toHaveBeenCalled(); @@ -214,29 +194,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.update(...updateArgs); expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); }); - it('should call client.update if groups match dashboard admin', async () => { + it('should call client.update if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - const updateArgs = [ - 'dashboard', - 'not-permitted-dashboard', - { - bar: 'for', - }, - ] as const; - await wrapper.update(...updateArgs); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); - }); - it('should call client.update if user ids match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); const updateArgs = [ 'dashboard', 'not-permitted-dashboard', @@ -283,25 +246,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.bulkUpdate(objectsToUpdate, {}); expect(clientMock.bulkUpdate).toHaveBeenCalledWith(objectsToUpdate, {}); }); - it('should call client.bulkUpdate if group match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - const bulkUpdateArgs = [ - { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, - ]; - await wrapper.bulkUpdate(bulkUpdateArgs); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(clientMock.bulkUpdate).toHaveBeenCalledWith(bulkUpdateArgs); - }); - it('should call client.bulkUpdate if user ids match dashboard admin', async () => { + it('should call client.bulkUpdate if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); const bulkUpdateArgs = [ { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, ]; @@ -393,31 +343,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); - it('should call client.bulkCreate if groups match dashboard admin', async () => { + it('should call client.bulkCreate if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - const objectsToBulkCreate = [ - { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, - ]; - await wrapper.bulkCreate(objectsToBulkCreate, { - overwrite: true, - workspaces: ['not-permitted-workspace'], - }); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { - overwrite: true, - workspaces: ['not-permitted-workspace'], - }); - }); - it('should call client.bulkCreate if user ids match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); const objectsToBulkCreate = [ { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, ]; @@ -505,36 +436,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); }); - it('should call client.create if groups match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - await wrapper.create( - 'dashboard', - { foo: 'bar' }, - { - id: 'not-permitted-dashboard', - overwrite: true, - } - ); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(clientMock.create).toHaveBeenCalledWith( - 'dashboard', - { foo: 'bar' }, - { - id: 'not-permitted-dashboard', - overwrite: true, - } - ); - }); - it('should call client.create if user ids match dashboard admin', async () => { + it('should call client.create if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); await wrapper.create( 'dashboard', { foo: 'bar' }, @@ -614,24 +521,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(clientMock.get).toHaveBeenCalledWith(...getArgs); expect(result).toMatchInlineSnapshot(`[Error: Not Found]`); }); - it('should call client.get and return result with arguments if groups match dashboard admin', async () => { + it('should call client.get and return result with arguments if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - const getArgs = ['dashboard', 'not-permitted-dashboard'] as const; - const result = await wrapper.get(...getArgs); - expect(clientMock.get).toHaveBeenCalledWith(...getArgs); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(result.id).toBe('not-permitted-dashboard'); - }); - it('should call client.get and return result with arguments if user ids match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); const getArgs = ['dashboard', 'not-permitted-dashboard'] as const; const result = await wrapper.get(...getArgs); expect(clientMock.get).toHaveBeenCalledWith(...getArgs); @@ -703,33 +598,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { {} ); }); - it('should call client.bulkGet and return result with arguments if groups match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - const bulkGetArgs = [ - { - type: 'dashboard', - id: 'foo', - }, - { - type: 'dashboard', - id: 'not-permitted-dashboard', - }, - ]; - const result = await wrapper.bulkGet(bulkGetArgs); - expect(clientMock.bulkGet).toHaveBeenCalledWith(bulkGetArgs); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - expect(result.saved_objects.length).toBe(2); - }); - it('should call client.bulkGet and return result with arguments if user ids match dashboard admin', async () => { + it('should call client.bulkGet and return result with arguments if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); const bulkGetArgs = [ { type: 'dashboard', @@ -820,28 +694,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }, }); }); - it('should call client.find with arguments if groups match dashboard admin', async () => { + it('should call client.find with arguments if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - await wrapper.find({ - type: 'dashboard', - workspaces: ['workspace-1', 'not-permitted-workspace'], - }); - expect(clientMock.find).toHaveBeenCalledWith({ - type: 'dashboard', - workspaces: ['workspace-1', 'not-permitted-workspace'], - }); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - }); - it('should call client.find with arguments if user ids match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); await wrapper.find({ type: 'dashboard', workspaces: ['workspace-1', 'not-permitted-workspace'], @@ -880,22 +738,12 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.deleteByWorkspace('workspace-1', {}); expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-1', {}); }); - it('should call client.deleteByWorkspace if groups match dashboard admin', async () => { - const { - wrapper, - clientMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(GROUP_MATCH_DASHBOARD_ADMIN); - await wrapper.deleteByWorkspace('not-permitted-workspace'); - expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('not-permitted-workspace'); - expect(permissionControlMock.validate).not.toHaveBeenCalled(); - }); - it('should call client.deleteByWorkspace if user ids match dashboard admin', async () => { + it('should call client.deleteByWorkspace if user/groups is dashboard admin', async () => { const { wrapper, clientMock, permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(USER_MATCH_DASHBOARD_ADMIN); + } = generateWorkspaceSavedObjectsClientWrapper(DASHBOARD_ADMIN); await wrapper.deleteByWorkspace('not-permitted-workspace'); expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('not-permitted-workspace'); expect(permissionControlMock.validate).not.toHaveBeenCalled(); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 84b3d14341db..523869321a22 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -4,8 +4,6 @@ */ import { i18n } from '@osd/i18n'; -import { Observable } from 'rxjs'; -import { first } from 'rxjs/operators'; import { getWorkspaceState } from '../../../../core/server/utils'; import { @@ -35,7 +33,6 @@ import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WorkspacePermissionMode, } from '../../common/constants'; -import { WorkspacePluginConfigType } from '../../config'; // Can't throw unauthorized for now, the page will be refreshed if unauthorized const generateWorkspacePermissionError = () => @@ -70,32 +67,8 @@ const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[] return !values || values.length === 0 ? defaultValues : values; }; -export const isRequestByDashboardAdmin = ( - request: OpenSearchDashboardsRequest, - adminGroups: string[], - adminUsers: string[], - permissionControl: SavedObjectsPermissionControlContract -): boolean => { - if (adminGroups.length === 0 && adminUsers.length === 0) return false; - - let groups: string[]; - let users: string[]; - - // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. - try { - ({ groups = [], users = [] } = permissionControl.getPrincipalsFromRequest(request)); - } catch (e) { - return false; - } - - const groupMatchAny = groups.some((group) => adminGroups.includes(group)) || false; - const userMatchAny = users.some((user) => adminUsers.includes(user)) || false; - return groupMatchAny || userMatchAny; -}; - export class WorkspaceSavedObjectsClientWrapper { private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; - private config?: WorkspacePluginConfigType; private formatWorkspacePermissionModeToStringArray( permission: WorkspacePermissionMode | WorkspacePermissionMode[] ): string[] { @@ -555,22 +528,7 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.deleteByWorkspace(workspace, options); }; - let isDashboardAdmin: boolean = false; - if (this.applicationConfig) { - const workspaceState = getWorkspaceState(wrapperOptions.request); - isDashboardAdmin = workspaceState?.isDashboardAdmin || false; - } else { - const config = this.config || ({} as WorkspacePluginConfigType); - const adminGroups = config.dashboardAdmin?.groups || []; - const adminUsers = config.dashboardAdmin?.users || []; - isDashboardAdmin = isRequestByDashboardAdmin( - wrapperOptions.request, - adminGroups, - adminUsers, - this.permissionControl - ); - } - + const isDashboardAdmin = getWorkspaceState(wrapperOptions.request)?.isDashboardAdmin; if (isDashboardAdmin) { return wrapperOptions.client; } @@ -593,21 +551,5 @@ export class WorkspaceSavedObjectsClientWrapper { }; }; - constructor( - private readonly permissionControl: SavedObjectsPermissionControlContract, - private readonly options: { - config$: Observable; - }, - private readonly applicationConfig: boolean - ) { - this.options?.config$.subscribe((config) => { - this.config = config; - }); - this.options?.config$ - .pipe(first()) - .toPromise() - .then((config) => { - this.config = config; - }); - } + constructor(private readonly permissionControl: SavedObjectsPermissionControlContract) {} } From a07dde07f2803e85e0bdea3838a2d5a804958b68 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 3 Apr 2024 10:35:24 +0800 Subject: [PATCH 09/21] optimize code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 128 +++++++++++++------------ 1 file changed, 67 insertions(+), 61 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 5c351eec1e47..c3c138b1e385 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -80,6 +80,72 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }); } + private async setupPermission( + core: CoreSetup, + config: WorkspacePluginConfigType, + { applicationConfig }: AppPluginSetupDependencies + ) { + this.permissionControl = new SavedObjectsPermissionControl(this.logger); + + core.http.registerOnPostAuth(async (request, response, toolkit) => { + let groups: string[]; + let users: string[]; + + // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. + try { + ({ groups = [], users = [] } = this.permissionControl!.getPrincipalsFromRequest(request)); + } catch (e) { + return toolkit.next(); + } + if (groups.length === 0 && users.length === 0) { + updateWorkspaceState(request, { + isDashboardAdmin: false, + }); + return toolkit.next(); + } + + this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig); + if (!!applicationConfig) { + const [coreStart] = await core.getStartServices(); + const scopeClient = coreStart.opensearch.client.asScoped(request); + const applicationConfigClient = applicationConfig.getConfigurationClient(scopeClient); + + const [configGroups, configUsers] = await Promise.all([ + applicationConfigClient + .getEntityConfig('workspace.dashboardAdmin.groups') + .catch(() => undefined), + applicationConfigClient + .getEntityConfig('workspace.dashboardAdmin.users') + .catch(() => undefined), + ]); + + this.isRequestByDashboardAdmin( + request, + groups, + users, + configGroups ? [configGroups] : [], + configUsers ? [configUsers] : [] + ); + return toolkit.next(); + } + + const configGroups = config.dashboardAdmin.groups || []; + const configUsers = config.dashboardAdmin.users || []; + this.isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers); + return toolkit.next(); + }); + + this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( + this.permissionControl + ); + + core.savedObjects.addClientWrapper( + 0, + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + this.workspaceSavedObjectsClientWrapper.wrapperFactory + ); + } + constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); this.config$ = initializerContext.config.create(); @@ -105,67 +171,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled); - if (isPermissionControlEnabled) { - this.permissionControl = new SavedObjectsPermissionControl(this.logger); - - core.http.registerOnPostAuth(async (request, response, toolkit) => { - let groups: string[]; - let users: string[]; - - // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. - try { - ({ groups = [], users = [] } = this.permissionControl!.getPrincipalsFromRequest(request)); - } catch (e) { - return toolkit.next(); - } - if (groups.length === 0 && users.length === 0) { - updateWorkspaceState(request, { - isDashboardAdmin: false, - }); - return toolkit.next(); - } - - if (!!applicationConfig) { - this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig); - const [coreStart] = await core.getStartServices(); - const scopeClient = coreStart.opensearch.client.asScoped(request); - const applicationConfigClient = applicationConfig.getConfigurationClient(scopeClient); - - const [configGroups, configUsers] = await Promise.all([ - applicationConfigClient - .getEntityConfig('workspace.dashboardAdmin.groups') - .catch(() => undefined), - applicationConfigClient - .getEntityConfig('workspace.dashboardAdmin.users') - .catch(() => undefined), - ]); - - this.isRequestByDashboardAdmin( - request, - groups, - users, - configGroups ? [configGroups] : [], - configUsers ? [configUsers] : [] - ); - return toolkit.next(); - } - - const configGroups = config.dashboardAdmin.groups || []; - const configUsers = config.dashboardAdmin.users || []; - this.isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers); - return toolkit.next(); - }); - - this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( - this.permissionControl - ); - - core.savedObjects.addClientWrapper( - 0, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, - this.workspaceSavedObjectsClientWrapper.wrapperFactory - ); - } + if (isPermissionControlEnabled) this.setupPermission(core, config, { applicationConfig }); registerRoutes({ http: core.http, From 518cd753bec817f618a939b342010f35ca0ad958 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 3 Apr 2024 11:11:38 +0800 Subject: [PATCH 10/21] optimize code and add unit test Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 25 ++-------------- src/plugins/workspace/server/utils.test.ts | 33 +++++++++++++++++++++- src/plugins/workspace/server/utils.ts | 21 ++++++++++++++ 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index c3c138b1e385..f4db056cd593 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -32,6 +32,7 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; +import { isRequestByDashboardAdmin } from './utils'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -60,26 +61,6 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }); } - private isRequestByDashboardAdmin( - request: OpenSearchDashboardsRequest, - groups: string[], - users: string[], - configGroups: string[], - configUsers: string[] - ) { - if (configGroups.length === 0 && configUsers.length === 0) { - updateWorkspaceState(request, { - isDashboardAdmin: false, - }); - return; - } - const groupMatchAny = groups.some((group) => configGroups.includes(group)) || false; - const userMatchAny = users.some((user) => configUsers.includes(user)) || false; - updateWorkspaceState(request, { - isDashboardAdmin: groupMatchAny || userMatchAny, - }); - } - private async setupPermission( core: CoreSetup, config: WorkspacePluginConfigType, @@ -119,7 +100,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { .catch(() => undefined), ]); - this.isRequestByDashboardAdmin( + isRequestByDashboardAdmin( request, groups, users, @@ -131,7 +112,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { const configGroups = config.dashboardAdmin.groups || []; const configUsers = config.dashboardAdmin.users || []; - this.isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers); + isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers); return toolkit.next(); }); diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 1f6c3e58f122..bb367af57e79 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -5,7 +5,8 @@ import { AuthStatus } from '../../../core/server'; import { httpServerMock, httpServiceMock } from '../../../core/server/mocks'; -import { generateRandomId, getPrincipalsFromRequest } from './utils'; +import { generateRandomId, getPrincipalsFromRequest, isRequestByDashboardAdmin } from './utils'; +import { getWorkspaceState } from '../../../core/server/utils'; describe('workspace utils', () => { const mockAuth = httpServiceMock.createAuth(); @@ -73,4 +74,34 @@ describe('workspace utils', () => { 'UNEXPECTED_AUTHORIZATION_STATUS' ); }); + + it('should be dashboard admin when users match configUsers', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const groups: string[] = ['dashboard_admin']; + const users: string[] = []; + const configGroups: string[] = ['dashboard_admin']; + const configUsers: string[] = []; + isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers); + expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(true); + }); + + it('should be dashboard admin when groups match configGroups', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const groups: string[] = []; + const users: string[] = ['dashboard_admin']; + const configGroups: string[] = []; + const configUsers: string[] = ['dashboard_admin']; + isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers); + expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(true); + }); + + it('should be not dashboard admin when groups do not match configGroups', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const groups: string[] = ['dashboard_admin']; + const users: string[] = []; + const configGroups: string[] = []; + const configUsers: string[] = ['dashboard_admin']; + isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers); + expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false); + }); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 1c8d73953afa..449a14ba2967 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -12,6 +12,7 @@ import { PrincipalType, } from '../../../core/server'; import { AuthInfo } from './types'; +import { updateWorkspaceState } from '../../../core/server/utils'; /** * Generate URL friendly random ID @@ -50,3 +51,23 @@ export const getPrincipalsFromRequest = ( throw new Error('UNEXPECTED_AUTHORIZATION_STATUS'); }; + +export const isRequestByDashboardAdmin = ( + request: OpenSearchDashboardsRequest, + groups: string[], + users: string[], + configGroups: string[], + configUsers: string[] +) => { + if (configGroups.length === 0 && configUsers.length === 0) { + updateWorkspaceState(request, { + isDashboardAdmin: false, + }); + return; + } + const groupMatchAny = groups.some((group) => configGroups.includes(group)) || false; + const userMatchAny = users.some((user) => configUsers.includes(user)) || false; + updateWorkspaceState(request, { + isDashboardAdmin: groupMatchAny || userMatchAny, + }); +}; From dc779998be585ace0d03aef3e0f4a7cf9c104148 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 3 Apr 2024 13:14:32 +0800 Subject: [PATCH 11/21] optimize code according to comments Signed-off-by: yubonluo --- src/core/server/utils/workspace.test.ts | 2 ++ src/plugins/workspace/server/plugin.ts | 11 +++++------ src/plugins/workspace/server/utils.test.ts | 12 ++++++++---- src/plugins/workspace/server/utils.ts | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 49382cfac38f..79a538892bd5 100644 --- a/src/core/server/utils/workspace.test.ts +++ b/src/core/server/utils/workspace.test.ts @@ -11,9 +11,11 @@ describe('updateWorkspaceState', () => { const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(requestMock, { id: 'foo', + isDashboardAdmin: true, }); expect(getWorkspaceState(requestMock)).toEqual({ id: 'foo', + isDashboardAdmin: true, }); }); }); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index f4db056cd593..72fb6d41806a 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,7 +11,6 @@ import { Plugin, Logger, CoreStart, - OpenSearchDashboardsRequest, } from '../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, @@ -32,7 +31,7 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; -import { isRequestByDashboardAdmin } from './utils'; +import { updateDashboardAdminStateForRequest } from './utils'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -63,7 +62,6 @@ export class WorkspacePlugin implements Plugin<{}, {}> { private async setupPermission( core: CoreSetup, - config: WorkspacePluginConfigType, { applicationConfig }: AppPluginSetupDependencies ) { this.permissionControl = new SavedObjectsPermissionControl(this.logger); @@ -100,7 +98,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { .catch(() => undefined), ]); - isRequestByDashboardAdmin( + updateDashboardAdminStateForRequest( request, groups, users, @@ -110,9 +108,10 @@ export class WorkspacePlugin implements Plugin<{}, {}> { return toolkit.next(); } + const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise(); const configGroups = config.dashboardAdmin.groups || []; const configUsers = config.dashboardAdmin.users || []; - isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers); + updateDashboardAdminStateForRequest(request, groups, users, configGroups, configUsers); return toolkit.next(); }); @@ -152,7 +151,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled); - if (isPermissionControlEnabled) this.setupPermission(core, config, { applicationConfig }); + if (isPermissionControlEnabled) await this.setupPermission(core, { applicationConfig }); registerRoutes({ http: core.http, diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index bb367af57e79..d38f52d469a0 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -5,7 +5,11 @@ import { AuthStatus } from '../../../core/server'; import { httpServerMock, httpServiceMock } from '../../../core/server/mocks'; -import { generateRandomId, getPrincipalsFromRequest, isRequestByDashboardAdmin } from './utils'; +import { + generateRandomId, + getPrincipalsFromRequest, + updateDashboardAdminStateForRequest, +} from './utils'; import { getWorkspaceState } from '../../../core/server/utils'; describe('workspace utils', () => { @@ -81,7 +85,7 @@ describe('workspace utils', () => { const users: string[] = []; const configGroups: string[] = ['dashboard_admin']; const configUsers: string[] = []; - isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers); + updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers); expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(true); }); @@ -91,7 +95,7 @@ describe('workspace utils', () => { const users: string[] = ['dashboard_admin']; const configGroups: string[] = []; const configUsers: string[] = ['dashboard_admin']; - isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers); + updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers); expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(true); }); @@ -101,7 +105,7 @@ describe('workspace utils', () => { const users: string[] = []; const configGroups: string[] = []; const configUsers: string[] = ['dashboard_admin']; - isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers); + updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers); expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false); }); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 449a14ba2967..7f36c296d1b1 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -52,7 +52,7 @@ export const getPrincipalsFromRequest = ( throw new Error('UNEXPECTED_AUTHORIZATION_STATUS'); }; -export const isRequestByDashboardAdmin = ( +export const updateDashboardAdminStateForRequest = ( request: OpenSearchDashboardsRequest, groups: string[], users: string[], From 71c20d95673306d983b88632a38e84dbc68d427d Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 3 Apr 2024 15:59:41 +0800 Subject: [PATCH 12/21] change dashboardAdmin yml config to openseachDashboard Signed-off-by: yubonluo --- src/core/server/mocks.ts | 1 + src/core/server/opensearch_dashboards_config.ts | 8 ++++++++ src/core/server/plugins/plugin_context.test.ts | 1 + src/core/server/plugins/types.ts | 1 + src/legacy/server/config/schema.js | 4 ++++ src/plugins/workspace/config.ts | 8 -------- src/plugins/workspace/server/plugin.ts | 11 ++++++++--- 7 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 687d408e40a6..525235b79b25 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -80,6 +80,7 @@ export function pluginInitializerContextConfigMock(config: T) { configIndex: '.opensearch_dashboards_config_tests', autocompleteTerminateAfter: duration(100000), autocompleteTimeout: duration(1000), + dashboardAdmin: { groups: [], users: [] }, }, opensearch: { shardTimeout: duration('30s'), diff --git a/src/core/server/opensearch_dashboards_config.ts b/src/core/server/opensearch_dashboards_config.ts index 47fa8a126501..b823d4f83e2d 100644 --- a/src/core/server/opensearch_dashboards_config.ts +++ b/src/core/server/opensearch_dashboards_config.ts @@ -91,6 +91,14 @@ export const config = { defaultValue: 'https://survey.opensearch.org', }), }), + dashboardAdmin: schema.object({ + groups: schema.arrayOf(schema.string(), { + defaultValue: [], + }), + users: schema.arrayOf(schema.string(), { + defaultValue: [], + }), + }), }), deprecations, }; diff --git a/src/core/server/plugins/plugin_context.test.ts b/src/core/server/plugins/plugin_context.test.ts index 7a8ba042825b..24bbd372b194 100644 --- a/src/core/server/plugins/plugin_context.test.ts +++ b/src/core/server/plugins/plugin_context.test.ts @@ -101,6 +101,7 @@ describe('createPluginInitializerContext', () => { configIndex: '.opensearch_dashboards_config', autocompleteTerminateAfter: duration(100000), autocompleteTimeout: duration(1000), + dashboardAdmin: { groups: [], users: [] }, }, opensearch: { shardTimeout: duration(30, 's'), diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts index 59b9881279c3..8a02e2ea706d 100644 --- a/src/core/server/plugins/types.ts +++ b/src/core/server/plugins/types.ts @@ -292,6 +292,7 @@ export const SharedGlobalConfigKeys = { 'configIndex', 'autocompleteTerminateAfter', 'autocompleteTimeout', + 'dashboardAdmin', ] as const, opensearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const, path: ['data'] as const, diff --git a/src/legacy/server/config/schema.js b/src/legacy/server/config/schema.js index a102268effca..84d457f06ca4 100644 --- a/src/legacy/server/config/schema.js +++ b/src/legacy/server/config/schema.js @@ -251,6 +251,10 @@ export default () => survey: Joi.object({ url: Joi.any().default('/'), }), + dashboardAdmin: Joi.object({ + groups: Joi.array().items(Joi.string()).default([]), + users: Joi.array().items(Joi.string()).default([]), + }), }).default(), savedObjects: HANDLED_IN_NEW_PLATFORM, diff --git a/src/plugins/workspace/config.ts b/src/plugins/workspace/config.ts index c5cf89f97249..70c87ac00cfc 100644 --- a/src/plugins/workspace/config.ts +++ b/src/plugins/workspace/config.ts @@ -10,14 +10,6 @@ export const configSchema = schema.object({ permission: schema.object({ enabled: schema.boolean({ defaultValue: true }), }), - dashboardAdmin: schema.object({ - groups: schema.arrayOf(schema.string(), { - defaultValue: [], - }), - users: schema.arrayOf(schema.string(), { - defaultValue: [], - }), - }), }); export type WorkspacePluginConfigType = TypeOf; diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 72fb6d41806a..2d23a4067e8b 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,6 +11,7 @@ import { Plugin, Logger, CoreStart, + SharedGlobalConfig, } from '../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, @@ -39,6 +40,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; private permissionControl?: SavedObjectsPermissionControlContract; private readonly config$: Observable; + private readonly globalConfig$: Observable; private workspaceSavedObjectsClientWrapper?: WorkspaceSavedObjectsClientWrapper; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { @@ -108,9 +110,11 @@ export class WorkspacePlugin implements Plugin<{}, {}> { return toolkit.next(); } - const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise(); - const configGroups = config.dashboardAdmin.groups || []; - const configUsers = config.dashboardAdmin.users || []; + const globalConfig: SharedGlobalConfig = await this.globalConfig$.pipe(first()).toPromise(); + const configGroups = (globalConfig.opensearchDashboards.dashboardAdmin.groups || + []) as string[]; + const configUsers = (globalConfig.opensearchDashboards.dashboardAdmin.users || + []) as string[]; updateDashboardAdminStateForRequest(request, groups, users, configGroups, configUsers); return toolkit.next(); }); @@ -129,6 +133,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); this.config$ = initializerContext.config.create(); + this.globalConfig$ = initializerContext.config.legacy.globalConfig$; } public async setup(core: CoreSetup, { applicationConfig }: AppPluginSetupDependencies) { From c61dac979084ced1d435d1710b76262e2f51573e Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 3 Apr 2024 16:05:42 +0800 Subject: [PATCH 13/21] add missed code Signed-off-by: yubonluo --- config/opensearch_dashboards.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index e99f7b77690a..46a0c018c6ea 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -292,6 +292,6 @@ # workspace.enabled: false # Set the backend roles in groups or users, whoever has the backend roles or exactly match the user ids defined in this config will be regard as dashboard admin. -# Dashboard admin will have the access to all the workspaces and objects inside OpenSearch Dashboards. -# workspace.dashboardAdmin.groups: ["dashboard_admin"] -# workspace.dashboardAdmin.users: ["dashboard_admin"] \ No newline at end of file +# Dashboard admin will have the access to all the workspaces(workspace.enabled: true) and objects inside OpenSearch Dashboards. +# opensearchDashboards.dashboardAdmin.groups: ["dashboard_admin"] +# opensearchDashboards.dashboardAdmin.users: ["dashboard_admin"] \ No newline at end of file From f0c1b5d33974df914ca1b74663482aab0fc11ae1 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Mon, 8 Apr 2024 14:28:22 +0800 Subject: [PATCH 14/21] optimize code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index fec3c379ef5d..3d41f958714e 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -96,10 +96,10 @@ export class WorkspacePlugin implements Plugin undefined), applicationConfigClient - .getEntityConfig('workspace.dashboardAdmin.users') + .getEntityConfig('opensearchDashboards.dashboardAdmin.users') .catch(() => undefined), ]); From a69cc587a9f43252ba58a2e9d6102181e602f8ca Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 9 Apr 2024 11:01:35 +0800 Subject: [PATCH 15/21] delete useless code Signed-off-by: yubonluo --- src/plugins/workspace/server/types.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index cac4dd3af9f4..c24cb49793d5 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -129,15 +129,6 @@ export interface AuthInfo { user_name?: string; } -export type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); - export interface AppPluginSetupDependencies { applicationConfig: ApplicationConfigPluginSetup; } From 7d83f48b19595f74e70a8555e24ddfa5422b4b7c Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 9 Apr 2024 11:03:12 +0800 Subject: [PATCH 16/21] delete useless code Signed-off-by: yubonluo --- src/plugins/workspace/server/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index c24cb49793d5..6a9009a06375 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -14,7 +14,6 @@ import { SavedObjectsServiceStart, } from '../../../core/server'; import { WorkspaceAttributeWithPermission } from '../../../core/types'; -import { WorkspacePermissionMode } from '../common/constants'; export interface WorkspaceFindOptions { page?: number; From f50093f4ae57259118416884844d461ff4d9f941 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Wed, 10 Apr 2024 17:35:11 +0800 Subject: [PATCH 17/21] optimize code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 39 ++++++---------------- src/plugins/workspace/server/utils.test.ts | 13 ++++++++ src/plugins/workspace/server/utils.ts | 14 ++++++++ 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 2c51ceab8e4f..6a0b9d07198a 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -37,7 +37,7 @@ import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; -import { updateDashboardAdminStateForRequest } from './utils'; +import { stringToArray, updateDashboardAdminStateForRequest } from './utils'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; export class WorkspacePlugin implements Plugin { @@ -70,15 +70,14 @@ export class WorkspacePlugin implements Plugin { let groups: string[]; let users: string[]; + let configGroups: string[]; + let configUsers: string[]; // There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated. try { @@ -86,20 +85,13 @@ export class WorkspacePlugin implements Plugin undefined), @@ -108,21 +100,12 @@ export class WorkspacePlugin implements Plugin undefined), ]); - updateDashboardAdminStateForRequest( - request, - groups, - users, - configGroups ? [configGroups] : [], - configUsers ? [configUsers] : [] - ); - return toolkit.next(); + [configGroups, configUsers] = [stringToArray(groupsResult), stringToArray(usersResult)]; + } else { + const globalConfig: SharedGlobalConfig = await this.globalConfig$.pipe(first()).toPromise(); + configGroups = (globalConfig.opensearchDashboards.dashboardAdmin.groups || []) as string[]; + configUsers = (globalConfig.opensearchDashboards.dashboardAdmin.users || []) as string[]; } - - const globalConfig: SharedGlobalConfig = await this.globalConfig$.pipe(first()).toPromise(); - const configGroups = (globalConfig.opensearchDashboards.dashboardAdmin.groups || - []) as string[]; - const configUsers = (globalConfig.opensearchDashboards.dashboardAdmin.users || - []) as string[]; updateDashboardAdminStateForRequest(request, groups, users, configGroups, configUsers); return toolkit.next(); }); @@ -168,7 +151,7 @@ export class WorkspacePlugin implements Plugin { updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers); expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false); }); + + it('should convert string to array', () => { + const jsonString = '["test1","test2"]'; + const strToArray = stringToArray(jsonString); + expect(strToArray).toStrictEqual(new Array('test1', 'test2')); + }); + + it('should convert string to a null array if input is invalid', () => { + const jsonString = '["test1", test2]'; + const strToArray = stringToArray(jsonString); + expect(strToArray).toStrictEqual([]); + }); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 7f36c296d1b1..b21cdadbfba2 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -10,6 +10,7 @@ import { OpenSearchDashboardsRequest, Principals, PrincipalType, + SharedGlobalConfig, } from '../../../core/server'; import { AuthInfo } from './types'; import { updateWorkspaceState } from '../../../core/server/utils'; @@ -71,3 +72,16 @@ export const updateDashboardAdminStateForRequest = ( isDashboardAdmin: groupMatchAny || userMatchAny, }); }; + +export const stringToArray = (adminConfig: string | undefined) => { + if (!adminConfig) { + return []; + } + let adminConfigArray; + try { + adminConfigArray = JSON.parse(adminConfig); + } catch (e) { + return []; + } + return adminConfigArray; +}; From e9fd21a925f8dd3ddd9d787776eeedd6163df7b0 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Thu, 11 Apr 2024 11:29:50 +0800 Subject: [PATCH 18/21] delete useless code Signed-off-by: yubonluo --- src/plugins/workspace/server/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index b21cdadbfba2..633fa9f5e254 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -10,7 +10,6 @@ import { OpenSearchDashboardsRequest, Principals, PrincipalType, - SharedGlobalConfig, } from '../../../core/server'; import { AuthInfo } from './types'; import { updateWorkspaceState } from '../../../core/server/utils'; From 909af1cd6277a9372f04f49f2250f34ac41ee2c1 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Thu, 11 Apr 2024 14:39:42 +0800 Subject: [PATCH 19/21] add utils unit tests Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 26 +++---- src/plugins/workspace/server/utils.test.ts | 90 +++++++++++++++++++++- src/plugins/workspace/server/utils.ts | 33 +++++++- 3 files changed, 131 insertions(+), 18 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 6a0b9d07198a..a614a59611f4 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -37,7 +37,11 @@ import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; -import { stringToArray, updateDashboardAdminStateForRequest } from './utils'; +import { + getApplicationOSDAdminConfig, + getOSDAdminConfig, + updateDashboardAdminStateForRequest, +} from './utils'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; export class WorkspacePlugin implements Plugin { @@ -89,22 +93,12 @@ export class WorkspacePlugin implements Plugin undefined), - applicationConfigClient - .getEntityConfig('opensearchDashboards.dashboardAdmin.users') - .catch(() => undefined), - ]); - - [configGroups, configUsers] = [stringToArray(groupsResult), stringToArray(usersResult)]; + [configGroups, configUsers] = await getApplicationOSDAdminConfig( + { applicationConfig }, + scopeClient + ); } else { - const globalConfig: SharedGlobalConfig = await this.globalConfig$.pipe(first()).toPromise(); - configGroups = (globalConfig.opensearchDashboards.dashboardAdmin.groups || []) as string[]; - configUsers = (globalConfig.opensearchDashboards.dashboardAdmin.users || []) as string[]; + [configGroups, configUsers] = await getOSDAdminConfig(this.globalConfig$); } updateDashboardAdminStateForRequest(request, groups, users, configGroups, configUsers); return toolkit.next(); diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 844e4ca90db2..5f8251224abb 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -4,14 +4,19 @@ */ import { AuthStatus } from '../../../core/server'; -import { httpServerMock, httpServiceMock } from '../../../core/server/mocks'; +import { coreMock, httpServerMock, httpServiceMock } from '../../../core/server/mocks'; import { generateRandomId, + getApplicationOSDAdminConfig, + getOSDAdminConfig, getPrincipalsFromRequest, stringToArray, updateDashboardAdminStateForRequest, } from './utils'; import { getWorkspaceState } from '../../../core/server/utils'; +import { AppPluginSetupDependencies } from './types'; +import { Observable, of } from 'rxjs'; +import { error } from 'console'; describe('workspace utils', () => { const mockAuth = httpServiceMock.createAuth(); @@ -110,6 +115,16 @@ describe('workspace utils', () => { expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false); }); + it('should be not dashboard admin when groups and users are []', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const groups: string[] = []; + const users: string[] = []; + const configGroups: string[] = []; + const configUsers: string[] = []; + updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers); + expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false); + }); + it('should convert string to array', () => { const jsonString = '["test1","test2"]'; const strToArray = stringToArray(jsonString); @@ -121,4 +136,77 @@ describe('workspace utils', () => { const strToArray = stringToArray(jsonString); expect(strToArray).toStrictEqual([]); }); + + it('should get correct OSD admin config when application config is enabled', async () => { + const applicationConfigMock = { + getConfigurationClient: jest.fn().mockReturnValue({ + getEntityConfig: jest.fn().mockImplementation(async (entity: string) => { + if (entity === 'opensearchDashboards.dashboardAdmin.groups') { + return '["group1", "group2"]'; + } else if (entity === 'opensearchDashboards.dashboardAdmin.users') { + return '["user1", "user2"]'; + } else { + return undefined; + } + }), + }), + registerConfigurationClient: jest.fn().mockResolvedValue({}), + }; + + const mockDependencies: AppPluginSetupDependencies = { + applicationConfig: applicationConfigMock, + }; + const coreStart = coreMock.createInternalStart(); + const scopedClusterClientMock = coreStart.opensearch.client.asScoped(); + const [groups, users] = await getApplicationOSDAdminConfig( + mockDependencies, + scopedClusterClientMock + ); + expect(groups).toEqual(['group1', 'group2']); + expect(users).toEqual(['user1', 'user2']); + }); + + it('should get [] when application config is enabled and not defined ', async () => { + const applicationConfigMock = { + getConfigurationClient: jest.fn().mockReturnValue({ + getEntityConfig: jest.fn().mockImplementation(async (entity: string) => { + throw error; + }), + }), + registerConfigurationClient: jest.fn().mockResolvedValue({}), + }; + + const mockDependencies: AppPluginSetupDependencies = { + applicationConfig: applicationConfigMock, + }; + const coreStart = coreMock.createInternalStart(); + const scopedClusterClientMock = coreStart.opensearch.client.asScoped(); + const [groups, users] = await getApplicationOSDAdminConfig( + mockDependencies, + scopedClusterClientMock + ); + expect(groups).toEqual([]); + expect(users).toEqual([]); + }); + + it('should get correct admin config when admin config is enabled ', async () => { + const globalConfig$: Observable = of({ + opensearchDashboards: { + dashboardAdmin: { + groups: ['group1', 'group2'], + users: ['user1', 'user2'], + }, + }, + }); + const [groups, users] = await getOSDAdminConfig(globalConfig$); + expect(groups).toEqual(['group1', 'group2']); + expect(users).toEqual(['user1', 'user2']); + }); + + it('should get [] when admin config is not enabled', async () => { + const globalConfig$: Observable = of({}); + const [groups, users] = await getOSDAdminConfig(globalConfig$); + expect(groups).toEqual([]); + expect(users).toEqual([]); + }); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 633fa9f5e254..a262f1914126 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -4,14 +4,18 @@ */ import crypto from 'crypto'; +import { Observable } from 'rxjs'; +import { first } from 'rxjs/operators'; import { AuthStatus, HttpAuth, + IScopedClusterClient, OpenSearchDashboardsRequest, Principals, PrincipalType, + SharedGlobalConfig, } from '../../../core/server'; -import { AuthInfo } from './types'; +import { AppPluginSetupDependencies, AuthInfo } from './types'; import { updateWorkspaceState } from '../../../core/server/utils'; /** @@ -84,3 +88,30 @@ export const stringToArray = (adminConfig: string | undefined) => { } return adminConfigArray; }; + +export const getApplicationOSDAdminConfig = async ( + { applicationConfig }: AppPluginSetupDependencies, + scopeClient: IScopedClusterClient +) => { + const applicationConfigClient = applicationConfig.getConfigurationClient(scopeClient); + + const [groupsResult, usersResult] = await Promise.all([ + applicationConfigClient + .getEntityConfig('opensearchDashboards.dashboardAdmin.groups') + .catch(() => undefined), + applicationConfigClient + .getEntityConfig('opensearchDashboards.dashboardAdmin.users') + .catch(() => undefined), + ]); + + return [stringToArray(groupsResult), stringToArray(usersResult)]; +}; + +export const getOSDAdminConfig = async (globalConfig$: Observable) => { + const globalConfig = await globalConfig$.pipe(first()).toPromise(); + const groupsResult = (globalConfig.opensearchDashboards?.dashboardAdmin?.groups || + []) as string[]; + const usersResult = (globalConfig.opensearchDashboards?.dashboardAdmin?.users || []) as string[]; + + return [groupsResult, usersResult]; +}; From 27d95fe08f778f77f6247838811c0591c1b0e6f2 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Thu, 11 Apr 2024 16:45:01 +0800 Subject: [PATCH 20/21] optimize code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 4 +--- src/plugins/workspace/server/utils.test.ts | 18 +++++------------- src/plugins/workspace/server/utils.ts | 5 ++--- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index a614a59611f4..9f36314f92d6 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -91,11 +91,9 @@ export class WorkspacePlugin implements Plugin { const mockDependencies: AppPluginSetupDependencies = { applicationConfig: applicationConfigMock, }; - const coreStart = coreMock.createInternalStart(); - const scopedClusterClientMock = coreStart.opensearch.client.asScoped(); - const [groups, users] = await getApplicationOSDAdminConfig( - mockDependencies, - scopedClusterClientMock - ); + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const [groups, users] = await getApplicationOSDAdminConfig(mockDependencies, mockRequest); expect(groups).toEqual(['group1', 'group2']); expect(users).toEqual(['user1', 'user2']); }); @@ -179,12 +175,8 @@ describe('workspace utils', () => { const mockDependencies: AppPluginSetupDependencies = { applicationConfig: applicationConfigMock, }; - const coreStart = coreMock.createInternalStart(); - const scopedClusterClientMock = coreStart.opensearch.client.asScoped(); - const [groups, users] = await getApplicationOSDAdminConfig( - mockDependencies, - scopedClusterClientMock - ); + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const [groups, users] = await getApplicationOSDAdminConfig(mockDependencies, mockRequest); expect(groups).toEqual([]); expect(users).toEqual([]); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index a262f1914126..79fcc60aad5d 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -9,7 +9,6 @@ import { first } from 'rxjs/operators'; import { AuthStatus, HttpAuth, - IScopedClusterClient, OpenSearchDashboardsRequest, Principals, PrincipalType, @@ -91,9 +90,9 @@ export const stringToArray = (adminConfig: string | undefined) => { export const getApplicationOSDAdminConfig = async ( { applicationConfig }: AppPluginSetupDependencies, - scopeClient: IScopedClusterClient + request: OpenSearchDashboardsRequest ) => { - const applicationConfigClient = applicationConfig.getConfigurationClient(scopeClient); + const applicationConfigClient = applicationConfig.getConfigurationClient(request); const [groupsResult, usersResult] = await Promise.all([ applicationConfigClient From acfb16661c134fc7fb690dcb1e9bf3ace694348f Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 12 Apr 2024 12:35:04 +0800 Subject: [PATCH 21/21] Fix the wrong reference Signed-off-by: yubonluo --- src/plugins/workspace/server/utils.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 44020a446f87..639dfb3963e5 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -16,7 +16,6 @@ import { import { getWorkspaceState } from '../../../core/server/utils'; import { AppPluginSetupDependencies } from './types'; import { Observable, of } from 'rxjs'; -import { error } from 'console'; describe('workspace utils', () => { const mockAuth = httpServiceMock.createAuth(); @@ -166,7 +165,7 @@ describe('workspace utils', () => { const applicationConfigMock = { getConfigurationClient: jest.fn().mockReturnValue({ getEntityConfig: jest.fn().mockImplementation(async (entity: string) => { - throw error; + throw new Error('Not found'); }), }), registerConfigurationClient: jest.fn().mockResolvedValue({}),