From 0bfd2f901d2fb9b831f81946f8548e70dc43a3e4 Mon Sep 17 00:00:00 2001 From: Julian Gassner Date: Thu, 6 Feb 2025 16:47:56 +0000 Subject: [PATCH] Add possibility to remove mfa --- backend/internal/mfa.js | 35 +++++++++--- backend/routes/mfa.js | 56 ++++++++----------- backend/schema/paths/mfa/delete/delete.json | 44 +++++++++++++++ .../schema/paths/mfa/{ => enable}/post.json | 11 ++-- backend/schema/swagger.json | 9 ++- frontend/js/app/api.js | 3 + frontend/js/app/user/form.ejs | 14 ++++- frontend/js/app/user/form.js | 45 ++++++++++++--- frontend/js/i18n/messages.json | 8 ++- 9 files changed, 168 insertions(+), 57 deletions(-) create mode 100644 backend/schema/paths/mfa/delete/delete.json rename backend/schema/paths/mfa/{ => enable}/post.json (90%) diff --git a/backend/internal/mfa.js b/backend/internal/mfa.js index 4daf3bba..5f6865b9 100644 --- a/backend/internal/mfa.js +++ b/backend/internal/mfa.js @@ -1,5 +1,5 @@ const authModel = require('../models/auth'); -const error = require('../lib/error'); +const error = require('../lib/error'); const speakeasy = require('speakeasy'); module.exports = { @@ -13,10 +13,10 @@ module.exports = { throw new error.AuthError('MFA is not enabled for this user.'); } const verified = speakeasy.totp.verify({ - secret: auth.mfa_secret, + secret: auth.mfa_secret, encoding: 'base32', - token: token, - window: 2 + token: token, + window: 2 }); if (!verified) { throw new error.AuthError('Invalid MFA token.'); @@ -58,10 +58,10 @@ module.exports = { throw new error.AuthError('MFA is not set up for this user.'); } const verified = speakeasy.totp.verify({ - secret: auth.mfa_secret, + secret: auth.mfa_secret, encoding: 'base32', - token: token, - window: 2 + token: token, + window: 2 }); if (!verified) { throw new error.AuthError('Invalid MFA token.'); @@ -73,4 +73,25 @@ module.exports = { .then(() => true); }); }, + disableMfaForUser: (data, userId) => { + return authModel + .query() + .where('user_id', userId) + .first() + .then((auth) => { + if (!auth) { + throw new error.AuthError('User not found.'); + } + return auth.verifyPassword(data.secret) + .then((valid) => { + if (!valid) { + throw new error.AuthError('Invalid password.'); + } + return authModel + .query() + .where('user_id', userId) + .update({ mfa_enabled: false, mfa_secret: null }); + }); + }); + }, }; diff --git a/backend/routes/mfa.js b/backend/routes/mfa.js index 1181a18b..d142e3cd 100644 --- a/backend/routes/mfa.js +++ b/backend/routes/mfa.js @@ -1,37 +1,18 @@ -const express = require('express'); -const jwtdecode = require('../lib/express/jwt-decode'); -const apiValidator = require('../lib/validator/api'); -const internalToken = require('../internal/token'); -const schema = require('../schema'); -const internalMfa = require('../internal/mfa'); -const qrcode = require('qrcode'); -const speakeasy = require('speakeasy'); -const userModel = require('../models/user'); +const express = require('express'); +const jwtdecode = require('../lib/express/jwt-decode'); +const apiValidator = require('../lib/validator/api'); +const schema = require('../schema'); +const internalMfa = require('../internal/mfa'); +const qrcode = require('qrcode'); +const speakeasy = require('speakeasy'); +const userModel = require('../models/user'); let router = express.Router({ caseSensitive: true, - strict: true, - mergeParams: true + strict: true, + mergeParams: true }); -router - .route('/') - .options((_, res) => { - res.sendStatus(204); - }) - - .get(async (req, res, next) => { - internalToken.getFreshToken(res.locals.access, { - expiry: (typeof req.query.expiry !== 'undefined' ? req.query.expiry : null), - scope: (typeof req.query.scope !== 'undefined' ? req.query.scope : null) - }) - .then((data) => { - res.status(200) - .send(data); - }) - .catch(next); - }); - router .route('/create') .post(jwtdecode(), (req, res, next) => { @@ -54,7 +35,7 @@ router .then(({ secret, user }) => { const otpAuthUrl = speakeasy.otpauthURL({ secret: secret.ascii, - label: user.email, + label: user.email, issuer: 'Nginx Proxy Manager' }); qrcode.toDataURL(otpAuthUrl, (err, dataUrl) => { @@ -71,12 +52,13 @@ router router .route('/enable') .post(jwtdecode(), (req, res, next) => { - apiValidator(schema.getValidationSchema('/mfa', 'post'), req.body).then((params) => { + apiValidator(schema.getValidationSchema('/mfa/enable', 'post'), req.body).then((params) => { internalMfa.enableMfaForUser(res.locals.access.token.getUserId(), params.token) .then(() => res.status(200).send({ success: true })) .catch(next); } - );}); + ).catch(next); + }); router .route('/check') @@ -86,4 +68,14 @@ router .catch(next); }); +router + .route('/delete') + .delete(jwtdecode(), (req, res, next) => { + apiValidator(schema.getValidationSchema('/mfa/delete', 'delete'), req.body).then((params) => { + internalMfa.disableMfaForUser(params, res.locals.access.token.getUserId()) + .then(() => res.status(200).send({ success: true })) + .catch(next); + }).catch(next); + }); + module.exports = router; diff --git a/backend/schema/paths/mfa/delete/delete.json b/backend/schema/paths/mfa/delete/delete.json new file mode 100644 index 00000000..9b9b1415 --- /dev/null +++ b/backend/schema/paths/mfa/delete/delete.json @@ -0,0 +1,44 @@ +{ + "operationId": "disableMfa", + "summary": "Disable multi-factor authentication for a user", + "tags": [ + "MFA" + ], + "requestBody": { + "description": "Payload to disable MFA", + "required": true, + "content": { + "application/json": { + "schema": { + "additionalProperties": false, + "properties": { + "secret": { + "type": "string", + "minLength": 1 + } + }, + "required": [ + "secret" + ] + } + } + } + }, + "responses": { + "200": { + "description": "MFA disabled successfully", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "success": { + "type": "boolean" + } + } + } + } + } + } + } +} \ No newline at end of file diff --git a/backend/schema/paths/mfa/post.json b/backend/schema/paths/mfa/enable/post.json similarity index 90% rename from backend/schema/paths/mfa/post.json rename to backend/schema/paths/mfa/enable/post.json index 5d2bbde8..33228781 100644 --- a/backend/schema/paths/mfa/post.json +++ b/backend/schema/paths/mfa/enable/post.json @@ -1,14 +1,15 @@ { "operationId": "enableMfa", "summary": "Enable multi-factor authentication for a user", - "tags": ["MFA"], + "tags": [ + "MFA" + ], "requestBody": { "description": "MFA Token Payload", "required": true, "content": { "application/json": { "schema": { - "type": "object", "additionalProperties": false, "properties": { "token": { @@ -16,7 +17,9 @@ "minLength": 1 } }, - "required": ["token"] + "required": [ + "token" + ] } } } @@ -38,4 +41,4 @@ } } } -} +} \ No newline at end of file diff --git a/backend/schema/swagger.json b/backend/schema/swagger.json index 841068ea..b7f2b9a3 100644 --- a/backend/schema/swagger.json +++ b/backend/schema/swagger.json @@ -15,9 +15,14 @@ "$ref": "./paths/get.json" } }, - "/mfa": { + "/mfa/enable": { "post": { - "$ref": "./paths/mfa/post.json" + "$ref": "./paths/mfa/enable/post.json" + } + }, + "/mfa/delete": { + "delete": { + "$ref": "./paths/mfa/delete/delete.json" } }, "/audit-log": { diff --git a/frontend/js/app/api.js b/frontend/js/app/api.js index cfd2c342..12497a6c 100644 --- a/frontend/js/app/api.js +++ b/frontend/js/app/api.js @@ -211,6 +211,9 @@ module.exports = { }, check: function () { return fetch('get', 'mfa/check'); + }, + delete: function (secret) { + return fetch('delete', 'mfa/delete', {secret: secret}); } }, diff --git a/frontend/js/app/user/form.ejs b/frontend/js/app/user/form.ejs index 4d21acf7..1e7bf9ed 100644 --- a/frontend/js/app/user/form.ejs +++ b/frontend/js/app/user/form.ejs @@ -27,12 +27,22 @@
- - + + + +
diff --git a/frontend/js/app/user/form.js b/frontend/js/app/user/form.js index ffde610c..fe72d4c9 100644 --- a/frontend/js/app/user/form.js +++ b/frontend/js/app/user/form.js @@ -15,10 +15,14 @@ module.exports = Mn.View.extend({ cancel: 'button.cancel', save: 'button.save', error: '.secret-error', - addMfa: '.add-mfa', - mfaLabel: '.mfa-label', // added binding - mfaValidation: '.mfa-validation-container', // added binding - qrInstructions: '.qr-instructions' // added binding for instructions + mfaError: '.mfa-error', + addMfa: '.mfa-add', + mfaValidation: '.mfa-validation-container', + qrInstructions: '.qr-instructions', + removeMfa: '.mfa-remove', + removeMfaConfirmContainer: '.mfa-remove-confirm-container', + removeMfaConfirm: '.mfa-remove-confirm', + removeMfaPassword: '.mfa-remove-password-field' }, events: { @@ -75,7 +79,6 @@ module.exports = Mn.View.extend({ return App.Api.Mfa.enable(mfaToken) .then(() => result); } - console.log(result); return result; }) .then(result => { @@ -106,6 +109,31 @@ module.exports = Mn.View.extend({ .catch(err => { view.ui.error.text(err.message).show(); }); + }, + 'click @ui.removeMfa': function (e) { + // Show confirmation section with a password field and confirm button + this.ui.removeMfa.hide(); + this.ui.removeMfaConfirmContainer.show(); + }, + 'click @ui.removeMfaConfirm': function (e) { + let view = this; + let password = view.ui.removeMfaPassword.val(); + if (!password) { + view.ui.error.text('Password required to remove MFA').show(); + return; + } + App.Api.Mfa.delete(password) + .then(() => { + view.ui.addMfa.show(); + view.ui.qrInstructions.hide(); + view.ui.mfaValidation.hide(); + view.ui.removeMfaConfirmContainer.hide(); + view.ui.removeMfa.hide(); + view.ui.mfaValidation.find('input[name="mfa_validation"]').removeAttr('required'); + }) + .catch(err => { + view.ui.mfaError.text(err.message).show(); + }); } }, @@ -143,16 +171,17 @@ module.exports = Mn.View.extend({ .then(response => { if (response.active) { view.ui.addMfa.hide(); - view.ui.mfaLabel.hide(); view.ui.qrInstructions.hide(); view.ui.mfaValidation.hide(); - // Remove required attribute if MFA is active & field is hidden + view.ui.removeMfa.show(); + view.ui.removeMfaConfirmContainer.hide(); view.ui.mfaValidation.find('input[name="mfa_validation"]').removeAttr('required'); } else { view.ui.addMfa.show(); - view.ui.mfaLabel.show(); view.ui.qrInstructions.hide(); view.ui.mfaValidation.hide(); + view.ui.removeMfa.hide(); + view.ui.removeMfaConfirmContainer.hide(); view.ui.mfaValidation.find('input[name="mfa_validation"]').removeAttr('required'); } }) diff --git a/frontend/js/i18n/messages.json b/frontend/js/i18n/messages.json index cd0b2988..adb57b4e 100644 --- a/frontend/js/i18n/messages.json +++ b/frontend/js/i18n/messages.json @@ -39,9 +39,13 @@ }, "mfa": { "mfa": "Multi Factor Authentication", - "add-mfa": "Generate secret", + "mfa-add": "Add Multi Factor Authentication", + "mfa-remove": "Remove Multi Factor Authentication", "mfa-setup-instruction": "Scan this QR code in your authenticator app to set up MFA and then enter the current MFA code in the input field.", - "mfa-token": "Multi factor authentication token" + "mfa-token": "Multi factor authentication token", + "confirm-password": "Please enter your password to confirm", + "enter-password": "Enter Password", + "confirm-remove-mfa": "Confirm Multi Factor Authentication removal" }, "login": { "title": "Login to your account",