feat: api token migration, new ACP tokens list, token creation

public/language/en-GB/admin/settings/api.json

@@ -3,17 +3,20 @@
 	"settings": "Settings",
 	"lead-text": "From this page you can configure access to the Write API in NodeBB.",
 	"intro": "By default, the Write API authenticates users based on their session cookie, but NodeBB also supports Bearer authentication via tokens generated via this page.",
+	"warning": "<strong>Be advised</strong> — treat tokens like passwords. If they are leaked, your account should be considered compromised.",
 	"docs": "Click here to access the full API specification",
 
 	"require-https": "Require API usage via HTTPS only",
 	"require-https-caveat": "<strong>Note</strong>: Some installations involving load balancers may proxy their requests to NodeBB using HTTP, in which case this option should remain disabled.",
 
 	"uid": "User ID",
+	"token": "Token",
 	"uid-help-text": "Specify a User ID to associate with this token. If the user ID is <code>0</code>, it will be considered a <em>master</em> token, which can assume the identity of other users based on the <code>_uid</code> parameter",
 	"description": "Description",
-	"last-seen-ago": "Last used <span class=\"timeago\" title=\"%1\"></span>.",
+	"last-seen": "Last seen",
-	"last-seen-on": "Last used on <span class=\"timeago\" title=\"%1\"></span>.",
+	"created": "Created",
+	"create-token": "Create Token",
+	"master-token": "Master token",
 	"last-seen-never": "This key has never been used.",
-	"no-description": "No description specified.",
+	"no-description": "No description specified."
-	"token-on-save": "Token will be generated once form is saved"
 }

public/scss/admin/admin.scss

@@ -9,6 +9,7 @@
 @import "./manage/privileges";
 @import "./manage/tags";
 @import "./manage/groups";
+@import "./settings/api";
 @import "./appearance/customise";
 @import "./extend/plugins";
 @import "./extend/rewards";

public/scss/admin/settings/api.scss

@@ -0,0 +1,7 @@
+.template-admin-settings-api {
+	[data-action="copy"]:active {
+		i::before {
+			content: '\f00c';
+		}
+	}
+}

public/src/admin/settings/api.js

@@ -1,53 +1,78 @@
 'use strict';
 
-define('admin/settings/api', ['settings', 'alerts', 'hooks'], function (settings, alerts, hooks) {
+define('admin/settings/api', ['settings', 'clipboard', 'bootbox', 'benchpress', 'api'], function (settings, clipboard, bootbox, Benchpress, api) {
 	const ACP = {};
 
 	ACP.init = function () {
 		settings.load('core.api', $('.core-api-settings'));
-		$('#save').on('click', saveSettings);
+		$('#save').on('click', () => {
+			settings.save('core.api', $('.core-api-settings'));
+		});
 
-		hooks.on('filter:settings.sorted-list.loadItem', ({ item }) => {
+		// Click to copy
-			if (!ajaxify.data.lastSeen[item.token]) {
+		const copyEls = document.querySelectorAll('[data-component="acp/tokens"] [data-action="copy"]');
-				item.lastSeen = '[[admin/settings/api:last-seen-never]]';
+		new clipboard(copyEls);
-				return { item };
-			}
 
-			const cutoffMs = 1000 * 60 * 60 * 24 * Math.max(0, parseInt(config.timeagoCutoff, 10));
+		handleTokenCreation();
-			let translationSuffix = 'ago';
+	};
-			if (cutoffMs > 0 && Date.now() - ajaxify.data.lastSeen[item.token] > cutoffMs) {
-				translationSuffix = 'on';
-			}
-			item.lastSeen = `[[admin/settings/api:last-seen-${translationSuffix}, ${ajaxify.data.lastSeenISO[item.token]}]]`;
 
-			return { item };
+	async function handleTokenCreation() {
+		const createEl = document.querySelector('[data-action="create"]');
+		if (createEl) {
+			createEl.addEventListener('click', async () => {
+				const html = await Benchpress.render('admin/partials/edit-token-modal', {});
+				bootbox.dialog({
+					title: '[[admin/settings/api:create-token]]',
+					message: html,
+					buttons: {
+						submit: {
+							label: '[[modules:bootbox.submit]]',
+							className: 'btn-primary',
+							callback: parseCreateForm,
+						},
+					},
 				});
-
-		hooks.on('action:settings.sorted-list.loaded', ({ listEl }) => {
-			$(listEl).find('.timeago').timeago();
 			});
-
-		hooks.on('action:settings.sorted-list.itemLoaded', ({ element }) => {
-			element.addEventListener('click', (ev) => {
-				if (ev.target.closest('input[readonly]')) {
-					// Select entire input text
-					ev.target.selectionStart = 0;
-					ev.target.selectionEnd = ev.target.value.length;
 		}
-			});
+	}
-		});
-	};
 
-	function saveSettings() {
+	async function parseCreateForm() {
-		settings.save('core.api', $('.core-api-settings'), function () {
+		const modal = this;
-			alerts.alert({
+		const formEl = this.get(0).querySelector('form');
-				type: 'success',
+		const tokensTableBody = document.querySelector('[data-component="acp/tokens"] tbody');
-				alert_id: 'core.api-saved',
+		const valid = formEl.reportValidity();
-				title: 'Settings Saved',
+		if (formEl && valid) {
-				timeout: 5000,
+			const formData = new FormData(formEl);
-			});
+			const uid = formData.get('uid');
+			const description = formData.get('description');
+			// const qs = new URLSearchParams(payload).toString();
+
+			const token = await api.post('/admin/tokens', { uid, description }).catch(app.alertError);
+
+			const now = new Date();
+			const rowEl = (await app.parseAndTranslate(ajaxify.data.template.name, 'tokens', {
+				tokens: [{
+					token,
+					uid,
+					description,
+					timestamp: now.getTime(),
+					timestampISO: now.toISOString(),
+					lastSeen: null,
+					lastSeenISO: new Date(0).toISOString(),
+				}],
+			})).get(0);
+
+			if (tokensTableBody) {
+				tokensTableBody.append(rowEl);
+				$(rowEl).find('.timeago').timeago();
+			} else {
 				ajaxify.refresh();
-		});
+			}
+
+			modal.modal('hide');
+		}
+
+		return false;
 	}
 
 	return ACP;

src/api/users.js

@@ -20,6 +20,8 @@ const translator = require('../translator');
 const sockets = require('../socket.io');
 const utils = require('../utils');
 
+const api = require('.');
+
 const usersAPI = module.exports;
 
 const hasAdminPrivilege = async (uid, privilege) => {
@@ -313,19 +315,7 @@ usersAPI.generateToken = async (caller, { uid, description }) => {
 		throw new Error('[[error:invalid-uid]]');
 	}
 
-	const settings = await meta.settings.get('core.api');
+	return await api.utils.tokens.generate({ uid, description });
-	settings.tokens = settings.tokens || [];
-
-	const newToken = {
-		token: utils.generateUUID(),
-		uid: caller.uid,
-		description: description || '',
-		timestamp: Date.now(),
-	};
-	settings.tokens.push(newToken);
-	await meta.settings.set('core.api', settings);
-
-	return newToken;
 };
 
 usersAPI.deleteToken = async (caller, { uid, token }) => {
@@ -334,15 +324,8 @@ usersAPI.deleteToken = async (caller, { uid, token }) => {
 		throw new Error('[[error:invalid-uid]]');
 	}
 
-	const settings = await meta.settings.get('core.api');
+	await api.utils.tokens.delete(token);
-	const beforeLen = settings.tokens.length;
-	settings.tokens = settings.tokens.filter(tokenObj => tokenObj.token !== token);
-	if (beforeLen !== settings.tokens.length) {
-		await meta.settings.set('core.api', settings);
 	return true;
-	}
-
-	return false;
 };
 
 const getSessionAsync = util.promisify((sid, callback) => {

src/api/utils.js

@@ -34,16 +34,16 @@ 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.timestampISO = new Date(parseInt(tokenObj.timestamp, 10)).toISOString();
 	});
 
 	return singular ? tokenObjs[0] : tokenObjs;
 };
 
 utils.tokens.generate = async ({ uid, description }) => {
-	const token = srcUtils.generateUUID();
-	const timestamp = Date.now();
-
 	if (parseInt(uid, 10) !== 0) {
 		const uidExists = await user.exists(uid);
 		if (!uidExists) {
@@ -51,6 +51,17 @@ utils.tokens.generate = async ({ uid, description }) => {
 		}
 	}
 
+	const token = srcUtils.generateUUID();
+	const timestamp = Date.now();
+
+	return utils.tokens.add({ token, uid, description, timestamp });
+};
+
+utils.tokens.add = async ({ token, uid, description = '', timestamp = Date.now() }) => {
+	if (!token || uid === undefined) {
+		throw new Error('[[error:invalid-data]]');
+	}
+
 	await Promise.all([
 		db.setObject(`token:${token}`, { uid, description, timestamp }),
 		db.sortedSetAdd(`tokens:createtime`, timestamp, token),
@@ -60,8 +71,11 @@ utils.tokens.generate = async ({ uid, description }) => {
 	return token;
 };
 
-utils.tokens.update = async (token, { description }) => {
+utils.tokens.update = async (token, { uid, description }) => {
-	await db.setObject(`token:${token}`, { description });
+	await Promise.all([
+		db.setObject(`token:${token}`, { uid, description }),
+		db.sortedSetAdd(`tokens:uid`, uid, token),
+	]);
 
 	return await utils.tokens.get(token);
 };

src/controllers/admin/settings.js

@@ -111,14 +111,6 @@ settingsController.social = async function (req, res) {
 };
 
 settingsController.api = async (req, res) => {
-	const { tokens } = await meta.settings.get('core.api');
+	const tokens = await api.utils.tokens.list();
-	const scores = await api.utils.tokens.getLastSeen(tokens.map(t => t.token));
+	res.render('admin/settings/api', { tokens });
-
-	const [lastSeen, lastSeenISO] = tokens.reduce((memo, cur, idx) => {
-		memo[0][cur.token] = scores[idx];
-		memo[1][cur.token] = new Date(scores[idx]).toISOString();
-		return memo;
-	}, [{}, {}]);
-
-	res.render('admin/settings/api', { lastSeen, lastSeenISO });
 };

src/controllers/write/admin.js

@@ -28,3 +28,21 @@ Admin.getAnalyticsData = async (req, res) => {
 		units: req.query.units,
 	}));
 };
+
+Admin.generateToken = async (req, res) => {
+	const { uid, description } = req.body;
+	helpers.formatApiResponse(200, res, await api.utils.tokens.generate({ uid, description }));
+};
+
+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.deleteToken = async (req, res) => {
+	const { token } = req.params;
+	helpers.formatApiResponse(200, res, await api.utils.tokens.delete(token));
+};

src/routes/write/admin.js

@@ -15,5 +15,9 @@ module.exports = function () {
 	setupApiRoute(router, 'get', '/analytics', [...middlewares], controllers.write.admin.getAnalyticsKeys);
 	setupApiRoute(router, 'get', '/analytics/:set', [...middlewares], controllers.write.admin.getAnalyticsData);
 
+	setupApiRoute(router, 'post', '/tokens', [...middlewares], controllers.write.admin.generateToken);
+	setupApiRoute(router, 'put', '/tokens/:token', [...middlewares], controllers.write.admin.updateToken);
+	setupApiRoute(router, 'delete', '/tokens/:token', [...middlewares], controllers.write.admin.deleteToken);
+
 	return router;
 };

src/upgrades/3.1.0/migrate_api_tokens.js

@@ -0,0 +1,38 @@
+'use strict';
+
+const assert = require('assert');
+const winston = require('winston');
+
+const db = require('../../database');
+const meta = require('../../meta');
+const api = require('../../api');
+
+module.exports = {
+	name: 'Migrate tokens away from sorted-list implementation',
+	timestamp: Date.UTC(2023, 4, 2),
+	method: async () => {
+		const { tokens = [] } = await meta.settings.get('core.api');
+
+		await Promise.all(tokens.map(async (tokenObj) => {
+			const { token, uid, description } = tokenObj;
+			await api.utils.tokens.add({ token, uid, description });
+		}));
+
+		// Validate
+		const oldCount = await db.sortedSetCard('settings:core.api:sorted-list:tokens');
+		const newCount = await db.sortedSetCard('tokens:createtime');
+		try {
+			if (oldCount > 0) {
+				assert.strictEqual(oldCount, newCount);
+			}
+
+			// Delete old tokens
+			await meta.settings.set('core.api', {
+				tokens: [],
+			});
+			await db.delete('settings:core.api:sorted-lists');
+		} catch (e) {
+			winston.warn('Old token count does not match migrated tokens count, leaving old tokens behind.');
+		}
+	},
+};

src/views/admin/partials/api/sorted-list/item.tpl

@@ -1,21 +0,0 @@
-<li data-type="item" class="list-group-item">
-    <div class="row">
-        <div class="col-9">
-            <span class="badge bg-primary">{{{ if uid }}}uid {uid}{{{ else }}}master{{{ end }}}</span>
-            {{{ if token }}}<input class="form-control-plaintext" type="text" readonly="readonly" value="{token}" size="32" />{{{ else }}}<em class="text-warning">[[admin/settings/api:token-on-save]]</em>{{{ end }}}<br />
-            <p>
-                {{{ if description }}}
-                {description}
-                {{{ else }}}
-                <em>[[admin/settings/api:no-description]]</em>
-                {{{ end }}}
-                <br />
-                <small class="text-info">{./lastSeen}</small>
-            </p>
-        </div>
-        <div class="col-3 text-end">
-            <button type="button" data-type="edit" class="btn btn-info">Edit</button>
-            <button type="button" data-type="remove" class="btn btn-danger">Delete</button>
-        </div>
-    </div>
-</li>

src/views/admin/partials/edit-token-modal.tpl

@@ -1,15 +1,13 @@
-<form>
+<form role="form">
-    <input type="hidden" name="token" />
-    <input type="hidden" name="timestamp" />
     <div class="mb-3">
         <label class="form-label" for="uid">[[admin/settings/api:uid]]</label>
-        <input type="text" inputmode="numeric" pattern="\d+" name="uid" class="form-control" placeholder="1" />
+        <input type="text" inputmode="numeric" pattern="\d+" name="uid" class="form-control" placeholder="0" value="{./uid}" />
         <p class="form-text">
             [[admin/settings/api:uid-help-text]]
         </p>
     </div>
     <div class="mb-3">
         <label class="form-label" for="description">[[admin/settings/api:description]]</label>
-        <input type="text" name="description" class="form-control" placeholder="Description" />
+        <input type="text" name="description" class="form-control" placeholder="Description" value="{./description}" />
     </div>
 </form>

src/views/admin/settings/api.tpl

@@ -1,14 +1,64 @@
 <form role="form" class="core-api-settings">
 	<p class="lead">[[admin/settings/api:lead-text]]</p>
 	<p>[[admin/settings/api:intro]]</p>
-	<p>
+	<p class="text-danger">[[admin/settings/api:warning]]</p>
-		<a href="https://docs.nodebb.org/api">
+	<p class="d-flex align-items-center">
+		<a class="flex-grow-1" href="https://docs.nodebb.org/api">
 			<i class="fa fa-external-link"></i>
 			[[admin/settings/api:docs]]
 		</a>
+		<button type="button" class="btn btn-primary float-end" data-action="create">
+			<i class="fa fa-plus"></i>
+			[[admin/settings/api:create-token]]
+		</button>
 	</p>
 
-	<hr />
+	<table class="table mb-5" data-component="acp/tokens">
+		<thead>
+			<tr>
+				<th>[[admin/settings/api:token]]</th>
+				<th>[[admin/settings/api:description]]</th>
+				<th>[[admin/settings/api:uid]]</th>
+				<th>[[admin/settings/api:last-seen]]</th>
+				<th>[[admin/settings/api:created]]</th>
+			</tr>
+		</thead>
+		<tbody>
+			{{{ each tokens }}}
+			<tr>
+				<td>
+					<button type="button" class="btn btn-link" data-action="copy" data-clipboard-text="{./token}"><i class="fa fa-fw fa-clipboard" aria-hidden="true"></i></button>
+					<div class="vr me-3" aria-hidden="true"></div>
+					<span class="user-select-all">{./token}</span>
+				</td>
+				<td class="align-middle">
+					{{{ if ./description }}}
+					{./description}
+					{{{ else }}}
+					<em class="text-secondary">[[admin/settings/api:no-description]]</em>
+					{{{ end }}}
+				</td>
+				<td class="align-middle">
+					{{{ if (./uid == "0") }}}
+					<em>[[admin/settings/api:master-token]]</em>
+					{{{ else }}}
+					{./uid}
+					{{{ end }}}
+				</td>
+				<td class="align-middle">
+					{{{ if ./lastSeen }}}
+					<span class="timeago" title="{./lastSeenISO}"></span>
+					{{{ else }}}
+					<em class="text-secondary">[[admin/settings/api:last-seen-never]]</em>
+					{{{ end }}}
+				</td>
+				<td class="align-middle">
+					<span class="timeago" title="{./timestampISO}"></span>
+				</td>
+			</tr>
+			{{{ end }}}
+		</tbody>
+	</table>
 
 	<div class="row mb-4">
 		<div class="col-sm-2 col-12 settings-header">[[admin/settings/api:settings]]</div>
@@ -20,17 +70,6 @@
 			</div>
 		</div>
 	</div>
-
-	<div class="row mb-4">
-		<div class="col-sm-2 col-12 settings-header">[[admin/settings/api:tokens]]</div>
-		<div class="col-sm-10 col-12">
-			<div class="form-group" data-type="sorted-list" data-sorted-list="tokens" data-item-template="admin/partials/api/sorted-list/item" data-form-template="admin/partials/api/sorted-list/form">
-				<input type="hidden" name="tokens">
-				<ul data-type="list" class="list-group mb-3"></ul>
-				<button type="button" data-type="add" class="btn btn-info">Create Token</button>
-			</div>
-		</div>
-	</div>
 </form>
 
 <!-- IMPORT admin/partials/save_button.tpl -->

test/tokens.js

@@ -81,10 +81,22 @@ describe('API tokens', () => {
 
 	describe('.update()', () => {
 		it('should update the description of a token', async () => {
-			await api.utils.tokens.update(token, { description: 'foobar' });
+			await api.utils.tokens.update(token, { uid: 0, description: 'foobar' });
 			const tokenObj = await api.utils.tokens.get(token);
 
 			assert(tokenObj);
+			assert.strictEqual(parseInt(tokenObj.uid, 10), 0);
+			assert.strictEqual(tokenObj.description, 'foobar');
+		});
+
+		it('should update the uid of a token', async () => {
+			await api.utils.tokens.update(token, { uid: 1, description: 'foobar' });
+			const tokenObj = await api.utils.tokens.get(token);
+			const uid = await db.sortedSetScore('tokens:uid', token);
+
+			assert(tokenObj);
+			assert.strictEqual(parseInt(tokenObj.uid, 10), 1);
+			assert.strictEqual(parseInt(uid, 10), 1);
 			assert.strictEqual(tokenObj.description, 'foobar');
 		});
 	});