diff --git a/docs/changelog.md b/docs/changelog.md new file mode 100644 index 0000000..d876c4d --- /dev/null +++ b/docs/changelog.md @@ -0,0 +1,19 @@ +# Changelog + +This file provides a running tally of breaking changes. +It is not meant to be exhaustive — if you notice a breaking change that is not included in this list, please [open an issue](https://github.com/julianlam/NodeBB-plugin-2factor/issues/new) and let me know. + +## v7.x + +* If an account is protected by Two-Factor Authentication, you will still be considered a guest when the page is loaded — specifically, `app.user` is no longer showing logged-in user data. +* Requests to the API (both the Read and Write API) now return a standard API-style (an object with `status` and `payload`) `401 Unauthorized` error if you have not passed the 2FA check. + +## v6.x + +* The plugin was updated to support a minimum NodeBB version of v3.x. + * The plugin should still work under v2.x, but the UI may not look as polished as the front-end elements have been changed to use Bootstrap 5 class names. + +## v5.x + +* Users are now presented with a list of choices based on configured 2FA methods, instead of a priority-based challenge +* Users can now set up multiple 2FA methods (e.g. both TOTP and Hardware key at the same time.) \ No newline at end of file diff --git a/lib/controllers.js b/lib/controllers.js index ad9d5ad..cabf4c2 100644 --- a/lib/controllers.js +++ b/lib/controllers.js @@ -30,10 +30,11 @@ async function getGroups(set) { } Controllers.renderChoices = async (req, res) => { + const uid = res.locals['2factor'] || req.uid; const [hasAuthn, hasTotp, hasBackupCodes] = await Promise.all([ - parent.hasAuthn(req.uid), - parent.hasTotp(req.uid), - parent.hasBackupCodes(req.uid), + parent.hasAuthn(uid), + parent.hasTotp(uid), + parent.hasBackupCodes(uid), ]); const count = [hasAuthn, hasTotp, hasBackupCodes].reduce((count, cur) => count + (cur ? 1 : 0), 0); @@ -65,13 +66,15 @@ Controllers.renderChoices = async (req, res) => { }; Controllers.renderTotpChallenge = async (req, res, next) => { + const uid = res.locals['2factor'] || req.uid; + if (req.session.tfa === true && ((req.query.next && !req.query.next.startsWith('/admin')) || !req.session.tfaForce)) { return res.redirect(nconf.get('relative_path') + (req.query.next || '/')); } const error = req.flash('error'); - if (!await parent.hasTotp(req.uid)) { + if (!await parent.hasTotp(uid)) { return next(); } @@ -84,15 +87,17 @@ Controllers.renderTotpChallenge = async (req, res, next) => { }; Controllers.renderAuthnChallenge = async (req, res, next) => { + const uid = res.locals['2factor'] || req.uid; + if (req.session.tfa === true && ((req.query.next && !req.query.next.startsWith('/admin')) || !req.session.tfaForce)) { return res.redirect(nconf.get('relative_path') + (req.query.next || '/')); } - if (!await parent.hasAuthn(req.uid)) { + if (!await parent.hasAuthn(uid)) { return next(); } - const keyIds = await parent.getAuthnKeyIds(req.uid); + const keyIds = await parent.getAuthnKeyIds(uid); let authnOptions; if (keyIds.length) { authnOptions = await parent._f2l.assertionOptions(); @@ -120,13 +125,15 @@ Controllers.processTotpLogin = function (req, res, next) { }; Controllers.renderBackup = async (req, res, next) => { + const uid = res.locals['2factor'] || req.uid; + if (req.session.tfa === true && ((req.query.next && !req.query.next.startsWith('/admin')) || !req.session.tfaForce)) { return res.redirect(nconf.get('relative_path') + (req.query.next || '/')); } const error = req.flash('error'); - if (!await parent.hasKey(req.user.uid)) { + if (!await parent.hasKey(uid)) { return next(); } diff --git a/lib/middlewares.js b/lib/middlewares.js index a7ea47b..42c5d51 100644 --- a/lib/middlewares.js +++ b/lib/middlewares.js @@ -8,7 +8,7 @@ Middlewares.requireSecondFactor = function (req, res, next) { if (req.session.hasOwnProperty('tfa') && req.session.tfa === true) { next(); } else { - translator.translate('[[2factor:second-factor-required]]', function (translated) { + translator.translate('[[2factor:second-factor-required]]', (translated) => { res.status(403).json({ error: translated, }); diff --git a/library.js b/library.js index 7e32be6..23f00f5 100644 --- a/library.js +++ b/library.js @@ -310,6 +310,17 @@ plugin.disassociate = async (uid) => { await db.delete(`2factor:webauthn:${uid}`); }; +plugin.overrideUid = async ({ req, locals }) => { + if (req.uid && await plugin.hasKey(req.uid) && req.session.tfa !== true) { + locals['2factor'] = req.uid; + req.uid = 0; + delete req.user; + delete req.loggedIn; + } + + return { req, locals }; +}; + plugin.check = async ({ req, res }) => { if (!req.user || req.session.tfa === true) { return; @@ -317,10 +328,10 @@ plugin.check = async ({ req, res }) => { const requestPath = req.baseUrl + req.path; const exemptPaths = ['/login/2fa', '/login/2fa/authn', '/login/2fa/totp', '/login/2fa/backup', '/2factor/authn/verify', '/register/complete']; - const exemptPrefixes = ['/api/v3/']; if ( - exemptPaths.some(path => requestPath === nconf.get('relative_path') + path || requestPath === `${nconf.get('relative_path')}/api${path}`) || - exemptPrefixes.some(prefix => requestPath.startsWith(nconf.get('relative_path') + prefix)) + exemptPaths.some(path => requestPath === nconf.get('relative_path') + path || + requestPath === `${nconf.get('relative_path')}/api${path}` || + requestPath === `${nconf.get('relative_path')}/api/v3/plugins${path}`) ) { return; } @@ -333,8 +344,12 @@ plugin.check = async ({ req, res }) => { .replace(nconf.get('relative_path'), ''); if (await plugin.hasKey(req.user.uid)) { - // Account has TFA, redirect to login - controllerHelpers.redirect(res, `/login/2fa?next=${redirect}`); + if (!res.locals.isAPI) { + // Account has TFA, redirect to login + controllerHelpers.redirect(res, `/login/2fa?next=${redirect}`); + } else { + await controllerHelpers.formatApiResponse(401, res, new Error('[[2factor:second-factor-required]]')); + } } else if (tfaEnforcedGroups.length && (await groups.isMemberOfGroups(req.uid, tfaEnforcedGroups)).includes(true)) { if (req.url.startsWith('/admin') || (!req.url.startsWith('/admin') && !req.url.match('2factor'))) { controllerHelpers.redirect(res, `/me/2factor?next=${redirect}`); diff --git a/plugin.json b/plugin.json index e60a9fb..04b3748 100644 --- a/plugin.json +++ b/plugin.json @@ -8,6 +8,7 @@ { "hook": "filter:admin.header.build", "method": "addAdminNavigation" }, { "hook": "filter:user.profileMenu", "method": "addProfileItem" }, { "hook": "response:router.page", "method": "check" }, + { "hook": "filter:middleware.buildHeader", "method": "overrideUid" }, { "hook": "static:user.loggedOut", "method": "clearSession" }, { "hook": "filter:middleware.render", "method": "updateTitle" }, { "hook": "static:sockets.validateSession", "method": "checkSocket" },