Validate permissions for api-tokens & throw on unknown permissions

packages/core/admin/server/controllers/__tests__/api-token.test.js

@@ -18,6 +18,17 @@ describe('API Token Controller', () => {
       const ctx = createContext({ body });
 
       global.strapi = {
+        contentAPI: {
+          permissions: {
+            providers: {
+              action: {
+                keys() {
+                  return ['foo', 'bar'];
+                },
+              },
+            },
+          },
+        },
         admin: {
           services: {
             'api-token': {

packages/core/admin/server/services/__tests__/api-token.test.js

@@ -1,10 +1,16 @@
 'use strict';
 
-const { NotFoundError } = require('@strapi/utils/lib/errors');
+const { NotFoundError, ApplicationError } = require('@strapi/utils/lib/errors');
 const crypto = require('crypto');
 const { omit, uniq } = require('lodash/fp');
 const apiTokenService = require('../api-token');
 
+const getActionProvider = (actions = []) => {
+  return {
+    contentAPI: { permissions: { providers: { action: { keys: jest.fn(() => actions) } } } },
+  };
+};
+
 describe('API Token', () => {
   const mockedApiToken = {
     randomBytes: 'api-token_test-random-bytes',
@@ -133,6 +139,7 @@ describe('API Token', () => {
       );
 
       global.strapi = {
+        ...getActionProvider(['admin::content.content.read']),
         query() {
           return {
             findOne,
@@ -215,6 +222,7 @@ describe('API Token', () => {
       );
 
       global.strapi = {
+        ...getActionProvider(['api::foo.foo.find', 'api::foo.foo.create']),
         query() {
           return {
             findOne,
@@ -234,6 +242,56 @@ describe('API Token', () => {
       expect(res.permissions).toHaveLength(2);
       expect(res.permissions).toEqual(['api::foo.foo.find', 'api::foo.foo.create']);
     });
+
+    test('Creates a custom token with invalid permissions should throw', async () => {
+      const attributes = {
+        name: 'api-token_tests-name',
+        description: 'api-token_tests-description',
+        type: 'custom',
+        permissions: ['valid-permission', 'unknown-permission-A', 'unknown-permission-B'],
+      };
+      const createTokenResult = {
+        ...attributes,
+        lifespan: null,
+        expiresAt: null,
+        id: 1,
+      };
+
+      const create = jest.fn().mockResolvedValue(createTokenResult);
+      const load = jest.fn().mockResolvedValueOnce(
+        Promise.resolve(
+          uniq(attributes.permissions).map((p) => {
+            return {
+              action: p,
+            };
+          })
+        )
+      );
+
+      global.strapi = {
+        ...getActionProvider(['valid-permission']),
+        query() {
+          return {
+            create,
+          };
+        },
+        config: {
+          get: jest.fn(() => ''),
+        },
+        entityService: {
+          load,
+        },
+      };
+
+      expect(() => apiTokenService.create(attributes)).rejects.toThrowError(
+        new ApplicationError(
+          `Unknown permissions provided: unknown-permission-A, unknown-permission-B`
+        )
+      );
+
+      expect(load).not.toHaveBeenCalled();
+      expect(create).not.toHaveBeenCalled();
+    });
   });
 
   describe('checkSaltIsDefined', () => {
@@ -580,6 +638,12 @@ describe('API Token', () => {
       );
 
     global.strapi = {
+      ...getActionProvider([
+        'admin::subject.keepThisAction',
+        'admin::subject.newAction',
+        'admin::subject.newAction',
+        'admin::subject.otherAction',
+      ]),
       query() {
         return {
           update,
@@ -717,6 +781,45 @@ describe('API Token', () => {
     });
   });
 
+  test('Updates permissions field of a custom token with unknown permissions', async () => {
+    const id = 1;
+
+    const updatedAttributes = {
+      permissions: ['valid-permission-A', 'unknown-permission'],
+    };
+
+    const update = jest.fn(({ data }) => Promise.resolve(data));
+    const deleteFn = jest.fn();
+    const create = jest.fn();
+    const load = jest.fn();
+
+    global.strapi = {
+      ...getActionProvider(['valid-permission-A']),
+      query() {
+        return {
+          update,
+          delete: deleteFn,
+          create,
+        };
+      },
+      config: {
+        get: jest.fn(() => ''),
+      },
+      entityService: {
+        load,
+      },
+    };
+
+    expect(() => apiTokenService.update(id, updatedAttributes)).rejects.toThrowError(
+      new ApplicationError(`Unknown permissions provided: unknown-permission`)
+    );
+
+    expect(update).not.toHaveBeenCalled();
+    expect(deleteFn).not.toHaveBeenCalled();
+    expect(create).not.toHaveBeenCalled();
+    expect(load).not.toHaveBeenCalled();
+  });
+
   describe('getByName', () => {
     const token = {
       id: 1,

packages/core/admin/server/services/api-token.js

@@ -65,6 +65,16 @@ const assertCustomTokenPermissionsValidity = (attributes) => {
   if (attributes.type === constants.API_TOKEN_TYPE.CUSTOM && isEmpty(attributes.permissions)) {
     throw new ValidationError('Missing permissions attribute for custom token');
   }
+
+  // Permissions provided for a custom type token should be valid/registered permissions UID
+  if (attributes.type === constants.API_TOKEN_TYPE.CUSTOM) {
+    const validPermissions = strapi.contentAPI.permissions.providers.action.keys();
+    const invalidPermissions = difference(attributes.permissions, validPermissions);
+
+    if (!isEmpty(invalidPermissions)) {
+      throw new ValidationError(`Unknown permissions provided: ${invalidPermissions.join(', ')}`);
+    }
+  }
 };
 
 /**

packages/core/admin/server/tests/admin-api-token-crud.test.e2e.js

@@ -304,11 +304,6 @@ describe('Admin API Token v2 CRUD (e2e)', () => {
     });
   });
 
-  /**
-   * TODO: Discuss: Which behaviour do we want? Should an empty array be treated the same as omitted/undefined?
-   * Easy to change in assertCustomTokenPermissionsValidity by checking isEmpty (to allow empty) vs !attributes.permissions
-   */
-
   test('Creates a non-custom api token with empty permissions attribute', async () => {
     const body = {
       name: 'api-token_tests-fullAccessFailWithEmptyPermissions',
@@ -340,6 +335,11 @@ describe('Admin API Token v2 CRUD (e2e)', () => {
   });
 
   test('Creates a custom api token', async () => {
+    strapi.contentAPI.permissions.providers.action.keys = jest.fn(() => [
+      'admin::subject.action',
+      'plugin::foo.bar.action',
+    ]);
+
     const body = {
       name: 'api-token_tests-customSuccess',
       description: 'api-token_tests-description',
@@ -395,6 +395,34 @@ describe('Admin API Token v2 CRUD (e2e)', () => {
     });
   });
 
+  test('Fails to create a custom api token with unknown permissions', async () => {
+    strapi.contentAPI.permissions.providers.action.keys = jest.fn(() => ['action-A', 'action-B']);
+
+    const body = {
+      name: 'api-token_tests-customFail',
+      description: 'api-token_tests-description',
+      type: 'custom',
+      permissions: ['action-A', 'action-C'],
+    };
+
+    const res = await rq({
+      url: '/admin/api-tokens',
+      method: 'POST',
+      body,
+    });
+
+    expect(res.statusCode).toBe(400);
+    expect(res.body).toMatchObject({
+      data: null,
+      error: {
+        status: 400,
+        name: 'ValidationError',
+        message: 'Unknown permissions provided: action-C',
+        details: {},
+      },
+    });
+  });
+
   test('Creates an api token without a description (successfully)', async () => {
     const body = {
       name: 'api-token_tests-without-description',
@@ -455,6 +483,11 @@ describe('Admin API Token v2 CRUD (e2e)', () => {
   test('List all tokens (successfully)', async () => {
     await deleteAllTokens();
 
+    strapi.contentAPI.permissions.providers.action.keys = jest.fn(() => [
+      'admin::model.model.read',
+      'admin::model.model.create',
+    ]);
+
     // create 4 tokens
     const tokens = [];
     tokens.push(