From e28c9e7ec9472eba798b1ce358a1efa4249b4f03 Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 18 Aug 2022 10:22:09 +0200 Subject: [PATCH] add lastUsed --- .../services/__tests__/api-token.test.js | 97 +++++++++++++++++-- .../core/admin/server/services/api-token.js | 32 ++++-- 2 files changed, 110 insertions(+), 19 deletions(-) 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 c568a2703f..afbee17033 100644 --- a/packages/core/admin/server/services/__tests__/api-token.test.js +++ b/packages/core/admin/server/services/__tests__/api-token.test.js @@ -10,6 +10,8 @@ describe('API Token', () => { hexedString: '6170692d746f6b656e5f746573742d72616e646f6d2d6279746573', }; + const SELECT_FIELDS = ['id', 'name', 'description', 'lastUsed', 'type', 'createdAt', 'updatedAt']; + beforeAll(() => { jest .spyOn(crypto, 'randomBytes') @@ -42,7 +44,7 @@ describe('API Token', () => { const res = await apiTokenService.create(attributes); expect(create).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, data: { ...attributes, accessKey: apiTokenService.hash(mockedApiToken.hexedString), @@ -138,7 +140,7 @@ describe('API Token', () => { const res = await apiTokenService.list(); expect(findMany).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, orderBy: { name: 'ASC' }, populate: ['permissions'], }); @@ -166,7 +168,7 @@ describe('API Token', () => { const res = await apiTokenService.revoke(token.id); expect(mockedDelete).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { id: token.id }, populate: ['permissions'], }); @@ -185,7 +187,7 @@ describe('API Token', () => { const res = await apiTokenService.revoke(42); expect(mockedDelete).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { id: 42 }, populate: ['permissions'], }); @@ -213,7 +215,7 @@ describe('API Token', () => { const res = await apiTokenService.getById(token.id); expect(findOne).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { id: token.id }, populate: ['permissions'], }); @@ -232,7 +234,7 @@ describe('API Token', () => { const res = await apiTokenService.getById(42); expect(findOne).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { id: 42 }, populate: ['permissions'], }); @@ -285,7 +287,7 @@ describe('API Token', () => { }, }); expect(update).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { id }, data: attributes, populate: ['permissions'], @@ -397,7 +399,7 @@ describe('API Token', () => { }); expect(update).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { id }, data: omit(['permissions'], updatedAttributes), populate: expect.anything(), // it doesn't matter how this is used @@ -406,6 +408,81 @@ describe('API Token', () => { expect(res).toEqual(updatedAttributes); }); + test('Updates a non-permissions field of a custom token', async () => { + const id = 1; + + const originalToken = { + id, + name: 'api-token_tests-name', + description: 'api-token_tests-description', + type: 'custom', + permissions: ['admin::subject.keepThisAction', 'admin::subject.oldAction'], + }; + + const updatedAttributes = { + name: 'api-token_tests-updated-name', + type: 'custom', + }; + + const update = jest.fn(({ data }) => Promise.resolve(data)); + const findOne = jest.fn().mockResolvedValue(omit('permissions', originalToken)); + const deleteFn = jest.fn(); + const create = jest.fn(); + const load = jest + .fn() + // first call to load original permissions + .mockResolvedValueOnce( + Promise.resolve( + originalToken.permissions.map(p => { + return { + action: p, + }; + }) + ) + ) + // second call to check new permissions + .mockResolvedValueOnce( + Promise.resolve( + originalToken.permissions.map(p => { + return { + action: p, + }; + }) + ) + ); + + global.strapi = { + query() { + return { + update, + findOne, + delete: deleteFn, + create, + }; + }, + config: { + get: jest.fn(() => ''), + }, + entityService: { + load, + }, + }; + + const res = await apiTokenService.update(id, updatedAttributes); + + expect(update).toHaveBeenCalledWith({ + select: SELECT_FIELDS, + where: { id }, + data: omit(['permissions'], updatedAttributes), + populate: expect.anything(), // it doesn't matter how this is used + }); + + expect(res).toEqual({ + permissions: originalToken.permissions, + ...updatedAttributes, + }); + }); + describe('getByName', () => { const token = { id: 1, @@ -426,7 +503,7 @@ describe('API Token', () => { const res = await apiTokenService.getByName(token.name); expect(findOne).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { name: token.name }, populate: ['permissions'], }); @@ -445,7 +522,7 @@ describe('API Token', () => { const res = await apiTokenService.getByName('unexistant-name'); expect(findOne).toHaveBeenCalledWith({ - select: ['id', 'name', 'description', 'type', 'createdAt'], + select: SELECT_FIELDS, where: { name: 'unexistant-name' }, populate: ['permissions'], }); diff --git a/packages/core/admin/server/services/api-token.js b/packages/core/admin/server/services/api-token.js index f36d3c1ed8..c3cf165675 100644 --- a/packages/core/admin/server/services/api-token.js +++ b/packages/core/admin/server/services/api-token.js @@ -16,6 +16,7 @@ const constants = require('../services/constants'); * @property {string} name * @property {string} [description] * @property {string} accessKey + * @property {number} lastUsed * @property {TokenType} type * @property {(number|ApiTokenPermission)[]} [permissions] */ @@ -29,7 +30,7 @@ const constants = require('../services/constants'); */ /** @constant {Array} */ -const SELECT_FIELDS = ['id', 'name', 'description', 'type', 'createdAt']; +const SELECT_FIELDS = ['id', 'name', 'description', 'lastUsed', 'type', 'createdAt', 'updatedAt']; /** @constant {Array} */ const POPULATE_FIELDS = ['permissions']; @@ -50,6 +51,7 @@ const assertCustomTokenPermissionsValidity = attributes => { * @param {Object} whereParams * @param {string|number} [whereParams.id] * @param {string} [whereParams.name] + * @param {number} [whereParams.lastUsed] * @param {string} [whereParams.description] * @param {string} [whereParams.accessKey] * @@ -77,6 +79,7 @@ const hash = accessKey => { * @param {Object} attributes * @param {TokenType} attributes.type * @param {string} attributes.name + * @param {number} attributes.lastUsed * @param {string[]} [attributes.permissions] * @param {string} [attributes.description] * @@ -199,6 +202,8 @@ const getByName = async name => { * @param {Object} attributes * @param {TokenType} attributes.type * @param {string} attributes.name + * @param {number} attributes.lastUsed + * @param {string[]} [attributes.permissions] * @param {string} [attributes.description] * * @returns {Promise>} @@ -211,12 +216,19 @@ const update = async (id, attributes) => { throw new NotFoundError('Token not found'); } - // TODO: allow updating only the non-permissions attributes of a custom token - assertCustomTokenPermissionsValidity({ - ...omit(['permissions'], originalToken), - ...attributes, - type: attributes.type || originalToken.type, - }); + const changingTypeToCustom = + attributes.type === constants.API_TOKEN_TYPE.custom && + originalToken.type !== constants.API_TOKEN_TYPE.custom; + + // if we're updating the permissions on any token type, or changing from non-custom to custom, ensure they're still valid + // if neither type nor permissions are changing, we don't need to validate again or else we can't allow partial update + if (attributes.permissions || changingTypeToCustom) { + assertCustomTokenPermissionsValidity({ + ...originalToken, + ...attributes, + type: attributes.type || originalToken.type, + }); + } const updatedToken = await strapi.query('admin::api-token').update({ select: SELECT_FIELDS, @@ -225,7 +237,8 @@ const update = async (id, attributes) => { data: omit('permissions', attributes), }); - if (updatedToken.type === constants.API_TOKEN_TYPE.CUSTOM) { + // custom tokens need to have their permissions updated as well + if (updatedToken.type === constants.API_TOKEN_TYPE.CUSTOM && attributes.permissions) { const currentPermissionsResult = (await strapi.entityService.load('admin::api-token', updatedToken, 'permissions')) || []; @@ -284,7 +297,7 @@ const update = async (id, attributes) => { // method attempting to createMany permissions, then update token with those permissions -- createMany doesn't return the ids, and we can't query for them } // if type is not custom, make sure any old permissions get removed - else { + else if (updatedToken.type !== constants.API_TOKEN_TYPE.CUSTOM) { await strapi.query('admin::token-permission').delete({ where: { token: id }, }); @@ -307,6 +320,7 @@ const update = async (id, attributes) => { * @param {Object} whereParams * @param {string|number} [whereParams.id] * @param {string} [whereParams.name] + * @param {number} [whereParams.lastUsed] * @param {string} [whereParams.description] * @param {string} [whereParams.accessKey] *