From 4f524e9f94a4ca23c4ae12f15d9f5149ec02aad3 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Wed, 10 May 2023 15:16:12 -0400 Subject: [PATCH] feat: token rolling API for admins + tests --- public/language/en-GB/admin/settings/api.json | 3 +- .../components/schemas/admin/tokenObject.yaml | 24 +++++ public/openapi/read/admin/settings/api.yaml | 14 +-- public/openapi/write.yaml | 6 ++ public/openapi/write/admin/tokens.yaml | 32 +++++++ public/openapi/write/admin/tokens/token.yaml | 89 +++++++++++++++++++ .../write/admin/tokens/token/roll.yaml | 25 ++++++ public/src/admin/settings/api.js | 48 ++++++---- src/api/users.js | 8 +- src/api/utils.js | 24 ++++- src/controllers/write/admin.js | 11 ++- src/routes/authentication.js | 5 +- src/routes/write/admin.js | 1 + src/views/admin/settings/api.tpl | 3 + test/api.js | 41 ++++++++- 15 files changed, 297 insertions(+), 37 deletions(-) create mode 100644 public/openapi/components/schemas/admin/tokenObject.yaml create mode 100644 public/openapi/write/admin/tokens.yaml create mode 100644 public/openapi/write/admin/tokens/token.yaml create mode 100644 public/openapi/write/admin/tokens/token/roll.yaml diff --git a/public/language/en-GB/admin/settings/api.json b/public/language/en-GB/admin/settings/api.json index e7a3cfe375..6fe1c23cc9 100644 --- a/public/language/en-GB/admin/settings/api.json +++ b/public/language/en-GB/admin/settings/api.json @@ -22,5 +22,6 @@ "no-description": "No description specified.", "actions": "Actions", - "delete-confirm": "Are you sure you wish to delete this token? It will not be recoverable." + "delete-confirm": "Are you sure you wish to delete this token? It will not be recoverable.", + "roll-confirm": "Are you sure you wish to regenerate this token? The old token will be immediately revoked and will not be recoverable." } \ No newline at end of file diff --git a/public/openapi/components/schemas/admin/tokenObject.yaml b/public/openapi/components/schemas/admin/tokenObject.yaml new file mode 100644 index 0000000000..fab6c269c4 --- /dev/null +++ b/public/openapi/components/schemas/admin/tokenObject.yaml @@ -0,0 +1,24 @@ +TokenObject: + type: object + properties: + uid: + type: number + description: A valid user id + description: + type: string + description: Optional descriptor to differentiate tokens. + token: + type: string + description: An API token that can be called against this API via Bearer Authentication. + timestamp: + type: number + timestampISO: + type: string + description: An ISO 8601 formatted date string (complementing `timestamp`) + lastSeen: + type: number + nullable: true + lastSeenISO: + type: string + description: An ISO 8601 formatted date string (complementing `lastSeen`) + nullable: true \ No newline at end of file diff --git a/public/openapi/read/admin/settings/api.yaml b/public/openapi/read/admin/settings/api.yaml index c36f11d9a6..1e955d32c8 100644 --- a/public/openapi/read/admin/settings/api.yaml +++ b/public/openapi/read/admin/settings/api.yaml @@ -11,14 +11,8 @@ get: allOf: - type: object properties: - lastSeen: - type: object - description: A key-value set of API tokens and a UNIX timestamp of when it was last used - properties: {} - additionalProperties: {} - lastSeenISO: - type: object - description: A key-value set of API tokens and an ISO 8601 formatted date string of when it was last used - properties: {} - additionalProperties: {} + tokens: + type: array + items: + $ref: ../../../components/schemas/admin/tokenObject.yaml#/TokenObject - $ref: ../../../components/schemas/CommonProps.yaml#/CommonProps \ No newline at end of file diff --git a/public/openapi/write.yaml b/public/openapi/write.yaml index efc9c2bc46..101da200d1 100644 --- a/public/openapi/write.yaml +++ b/public/openapi/write.yaml @@ -198,6 +198,12 @@ paths: $ref: 'write/admin/analytics.yaml' /admin/analytics/{set}: $ref: 'write/admin/analytics/set.yaml' + /admin/tokens: + $ref: 'write/admin/tokens.yaml' + /admin/tokens/{token}: + $ref: 'write/admin/tokens/token.yaml' + /admin/tokens/{token}/roll: + $ref: 'write/admin/tokens/token/roll.yaml' /files/: $ref: 'write/files.yaml' /files/folder: diff --git a/public/openapi/write/admin/tokens.yaml b/public/openapi/write/admin/tokens.yaml new file mode 100644 index 0000000000..c5d9cb46df --- /dev/null +++ b/public/openapi/write/admin/tokens.yaml @@ -0,0 +1,32 @@ +post: + tags: + - admin + summary: create token + description: This operation creates a new API token for access to the Read and Write APIs. + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + uid: + type: number + description: The generated token will make calls against NodeBB as this user. + example: 1 + description: + type: string + description: Optional descriptor to differentiate tokens. + example: 'My new token.' + responses: + '200': + description: token successfully created + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../components/schemas/Status.yaml#/Status + response: + $ref: ../../components/schemas/admin/tokenObject.yaml#/TokenObject diff --git a/public/openapi/write/admin/tokens/token.yaml b/public/openapi/write/admin/tokens/token.yaml new file mode 100644 index 0000000000..99f6718d93 --- /dev/null +++ b/public/openapi/write/admin/tokens/token.yaml @@ -0,0 +1,89 @@ +get: + tags: + - admin + summary: get token + description: This operation retrieves an API token and its associated metadata + parameters: + - in: path + name: token + schema: + type: string + required: true + description: a valid API token + example: 4eb506f8-a173-4693-a41b-e23604bc973a + responses: + '200': + description: token successfully retrieved + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../components/schemas/Status.yaml#/Status + response: + $ref: ../../../components/schemas/admin/tokenObject.yaml#/TokenObject +put: + tags: + - admin + summary: update token + description: This operation updates a token's metadata. + parameters: + - in: path + name: token + schema: + type: string + required: true + description: a valid API token + example: 4eb506f8-a173-4693-a41b-e23604bc973a + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + uid: + type: number + description: The generated token will make calls against NodeBB as this user. + example: 1 + description: + type: string + description: Optional descriptor to differentiate tokens. + example: 'My new token.' + responses: + '200': + description: Token metadata updated. + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../components/schemas/Status.yaml#/Status + response: + $ref: ../../../components/schemas/admin/tokenObject.yaml#/TokenObject +delete: + tags: + - admin + summary: revoke token + description: This operation revokes a token and removes it from the database + parameters: + - in: path + name: token + schema: + type: string + required: true + description: a valid API token + example: 4eb506f8-a173-4693-a41b-e23604bc973a + responses: + '200': + description: Token metadata updated. + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../components/schemas/Status.yaml#/Status + response: {} \ No newline at end of file diff --git a/public/openapi/write/admin/tokens/token/roll.yaml b/public/openapi/write/admin/tokens/token/roll.yaml new file mode 100644 index 0000000000..f0bc7ae4f2 --- /dev/null +++ b/public/openapi/write/admin/tokens/token/roll.yaml @@ -0,0 +1,25 @@ +post: + tags: + - admin + summary: regenerate token + description: This operation regenerates an existing token. The previous token is immediately invalidated. + parameters: + - in: path + name: token + schema: + type: string + required: true + description: a valid API token + example: 4eb506f8-a173-4693-a41b-e23604bc973a + responses: + '200': + description: Token regenerated. + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../../components/schemas/Status.yaml#/Status + response: + $ref: ../../../../components/schemas/admin/tokenObject.yaml#/TokenObject \ No newline at end of file diff --git a/public/src/admin/settings/api.js b/public/src/admin/settings/api.js index a3687ee87f..4420e9a300 100644 --- a/public/src/admin/settings/api.js +++ b/public/src/admin/settings/api.js @@ -1,6 +1,6 @@ 'use strict'; -define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', 'api'], function (settings, clipboard, bootbox, Benchpress, api) { +define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', 'api', 'alerts'], function (settings, clipboard, bootbox, Benchpress, api, alerts) { const ACP = {}; ACP.init = function () { @@ -39,6 +39,10 @@ define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', case 'delete': handleTokenDeletion(subselector); break; + + case 'roll': + handleTokenRolling(subselector); + break; } } }); @@ -57,24 +61,13 @@ define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', const description = formData.get('description'); try { - const token = await api.post('/admin/tokens', { uid, description }); - + const tokenObj = await api.post('/admin/tokens', { uid, description }); if (!tokensTableBody) { modal.modal('hide'); return ajaxify.refresh(); } - const now = new Date(); - const tokenObj = { - token, - uid, - description, - timestamp: now.getTime(), - timestampISO: now.toISOString(), - lastSeen: null, - lastSeenISO: new Date(0).toISOString(), - }; - ajaxify.data.tokens.append(tokenObj); + ajaxify.data.tokens.push(tokenObj); const rowEl = (await app.parseAndTranslate(ajaxify.data.template.name, 'tokens', { tokens: [tokenObj], })).get(0); @@ -83,7 +76,7 @@ define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', $(rowEl).find('.timeago').timeago(); modal.modal('hide'); } catch (e) { - app.alertError(e); + alerts.error(e); } } @@ -126,7 +119,7 @@ define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', $(newEl).find('.timeago').timeago(); modal.modal('hide'); } catch (e) { - app.alertError(e); + alerts.error(e); } } @@ -156,7 +149,7 @@ define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', try { await api.del(`/admin/tokens/${token}`); } catch (e) { - app.alertError(e); + alerts.error(e); } rowEl.remove(); @@ -164,5 +157,26 @@ define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', }); } + async function handleTokenRolling(el) { + const rowEl = el.closest('[data-token]'); + const token = rowEl.getAttribute('data-token'); + + bootbox.confirm('[[admin/settings/api:roll-confirm]]', async (ok) => { + if (ok) { + try { + const tokenObj = await api.post(`/admin/tokens/${token}/roll`); + const newEl = (await app.parseAndTranslate(ajaxify.data.template.name, 'tokens', { + tokens: [tokenObj], + })).get(0); + + rowEl.replaceWith(newEl); + $(newEl).find('.timeago').timeago(); + } catch (e) { + alerts.error(e); + } + } + }); + } + return ACP; }); diff --git a/src/api/users.js b/src/api/users.js index ef0ef52500..84fe4b2b3b 100644 --- a/src/api/users.js +++ b/src/api/users.js @@ -18,9 +18,8 @@ const plugins = require('../plugins'); const events = require('../events'); const translator = require('../translator'); const sockets = require('../socket.io'); -const utils = require('../utils'); -const api = require('.'); +// const api = require('.'); const usersAPI = module.exports; @@ -310,15 +309,18 @@ usersAPI.unmute = async function (caller, data) { }; usersAPI.generateToken = async (caller, { uid, description }) => { + const api = require('.'); await hasAdminPrivilege(caller.uid, 'settings'); if (parseInt(uid, 10) !== parseInt(caller.uid, 10)) { throw new Error('[[error:invalid-uid]]'); } - return await api.utils.tokens.generate({ uid, description }); + const tokenObj = await api.utils.tokens.generate({ uid, description }); + return tokenObj.token; }; usersAPI.deleteToken = async (caller, { uid, token }) => { + const api = require('.'); await hasAdminPrivilege(caller.uid, 'settings'); if (parseInt(uid, 10) !== parseInt(caller.uid, 10)) { throw new Error('[[error:invalid-uid]]'); diff --git a/src/api/utils.js b/src/api/utils.js index 7bfc7c4e6e..d0bdc82f85 100644 --- a/src/api/utils.js +++ b/src/api/utils.js @@ -36,7 +36,7 @@ utils.tokens.get = async (tokens) => { tokenObjs.forEach((tokenObj, idx) => { tokenObj.token = tokens[idx]; tokenObj.lastSeen = lastSeen[idx]; - tokenObj.lastSeenISO = new Date(lastSeen[idx]).toISOString(); + tokenObj.lastSeenISO = lastSeen[idx] ? new Date(lastSeen[idx]).toISOString() : null; tokenObj.timestampISO = new Date(parseInt(tokenObj.timestamp, 10)).toISOString(); }); @@ -80,6 +80,28 @@ utils.tokens.update = async (token, { uid, description }) => { return await utils.tokens.get(token); }; +utils.tokens.roll = async (token) => { + const [createTime, uid, lastSeen] = await db.sortedSetsScore([`tokens:createtime`, `tokens:uid`, `tokens:lastSeen`], token); + const newToken = srcUtils.generateUUID(); + + const updates = [ + db.rename(`token:${token}`, `token:${newToken}`), + db.sortedSetRemove(`tokens:createtime`, token), + db.sortedSetRemove(`tokens:uid`, token), + db.sortedSetRemove(`tokens:lastSeen`, token), + db.sortedSetAdd(`tokens:createtime`, createTime, newToken), + db.sortedSetAdd(`tokens:uid`, uid, newToken), + ]; + + if (lastSeen) { + updates.push(db.sortedSetAdd(`tokens:lastSeen`, lastSeen, newToken)); + } + + await Promise.all(updates); + + return newToken; +}; + utils.tokens.delete = async (token) => { await Promise.all([ db.delete(`token:${token}`), diff --git a/src/controllers/write/admin.js b/src/controllers/write/admin.js index a0c6f082e9..b7ba39db10 100644 --- a/src/controllers/write/admin.js +++ b/src/controllers/write/admin.js @@ -31,7 +31,8 @@ Admin.getAnalyticsData = async (req, res) => { Admin.generateToken = async (req, res) => { const { uid, description } = req.body; - helpers.formatApiResponse(200, res, await api.utils.tokens.generate({ uid, description })); + const token = await api.utils.tokens.generate({ uid, description }); + helpers.formatApiResponse(200, res, await api.utils.tokens.get(token)); }; Admin.getToken = async (req, res) => { @@ -39,13 +40,19 @@ Admin.getToken = async (req, res) => { }; Admin.updateToken = async (req, res) => { - // todo: token rolling via req.body const { uid, description } = req.body; const { token } = req.params; helpers.formatApiResponse(200, res, await api.utils.tokens.update(token, { uid, description })); }; +Admin.rollToken = async (req, res) => { + let { token } = req.params; + + token = await api.utils.tokens.roll(token); + helpers.formatApiResponse(200, res, await api.utils.tokens.get(token)); +}; + Admin.deleteToken = async (req, res) => { const { token } = req.params; helpers.formatApiResponse(200, res, await api.utils.tokens.delete(token)); diff --git a/src/routes/authentication.js b/src/routes/authentication.js index e2b1cd73cf..74c778b02e 100644 --- a/src/routes/authentication.js +++ b/src/routes/authentication.js @@ -10,6 +10,7 @@ const meta = require('../meta'); const controllers = require('../controllers'); const helpers = require('../controllers/helpers'); const plugins = require('../plugins'); +const api = require('../api'); const { generateToken } = require('../middleware/csrf'); let loginStrategies = []; @@ -45,8 +46,8 @@ Auth.getLoginStrategies = function () { }; Auth.verifyToken = async function (token, done) { - const { tokens = [] } = await meta.settings.get('core.api'); - const tokenObj = tokens.find(t => t.token === token); + const tokens = await api.utils.tokens.list(); + const tokenObj = tokens.filter((t => t.token === token)).pop(); const uid = tokenObj ? tokenObj.uid : undefined; if (uid !== undefined) { diff --git a/src/routes/write/admin.js b/src/routes/write/admin.js index 63645d40fc..2571b8dd01 100644 --- a/src/routes/write/admin.js +++ b/src/routes/write/admin.js @@ -19,6 +19,7 @@ module.exports = function () { setupApiRoute(router, 'get', '/tokens/:token', [...middlewares], controllers.write.admin.getToken); setupApiRoute(router, 'put', '/tokens/:token', [...middlewares], controllers.write.admin.updateToken); setupApiRoute(router, 'delete', '/tokens/:token', [...middlewares], controllers.write.admin.deleteToken); + setupApiRoute(router, 'post', '/tokens/:token/roll', [...middlewares], controllers.write.admin.rollToken); return router; }; diff --git a/src/views/admin/settings/api.tpl b/src/views/admin/settings/api.tpl index 2734b2989a..b348f22d82 100644 --- a/src/views/admin/settings/api.tpl +++ b/src/views/admin/settings/api.tpl @@ -60,6 +60,9 @@ + diff --git a/test/api.js b/test/api.js index fb850da632..6eddd17f52 100644 --- a/test/api.js +++ b/test/api.js @@ -56,8 +56,23 @@ describe('API', async () => { example: '', // to be defined later... }, ], + '/admin/tokens/{token}': [ + { + in: 'path', + name: 'token', + example: '', // to be defined later... + }, + ], + }, + post: { + '/admin/tokens/{token}/roll': [ + { + in: 'path', + name: 'token', + example: '', // to be defined later... + }, + ], }, - post: {}, put: { '/groups/{slug}/pending/{uid}': [ { @@ -71,6 +86,13 @@ describe('API', async () => { example: '', // to be defined later... }, ], + '/admin/tokens/{token}': [ + { + in: 'path', + name: 'token', + example: '', // to be defined later... + }, + ], }, patch: {}, delete: { @@ -134,6 +156,13 @@ describe('API', async () => { example: '', // to be defined later... }, ], + '/admin/tokens/{token}': [ + { + in: 'path', + name: 'token', + example: '', // to be defined later... + }, + ], }, }; @@ -170,6 +199,16 @@ describe('API', async () => { } await groups.join('administrators', adminUid); + // Create api token for testing read/updating/deletion + const token = await api.utils.tokens.generate({ uid: adminUid }); + mocks.get['/admin/tokens/{token}'][0].example = token; + mocks.put['/admin/tokens/{token}'][0].example = token; + mocks.delete['/admin/tokens/{token}'][0].example = token; + + // Create another token for testing rolling + const token2 = await api.utils.tokens.generate({ uid: adminUid }); + mocks.post['/admin/tokens/{token}/roll'][0].example = token2; + // Create sample group await groups.create({ name: 'Test Group',