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 <suzhou@amazon.com> * feat: make lint pass Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
ruanyl · Sep 16, 2023 · f937a8d · f937a8d
1 parent 7937571
commit f937a8d
Show file tree

Hide file tree

Showing 13 changed files with 57 additions and 51 deletions.
diff --git 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
@@ -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
@@ -205,7 +205,6 @@ export function createPluginSetupContext<TPlugin, TPluginDependencies>(
       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
@@ -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
@@ -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<SavedObjectsService>;
 
@@ -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
@@ -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/...objects/permission_control/client.mock.ts → .../server/permission_control/client.mock.ts b/...objects/permission_control/client.mock.ts → .../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/...aved_objects/permission_control/client.ts → ...space/server/permission_control/client.ts b/...aved_objects/permission_control/client.ts → ...space/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/...bjects/permission_control/routes/index.ts → ...server/permission_control/routes/index.ts b/...bjects/permission_control/routes/index.ts → ...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/...s/permission_control/routes/principals.ts → ...r/permission_control/routes/principals.ts b/...s/permission_control/routes/principals.ts → ...r/permission_control/routes/principals.ts
@@ -4,16 +4,17 @@
  */
 
 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,
   permissionControl: SavedObjectsPermissionControlContract
 ) => {
   router.post(
     {
-      path: '/principals',
+      path: `${WORKSPACES_API_BASE_URL}/principals`,
       validate: {
         body: schema.object({
           objects: schema.arrayOf(

diff --git a/...cts/permission_control/routes/validate.ts → ...ver/permission_control/routes/validate.ts b/...cts/permission_control/routes/validate.ts → ...ver/permission_control/routes/validate.ts
@@ -4,16 +4,17 @@
  */
 
 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,
   permissionControl: SavedObjectsPermissionControlContract
 ) => {
   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
@@ -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<ConfigSchema>;
+  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
@@ -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),