Address pr comments

Signed-off-by: Lin Wang <[email protected]>
wanglam · Mar 1, 2024 · 898fe4c · 898fe4c
1 parent e5430b0
commit 898fe4c
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 17 deletions.
diff --git a/src/core/server/index.ts b/src/core/server/index.ts
@@ -327,6 +327,7 @@ export {
   Principals,
   TransformedPermission,
   PrincipalType,
+  Permissions,
 } from './saved_objects';
 
 export {

diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts
@@ -14,6 +14,7 @@ import {
   Principals,
   SavedObject,
   WORKSPACE_TYPE,
+  Permissions,
 } from '../../../../core/server';
 import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants';
 import { getPrincipalsFromRequest } from '../utils';
@@ -84,7 +85,7 @@ export class SavedObjectsPermissionControl {
       SavedObject<unknown>,
       'id' | 'type' | 'workspaces' | 'permissions'
     >> = [];
-    const hasAllPermission = savedObjects.every((savedObject) => {
+    const hasPermissionToAllObjects = savedObjects.every((savedObject) => {
       // for object that doesn't contain ACL like config, return true
       if (!savedObject.permissions) {
         return true;
@@ -97,10 +98,10 @@ export class SavedObjectsPermissionControl {
       }
       return hasPermission;
     });
-    if (!hasAllPermission) {
+    if (!hasPermissionToAllObjects) {
       this.logNotPermitted(notPermittedSavedObjects, principals, permissionModes);
     }
-    return hasAllPermission;
+    return hasPermissionToAllObjects;
   }
 
   /**
@@ -136,7 +137,12 @@ export class SavedObjectsPermissionControl {
     }
 
     const principals = getPrincipalsFromRequest(request);
-    let savedObjectsBasicInfo: any[] = [];
+    const deniedObjects: Array<
+      Pick<SavedObjectsBulkGetObject, 'id' | 'type'> & {
+        workspaces?: string[];
+        permissions?: Permissions;
+      }
+    > = [];
     const hasAllPermission = savedObjectsGet.every((item) => {
       // for object that doesn't contain ACL like config, return true
       if (!item.permissions) {
@@ -145,15 +151,12 @@ export class SavedObjectsPermissionControl {
       const aclInstance = new ACL(item.permissions);
       const hasPermission = aclInstance.hasPermission(permissionModes, principals);
       if (!hasPermission) {
-        savedObjectsBasicInfo = [
-          ...savedObjectsBasicInfo,
-          {
-            id: item.id,
-            type: item.type,
-            workspaces: item.workspaces,
-            permissions: item.permissions,
-          },
-        ];
+        deniedObjects.push({
+          id: item.id,
+          type: item.type,
+          workspaces: item.workspaces,
+          permissions: item.permissions,
+        });
       }
       return hasPermission;
     });
@@ -162,7 +165,7 @@ export class SavedObjectsPermissionControl {
         `Authorization failed, principals: ${JSON.stringify(
           principals
         )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify(
-          savedObjectsBasicInfo
+          deniedObjects
         )}`
       );
     }

diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts
@@ -62,9 +62,9 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
 
     await this.client.setup(core);
 
+    this.proxyWorkspaceTrafficToRealHandler(core);
     this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled);
     if (isPermissionControlEnabled) {
-      this.proxyWorkspaceTrafficToRealHandler(core);
       this.permissionControl = new SavedObjectsPermissionControl(this.logger);
 
       this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper(

diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts
@@ -6,7 +6,7 @@
 import { schema } from '@osd/config-schema';
 import { CoreSetup, Logger, ensureRawRequest } from '../../../../core/server';
 import { WorkspacePermissionMode } from '../../common/constants';
-import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types';
+import { AuthInfo, IWorkspaceClientImpl, WorkspacePermissionItem } from '../types';
 
 const WORKSPACES_API_BASE_URL = '/api/workspaces';
 
@@ -125,7 +125,7 @@ export function registerRoutes({
     router.handleLegacyErrors(async (context, req, res) => {
       const { attributes, permissions: permissionsInRequest } = req.body;
       const rawRequest = ensureRawRequest(req);
-      const authInfo = rawRequest?.auth?.credentials?.authInfo as { user_name?: string } | null;
+      const authInfo = rawRequest?.auth?.credentials?.authInfo as AuthInfo | null;
       let permissions: WorkspacePermissionItem[] = [];
       if (permissionsInRequest) {
         permissions = Array.isArray(permissionsInRequest)