diff --git a/packages/core/admin/server/controllers/__tests__/api-token.test.js b/packages/core/admin/server/controllers/__tests__/api-token.test.js index f3c58ac530..44b73b12d5 100644 --- a/packages/core/admin/server/controllers/__tests__/api-token.test.js +++ b/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': { diff --git a/packages/core/admin/server/services/__tests__/api-token.test.js b/packages/core/admin/server/services/__tests__/api-token.test.js index 8c861fb761..d33eaf5010 100644 --- a/packages/core/admin/server/services/__tests__/api-token.test.js +++ b/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, diff --git a/packages/core/admin/server/services/api-token.js b/packages/core/admin/server/services/api-token.js index e17cda29d3..a323a108a5 100644 --- a/packages/core/admin/server/services/api-token.js +++ b/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(', ')}`); + } + } }; /** diff --git a/packages/core/admin/server/tests/admin-api-token-crud.test.e2e.js b/packages/core/admin/server/tests/admin-api-token-crud.test.e2e.js index 70a63d31c8..8664527f00 100644 --- a/packages/core/admin/server/tests/admin-api-token-crud.test.e2e.js +++ b/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(