From 9488790fc5dc1de77f65cb2925bcca548fa6d6a6 Mon Sep 17 00:00:00 2001 From: yuboluo <15242088755@163.com> Date: Fri, 12 Apr 2024 15:05:31 +0800 Subject: [PATCH 01/15] [Workspace] dashboard admin(groups/users) implementation and integrating with dynamic application config Signed-off-by: yubonluo --- config/opensearch_dashboards.yml | 5 + src/core/server/mocks.ts | 1 + .../server/opensearch_dashboards_config.ts | 8 + .../server/plugins/plugin_context.test.ts | 1 + src/core/server/plugins/types.ts | 1 + src/core/server/utils/workspace.test.ts | 2 + src/core/server/utils/workspace.ts | 4 +- src/legacy/server/config/schema.js | 4 + .../workspace/opensearch_dashboards.json | 2 +- src/plugins/workspace/server/plugin.test.ts | 14 +- src/plugins/workspace/server/plugin.ts | 67 +++++-- ...space_saved_objects_client_wrapper.test.ts | 188 ++++++++++++++++++ ...space_saved_objects_client_wrapper.test.ts | 162 ++++++++++++++- .../workspace_saved_objects_client_wrapper.ts | 6 + src/plugins/workspace/server/types.ts | 4 + src/plugins/workspace/server/utils.test.ts | 129 +++++++++++- src/plugins/workspace/server/utils.ts | 66 +++++- 17 files changed, 639 insertions(+), 25 deletions(-) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 40d643b014fd..69beaccf8aba 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -313,3 +313,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 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(workspace.enabled: true) and objects inside OpenSearch Dashboards. +# opensearchDashboards.dashboardAdmin.groups: ["dashboard_admin"] +# opensearchDashboards.dashboardAdmin.users: ["dashboard_admin"] \ No newline at end of file diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index dce39d03da7f..87868148e0e8 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 57aa372514de..ac793967d96b 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 c225a24aa386..e9c7591f6c56 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/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 7dfcff9e5d18..19f8bad4f866 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, { requestWorkspaceId: 'foo', + isDashboardAdmin: true, }); expect(getWorkspaceState(requestMock)).toEqual({ requestWorkspaceId: 'foo', + isDashboardAdmin: true, }); }); }); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index 2003e615d501..89f2b7975964 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -7,6 +7,7 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; export interface WorkspaceState { requestWorkspaceId?: string; + isDashboardAdmin?: boolean; } /** @@ -29,8 +30,9 @@ export const updateWorkspaceState = ( }; export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => { - const { requestWorkspaceId } = ensureRawRequest(request).app as WorkspaceState; + const { requestWorkspaceId, isDashboardAdmin } = ensureRawRequest(request).app as WorkspaceState; return { requestWorkspaceId, + isDashboardAdmin, }; }; 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/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index e6ff1e2d0173..c0a31c59169e 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -7,6 +7,6 @@ "savedObjects", "opensearchDashboardsReact" ], - "optionalPlugins": ["savedObjectsManagement"], + "optionalPlugins": ["savedObjectsManagement", "applicationConfig"], "requiredBundles": ["opensearchDashboardsReact"] } diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index 0ad72b51b6dc..f3f13fa27513 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -6,9 +6,17 @@ import { OnPreRoutingHandler } from 'src/core/server'; import { coreMock, httpServerMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; +import { AppPluginSetupDependencies } from './types'; import { getWorkspaceState } from '../../../core/server/utils'; describe('Workspace server plugin', () => { + const mockApplicationConfig = { + getConfigurationClient: jest.fn().mockResolvedValue({}), + registerConfigurationClient: jest.fn().mockResolvedValue({}), + }; + const mockDependencies: AppPluginSetupDependencies = { + applicationConfig: mockApplicationConfig, + }; it('#setup', async () => { let value; const setupMock = coreMock.createSetup(); @@ -17,7 +25,7 @@ describe('Workspace server plugin', () => { }); 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 { @@ -43,7 +51,7 @@ describe('Workspace server plugin', () => { return fn; }); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); - await workspacePlugin.setup(setupMock); + await workspacePlugin.setup(setupMock, mockDependencies); const toolKitMock = httpServerMock.createToolkit(); const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ @@ -78,7 +86,7 @@ describe('Workspace server plugin', () => { }); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); - await workspacePlugin.setup(setupMock); + await workspacePlugin.setup(setupMock, mockDependencies); await workspacePlugin.start(startMock); expect(startMock.savedObjects.createSerializer).toBeCalledTimes(1); }); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index db0921483eb4..d2d670a1302f 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -21,7 +21,12 @@ import { PRIORITY_FOR_WORKSPACE_ID_CONSUMER_WRAPPER, PRIORITY_FOR_PERMISSION_CONTROL_WRAPPER, } from '../common/constants'; -import { IWorkspaceClientImpl, WorkspacePluginSetup, WorkspacePluginStart } from './types'; +import { + IWorkspaceClientImpl, + WorkspacePluginSetup, + WorkspacePluginStart, + AppPluginSetupDependencies, +} from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; @@ -35,6 +40,11 @@ import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; +import { + getApplicationOSDAdminConfig, + getOSDAdminConfig, + updateDashboardAdminStateForRequest, +} from './utils'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; export class WorkspacePlugin implements Plugin { @@ -67,12 +77,51 @@ 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 { + ({ groups = [], users = [] } = this.permissionControl!.getPrincipalsFromRequest(request)); + } catch (e) { + return toolkit.next(); + } + + if (!!applicationConfig) { + [configGroups, configUsers] = await getApplicationOSDAdminConfig( + { applicationConfig }, + request + ); + } else { + [configGroups, configUsers] = await getOSDAdminConfig(this.globalConfig$); + } + updateDashboardAdminStateForRequest(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.globalConfig$ = initializerContext.config.legacy.globalConfig$; } - public async setup(core: CoreSetup) { + public async setup(core: CoreSetup, { applicationConfig }: AppPluginSetupDependencies) { this.logger.debug('Setting up Workspaces service'); const globalConfig = await this.globalConfig$.pipe(first()).toPromise(); const isPermissionControlEnabled = globalConfig.savedObjects.permission.enabled === true; @@ -98,19 +147,7 @@ export class WorkspacePlugin implements Plugin { const savedObjects: Array<{ type: string; id: string }> = []; @@ -51,6 +52,7 @@ const repositoryKit = (() => { const permittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); const notPermittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); +const dashboardAdminRequest = httpServerMock.createOpenSearchDashboardsRequest(); describe('WorkspaceSavedObjectsClientWrapper', () => { let internalSavedObjectsRepository: ISavedObjectsRepository; @@ -59,6 +61,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { let osd: TestOpenSearchDashboardsUtils; let permittedSavedObjectedClient: SavedObjectsClientContract; let notPermittedSavedObjectedClient: SavedObjectsClientContract; + let dashboardAdminSavedObjectedClient: SavedObjectsClientContract; beforeAll(async function () { servers = createTestServers({ @@ -133,6 +136,10 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { notPermittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( notPermittedRequest ); + updateWorkspaceState(dashboardAdminRequest, { isDashboardAdmin: true }); + dashboardAdminSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( + dashboardAdminRequest + ); }); afterAll(async () => { @@ -172,6 +179,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { (await permittedSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2')).error ).toBeUndefined(); }); + + it('should return consistent dashboard when groups/users 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 +233,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).saved_objects.length ).toEqual(1); }); + + it('should return consistent dashboard when groups/users 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 +281,19 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { true ); }); + + 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, + page: 1, + }); + + expect(result.saved_objects.some((item) => item.id === 'inner-workspace-dashboard-1')).toBe( + true + ); + }); }); describe('create', () => { @@ -278,6 +326,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', createResult.id); }); + 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', + {}, + { + 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 +369,20 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.error).toBeUndefined(); }); + + it('should able to create with override when groups/users 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 +411,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', objectId); }); + 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 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 +463,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.saved_objects).toHaveLength(1); }); + + 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', + type: 'dashboard', + attributes: {}, + }, + ], + { + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.saved_objects).toHaveLength(1); + }); }); describe('update', () => { @@ -414,6 +518,27 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { .error ).toBeUndefined(); }); + + it('should update saved objects for any workspaces when groups/users 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 +584,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ).saved_objects.length ).toEqual(1); }); + + it('should bulk update saved objects for any workspaces when groups/users 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,6 +668,52 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); + + it('should be able to delete any data when groups/users is dashboard admin', async () => { + const createPermittedResult = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + permissions: { + read: { users: ['foo'] }, + write: { users: ['foo'] }, + }, + } + ); + + await dashboardAdminSavedObjectedClient.delete('dashboard', createPermittedResult.id); + + let permittedError; + try { + permittedError = await dashboardAdminSavedObjectedClient.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 dashboardAdminSavedObjectedClient.delete('dashboard', createACLResult.id); + + let ACLError; + try { + ACLError = await dashboardAdminSavedObjectedClient.get('dashboard', createACLResult.id); + } catch (e) { + ACLError = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(ACLError)).toBe(true); + }); }); describe('deleteByWorkspace', () => { 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 186ecda0d8ba..e98189cbb92d 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,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +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 generateWorkspaceSavedObjectsClientWrapper = () => { +const DASHBOARD_ADMIN = 'dashnoard_admin'; +const NO_DASHBOARD_ADMIN = 'no_dashnoard_admin'; + +const generateWorkspaceSavedObjectsClientWrapper = (role = NO_DASHBOARD_ADMIN) => { const savedObjectsStore = [ { type: 'dashboard', @@ -75,7 +80,8 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { 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, @@ -91,8 +97,11 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }), validateSavedObjectsACL: jest.fn(), batchValidate: jest.fn(), - getPrincipalsFromRequest: jest.fn().mockImplementation(() => ({ users: ['user-1'] })), + getPrincipalsFromRequest: jest.fn().mockImplementation(() => { + return { users: ['user-1'] }; + }), }; + const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); const scopedClientMock = { find: jest.fn().mockImplementation(async () => ({ @@ -152,6 +161,21 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.delete(...deleteArgs); expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); }); + it('should call client.delete if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + 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(); + expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); + }); }); describe('update', () => { @@ -206,6 +230,23 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.update(...updateArgs); expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); }); + it('should call client.update if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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); + }); }); describe('bulk update', () => { @@ -241,6 +282,19 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.bulkUpdate(objectsToUpdate, {}); expect(clientMock.bulkUpdate).toHaveBeenCalledWith(objectsToUpdate, {}); }); + it('should call client.bulkUpdate if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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); + }); }); describe('bulk create', () => { @@ -343,6 +397,25 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); + it('should call client.bulkCreate if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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'], + }); + }); }); describe('create', () => { @@ -417,6 +490,30 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); }); + it('should call client.create if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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, + } + ); + }); }); describe('get', () => { it('should return saved object if no need to validate permission', async () => { @@ -478,6 +575,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(clientMock.get).toHaveBeenCalledWith(...getArgs); expect(result).toMatchInlineSnapshot(`[Error: Not Found]`); }); + it('should call client.get and return result with arguments if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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'); + }); }); describe('bulk get', () => { it("should call permission validate with object's workspace and throw permission error", async () => { @@ -543,6 +652,27 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { {} ); }); + it('should call client.bulkGet and return result with arguments if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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); + }); }); describe('find', () => { it('should call client.find with ACLSearchParams for workspace type', async () => { @@ -634,6 +764,22 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }, }); }); + it('should call client.find with arguments if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(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(); + }); }); describe('deleteByWorkspace', () => { @@ -663,6 +809,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await wrapper.deleteByWorkspace('workspace-1', {}); expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-1', {}); }); + it('should call client.deleteByWorkspace if user/groups is dashboard admin', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = 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 4d5d03641b5f..3101b74598bb 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 @@ -5,6 +5,7 @@ import { i18n } from '@osd/i18n'; +import { getWorkspaceState } from '../../../../core/server/utils'; import { OpenSearchDashboardsRequest, SavedObject, @@ -519,6 +520,11 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.deleteByWorkspace(workspace, options); }; + const isDashboardAdmin = getWorkspaceState(wrapperOptions.request)?.isDashboardAdmin; + if (isDashboardAdmin) { + return wrapperOptions.client; + } + return { ...wrapperOptions.client, get: getWithWorkspacePermissionControl, diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index 2973ea4dbc31..e62193fab2b6 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, @@ -128,6 +129,9 @@ export interface AuthInfo { user_name?: string; } +export interface AppPluginSetupDependencies { + applicationConfig: ApplicationConfigPluginSetup; +} export interface WorkspacePluginSetup { client: IWorkspaceClientImpl; } diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 1f6c3e58f122..639dfb3963e5 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -5,7 +5,17 @@ import { AuthStatus } from '../../../core/server'; import { httpServerMock, httpServiceMock } from '../../../core/server/mocks'; -import { generateRandomId, getPrincipalsFromRequest } from './utils'; +import { + generateRandomId, + getApplicationOSDAdminConfig, + getOSDAdminConfig, + getPrincipalsFromRequest, + stringToArray, + updateDashboardAdminStateForRequest, +} from './utils'; +import { getWorkspaceState } from '../../../core/server/utils'; +import { AppPluginSetupDependencies } from './types'; +import { Observable, of } from 'rxjs'; describe('workspace utils', () => { const mockAuth = httpServiceMock.createAuth(); @@ -73,4 +83,121 @@ 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[] = []; + updateDashboardAdminStateForRequest(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']; + updateDashboardAdminStateForRequest(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']; + updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers); + 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); + 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([]); + }); + + 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 mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const [groups, users] = await getApplicationOSDAdminConfig(mockDependencies, mockRequest); + 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 new Error('Not found'); + }), + }), + registerConfigurationClient: jest.fn().mockResolvedValue({}), + }; + + const mockDependencies: AppPluginSetupDependencies = { + applicationConfig: applicationConfigMock, + }; + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const [groups, users] = await getApplicationOSDAdminConfig(mockDependencies, mockRequest); + 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 1c8d73953afa..79fcc60aad5d 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, OpenSearchDashboardsRequest, Principals, PrincipalType, + SharedGlobalConfig, } from '../../../core/server'; -import { AuthInfo } from './types'; +import { AppPluginSetupDependencies, AuthInfo } from './types'; +import { updateWorkspaceState } from '../../../core/server/utils'; /** * Generate URL friendly random ID @@ -50,3 +54,63 @@ export const getPrincipalsFromRequest = ( throw new Error('UNEXPECTED_AUTHORIZATION_STATUS'); }; + +export const updateDashboardAdminStateForRequest = ( + 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, + }); +}; + +export const stringToArray = (adminConfig: string | undefined) => { + if (!adminConfig) { + return []; + } + let adminConfigArray; + try { + adminConfigArray = JSON.parse(adminConfig); + } catch (e) { + return []; + } + return adminConfigArray; +}; + +export const getApplicationOSDAdminConfig = async ( + { applicationConfig }: AppPluginSetupDependencies, + request: OpenSearchDashboardsRequest +) => { + const applicationConfigClient = applicationConfig.getConfigurationClient(request); + + 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 4209c2ae3b6463ead434b923b49e1809c4effb1f Mon Sep 17 00:00:00 2001 From: yubonluo Date: Mon, 15 Apr 2024 15:35:24 +0800 Subject: [PATCH 02/15] Modify change log Signed-off-by: yubonluo --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d4ec919e3d..94d40ffe95d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Dynamic Configurations] Improve dynamic configurations by adding cache and simplifying client fetch ([#6364](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6364)) - [MD] Add OpenSearch cluster group label to top of single selectable dropdown ([#6400](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6400)) - [Workspace] Support workspace in saved objects client in server side. ([#6365](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6365)) +- [Workspace] Dashboard admin(groups/users) implementation. ([#6454](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6454)) ### 🐛 Bug Fixes From 87c4288a5f3b7c873126d36df4aa7b0d8ccf2d85 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 19 Apr 2024 14:32:19 +0800 Subject: [PATCH 03/15] optimize the code Signed-off-by: yubonluo --- ...space_saved_objects_client_wrapper.test.ts | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) 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 819e2cf3c2cb..432760aa692a 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 @@ -344,18 +344,6 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { await permittedSavedObjectedClient.delete('dashboard', objectId); }); - 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 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 { @@ -624,7 +612,7 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { .error ).toBeUndefined(); }); - it('should return consistent dashboard after bulkget called', async () => { + it('should return consistent dashboard after bulkGet called', async () => { expect( ( await dashboardAdminSavedObjectedClient.bulkGet([ @@ -693,6 +681,17 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.saved_objects).toHaveLength(1); }); + it('should able to create saved objects into any workspaces after bulkCreate called', 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 update saved objects for any workspaces after update called', async () => { expect( ( From c4cd56ab89a18c834c260a416371df80c44f7a96 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 19 Apr 2024 15:32:15 +0800 Subject: [PATCH 04/15] modify change log Signed-off-by: yubonluo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7c50f9e9754..5655f11ee760 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,7 +99,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add error state to all data source menu components to show error component and consolidate all fetch errors ([#6440](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6440)) - [Multiple Datasource] UI change for datasource view picker to enable selectable([#6497](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6497)) - [Workspace] Support workspace in saved objects client in server side. ([#6365](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6365)) -- [Workspace] Dashboard admin(groups/users) implementation. ([#6454](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6454)) +- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) - [MD] Add dropdown header to data source single selector ([#6431](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6431)) - [Workspace] Add permission tab to workspace create update page ([#6378](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6378)) - [Workspace] Hide datasource and advanced settings menu in dashboard management when in workspace. ([#6455](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6455)) From 34f0164a19e9064c99f44ae76c29a2714176a476 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 19 Apr 2024 15:44:48 +0800 Subject: [PATCH 05/15] modify change log Signed-off-by: yubonluo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86f842ab64bf..265ff3775c9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,12 +100,12 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add error state to all data source menu components to show error component and consolidate all fetch errors ([#6440](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6440)) - [Multiple Datasource] UI change for datasource view picker to enable selectable([#6497](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6497)) - [Workspace] Support workspace in saved objects client in server side. ([#6365](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6365)) -- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) - [MD] Add dropdown header to data source single selector ([#6431](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6431)) - [Workspace] Add permission tab to workspace create update page ([#6378](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6378)) - [Workspace] Hide datasource and advanced settings menu in dashboard management when in workspace. ([#6455](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6455)) - [Multiple Datasource] Modify selectable picker to remove group label and close popover after selection ([#6515](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6515)) - [Workspace] Add workspaces filter to saved objects page. ([#6458](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6458)) +- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) ### 🐛 Bug Fixes From f434c84cf22c03f935f14a60f5ced4238bbf07e1 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 19 Apr 2024 16:35:43 +0800 Subject: [PATCH 06/15] solve change log issue Signed-off-by: yubonluo --- changelogs/fragments/6554.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/6554.yml diff --git a/changelogs/fragments/6554.yml b/changelogs/fragments/6554.yml new file mode 100644 index 000000000000..a7a2ffab95b0 --- /dev/null +++ b/changelogs/fragments/6554.yml @@ -0,0 +1,2 @@ +skip: +- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) \ No newline at end of file From 67237c5b0cbe8b03f6248d086e18f0e0d69edda9 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Mon, 22 Apr 2024 03:28:37 +0000 Subject: [PATCH 07/15] Changeset file for PR #6554 created/updated --- changelogs/fragments/6554.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelogs/fragments/6554.yml b/changelogs/fragments/6554.yml index a7a2ffab95b0..5b077c2eb28b 100644 --- a/changelogs/fragments/6554.yml +++ b/changelogs/fragments/6554.yml @@ -1,2 +1,2 @@ -skip: -- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) \ No newline at end of file +feat: +- Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) \ No newline at end of file From ee48ea369195870122a78314c7797bdc52d9ffbf Mon Sep 17 00:00:00 2001 From: yubonluo Date: Mon, 22 Apr 2024 11:29:36 +0800 Subject: [PATCH 08/15] [Workspace] delete useless code Signed-off-by: yubonluo --- changelogs/fragments/6554.yml | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 changelogs/fragments/6554.yml diff --git a/changelogs/fragments/6554.yml b/changelogs/fragments/6554.yml deleted file mode 100644 index a7a2ffab95b0..000000000000 --- a/changelogs/fragments/6554.yml +++ /dev/null @@ -1,2 +0,0 @@ -skip: -- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) \ No newline at end of file From 5105dd9abd4263fbacdb06a2dbc7bbdbaf0775b8 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Wed, 24 Apr 2024 02:47:29 +0000 Subject: [PATCH 09/15] Changeset file for PR #6554 created/updated --- changelogs/fragments/6554.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/6554.yml b/changelogs/fragments/6554.yml index 5b077c2eb28b..b3944b967773 100644 --- a/changelogs/fragments/6554.yml +++ b/changelogs/fragments/6554.yml @@ -1,2 +1,2 @@ feat: -- Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) \ No newline at end of file +- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) \ No newline at end of file From f4a9dc3b6f1ee0479ac82da3aa979764b3631b00 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 30 Apr 2024 15:05:28 +0800 Subject: [PATCH 10/15] delete useless code Signed-off-by: yubonluo --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb2f09b9d5fb..6b9a0b11f941 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,7 +105,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Workspace] Hide datasource and advanced settings menu in dashboard management when in workspace. ([#6455](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6455)) - [Multiple Datasource] Modify selectable picker to remove group label and close popover after selection ([#6515](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6515)) - [Workspace] Add workspaces filter to saved objects page. ([#6458](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6458)) -- [Workspace] Dashboard admin(groups/users) implementation. ([#6554](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6554)) ### 🐛 Bug Fixes From 797698feb4fdf7b8996e653b87d69843739e8d8e Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 17 May 2024 14:49:27 +0800 Subject: [PATCH 11/15] Optimize the code Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 8 ++++---- src/plugins/workspace/server/types.ts | 2 +- src/plugins/workspace/server/utils.test.ts | 18 ++++++++++++------ src/plugins/workspace/server/utils.ts | 16 +++++++++------- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 91cb1c99da9b..0de293911a1d 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -43,8 +43,8 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { - getApplicationOSDAdminConfig, - getOSDAdminConfig, + getOSDAdminConfigFromApplicationConfig, + getOSDAdminConfigFromYMLConfig, updateDashboardAdminStateForRequest, } from './utils'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; @@ -98,12 +98,12 @@ export class WorkspacePlugin implements Plugin { applicationConfig: applicationConfigMock, }; const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - const [groups, users] = await getApplicationOSDAdminConfig(mockDependencies, mockRequest); + const [groups, users] = await getOSDAdminConfigFromApplicationConfig( + mockDependencies, + mockRequest + ); expect(groups).toEqual(['group1', 'group2']); expect(users).toEqual(['user1', 'user2']); }); @@ -175,7 +178,10 @@ describe('workspace utils', () => { applicationConfig: applicationConfigMock, }; const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - const [groups, users] = await getApplicationOSDAdminConfig(mockDependencies, mockRequest); + const [groups, users] = await getOSDAdminConfigFromApplicationConfig( + mockDependencies, + mockRequest + ); expect(groups).toEqual([]); expect(users).toEqual([]); }); @@ -189,14 +195,14 @@ describe('workspace utils', () => { }, }, }); - const [groups, users] = await getOSDAdminConfig(globalConfig$); + const [groups, users] = await getOSDAdminConfigFromYMLConfig(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$); + const [groups, users] = await getOSDAdminConfigFromYMLConfig(globalConfig$); expect(groups).toEqual([]); expect(users).toEqual([]); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 79fcc60aad5d..60a0d114d951 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -68,8 +68,8 @@ export const updateDashboardAdminStateForRequest = ( }); return; } - const groupMatchAny = groups.some((group) => configGroups.includes(group)) || false; - const userMatchAny = users.some((user) => configUsers.includes(user)) || false; + const groupMatchAny = groups.some((group) => configGroups.includes(group)); + const userMatchAny = users.some((user) => configUsers.includes(user)); updateWorkspaceState(request, { isDashboardAdmin: groupMatchAny || userMatchAny, }); @@ -88,17 +88,17 @@ export const stringToArray = (adminConfig: string | undefined) => { return adminConfigArray; }; -export const getApplicationOSDAdminConfig = async ( +export const getOSDAdminConfigFromApplicationConfig = async ( { applicationConfig }: AppPluginSetupDependencies, request: OpenSearchDashboardsRequest ) => { - const applicationConfigClient = applicationConfig.getConfigurationClient(request); + const applicationConfigClient = applicationConfig?.getConfigurationClient(request); const [groupsResult, usersResult] = await Promise.all([ - applicationConfigClient + applicationConfigClient! .getEntityConfig('opensearchDashboards.dashboardAdmin.groups') .catch(() => undefined), - applicationConfigClient + applicationConfigClient! .getEntityConfig('opensearchDashboards.dashboardAdmin.users') .catch(() => undefined), ]); @@ -106,7 +106,9 @@ export const getApplicationOSDAdminConfig = async ( return [stringToArray(groupsResult), stringToArray(usersResult)]; }; -export const getOSDAdminConfig = async (globalConfig$: Observable) => { +export const getOSDAdminConfigFromYMLConfig = async ( + globalConfig$: Observable +) => { const globalConfig = await globalConfig$.pipe(first()).toPromise(); const groupsResult = (globalConfig.opensearchDashboards?.dashboardAdmin?.groups || []) as string[]; From 1e0b749a3fbc614f6d90fcd5672ee8197cff3053 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Mon, 20 May 2024 13:26:22 +0800 Subject: [PATCH 12/15] Add unit test to cover setupPermission in plugin. Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.test.ts | 68 ++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index bb23cb335ce1..ae58f2d578a1 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -3,11 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { OnPreRoutingHandler } from 'src/core/server'; +import { OnPostAuthHandler, OnPreRoutingHandler } from 'src/core/server'; import { coreMock, httpServerMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; import { AppPluginSetupDependencies } from './types'; import { getWorkspaceState } from '../../../core/server/utils'; +import * as utilsExports from './utils'; describe('Workspace server plugin', () => { const mockApplicationConfig = { @@ -75,6 +76,71 @@ describe('Workspace server plugin', () => { expect(toolKitMock.next).toBeCalledTimes(1); }); + describe('#setupPermission', () => { + const setupMock = coreMock.createSetup(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + let registerOnPostAuthFn: OnPostAuthHandler = () => httpServerMock.createResponseFactory().ok(); + setupMock.http.registerOnPostAuth.mockImplementation((fn) => { + registerOnPostAuthFn = fn; + return fn; + }); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/w/foo/app', + }); + + it('catch error', async () => { + await workspacePlugin.setup(setupMock, mockDependencies); + const toolKitMock = httpServerMock.createToolkit(); + + await registerOnPostAuthFn( + requestWithWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); + + it('with yml config', async () => { + jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); + jest + .spyOn(utilsExports, 'getOSDAdminConfigFromYMLConfig') + .mockResolvedValue([['group1'], ['user1']]); + const mockNoDependencies: AppPluginSetupDependencies = {}; + + await workspacePlugin.setup(setupMock, mockNoDependencies); + const toolKitMock = httpServerMock.createToolkit(); + + await registerOnPostAuthFn( + requestWithWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); + + it('with application config', async () => { + jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); + jest + .spyOn(utilsExports, 'getOSDAdminConfigFromApplicationConfig') + .mockResolvedValue([['group1'], ['user1']]); + await workspacePlugin.setup(setupMock, mockDependencies); + const toolKitMock = httpServerMock.createToolkit(); + + await registerOnPostAuthFn( + requestWithWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); + }); + it('#start', async () => { const setupMock = coreMock.createSetup(); const startMock = coreMock.createStart(); From 570e684a178eeecd46b5976c6b9d10764a346c7e Mon Sep 17 00:00:00 2001 From: yubonluo Date: Tue, 21 May 2024 17:08:18 +0800 Subject: [PATCH 13/15] delete the logic of dynamic application config Signed-off-by: yubonluo --- .../workspace/opensearch_dashboards.json | 2 +- src/plugins/workspace/server/plugin.test.ts | 35 ++-------- src/plugins/workspace/server/plugin.ts | 30 ++------- src/plugins/workspace/server/types.ts | 4 -- src/plugins/workspace/server/utils.test.ts | 65 ------------------- src/plugins/workspace/server/utils.ts | 33 +--------- 6 files changed, 13 insertions(+), 156 deletions(-) diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 0b84b780ffed..cf67547f788c 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -7,6 +7,6 @@ "savedObjects", "opensearchDashboardsReact" ], - "optionalPlugins": ["savedObjectsManagement","management","applicationConfig"], + "optionalPlugins": ["savedObjectsManagement","management"], "requiredBundles": ["opensearchDashboardsReact"] } diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index ae58f2d578a1..74793187c27e 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -6,18 +6,10 @@ import { OnPostAuthHandler, OnPreRoutingHandler } from 'src/core/server'; import { coreMock, httpServerMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; -import { AppPluginSetupDependencies } from './types'; import { getWorkspaceState } from '../../../core/server/utils'; import * as utilsExports from './utils'; describe('Workspace server plugin', () => { - const mockApplicationConfig = { - getConfigurationClient: jest.fn().mockResolvedValue({}), - registerConfigurationClient: jest.fn().mockResolvedValue({}), - }; - const mockDependencies: AppPluginSetupDependencies = { - applicationConfig: mockApplicationConfig, - }; it('#setup', async () => { let value; const setupMock = coreMock.createSetup(); @@ -26,7 +18,7 @@ describe('Workspace server plugin', () => { }); setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn())); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); - await workspacePlugin.setup(setupMock, mockDependencies); + await workspacePlugin.setup(setupMock); expect(value).toMatchInlineSnapshot(` Object { "workspaces": Object { @@ -52,7 +44,7 @@ describe('Workspace server plugin', () => { return fn; }); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); - await workspacePlugin.setup(setupMock, mockDependencies); + await workspacePlugin.setup(setupMock); const toolKitMock = httpServerMock.createToolkit(); const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ @@ -95,7 +87,7 @@ describe('Workspace server plugin', () => { }); it('catch error', async () => { - await workspacePlugin.setup(setupMock, mockDependencies); + await workspacePlugin.setup(setupMock); const toolKitMock = httpServerMock.createToolkit(); await registerOnPostAuthFn( @@ -111,25 +103,8 @@ describe('Workspace server plugin', () => { jest .spyOn(utilsExports, 'getOSDAdminConfigFromYMLConfig') .mockResolvedValue([['group1'], ['user1']]); - const mockNoDependencies: AppPluginSetupDependencies = {}; - await workspacePlugin.setup(setupMock, mockNoDependencies); - const toolKitMock = httpServerMock.createToolkit(); - - await registerOnPostAuthFn( - requestWithWorkspaceInUrl, - httpServerMock.createResponseFactory(), - toolKitMock - ); - expect(toolKitMock.next).toBeCalledTimes(1); - }); - - it('with application config', async () => { - jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); - jest - .spyOn(utilsExports, 'getOSDAdminConfigFromApplicationConfig') - .mockResolvedValue([['group1'], ['user1']]); - await workspacePlugin.setup(setupMock, mockDependencies); + await workspacePlugin.setup(setupMock); const toolKitMock = httpServerMock.createToolkit(); await registerOnPostAuthFn( @@ -152,7 +127,7 @@ describe('Workspace server plugin', () => { }); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); - await workspacePlugin.setup(setupMock, mockDependencies); + await workspacePlugin.setup(setupMock); await workspacePlugin.start(startMock); expect(startMock.savedObjects.createSerializer).toBeCalledTimes(1); }); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 0de293911a1d..2ec8447ad817 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -23,12 +23,7 @@ import { WORKSPACE_UI_SETTINGS_CLIENT_WRAPPER_ID, PRIORITY_FOR_WORKSPACE_UI_SETTINGS_WRAPPER, } from '../common/constants'; -import { - IWorkspaceClientImpl, - WorkspacePluginSetup, - WorkspacePluginStart, - AppPluginSetupDependencies, -} from './types'; +import { IWorkspaceClientImpl, WorkspacePluginSetup, WorkspacePluginStart } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; @@ -42,11 +37,7 @@ import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; -import { - getOSDAdminConfigFromApplicationConfig, - getOSDAdminConfigFromYMLConfig, - updateDashboardAdminStateForRequest, -} from './utils'; +import { getOSDAdminConfigFromYMLConfig, updateDashboardAdminStateForRequest } from './utils'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; import { WorkspaceUiSettingsClientWrapper } from './saved_objects/workspace_ui_settings_client_wrapper'; @@ -81,14 +72,12 @@ 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 { @@ -97,14 +86,7 @@ export class WorkspacePlugin implements Plugin { @@ -124,68 +121,6 @@ describe('workspace utils', () => { 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([]); - }); - - 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 mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - const [groups, users] = await getOSDAdminConfigFromApplicationConfig( - mockDependencies, - mockRequest - ); - 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 new Error('Not found'); - }), - }), - registerConfigurationClient: jest.fn().mockResolvedValue({}), - }; - - const mockDependencies: AppPluginSetupDependencies = { - applicationConfig: applicationConfigMock, - }; - const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - const [groups, users] = await getOSDAdminConfigFromApplicationConfig( - mockDependencies, - mockRequest - ); - expect(groups).toEqual([]); - expect(users).toEqual([]); - }); - it('should get correct admin config when admin config is enabled ', async () => { const globalConfig$: Observable = of({ opensearchDashboards: { diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 60a0d114d951..13fced2e6d64 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -14,7 +14,7 @@ import { PrincipalType, SharedGlobalConfig, } from '../../../core/server'; -import { AppPluginSetupDependencies, AuthInfo } from './types'; +import { AuthInfo } from './types'; import { updateWorkspaceState } from '../../../core/server/utils'; /** @@ -75,37 +75,6 @@ export const updateDashboardAdminStateForRequest = ( }); }; -export const stringToArray = (adminConfig: string | undefined) => { - if (!adminConfig) { - return []; - } - let adminConfigArray; - try { - adminConfigArray = JSON.parse(adminConfig); - } catch (e) { - return []; - } - return adminConfigArray; -}; - -export const getOSDAdminConfigFromApplicationConfig = async ( - { applicationConfig }: AppPluginSetupDependencies, - request: OpenSearchDashboardsRequest -) => { - const applicationConfigClient = applicationConfig?.getConfigurationClient(request); - - 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 getOSDAdminConfigFromYMLConfig = async ( globalConfig$: Observable ) => { From fd10be4166e338ffd7952bd0e24d316b5810ac49 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Thu, 23 May 2024 22:52:37 +0800 Subject: [PATCH 14/15] Default to OSD admin if security uninstall Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.test.ts | 18 +++++++++++++++++- src/plugins/workspace/server/plugin.ts | 7 +++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index 74793187c27e..0b943cf47252 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -99,7 +99,9 @@ describe('Workspace server plugin', () => { }); it('with yml config', async () => { - jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); + jest + .spyOn(utilsExports, 'getPrincipalsFromRequest') + .mockImplementation(() => ({ users: [`user1`] })); jest .spyOn(utilsExports, 'getOSDAdminConfigFromYMLConfig') .mockResolvedValue([['group1'], ['user1']]); @@ -114,6 +116,20 @@ describe('Workspace server plugin', () => { ); expect(toolKitMock.next).toBeCalledTimes(1); }); + + it('uninstall security plugin', async () => { + jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({})); + + await workspacePlugin.setup(setupMock); + const toolKitMock = httpServerMock.createToolkit(); + + await registerOnPostAuthFn( + requestWithWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); }); it('#start', async () => { diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 2ec8447ad817..268d9e69709a 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -85,6 +85,13 @@ export class WorkspacePlugin implements Plugin Date: Fri, 24 May 2024 10:15:03 +0800 Subject: [PATCH 15/15] Default to OSD admin if security uninstall Signed-off-by: yubonluo --- src/plugins/workspace/server/plugin.ts | 7 ------- src/plugins/workspace/server/utils.test.ts | 12 +++++++++++- src/plugins/workspace/server/utils.ts | 12 ++++++++---- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 268d9e69709a..2ec8447ad817 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -85,13 +85,6 @@ export class WorkspacePlugin implements Plugin { expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false); }); - it('should be not dashboard admin when groups and users are []', () => { + it('should be 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(true); + }); + + it('should be dashboard admin when configGroups and configUsers are []', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const groups: string[] = ['user1']; + const users: string[] = []; + const configGroups: string[] = []; + const configUsers: string[] = []; + 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 13fced2e6d64..9037038f16af 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -62,10 +62,14 @@ export const updateDashboardAdminStateForRequest = ( configGroups: string[], configUsers: string[] ) => { - if (configGroups.length === 0 && configUsers.length === 0) { - updateWorkspaceState(request, { - isDashboardAdmin: false, - }); + // If the security plugin is not installed, login defaults to OSD Admin + if (!groups.length && !users.length) { + updateWorkspaceState(request, { isDashboardAdmin: true }); + return; + } + + if (!configGroups.length && !configUsers.length) { + updateWorkspaceState(request, { isDashboardAdmin: false }); return; } const groupMatchAny = groups.some((group) => configGroups.includes(group));