From f937a8dc69f5e8eadf08f75e1e578fd53fcb6497 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 15 Sep 2023 13:51:21 +0800 Subject: [PATCH] Refractor: move permission control service to workspace directory and keep saved objects as clean as possible (#166) * refractor: move permission control service to workspace directory and keep saved objects as clean as possible Signed-off-by: SuZhou-Joe * feat: make lint pass Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe --- src/core/server/index.ts | 4 +- src/core/server/legacy/legacy_service.ts | 1 - src/core/server/plugins/plugin_context.ts | 1 - src/core/server/saved_objects/index.ts | 9 ++++- .../saved_objects_service.mock.ts | 2 - .../saved_objects/saved_objects_service.ts | 17 -------- .../server}/permission_control/client.mock.ts | 1 - .../server}/permission_control/client.ts | 39 +++++++++++-------- .../permission_control/routes/index.ts | 6 +-- .../permission_control/routes/principals.ts | 5 ++- .../permission_control/routes/validate.ts | 5 ++- src/plugins/workspace/server/plugin.ts | 16 +++++++- src/plugins/workspace/server/routes/index.ts | 2 +- 13 files changed, 57 insertions(+), 51 deletions(-) rename src/{core/server/saved_objects => plugins/workspace/server}/permission_control/client.mock.ts (95%) rename src/{core/server/saved_objects => plugins/workspace/server}/permission_control/client.ts (83%) rename src/{core/server/saved_objects => plugins/workspace/server}/permission_control/routes/index.ts (74%) rename src/{core/server/saved_objects => plugins/workspace/server}/permission_control/routes/principals.ts (82%) rename src/{core/server/saved_objects => plugins/workspace/server}/permission_control/routes/validate.ts (83%) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 2a5b46a320d1..a103a90bf16c 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -326,7 +326,9 @@ export { SavedObjectsDeleteByWorkspaceOptions, Permissions, ACL, - SavedObjectsPermissionControlContract, + Principals, + TransformedPermission, + PrincipalType, } from './saved_objects'; export { diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 1f6bbc3a3b49..a48ff12e859a 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -276,7 +276,6 @@ export class LegacyService implements CoreService { registerType: setupDeps.core.savedObjects.registerType, getImportExportObjectLimit: setupDeps.core.savedObjects.getImportExportObjectLimit, setRepositoryFactoryProvider: setupDeps.core.savedObjects.setRepositoryFactoryProvider, - permissionControl: setupDeps.core.savedObjects.permissionControl, }, status: { isStatusPageAnonymous: setupDeps.core.status.isStatusPageAnonymous, diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index bcea61fdd797..7782fd93041e 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -205,7 +205,6 @@ export function createPluginSetupContext( registerType: deps.savedObjects.registerType, getImportExportObjectLimit: deps.savedObjects.getImportExportObjectLimit, setRepositoryFactoryProvider: deps.savedObjects.setRepositoryFactoryProvider, - permissionControl: deps.savedObjects.permissionControl, }, status: { core$: deps.status.core$, diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts index 30d774464bea..11809c5b88c9 100644 --- a/src/core/server/saved_objects/index.ts +++ b/src/core/server/saved_objects/index.ts @@ -85,5 +85,10 @@ export { export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config'; export { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry'; -export { Permissions, ACL } from './permission_control/acl'; -export { SavedObjectsPermissionControlContract } from './permission_control/client'; +export { + Permissions, + ACL, + Principals, + TransformedPermission, + PrincipalType, +} from './permission_control/acl'; diff --git a/src/core/server/saved_objects/saved_objects_service.mock.ts b/src/core/server/saved_objects/saved_objects_service.mock.ts index bf4509adad39..74168c436c3d 100644 --- a/src/core/server/saved_objects/saved_objects_service.mock.ts +++ b/src/core/server/saved_objects/saved_objects_service.mock.ts @@ -45,7 +45,6 @@ import { typeRegistryMock } from './saved_objects_type_registry.mock'; import { migrationMocks } from './migrations/mocks'; import { ServiceStatusLevels } from '../status'; import { ISavedObjectTypeRegistry } from './saved_objects_type_registry'; -import { savedObjectsPermissionControlMock } from './permission_control/client.mock'; type SavedObjectsServiceContract = PublicMethodsOf; @@ -81,7 +80,6 @@ const createSetupContractMock = () => { registerType: jest.fn(), getImportExportObjectLimit: jest.fn(), setRepositoryFactoryProvider: jest.fn(), - permissionControl: savedObjectsPermissionControlMock, }; setupContract.getImportExportObjectLimit.mockReturnValue(100); diff --git a/src/core/server/saved_objects/saved_objects_service.ts b/src/core/server/saved_objects/saved_objects_service.ts index 5c43154be841..2f91ba4fa8d6 100644 --- a/src/core/server/saved_objects/saved_objects_service.ts +++ b/src/core/server/saved_objects/saved_objects_service.ts @@ -65,11 +65,6 @@ import { registerRoutes } from './routes'; import { ServiceStatus } from '../status'; import { calculateStatus$ } from './status'; import { createMigrationOpenSearchClient } from './migrations/core/'; -import { - SavedObjectsPermissionControl, - SavedObjectsPermissionControlContract, -} from './permission_control/client'; -import { registerPermissionCheckRoutes } from './permission_control/routes'; /** * Saved Objects is OpenSearchDashboards's data persistence mechanism allowing plugins to * use OpenSearch for storing and querying state. The SavedObjectsServiceSetup API exposes methods @@ -180,8 +175,6 @@ export interface SavedObjectsServiceSetup { setRepositoryFactoryProvider: ( respositoryFactoryProvider: SavedObjectRepositoryFactoryProvider ) => void; - - permissionControl: SavedObjectsPermissionControlContract; } /** @@ -308,7 +301,6 @@ export class SavedObjectsService private started = false; private respositoryFactoryProvider?: SavedObjectRepositoryFactoryProvider; - private permissionControl?: SavedObjectsPermissionControlContract; constructor(private readonly coreContext: CoreContext) { this.logger = coreContext.logger.get('savedobjects-service'); @@ -336,13 +328,6 @@ export class SavedObjectsService migratorPromise: this.migrator$.pipe(first()).toPromise(), }); - this.permissionControl = new SavedObjectsPermissionControl(this.logger); - - registerPermissionCheckRoutes({ - http: setupDeps.http, - permissionControl: this.permissionControl, - }); - return { status$: calculateStatus$( this.migrator$.pipe(switchMap((migrator) => migrator.getStatus$())), @@ -383,7 +368,6 @@ export class SavedObjectsService } this.respositoryFactoryProvider = repositoryProvider; }, - permissionControl: this.permissionControl, }; } @@ -492,7 +476,6 @@ export class SavedObjectsService this.started = true; const getScopedClient = clientProvider.getClient.bind(clientProvider); - this.permissionControl?.setup(repositoryFactory.createInternalRepository); return { getScopedClient, diff --git a/src/core/server/saved_objects/permission_control/client.mock.ts b/src/plugins/workspace/server/permission_control/client.mock.ts similarity index 95% rename from src/core/server/saved_objects/permission_control/client.mock.ts rename to src/plugins/workspace/server/permission_control/client.mock.ts index 4cae55f62890..13ce45c7975d 100644 --- a/src/core/server/saved_objects/permission_control/client.mock.ts +++ b/src/plugins/workspace/server/permission_control/client.mock.ts @@ -5,7 +5,6 @@ import { SavedObjectsPermissionControlContract } from './client'; export const savedObjectsPermissionControlMock: SavedObjectsPermissionControlContract = { - setup: jest.fn(), validate: jest.fn(), batchValidate: jest.fn(), getPrincipalsOfObjects: jest.fn(), diff --git a/src/core/server/saved_objects/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts similarity index 83% rename from src/core/server/saved_objects/permission_control/client.ts rename to src/plugins/workspace/server/permission_control/client.ts index f7c0c800ec4c..2d8bf80491c3 100644 --- a/src/core/server/saved_objects/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -3,13 +3,18 @@ * SPDX-License-Identifier: Apache-2.0 */ import { i18n } from '@osd/i18n'; -import { OpenSearchDashboardsRequest } from '../../http'; -import { ensureRawRequest } from '../../http/router'; -import { SavedObjectsServiceStart } from '../saved_objects_service'; -import { SavedObjectsBulkGetObject } from '../service'; -import { ACL, Principals, TransformedPermission, PrincipalType } from './acl'; -import { Logger } from '../../logging'; -import { WORKSPACE_TYPE } from '../../../utils'; +import { ensureRawRequest, OpenSearchDashboardsRequest } from '../../../../core/server'; +import { + ACL, + Principals, + TransformedPermission, + PrincipalType, + SavedObjectsBulkGetObject, + SavedObjectsServiceStart, + Logger, + WORKSPACE_TYPE, +} from '../../../../core/server'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants'; export type SavedObjectsPermissionControlContract = Pick< SavedObjectsPermissionControl, @@ -25,9 +30,11 @@ export interface AuthInfo { export class SavedObjectsPermissionControl { private readonly logger: Logger; - private createInternalRepository?: SavedObjectsServiceStart['createInternalRepository']; - private getInternalRepository() { - return this.createInternalRepository?.(); + private _getScopedClient?: SavedObjectsServiceStart['getScopedClient']; + private getScopedClient(request: OpenSearchDashboardsRequest) { + return this._getScopedClient?.(request, { + excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + }); } constructor(logger: Logger) { @@ -65,12 +72,10 @@ export class SavedObjectsPermissionControl { request: OpenSearchDashboardsRequest, savedObjects: SavedObjectsBulkGetObject[] ) { - return (await this.getInternalRepository()?.bulkGet(savedObjects))?.saved_objects || []; + return (await this.getScopedClient?.(request)?.bulkGet(savedObjects))?.saved_objects || []; } - public async setup( - createInternalRepository: SavedObjectsServiceStart['createInternalRepository'] - ) { - this.createInternalRepository = createInternalRepository; + public async setup(getScopedClient: SavedObjectsServiceStart['getScopedClient']) { + this._getScopedClient = getScopedClient; } public async validate( request: OpenSearchDashboardsRequest, @@ -164,9 +169,9 @@ export class SavedObjectsPermissionControl { permissionModes: SavedObjectsPermissionModes ) { const principals = this.getPrincipalsFromRequest(request); - const repository = this.getInternalRepository(); + const savedObjectClient = this.getScopedClient?.(request); try { - const result = await repository?.find({ + const result = await savedObjectClient?.find({ type: [WORKSPACE_TYPE], ACLSearchParams: { permissionModes, diff --git a/src/core/server/saved_objects/permission_control/routes/index.ts b/src/plugins/workspace/server/permission_control/routes/index.ts similarity index 74% rename from src/core/server/saved_objects/permission_control/routes/index.ts rename to src/plugins/workspace/server/permission_control/routes/index.ts index 405bd22bdbe9..8bf96a159d34 100644 --- a/src/core/server/saved_objects/permission_control/routes/index.ts +++ b/src/plugins/workspace/server/permission_control/routes/index.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { InternalHttpServiceSetup } from '../../../http'; +import { HttpServiceSetup } from '../../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../client'; import { registerListRoute } from './principals'; import { registerValidateRoute } from './validate'; @@ -12,10 +12,10 @@ export function registerPermissionCheckRoutes({ http, permissionControl, }: { - http: InternalHttpServiceSetup; + http: HttpServiceSetup; permissionControl: SavedObjectsPermissionControlContract; }) { - const router = http.createRouter('/api/saved_objects_permission_control/'); + const router = http.createRouter(); registerValidateRoute(router, permissionControl); registerListRoute(router, permissionControl); diff --git a/src/core/server/saved_objects/permission_control/routes/principals.ts b/src/plugins/workspace/server/permission_control/routes/principals.ts similarity index 82% rename from src/core/server/saved_objects/permission_control/routes/principals.ts rename to src/plugins/workspace/server/permission_control/routes/principals.ts index e21e4151146b..a10517684d08 100644 --- a/src/core/server/saved_objects/permission_control/routes/principals.ts +++ b/src/plugins/workspace/server/permission_control/routes/principals.ts @@ -4,8 +4,9 @@ */ import { schema } from '@osd/config-schema'; -import { IRouter } from '../../../http'; +import { IRouter } from '../../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../client'; +import { WORKSPACES_API_BASE_URL } from '../../routes'; export const registerListRoute = ( router: IRouter, @@ -13,7 +14,7 @@ export const registerListRoute = ( ) => { router.post( { - path: '/principals', + path: `${WORKSPACES_API_BASE_URL}/principals`, validate: { body: schema.object({ objects: schema.arrayOf( diff --git a/src/core/server/saved_objects/permission_control/routes/validate.ts b/src/plugins/workspace/server/permission_control/routes/validate.ts similarity index 83% rename from src/core/server/saved_objects/permission_control/routes/validate.ts rename to src/plugins/workspace/server/permission_control/routes/validate.ts index 746608d2a74c..37a5b0d3144e 100644 --- a/src/core/server/saved_objects/permission_control/routes/validate.ts +++ b/src/plugins/workspace/server/permission_control/routes/validate.ts @@ -4,8 +4,9 @@ */ import { schema } from '@osd/config-schema'; -import { IRouter } from '../../../http'; +import { IRouter } from '../../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../client'; +import { WORKSPACES_API_BASE_URL } from '../../routes'; export const registerValidateRoute = ( router: IRouter, @@ -13,7 +14,7 @@ export const registerValidateRoute = ( ) => { router.post( { - path: '/validate/{type}/{id}', + path: `${WORKSPACES_API_BASE_URL}/validate/{type}/{id}`, validate: { params: schema.object({ type: schema.string(), diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index e6ac24961e01..c9a951b7580a 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -31,11 +31,17 @@ import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, } from '../common/constants'; import { ConfigSchema } from '../config'; +import { + SavedObjectsPermissionControl, + SavedObjectsPermissionControlContract, +} from './permission_control/client'; +import { registerPermissionCheckRoutes } from './permission_control/routes'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; private client?: IWorkspaceDBImpl; private config$: Observable; + private permissionControl?: SavedObjectsPermissionControlContract; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { /** @@ -65,8 +71,15 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.client = new WorkspaceClientWithSavedObject(core); await this.client.setup(core); + this.permissionControl = new SavedObjectsPermissionControl(this.logger); + + registerPermissionCheckRoutes({ + http: core.http, + permissionControl: this.permissionControl, + }); + const workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( - core.savedObjects.permissionControl, + this.permissionControl, { config$: this.config$, } @@ -171,6 +184,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { public start(core: CoreStart) { this.logger.debug('Starting SavedObjects service'); + this.permissionControl?.setup(core.savedObjects.getScopedClient); this.client?.setSavedObjectes(core.savedObjects); this.setupWorkspaces(core); diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 8ad83ec113be..b2b3f69b0534 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -14,7 +14,7 @@ import { } from '../../../../core/server'; import { IWorkspaceDBImpl, WorkspaceRoutePermissionItem } from '../types'; -const WORKSPACES_API_BASE_URL = '/api/workspaces'; +export const WORKSPACES_API_BASE_URL = '/api/workspaces'; const workspacePermissionMode = schema.oneOf([ schema.literal(WorkspacePermissionMode.LibraryRead),