Rebase to new dashboard permission check

merico-dev · Apr 26, 2023 · 7579367 · 7579367
1 parent fd9a785
commit 7579367
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 27 deletions.
diff --git a/api/src/controller/dashboard_content.controller.ts b/api/src/controller/dashboard_content.controller.ts
@@ -54,10 +54,10 @@ export class DashboardContentController implements interfaces.Controller {
       await DashboardPermissionService.checkPermission(
         dashboard_id,
         'VIEW',
-        req.body.auth?.role_id >= ROLE_TYPES.ADMIN,
         req.locale,
-        req.body.auth ? (req.body.auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
         req.body.auth?.id,
+        req.body.auth ? (req.body.auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
+        req.body.auth?.role_id,
       );
       const result = await this.dashboardContentService.list(dashboard_id, filter, sort, pagination);
       res.json(result);
@@ -84,10 +84,10 @@ export class DashboardContentController implements interfaces.Controller {
       await DashboardPermissionService.checkPermission(
         dashboard_id,
         'EDIT',
-        req.body.auth?.role_id >= ROLE_TYPES.ADMIN,
         req.locale,
-        req.body.auth ? (req.body.auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
         req.body.auth?.id,
+        req.body.auth ? (req.body.auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
+        req.body.auth?.role_id,
       );
       const result = await this.dashboardContentService.create(dashboard_id, name, content, req.locale);
       res.json(result);
@@ -116,10 +116,10 @@ export class DashboardContentController implements interfaces.Controller {
       await DashboardPermissionService.checkPermission(
         result.dashboard_id,
         'VIEW',
-        req.body.auth?.role_id >= ROLE_TYPES.ADMIN,
         req.locale,
-        req.body.auth ? (req.body.auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
         req.body.auth?.id,
+        req.body.auth ? (req.body.auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
+        req.body.auth?.role_id,
       );
       res.json(result);
     } catch (err) {

diff --git a/api/src/controller/dashboard_permission.controller.ts b/api/src/controller/dashboard_permission.controller.ts
@@ -79,10 +79,10 @@ export class DashboardPermissionController implements interfaces.Controller {
       500: { description: 'SERVER ERROR', type: SwaggerDefinitionConstant.Response.Type.OBJECT, model: 'ApiError' },
     },
   })
-  @httpPost('/get', ensureAuthEnabled, permission(ROLE_TYPES.READER))
+  @httpPost('/get', ensureAuthEnabled, permission(ROLE_TYPES.READER), validate(DashboardPermissionGetRequest))
   public async get(req: express.Request, res: express.Response, next: express.NextFunction): Promise<void> {
     try {
-      const { id } = validate(DashboardPermissionGetRequest, req.body);
+      const { id } = req.body as DashboardPermissionGetRequest;
       const result = await this.dashboardPermissionService.get(id);
       res.json(result);
     } catch (err) {

diff --git a/api/src/services/dashboard_content.service.ts b/api/src/services/dashboard_content.service.ts
@@ -95,10 +95,10 @@ export class DashboardContentService {
     await DashboardPermissionService.checkPermission(
       dashboard.id,
       'EDIT',
-      auth ? auth.role_id >= ROLE_TYPES.ADMIN : false,
       locale,
-      auth ? (auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
       auth?.id,
+      auth ? (auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
+      auth?.role_id,
     );
     if (AUTH_ENABLED && dashboard.is_preset && (!auth?.role_id || auth.role_id < ROLE_TYPES.SUPERADMIN)) {
       throw new ApiError(BAD_REQUEST, { message: translate('DASHBOARD_CONTENT_EDIT_REQUIRES_SUPERADMIN', locale) });
@@ -136,10 +136,10 @@ export class DashboardContentService {
     await DashboardPermissionService.checkPermission(
       dashboard.id,
       'EDIT',
-      auth ? auth.role_id >= ROLE_TYPES.ADMIN : false,
       locale,
-      auth ? (auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
       auth?.id,
+      auth ? (auth instanceof ApiKey ? 'APIKEY' : 'ACCOUNT') : undefined,
+      auth?.role_id,
     );
     if (AUTH_ENABLED && dashboard.is_preset && (!auth?.role_id || auth.role_id < ROLE_TYPES.SUPERADMIN)) {
       throw new ApiError(BAD_REQUEST, {

diff --git a/api/tests/e2e/04_dashboard.test.ts b/api/tests/e2e/04_dashboard.test.ts
@@ -280,7 +280,9 @@ describe('DashboardController', () => {
         .send(query);
 
       expect(response.body.code).toEqual('NOT_FOUND');
-      expect(response.body.detail.message).toContain('Could not find any entity of type "Dashboard" matching');
+      expect(response.body.detail.message).toContain(
+        'Could not find any entity of type "DashboardPermission" matching',
+      );
       expect(response.body.detail.message).toContain(notFoundId);
     });
   });
@@ -354,7 +356,9 @@ describe('DashboardController', () => {
         .send(query);
 
       expect(response.body.code).toEqual('NOT_FOUND');
-      expect(response.body.detail.message).toContain('Could not find any entity of type "Dashboard" matching');
+      expect(response.body.detail.message).toContain(
+        'Could not find any entity of type "DashboardPermission" matching',
+      );
       expect(response.body.detail.message).toContain(notFoundId);
     });
 
@@ -432,7 +436,9 @@ describe('DashboardController', () => {
         .send(query);
 
       expect(response.body.code).toEqual('NOT_FOUND');
-      expect(response.body.detail.message).toContain('Could not find any entity of type "Dashboard" matching');
+      expect(response.body.detail.message).toContain(
+        'Could not find any entity of type "DashboardPermission" matching',
+      );
       expect(response.body.detail.message).toContain(notFoundId);
     });
 

diff --git a/api/tests/e2e/10_dashboard_permission.test.ts b/api/tests/e2e/10_dashboard_permission.test.ts
@@ -252,7 +252,7 @@ describe('DashboardPermissionController', () => {
       const query1: DashboardPermissionUpdateRequest = {
         id: dashboardId1,
         access: [
-          { type: 'ACCOUNT', id: readerAccount.id, permission: 'EDIT' },
+          { type: 'ACCOUNT', id: readerAccount.id, permission: 'VIEW' },
           { type: 'APIKEY', id: authorApiKey.id, permission: 'EDIT' },
           { type: 'ACCOUNT', id: authorAccount.id, permission: 'VIEW' },
           { type: 'APIKEY', id: readerApiKey.id, permission: 'VIEW' },
@@ -270,7 +270,7 @@ describe('DashboardPermissionController', () => {
         owner_id: superadminLogin.account.id,
         owner_type: 'ACCOUNT',
         access: [
-          { type: 'ACCOUNT', id: readerAccount.id, permission: 'EDIT' },
+          { type: 'ACCOUNT', id: readerAccount.id, permission: 'VIEW' },
           { type: 'APIKEY', id: authorApiKey.id, permission: 'EDIT' },
           { type: 'ACCOUNT', id: authorAccount.id, permission: 'VIEW' },
           { type: 'APIKEY', id: readerApiKey.id, permission: 'VIEW' },
@@ -296,7 +296,7 @@ describe('DashboardPermissionController', () => {
         owner_id: superadminLogin.account.id,
         owner_type: 'ACCOUNT',
         access: [
-          { type: 'ACCOUNT', id: readerAccount.id, permission: 'EDIT' },
+          { type: 'ACCOUNT', id: readerAccount.id, permission: 'VIEW' },
           { type: 'APIKEY', id: authorApiKey.id, permission: 'EDIT' },
         ],
       });
@@ -397,7 +397,6 @@ describe('DashboardPermissionController', () => {
       const query1: DashboardIDRequest = {
         id: dashboardId1,
       };
-
       const response1 = await server
         .post('/dashboard/details')
         .set('Authorization', `Bearer ${authorLogin.token}`)
@@ -406,11 +405,9 @@ describe('DashboardPermissionController', () => {
         code: 'FORBIDDEN',
         detail: { message: 'Insufficient privileges for this dashboard' },
       });
-
       const query2: DashboardIDRequest = {
         id: dashboardId2,
       };
-
       const response2 = await server
         .put('/dashboard/update')
         .set('Authorization', `Bearer ${authorLogin.token}`)
@@ -439,7 +436,7 @@ describe('DashboardPermissionController', () => {
         id: dashboardId1,
         owner_id: authorApiKey.id,
         owner_type: 'APIKEY',
-        access: [{ id: readerAccount.id, type: 'ACCOUNT', permission: 'EDIT' }],
+        access: [{ id: readerAccount.id, type: 'ACCOUNT', permission: 'VIEW' }],
       });
     });
 

diff --git a/api/tests/e2e/11_dashboard_content.test.ts b/api/tests/e2e/11_dashboard_content.test.ts
@@ -570,11 +570,10 @@ describe('DashboardContentController', () => {
         .set('Authorization', `Bearer ${superadminLogin.token}`)
         .send(query1);
 
-      expect(response2.body).toMatchObject({
-        total: 0,
-        offset: 0,
-        data: [],
-      });
+      expect(response2.body.code).toEqual('NOT_FOUND');
+      expect(response2.body.detail.message).toContain(
+        'Could not find any entity of type "DashboardPermission" matching',
+      );
     });
 
     it('should fail if not found', async () => {