From 31d74d2710d0dd178798cf474e23bf63e4ed1937 Mon Sep 17 00:00:00 2001 From: Dieter Stinglhamber Date: Wed, 8 Sep 2021 14:38:43 +0200 Subject: [PATCH] allow for partial payload to update a token --- .../controllers/__tests__/api-token.test.js | 12 +-- .../admin/server/controllers/api-token.js | 17 ++-- .../services/__tests__/api-token.test.js | 46 ++++++++++- .../core/admin/server/services/api-token.js | 10 +++ .../server/tests/admin-api-token.test.e2e.js | 81 +++++++++++++++---- .../admin/server/validation/api-tokens.js | 6 +- 6 files changed, 138 insertions(+), 34 deletions(-) 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 c4736fb894..469dfb5b2e 100644 --- a/packages/core/admin/server/controllers/__tests__/api-token.test.js +++ b/packages/core/admin/server/controllers/__tests__/api-token.test.js @@ -210,7 +210,7 @@ describe('API Token Controller', () => { test('Fails if the name is already taken', async () => { const getById = jest.fn(() => ({ id, ...body })); - const exists = jest.fn(() => true); + const getByName = jest.fn(() => ({ id: 2, name: body.name })); const badRequest = jest.fn(); const ctx = createContext({ body, params: { id } }, { badRequest }); @@ -218,8 +218,8 @@ describe('API Token Controller', () => { admin: { services: { 'api-token': { - exists, getById, + getByName, }, }, }, @@ -227,7 +227,7 @@ describe('API Token Controller', () => { await apiTokenController.update(ctx); - expect(exists).toHaveBeenCalledWith({ name: body.name }); + expect(getByName).toHaveBeenCalledWith(body.name); expect(badRequest).toHaveBeenCalledWith('Name already taken'); }); @@ -255,7 +255,7 @@ describe('API Token Controller', () => { test('Updates API Token Successfully', async () => { const update = jest.fn().mockResolvedValue(body); const getById = jest.fn(() => ({ id, ...body })); - const exists = jest.fn(() => false); + const getByName = jest.fn(() => null); const badRequest = jest.fn(); const notFound = jest.fn(); const send = jest.fn(); @@ -266,7 +266,7 @@ describe('API Token Controller', () => { services: { 'api-token': { getById, - exists, + getByName, update, }, }, @@ -276,7 +276,7 @@ describe('API Token Controller', () => { await apiTokenController.update(ctx); expect(getById).toHaveBeenCalledWith(id); - expect(exists).toHaveBeenCalledWith({ name: body.name }); + expect(getByName).toHaveBeenCalledWith(body.name); expect(badRequest).not.toHaveBeenCalled(); expect(notFound).not.toHaveBeenCalled(); expect(update).toHaveBeenCalledWith(id, body); diff --git a/packages/core/admin/server/controllers/api-token.js b/packages/core/admin/server/controllers/api-token.js index 8574170826..a761842583 100644 --- a/packages/core/admin/server/controllers/api-token.js +++ b/packages/core/admin/server/controllers/api-token.js @@ -73,16 +73,19 @@ module.exports = { const { id } = ctx.params; const apiTokenService = getService('api-token'); + const attributes = body; /** * We trim both field to avoid having issues with either: * - having a space at the end or start of the value. * - having only spaces as value; */ - const attributes = { - name: trim(body.name), - description: trim(body.description), - type: body.type, - }; + if (has(attributes, 'name')) { + attributes.name = trim(body.name); + } + + if (has(attributes, 'description') || attributes.description === null) { + attributes.description = trim(body.description); + } try { await validateApiTokenUpdateInput(attributes); @@ -96,8 +99,8 @@ module.exports = { } if (has(attributes, 'name')) { - const nameAlreadyTaken = await apiTokenService.exists({ name: attributes.name }); - if (nameAlreadyTaken) { + const nameAlreadyTaken = await apiTokenService.getByName(attributes.name); + if (!!nameAlreadyTaken && nameAlreadyTaken.id !== id) { return ctx.badRequest('Name already taken'); } } 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 9624d25306..d7039e1027 100644 --- a/packages/core/admin/server/services/__tests__/api-token.test.js +++ b/packages/core/admin/server/services/__tests__/api-token.test.js @@ -199,7 +199,7 @@ describe('API Token', () => { }); }); - describe('get', () => { + describe('getById', () => { const token = { id: 1, name: 'api-token_tests-name', @@ -274,4 +274,48 @@ describe('API Token', () => { expect(res).toEqual(attributes); }); }); + describe('getByName', () => { + const token = { + id: 1, + name: 'api-token_tests-name', + description: 'api-token_tests-description', + type: 'read-only', + }; + + test('It retrieves the token', async () => { + const findOne = jest.fn().mockResolvedValue(token); + + global.strapi = { + query() { + return { findOne }; + }, + }; + + const res = await apiTokenService.getByName(token.name); + + expect(findOne).toHaveBeenCalledWith({ + select: ['id', 'name', 'description', 'type'], + where: { name: token.name }, + }); + expect(res).toEqual(token); + }); + + test('It returns `null` if the resource does not exist', async () => { + const findOne = jest.fn().mockResolvedValue(null); + + global.strapi = { + query() { + return { findOne }; + }, + }; + + const res = await apiTokenService.getByName('unexistant-name'); + + expect(findOne).toHaveBeenCalledWith({ + select: ['id', 'name', 'description', 'type'], + where: { name: 'unexistant-name' }, + }); + expect(res).toEqual(null); + }); + }); }); diff --git a/packages/core/admin/server/services/api-token.js b/packages/core/admin/server/services/api-token.js index c9f074f77d..d2a0409b2e 100644 --- a/packages/core/admin/server/services/api-token.js +++ b/packages/core/admin/server/services/api-token.js @@ -116,6 +116,15 @@ const getById = async id => { return strapi.query('admin::api-token').findOne({ select: SELECT_FIELDS, where: { id } }); }; +/** + * @param {string} name + * + * @returns {Promise>} + */ +const getByName = async name => { + return strapi.query('admin::api-token').findOne({ select: SELECT_FIELDS, where: { name } }); +}; + /** * @param {string|number} id * @param {Object} attributes @@ -140,4 +149,5 @@ module.exports = { revoke, getById, update, + getByName, }; diff --git a/packages/core/admin/server/tests/admin-api-token.test.e2e.js b/packages/core/admin/server/tests/admin-api-token.test.e2e.js index 56015361c7..1c413a7f5b 100644 --- a/packages/core/admin/server/tests/admin-api-token.test.e2e.js +++ b/packages/core/admin/server/tests/admin-api-token.test.e2e.js @@ -8,8 +8,8 @@ const { createAuthRequest } = require('../../../../../test/helpers/request'); * * N° Description * ------------------------------------------- - * 1. Fails to creates an api token (missing parameters from the body) - * 2. Fails to creates an api token (invalid `type` in the body) + * 1. Fails to create an api token (missing parameters from the body) + * 2. Fails to create an api token (invalid `type` in the body) * 3. Creates an api token (successfully) * 4. Creates an api token without a description (successfully) * 5. Creates an api token with trimmed description and name (successfully) @@ -20,8 +20,10 @@ const { createAuthRequest } = require('../../../../../test/helpers/request'); * 10. Returns a 404 if the ressource to retrieve does not exist * 11. Updates a token (successfully) * 12. Returns a 404 if the ressource to update does not exist - * 13. Fails to creates an api token (missing parameters from the body) - * 14. Fails to creates an api token (invalid `type` in the body) + * 13. Updates a token with partial payload (successfully) + * 14. Fails to update an api token (invalid `type` in the body) + * 15. Updates a token when passing a `null` description (successfully) + * 16. Updates a token but not the description of no description is passed (successfully) */ describe('Admin API Token CRUD (e2e)', () => { @@ -262,12 +264,14 @@ describe('Admin API Token CRUD (e2e)', () => { type: body.type, id: apiTokens[0].id, }); + + apiTokens[0] = res.body.data; }); test('12. Returns a 404 if the ressource to update does not exist', async () => { const body = { name: 'api-token_tests-updated-name', - description: 'api-token_tests-description', + description: 'api-token_tests-updated-description', type: 'read-only', }; @@ -281,27 +285,26 @@ describe('Admin API Token CRUD (e2e)', () => { expect(res.body.data).toBeUndefined(); }); - test('13. Fails to update an api token (missing parameters from the body)', async () => { + test('13. Updates a token with partial payload (successfully)', async () => { const body = { - name: 'api-token_tests-updated-name', - description: 'api-token_tests-description', + description: 'api-token_tests-re-updated-description', }; const res = await rq({ - url: '/admin/api-tokens/1', + url: `/admin/api-tokens/${apiTokens[0].id}`, method: 'PUT', body, }); - expect(res.statusCode).toBe(400); - expect(res.body).toMatchObject({ - statusCode: 400, - error: 'Bad Request', - message: 'ValidationError', - data: { - type: ['type is a required field'], - }, + expect(res.statusCode).toBe(200); + expect(res.body.data).toMatchObject({ + name: apiTokens[0].name, + description: body.description, + type: apiTokens[0].type, + id: apiTokens[0].id, }); + + apiTokens[0] = res.body.data; }); test('14. Fails to update an api token (invalid `type` in the body)', async () => { @@ -327,4 +330,48 @@ describe('Admin API Token CRUD (e2e)', () => { }, }); }); + + test('15. Updates a token when passing a `null` description (successfully)', async () => { + const body = { + description: null, + }; + + const res = await rq({ + url: `/admin/api-tokens/${apiTokens[0].id}`, + method: 'PUT', + body, + }); + + expect(res.statusCode).toBe(200); + expect(res.body.data).toMatchObject({ + name: apiTokens[0].name, + description: '', + type: apiTokens[0].type, + id: apiTokens[0].id, + }); + + apiTokens[0] = res.body.data; + }); + + test('16. Updates a token but not the description of no description is passed (successfully)', async () => { + const body = { + name: 'api-token_tests-name', + }; + + const res = await rq({ + url: `/admin/api-tokens/${apiTokens[0].id}`, + method: 'PUT', + body, + }); + + expect(res.statusCode).toBe(200); + expect(res.body.data).toMatchObject({ + name: body.name, + description: apiTokens[0].description, + type: apiTokens[0].type, + id: apiTokens[0].id, + }); + + apiTokens[0] = res.body.data; + }); }); diff --git a/packages/core/admin/server/validation/api-tokens.js b/packages/core/admin/server/validation/api-tokens.js index 86aa22c8c5..d1e3bed4b3 100644 --- a/packages/core/admin/server/validation/api-tokens.js +++ b/packages/core/admin/server/validation/api-tokens.js @@ -32,12 +32,12 @@ const apiTokenUpdateSchema = yup name: yup .string() .min(1) - .required(), - description: yup.string().optional(), + .notNull(), + description: yup.string().nullable(), type: yup .string() .oneOf(Object.values(constants.API_TOKEN_TYPE)) - .required(), + .notNull(), }) .noUnknown();