From 608f1d8222918116a8b919709a296a92e603dce7 Mon Sep 17 00:00:00 2001 From: byn9826 Date: Thu, 26 Sep 2024 18:04:34 -0400 Subject: [PATCH] Manage sms mfa through admin panel and s2s api --- README.md | 2 +- admin-panel/app/[lang]/dashboard/page.tsx | 8 +- .../app/[lang]/users/[authId]/page.tsx | 72 +++++++++++++- .../app/api/users/[authId]/sms-mfa/route.ts | 27 ++++++ admin-panel/translations/en.json | 9 +- admin-panel/translations/fr.json | 9 +- docs/q&a.md | 5 + server/src/__tests__/normal/user.test.tsx | 95 ++++++++++++++++++- server/src/handlers/user.ts | 24 +++++ server/src/models/user.ts | 2 + server/src/routes/user.tsx | 52 +++++++++- server/src/scripts/schemas/user.cjs | 1 + server/src/scripts/swagger.json | 55 ++++++++++- server/src/services/user.ts | 93 ++++++++++++------ 14 files changed, 409 insertions(+), 45 deletions(-) create mode 100644 admin-panel/app/api/users/[authId]/sms-mfa/route.ts diff --git a/README.md b/README.md index ddd716ff..80cdb012 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ - Google Sign-In - Facebook Sign-In - GitHub Sign-In -- Multi-Factor Authentication: +- Multi-Factor Authentication [How to setup MFA](https://auth.valuemelody.com/q_a.html#how-to-setup-mfa): - Email MFA - OTP MFA - SMS MFA diff --git a/admin-panel/app/[lang]/dashboard/page.tsx b/admin-panel/app/[lang]/dashboard/page.tsx index 2d0df790..4aa8244e 100644 --- a/admin-panel/app/[lang]/dashboard/page.tsx +++ b/admin-panel/app/[lang]/dashboard/page.tsx @@ -205,6 +205,12 @@ const Page = () => { + + SMS_MFA_IS_REQUIRED + + + + EMAIL_MFA_IS_REQUIRED @@ -214,7 +220,7 @@ const Page = () => { ENFORCE_ONE_MFA_ENROLLMENT - + {configs.ENFORCE_ONE_MFA_ENROLLMENT.join(', ')} diff --git a/admin-panel/app/[lang]/users/[authId]/page.tsx b/admin-panel/app/[lang]/users/[authId]/page.tsx index fb344cfb..996ba06f 100644 --- a/admin-panel/app/[lang]/users/[authId]/page.tsx +++ b/admin-panel/app/[lang]/users/[authId]/page.tsx @@ -55,6 +55,7 @@ const Page = () => { const isEmailEnrolled = configs.EMAIL_MFA_IS_REQUIRED || user?.mfaTypes.includes('email') const isOtpEnrolled = configs.OTP_MFA_IS_REQUIRED || user?.mfaTypes.includes('otp') + const isSmsEnrolled = configs.SMS_MFA_IS_REQUIRED || user?.mfaTypes.includes('sms') const updateObj = useMemo( () => { @@ -189,6 +190,16 @@ const Page = () => { if (result) getUser() } + const handleResetSmsMfa = async () => { + const token = await acquireToken() + const result = await proxyTool.sendNextRequest({ + endpoint: `/api/users/${authId}/sms-mfa`, + method: 'DELETE', + token, + }) + if (result) getUser() + } + const handleResetEmailMfa = async () => { const token = await acquireToken() const result = await proxyTool.sendNextRequest({ @@ -209,6 +220,16 @@ const Page = () => { if (result) getUser() } + const handleEnrollSmsMfa = async () => { + const token = await acquireToken() + const result = await proxyTool.sendNextRequest({ + endpoint: `/api/users/${authId}/sms-mfa`, + method: 'POST', + token, + }) + if (result) getUser() + } + const handleEnrollEmailMfa = async () => { const token = await acquireToken() const result = await proxyTool.sendNextRequest({ @@ -279,7 +300,7 @@ const Page = () => { const renderOtpButtons = (user) => { return ( <> - {(user.otpVerified || user.mfaTypes.includes('otp')) && user.isActive && ( + {user.mfaTypes.includes('otp') && user.isActive && ( + )} + {user.isActive && !isSmsEnrolled && ( + + )} + + ) + } + const renderIpButtons = (lockedIPs) => { if (!lockedIPs.length) return null return ( @@ -369,7 +412,7 @@ const Page = () => { )} {!user.socialAccountId && ( - {t('users.authenticator')} + {t('users.otpMfa')}
{isOtpEnrolled && !user.otpVerified && ( @@ -392,6 +435,31 @@ const Page = () => { )} + {!user.socialAccountId && ( + + {t('users.smsMfa')} + +
+ {isSmsEnrolled && !user.smsPhoneNumberVerified && ( +
+ {t('users.smsMfaEnrolled')} +
+ )} + {isSmsEnrolled && user.smsPhoneNumberVerified && ( +
+ {t('users.smsMfaVerified')} +
+ )} +
+ {renderSmsButtons(user)} +
+
+ + + {renderSmsButtons(user)} + + + )} {t('users.locale')} diff --git a/admin-panel/app/api/users/[authId]/sms-mfa/route.ts b/admin-panel/app/api/users/[authId]/sms-mfa/route.ts new file mode 100644 index 00000000..895fccc7 --- /dev/null +++ b/admin-panel/app/api/users/[authId]/sms-mfa/route.ts @@ -0,0 +1,27 @@ +import { sendS2SRequest } from 'app/api/request' + +type Params = { + authId: string; +} + +export async function POST ( + request: Request, context: { params: Params }, +) { + const authId = context.params.authId + + return sendS2SRequest({ + method: 'POST', + uri: `/api/v1/users/${authId}/sms-mfa`, + }) +} + +export async function DELETE ( + request: Request, context: { params: Params }, +) { + const authId = context.params.authId + + return sendS2SRequest({ + method: 'DELETE', + uri: `/api/v1/users/${authId}/sms-mfa`, + }) +} diff --git a/admin-panel/translations/en.json b/admin-panel/translations/en.json index 0093545b..7c20d9f1 100644 --- a/admin-panel/translations/en.json +++ b/admin-panel/translations/en.json @@ -67,11 +67,14 @@ "emailVerified": "Email Verified", "emailNotVerified": "Email not Verified", "emailMfaEnrolled": "Email MFA Enrolled", - "otpMfaEnrolled": "Authenticator MFA Enrolled", - "otpMfaVerified": "Authenticator MFA Verified", + "otpMfaEnrolled": "OTP MFA Enrolled", + "otpMfaVerified": "OTP MFA Verified", + "smsMfaEnrolled": "SMS MFA Enrolled", + "smsMfaVerified": "SMS MFA Verified", "resetMfa": "Reset MFA", "enrollMfa": "Enroll MFA", - "authenticator": "Authenticator", + "otpMfa": "OTP MFA", + "smsMfa": "SMS MFA", "firstName": "First Name", "lastName": "Last Name", "roles": "Roles", diff --git a/admin-panel/translations/fr.json b/admin-panel/translations/fr.json index 7a1b45fd..5632c8ae 100644 --- a/admin-panel/translations/fr.json +++ b/admin-panel/translations/fr.json @@ -67,11 +67,14 @@ "emailVerified": "E-mail vérifié", "emailNotVerified": "E-mail non vérifié", "emailMfaEnrolled": "MFA par e-mail activé", - "otpMfaEnrolled": "MFA par authentificateur activé", - "otpMfaVerified": "MFA par authentificateur vérifié", + "otpMfaEnrolled": "MFA par OTP activé", + "otpMfaVerified": "MFA par OTP vérifié", + "smsMfaEnrolled": "MFA par SMS activé", + "smsMfaVerified": "MFA par SMS vérifié", "resetMfa": "Réinitialiser MFA", "enrollMfa": "Inscrire MFA", - "authenticator": "Authentificateur", + "otpMfa": "MFA par OTP", + "smsMfa": "MFA par SMS", "firstName": "Prénom", "lastName": "Nom", "roles": "Rôles", diff --git a/docs/q&a.md b/docs/q&a.md index 1a2dd3b6..40a4d8cc 100644 --- a/docs/q&a.md +++ b/docs/q&a.md @@ -96,3 +96,8 @@ npm run dev:secret:clean # For Cloudflare local env npm run prod:secret:clean # For Cloudflare remote env ``` After running these commands, the old secret will be removed, and any tokens signed with the old secret will no longer be valid. + +## How to setup MFA +- Enforcing specific MFA types: You can set OTP_MFA_IS_REQUIRED, SMS_MFA_IS_REQUIRED, or EMAIL_MFA_IS_REQUIRED to true to enforce those MFA methods as a login requirement. +- Letting users choose one of the supported MFA types: If OTP_MFA_IS_REQUIRED, SMS_MFA_IS_REQUIRED, and EMAIL_MFA_IS_REQUIRED are all set to false, you can set ENFORCE_ONE_MFA_ENROLLMENT to contain the MFA types you want to support. The user will then be required to enroll in one of the selected MFA types. +- You can also use the MFA enrollment functionality provided by the admin panel or the S2S API to customize your MFA enrollment flow. diff --git a/server/src/__tests__/normal/user.test.tsx b/server/src/__tests__/normal/user.test.tsx index d451ae9e..2909dc08 100644 --- a/server/src/__tests__/normal/user.test.tsx +++ b/server/src/__tests__/normal/user.test.tsx @@ -20,7 +20,7 @@ import { } from 'models' import { attachIndividualScopes, - dbTime, disableUser, enrollEmailMfa, enrollOtpMfa, getS2sToken, + dbTime, disableUser, enrollEmailMfa, enrollOtpMfa, getS2sToken, enrollSmsMfa, } from 'tests/util' let db: Database @@ -57,6 +57,7 @@ const user1 = { locale: 'en', emailVerified: false, otpVerified: false, + smsPhoneNumberVerified: false, mfaTypes: [], isActive: true, loginCount: 0, @@ -76,6 +77,7 @@ const user2 = { locale: 'fr', emailVerified: false, otpVerified: false, + smsPhoneNumberVerified: false, mfaTypes: [], isActive: true, loginCount: 0, @@ -1202,3 +1204,94 @@ describe( ) }, ) + +describe( + 'enroll sms mfa', + () => { + const enrollAndCheckUser = async () => { + await insertUsers() + + await app.request( + `${BaseRoute}/1-1-1-1/sms-mfa`, + { + method: 'POST', + headers: { Authorization: `Bearer ${await getS2sToken(db)}` }, + }, + mock(db), + ) + + const userRes = await app.request( + `${BaseRoute}/1-1-1-1`, + { headers: { Authorization: `Bearer ${await getS2sToken(db)}` } }, + mock(db), + ) + return userRes + } + + test( + 'should enroll sms mfa', + async () => { + const userRes = await enrollAndCheckUser() + + const userJson = await userRes.json() as { user: userModel.Record } + expect(userJson.user.mfaTypes).toStrictEqual(['sms']) + }, + ) + + test( + 'if sms is enforced by config', + async () => { + global.process.env.SMS_MFA_IS_REQUIRED = true as unknown as string + const userRes = await enrollAndCheckUser() + + const userJson = await userRes.json() as { user: userModel.Record } + expect(userJson.user.mfaTypes).toStrictEqual([]) + global.process.env.SMS_MFA_IS_REQUIRED = false as unknown as string + }, + ) + }, +) + +describe( + 'Unenroll sms mfa', + () => { + const handleUnenrollCheck = async () => { + const res = await app.request( + `${BaseRoute}/1-1-1-1/sms-mfa`, + { + method: 'DELETE', + headers: { Authorization: `Bearer ${await getS2sToken(db)}` }, + }, + mock(db), + ) + expect(res.status).toBe(204) + + const userRes = await app.request( + `${BaseRoute}/1-1-1-1`, + { headers: { Authorization: `Bearer ${await getS2sToken(db)}` } }, + mock(db), + ) + + const userJson = await userRes.json() as { user: userModel.Record } + expect(userJson.user.mfaTypes).toStrictEqual([]) + } + + test( + 'should unenroll sms mfa', + async () => { + await insertUsers() + await enrollSmsMfa(db) + + await handleUnenrollCheck() + }, + ) + + test( + 'If user is not enrolled', + async () => { + await insertUsers() + await handleUnenrollCheck() + }, + ) + }, +) diff --git a/server/src/handlers/user.ts b/server/src/handlers/user.ts index 2d5de7eb..dd5b889c 100644 --- a/server/src/handlers/user.ts +++ b/server/src/handlers/user.ts @@ -108,6 +108,18 @@ export const postUserOtpMfa = async (c: Context) => { return c.body(null) } +export const postUserSmsMfa = async (c: Context) => { + const authId = c.req.param('authId') + + await userService.enrollUserMfa( + c, + authId, + userModel.MfaType.Sms, + ) + c.status(204) + return c.body(null) +} + export const deleteUserEmailMfa = async (c: Context) => { const authId = c.req.param('authId') @@ -132,6 +144,18 @@ export const deleteUserOtpMfa = async (c: Context) => { return c.body(null) } +export const deleteUserSmsMfa = async (c: Context) => { + const authId = c.req.param('authId') + + await userService.resetUserMfa( + c, + authId, + userModel.MfaType.Sms, + ) + c.status(204) + return c.body(null) +} + export const deleteUserAppConsent = async (c: Context) => { const authId = c.req.param('authId') const appId = c.req.param('appId') diff --git a/server/src/models/user.ts b/server/src/models/user.ts index 576e4419..b15a1f94 100644 --- a/server/src/models/user.ts +++ b/server/src/models/user.ts @@ -61,6 +61,7 @@ export interface ApiRecord { lastName?: string | null; emailVerified: boolean; otpVerified: boolean; + smsPhoneNumberVerified: boolean; loginCount: number; mfaTypes: string[]; isActive: boolean; @@ -131,6 +132,7 @@ export const convertToApiRecord = ( email: record.email, locale: record.locale, emailVerified: record.emailVerified, + smsPhoneNumberVerified: record.smsPhoneNumberVerified, otpVerified: record.otpVerified, mfaTypes: record.mfaTypes, isActive: record.isActive, diff --git a/server/src/routes/user.tsx b/server/src/routes/user.tsx index d0241ae0..b60d08ca 100644 --- a/server/src/routes/user.tsx +++ b/server/src/routes/user.tsx @@ -349,7 +349,7 @@ userRoutes.delete( * @swagger * /api/v1/users/{authId}/otp-mfa: * post: - * summary: enroll user for authenticator MFA. + * summary: enroll user for OTP MFA. * description: Required scope - write_user * tags: [Users] * parameters: @@ -373,7 +373,7 @@ userRoutes.post( * @swagger * /api/v1/users/{authId}/otp-mfa: * delete: - * summary: Remove user's current authenticator MFA setup. + * summary: Remove user's current OTP MFA setup. * description: Required scope - write_user * tags: [Users] * parameters: @@ -392,3 +392,51 @@ userRoutes.delete( authMiddleware.s2sWriteUser, userHandler.deleteUserOtpMfa, ) + +/** + * @swagger + * /api/v1/users/{authId}/sms-mfa: + * post: + * summary: enroll user for SMS MFA. + * description: Required scope - write_user + * tags: [Users] + * parameters: + * - in: path + * name: authId + * required: true + * schema: + * type: string + * description: The authId of the user + * responses: + * 204: + * description: Successful operation with no content to return + */ +userRoutes.post( + `${BaseRoute}/:authId/sms-mfa`, + authMiddleware.s2sWriteUser, + userHandler.postUserSmsMfa, +) + +/** + * @swagger + * /api/v1/users/{authId}/sms-mfa: + * delete: + * summary: Remove user's current SMS MFA setup. + * description: Required scope - write_user + * tags: [Users] + * parameters: + * - in: path + * name: authId + * required: true + * schema: + * type: string + * description: The authId of the user + * responses: + * 204: + * description: Successful operation with no content to return + */ +userRoutes.delete( + `${BaseRoute}/:authId/sms-mfa`, + authMiddleware.s2sWriteUser, + userHandler.deleteUserSmsMfa, +) diff --git a/server/src/scripts/schemas/user.cjs b/server/src/scripts/schemas/user.cjs index 6d90fb6e..5f9730d1 100644 --- a/server/src/scripts/schemas/user.cjs +++ b/server/src/scripts/schemas/user.cjs @@ -26,6 +26,7 @@ const User = { locale: { type: 'string' }, emailVerified: { type: 'boolean' }, otpVerified: { type: 'boolean' }, + smsPhoneNumberVerified: { type: 'boolean' }, isActive: { type: 'boolean' }, createdAt: { type: 'string' }, updatedAt: { type: 'string' }, diff --git a/server/src/scripts/swagger.json b/server/src/scripts/swagger.json index 22d087e4..0319cd7e 100644 --- a/server/src/scripts/swagger.json +++ b/server/src/scripts/swagger.json @@ -384,6 +384,9 @@ "otpVerified": { "type": "boolean" }, + "smsPhoneNumberVerified": { + "type": "boolean" + }, "isActive": { "type": "boolean" }, @@ -1341,7 +1344,55 @@ }, "/api/v1/users/{authId}/otp-mfa": { "post": { - "summary": "enroll user for authenticator MFA.", + "summary": "enroll user for OTP MFA.", + "description": "Required scope - write_user", + "tags": [ + "Users" + ], + "parameters": [ + { + "in": "path", + "name": "authId", + "required": true, + "schema": { + "type": "string" + }, + "description": "The authId of the user" + } + ], + "responses": { + "204": { + "description": "Successful operation with no content to return" + } + } + }, + "delete": { + "summary": "Remove user's current OTP MFA setup.", + "description": "Required scope - write_user", + "tags": [ + "Users" + ], + "parameters": [ + { + "in": "path", + "name": "authId", + "required": true, + "schema": { + "type": "string" + }, + "description": "The authId of the user" + } + ], + "responses": { + "204": { + "description": "Successful operation with no content to return" + } + } + } + }, + "/api/v1/users/{authId}/sms-mfa": { + "post": { + "summary": "enroll user for SMS MFA.", "description": "Required scope - write_user", "tags": [ "Users" @@ -1364,7 +1415,7 @@ } }, "delete": { - "summary": "Remove user's current authenticator MFA setup.", + "summary": "Remove user's current SMS MFA setup.", "description": "Required scope - write_user", "tags": [ "Users" diff --git a/server/src/services/user.ts b/server/src/services/user.ts index c6289f8a..2f635181 100644 --- a/server/src/services/user.ts +++ b/server/src/services/user.ts @@ -420,6 +420,7 @@ export const enrollUserMfa = async ( const { OTP_MFA_IS_REQUIRED: otpMfaRequired, EMAIL_MFA_IS_REQUIRED: emailMfaRequired, + SMS_MFA_IS_REQUIRED: smsMfaRequired, } = env(c) const user = await userModel.getByAuthId( @@ -434,23 +435,33 @@ export const enrollUserMfa = async ( throw new errorConfig.Forbidden(localeConfig.Error.UserDisabled) } - const isOtp = mfaType === userModel.MfaType.Otp - if (isOtp && otpMfaRequired) return user - if (mfaType === userModel.MfaType.Email && emailMfaRequired) return user - if (user.mfaTypes.includes(mfaType)) return user - const newUser = await userModel.update( - c.env.DB, - user.id, - { - mfaTypes: [...user.mfaTypes, mfaType].join(','), - otpVerified: isOtp ? 0 : undefined, - otpSecret: isOtp ? cryptoUtil.genOtpSecret() : undefined, - }, - ) + if (mfaType === userModel.MfaType.Otp && !otpMfaRequired) { + return userModel.update( + c.env.DB, + user.id, + { + mfaTypes: [...user.mfaTypes, mfaType].join(','), + otpVerified: 0, + otpSecret: cryptoUtil.genOtpSecret(), + }, + ) + } else if (mfaType === userModel.MfaType.Email && !emailMfaRequired) { + return userModel.update( + c.env.DB, + user.id, + { mfaTypes: [...user.mfaTypes, mfaType].join(',') }, + ) + } else if (mfaType === userModel.MfaType.Sms && !smsMfaRequired) { + return userModel.update( + c.env.DB, + user.id, + { mfaTypes: [...user.mfaTypes, mfaType].join(',') }, + ) + } - return newUser + return user } export const resetUserMfa = async ( @@ -470,23 +481,45 @@ export const resetUserMfa = async ( throw new errorConfig.Forbidden(localeConfig.Error.UserDisabled) } - const isOtp = mfaType === userModel.MfaType.Otp - if ( - isOtp && - !user.mfaTypes.includes(userModel.MfaType.Otp) && - !user.otpVerified && !user.otpSecret - ) return true - if (mfaType === userModel.MfaType.Email && !user.mfaTypes.includes(userModel.MfaType.Email)) return true + if (mfaType === userModel.MfaType.Otp) { + if ( + !user.mfaTypes.includes(userModel.MfaType.Otp) && + !user.otpVerified && + !user.otpSecret + ) return true - await userModel.update( - c.env.DB, - user.id, - { - mfaTypes: user.mfaTypes.filter((type) => type !== mfaType).join(','), - otpVerified: isOtp ? 0 : undefined, - otpSecret: isOtp ? '' : undefined, - }, - ) + await userModel.update( + c.env.DB, + user.id, + { + mfaTypes: user.mfaTypes.filter((type) => type !== mfaType).join(','), + otpVerified: 0, + otpSecret: '', + }, + ) + } else if (mfaType === userModel.MfaType.Email) { + if (!user.mfaTypes.includes(userModel.MfaType.Email)) return true + await userModel.update( + c.env.DB, + user.id, + { mfaTypes: user.mfaTypes.filter((type) => type !== mfaType).join(',') }, + ) + } else if (mfaType === userModel.MfaType.Sms) { + if ( + !user.mfaTypes.includes(userModel.MfaType.Sms) && + !user.smsPhoneNumber && + !user.smsPhoneNumberVerified + ) return true + await userModel.update( + c.env.DB, + user.id, + { + mfaTypes: user.mfaTypes.filter((type) => type !== mfaType).join(','), + smsPhoneNumber: null, + smsPhoneNumberVerified: 0, + }, + ) + } return true }