From a71ae7de5cd2a05327befd9586618fac3f27d8f5 Mon Sep 17 00:00:00 2001 From: Baozier Date: Thu, 28 Nov 2024 21:42:04 -0500 Subject: [PATCH] Support reset_mfa policy (#195) --- README.md | 1 + admin-panel/app/[lang]/account/page.tsx | 11 +- admin-panel/translations/en.json | 3 +- admin-panel/translations/fr.json | 3 +- .../__tests__/normal/identity-policy.test.tsx | 242 +++++++++++++++++- server/src/__tests__/normal/oauth.test.tsx | 27 ++ server/src/configs/locale.ts | 23 ++ server/src/configs/route.ts | 1 + server/src/dtos/oauth.ts | 1 + server/src/handlers/identity/main.tsx | 14 +- server/src/handlers/identity/policy.tsx | 52 +++- server/src/handlers/oauth.ts | 3 + server/src/routes/identity.tsx | 10 + server/src/services/user.ts | 14 +- server/src/utils/identity.ts | 35 +-- server/src/views/ResetMfa.tsx | 86 +++++++ server/src/views/index.tsx | 2 + 17 files changed, 492 insertions(+), 36 deletions(-) create mode 100644 server/src/views/ResetMfa.tsx diff --git a/README.md b/README.md index 2c468e2..4af0517 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,7 @@ - sign_in_or_sign_up - change_password - change_email + - reset_mfa - Mailer Option: - SendGrid - Mailgun diff --git a/admin-panel/app/[lang]/account/page.tsx b/admin-panel/app/[lang]/account/page.tsx index a4e8ff6..b0f14ab 100644 --- a/admin-panel/app/[lang]/account/page.tsx +++ b/admin-panel/app/[lang]/account/page.tsx @@ -23,14 +23,23 @@ const Page = () => { }) } + const handleResetMfa = () => { + loginRedirect({ + locale: locale || undefined, policy: 'reset_mfa', + }) + } + return ( -
+
+
) } diff --git a/admin-panel/translations/en.json b/admin-panel/translations/en.json index 177f3aa..339361a 100644 --- a/admin-panel/translations/en.json +++ b/admin-panel/translations/en.json @@ -13,7 +13,8 @@ }, "account": { "changePassword": "Change Password", - "changeEmail": "Change Email" + "changeEmail": "Change Email", + "resetMfa": "Reset MFA" }, "common": { "enable": "Enable", diff --git a/admin-panel/translations/fr.json b/admin-panel/translations/fr.json index cf90860..6eb8079 100644 --- a/admin-panel/translations/fr.json +++ b/admin-panel/translations/fr.json @@ -13,7 +13,8 @@ }, "account": { "changePassword": "Changer le mot de passe", - "changeEmail": "Changer l'email" + "changeEmail": "Changer l'email", + "resetMfa": "Réinitialiser MFA" }, "common": { "enable": "Activer", diff --git a/server/src/__tests__/normal/identity-policy.test.tsx b/server/src/__tests__/normal/identity-policy.test.tsx index 7117a1c..c17a4f6 100644 --- a/server/src/__tests__/normal/identity-policy.test.tsx +++ b/server/src/__tests__/normal/identity-policy.test.tsx @@ -20,6 +20,9 @@ import { } from 'tests/identity' import { jwtService } from 'services' import { cryptoUtil } from 'utils' +import { + enrollEmailMfa, enrollOtpMfa, enrollSmsMfa, +} from 'tests/util' let db: Database @@ -149,7 +152,7 @@ describe( ) test( - 'should redirect if use wrong auth code', + 'should throw 400 if use wrong auth code', async () => { await insertUsers( db, @@ -169,8 +172,8 @@ describe( }, mock(db), ) - expect(res.status).toBe(302) - expect(res.headers.get('Location')).toBe(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=en`) + expect(res.status).toBe(400) + expect(await res.text()).toBe(localeConfig.Error.WrongAuthCode) }, ) }, @@ -422,7 +425,7 @@ describe( ) test( - 'should redirect if use wrong auth code', + 'should throw 400 if use wrong auth code', async () => { await insertUsers( db, @@ -443,9 +446,240 @@ describe( }, mock(db), ) + expect(res.status).toBe(400) + expect(await res.text()).toBe(localeConfig.Error.WrongAuthCode) + }, + ) + }, +) + +describe( + 'get /reset-mfa', + () => { + test( + 'should show reset mfa page', + async () => { + await insertUsers( + db, + false, + ) + const params = await prepareFollowUpParams(db) + + const res = await app.request( + `${routeConfig.IdentityRoute.ResetMfa}${params}`, + {}, + mock(db), + ) + + const html = await res.text() + const dom = new JSDOM(html) + const document = dom.window.document + expect(document.getElementsByTagName('button').length).toBe(1) + expect(document.getElementsByTagName('select').length).toBe(1) + }, + ) + + test( + 'should redirect if use wrong auth code', + async () => { + await insertUsers( + db, + false, + ) + await prepareFollowUpParams(db) + + const res = await app.request( + `${routeConfig.IdentityRoute.ResetMfa}?locale=en&code=abc`, + {}, + mock(db), + ) expect(res.status).toBe(302) expect(res.headers.get('Location')).toBe(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=en`) }, ) + + test( + 'could disable locale selector', + async () => { + global.process.env.ENABLE_LOCALE_SELECTOR = false as unknown as string + await insertUsers( + db, + false, + ) + const params = await prepareFollowUpParams(db) + + const res = await app.request( + `${routeConfig.IdentityRoute.ChangePassword}${params}`, + {}, + mock(db), + ) + + const html = await res.text() + const dom = new JSDOM(html) + const document = dom.window.document + expect(document.getElementsByTagName('select').length).toBe(0) + global.process.env.ENABLE_LOCALE_SELECTOR = true as unknown as string + }, + ) + }, +) + +describe( + 'post /reset-mfa', + () => { + test( + 'should reset email mfa', + async () => { + process.env.ENABLE_USER_APP_CONSENT = false as unknown as string + + await insertUsers( + db, + false, + ) + enrollEmailMfa(db) + const body = await prepareFollowUpBody(db) + + const res = await app.request( + routeConfig.IdentityRoute.ResetMfa, + { + method: 'POST', + body: JSON.stringify({ ...body }), + }, + mock(db), + ) + const json = await res.json() + expect(json).toStrictEqual({ success: true }) + + const appRecord = await getApp(db) + const reLoginRes = await postSignInRequest( + db, + appRecord, + { password: 'Password1!' }, + ) + const loginResJson = await reLoginRes.json() as { code: string } + expect(loginResJson).toStrictEqual({ + code: expect.any(String), + redirectUri: 'http://localhost:3000/en/dashboard', + state: '123', + scopes: ['profile', 'openid', 'offline_access'], + nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll, + }) + + process.env.ENABLE_USER_APP_CONSENT = true as unknown as string + }, + ) + + test( + 'should reset sms mfa', + async () => { + process.env.ENABLE_USER_APP_CONSENT = false as unknown as string + + await insertUsers( + db, + false, + ) + await enrollSmsMfa(db) + await db.prepare('update "user" set "smsPhoneNumber" = ?, "smsPhoneNumberVerified" = ?').run( + '+16471231234', + 1, + ) + const body = await prepareFollowUpBody(db) + + const res = await app.request( + routeConfig.IdentityRoute.ResetMfa, + { + method: 'POST', + body: JSON.stringify({ ...body }), + }, + mock(db), + ) + const json = await res.json() + expect(json).toStrictEqual({ success: true }) + + const appRecord = await getApp(db) + const reLoginRes = await postSignInRequest( + db, + appRecord, + { password: 'Password1!' }, + ) + const loginResJson = await reLoginRes.json() as { code: string } + expect(loginResJson).toStrictEqual({ + code: expect.any(String), + redirectUri: 'http://localhost:3000/en/dashboard', + state: '123', + scopes: ['profile', 'openid', 'offline_access'], + nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll, + }) + + process.env.ENABLE_USER_APP_CONSENT = true as unknown as string + }, + ) + + test( + 'should reset otp mfa', + async () => { + process.env.ENABLE_USER_APP_CONSENT = false as unknown as string + + await insertUsers( + db, + false, + ) + await enrollOtpMfa(db) + const body = await prepareFollowUpBody(db) + + const res = await app.request( + routeConfig.IdentityRoute.ResetMfa, + { + method: 'POST', + body: JSON.stringify({ ...body }), + }, + mock(db), + ) + const json = await res.json() + expect(json).toStrictEqual({ success: true }) + + const appRecord = await getApp(db) + const reLoginRes = await postSignInRequest( + db, + appRecord, + { password: 'Password1!' }, + ) + const loginResJson = await reLoginRes.json() as { code: string } + expect(loginResJson).toStrictEqual({ + code: expect.any(String), + redirectUri: 'http://localhost:3000/en/dashboard', + state: '123', + scopes: ['profile', 'openid', 'offline_access'], + nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll, + }) + + process.env.ENABLE_USER_APP_CONSENT = true as unknown as string + }, + ) + + test( + 'should redirect if use wrong auth code', + async () => { + await insertUsers( + db, + false, + ) + await prepareFollowUpBody(db) + + const res = await app.request( + routeConfig.IdentityRoute.ResetMfa, + { + method: 'POST', + body: JSON.stringify({ + locale: 'en', + code: 'abc', + }), + }, + mock(db), + ) + expect(res.status).toBe(400) + expect(await res.text()).toBe(localeConfig.Error.WrongAuthCode) + }, + ) }, ) diff --git a/server/src/__tests__/normal/oauth.test.tsx b/server/src/__tests__/normal/oauth.test.tsx index 0dfc430..0ec89e4 100644 --- a/server/src/__tests__/normal/oauth.test.tsx +++ b/server/src/__tests__/normal/oauth.test.tsx @@ -243,6 +243,33 @@ describe( }, ) + test( + 'could redirect to reset mfa through session', + async () => { + global.process.env.ENFORCE_ONE_MFA_ENROLLMENT = [] as unknown as string + const appRecord = await getApp(db) + await insertUsers(db) + await postSignInRequest( + db, + appRecord, + ) + + const url = routeConfig.OauthRoute.Authorize + const res = await getSignInRequest( + db, + url, + appRecord, + '&policy=reset_mfa', + ) + expect(res.status).toBe(302) + const path = res.headers.get('Location') + expect(path).toContain(`${routeConfig.IdentityRoute.ResetMfa}`) + expect(path).toContain('&code=') + + global.process.env.ENFORCE_ONE_MFA_ENROLLMENT = ['email', 'otp'] as unknown as string + }, + ) + test( 'could redirect to change email through session', async () => { diff --git a/server/src/configs/locale.ts b/server/src/configs/locale.ts index afd7b8d..9aa86ed 100644 --- a/server/src/configs/locale.ts +++ b/server/src/configs/locale.ts @@ -475,6 +475,29 @@ export const verifyEmail = Object.freeze({ }, }) +export const resetMfa = Object.freeze({ + title: { + en: 'Reset your MFA', + fr: 'Réinitialisez votre MFA', + }, + success: { + en: 'Reset success!', + fr: 'Réinitialisation réussie!', + }, + desc: { + en: 'Your current Multi-Factor Authentication (MFA) method will be reset. After this reset, you will need to set up MFA again to ensure continued secure access to your account.', + fr: "Votre méthode actuelle d'authentification multifactorielle (MFA) sera réinitialisée. Après cette réinitialisation, vous devrez configurer à nouveau votre MFA pour garantir un accès sécurisé continu à votre compte.", + }, + confirm: { + en: 'Confirm', + fr: 'Confirmer', + }, + redirect: { + en: 'Redirect back', + fr: 'Rediriger en arrière', + }, +}) + export const emailVerificationEmail = Object.freeze({ subject: { en: 'Welcome to Melody Auth! Please verify your email address', diff --git a/server/src/configs/route.ts b/server/src/configs/route.ts index ff5af18..3cfd9d2 100644 --- a/server/src/configs/route.ts +++ b/server/src/configs/route.ts @@ -40,4 +40,5 @@ export enum IdentityRoute { ChangePassword = `${InternalRoute.Identity}/change-password`, ChangeEmail = `${InternalRoute.Identity}/change-email`, ChangeEmailCode = `${InternalRoute.Identity}/change-email-code`, + ResetMfa = `${InternalRoute.Identity}/reset-mfa`, } diff --git a/server/src/dtos/oauth.ts b/server/src/dtos/oauth.ts index b4a8d64..21011c1 100644 --- a/server/src/dtos/oauth.ts +++ b/server/src/dtos/oauth.ts @@ -23,6 +23,7 @@ export enum Policy { SignInOrSignUp = 'sign_in_or_sign_up', ChangePassword = 'change_password', ChangeEmail = 'change_email', + ResetMfa = 'reset_mfa', } const parseScopes = (scopes: string[]) => scopes.map((s) => s.trim().toLowerCase()) diff --git a/server/src/handlers/identity/main.tsx b/server/src/handlers/identity/main.tsx index 697f3f7..19098d8 100644 --- a/server/src/handlers/identity/main.tsx +++ b/server/src/handlers/identity/main.tsx @@ -41,13 +41,13 @@ export const getAuthorizePassword = async (c: Context) => { GITHUB_AUTH_APP_NAME: githubAppName, } = env(c) - const isChangePassword = queryDto.policy === Policy.ChangePassword - const enablePasswordReset = isChangePassword ? false : allowPasswordReset - const enableSignUp = isChangePassword ? false : allowSignUp - const enablePasswordSignIn = isChangePassword ? true : allowPasswordSignIn - const googleClientId = isChangePassword ? '' : googleAuthId - const facebookClientId = isChangePassword ? '' : facebookAuthId - const githubClientId = isChangePassword ? '' : githubAuthId + const isBasePolicy = !queryDto.policy || queryDto.policy === Policy.SignInOrSignUp + const enablePasswordReset = isBasePolicy ? allowPasswordReset : false + const enableSignUp = isBasePolicy ? allowSignUp : false + const enablePasswordSignIn = isBasePolicy ? allowPasswordSignIn : true + const googleClientId = isBasePolicy ? googleAuthId : '' + const facebookClientId = isBasePolicy ? facebookAuthId : '' + const githubClientId = isBasePolicy ? githubAuthId : '' const queryString = requestUtil.getQueryString(c) diff --git a/server/src/handlers/identity/policy.tsx b/server/src/handlers/identity/policy.tsx index 2632f6e..1fba781 100644 --- a/server/src/handlers/identity/policy.tsx +++ b/server/src/handlers/identity/policy.tsx @@ -12,7 +12,7 @@ import { } from 'services' import { validateUtil } from 'utils' import { - ChangeEmail, ChangePassword, + ChangeEmail, ChangePassword, ResetMfa, } from 'views' import { userModel } from 'models' @@ -56,7 +56,7 @@ export const postChangePassword = async (c: Context) => { c.env.KV, bodyDto.code, ) - if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${bodyDto.locale}`) + if (!authInfo) throw new errorConfig.Forbidden(localeConfig.Error.WrongAuthCode) checkAccount(authInfo.user) await userService.changeUserPassword( @@ -102,7 +102,7 @@ export const postChangeEmail = async (c: Context) => { c.env.KV, bodyDto.code, ) - if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${bodyDto.locale}`) + if (!authInfo) throw new errorConfig.Forbidden(localeConfig.Error.WrongAuthCode) checkAccount(authInfo.user) const isCorrectCode = await kvService.verifyChangeEmailCode( @@ -133,7 +133,7 @@ export const postVerificationCode = async (c: Context) => { c.env.KV, bodyDto.code, ) - if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${bodyDto.locale}`) + if (!authInfo) throw new errorConfig.Forbidden(localeConfig.Error.WrongAuthCode) checkAccount(authInfo.user) const { CHANGE_EMAIL_EMAIL_THRESHOLD: emailThreshold } = env(c) @@ -169,3 +169,47 @@ export const postVerificationCode = async (c: Context) => { return c.json({ success: true }) } + +export const getResetMfa = async (c: Context) => { + const queryDto = await identityDto.parseGetAuthorizeFollowUpReq(c) + + const authInfo = await kvService.getAuthCodeBody( + c.env.KV, + queryDto.code, + ) + if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${queryDto.locale}`) + checkAccount(authInfo.user) + + const { + COMPANY_LOGO_URL: logoUrl, + SUPPORTED_LOCALES: locales, + ENABLE_LOCALE_SELECTOR: enableLocaleSelector, + } = env(c) + + return c.html() +} + +export const postResetMfa = async (c: Context) => { + const reqBody = await c.req.json() + + const bodyDto = new identityDto.PostAuthorizeFollowUpReqDto(reqBody) + await validateUtil.dto(bodyDto) + + const authCodeBody = await kvService.getAuthCodeBody( + c.env.KV, + bodyDto.code, + ) + if (!authCodeBody) throw new errorConfig.Forbidden(localeConfig.Error.WrongAuthCode) + + await userService.resetUserMfa( + c, + authCodeBody.user.authId, + ) + + return c.json({ success: true }) +} diff --git a/server/src/handlers/oauth.ts b/server/src/handlers/oauth.ts index 125e440..b3a494d 100644 --- a/server/src/handlers/oauth.ts +++ b/server/src/handlers/oauth.ts @@ -114,6 +114,9 @@ export const getAuthorize = async (c: Context) => { } else if (queryDto.policy === Policy.ChangeEmail) { const url = `${routeConfig.IdentityRoute.ChangeEmail}?state=${queryDto.state}&code=${authCode}&locale=${queryDto.locale}&redirect_uri=${queryDto.redirectUri}` return c.redirect(url) + } else if (queryDto.policy === Policy.ResetMfa) { + const url = `${routeConfig.IdentityRoute.ResetMfa}?state=${queryDto.state}&code=${authCode}&locale=${queryDto.locale}&redirect_uri=${queryDto.redirectUri}` + return c.redirect(url) } } diff --git a/server/src/routes/identity.tsx b/server/src/routes/identity.tsx index e343a86..89b96d0 100644 --- a/server/src/routes/identity.tsx +++ b/server/src/routes/identity.tsx @@ -221,3 +221,13 @@ identityRoutes.post( configMiddleware.enableEmailVerification, identityHandler.postChangeEmail, ) + +identityRoutes.get( + routeConfig.IdentityRoute.ResetMfa, + identityHandler.getResetMfa, +) + +identityRoutes.post( + routeConfig.IdentityRoute.ResetMfa, + identityHandler.postResetMfa, +) diff --git a/server/src/services/user.ts b/server/src/services/user.ts index 996ee23..de3bd42 100644 --- a/server/src/services/user.ts +++ b/server/src/services/user.ts @@ -512,7 +512,7 @@ export const enrollUserMfa = async ( export const resetUserMfa = async ( c: Context, authId: string, - mfaType: userModel.MfaType, + mfaType?: userModel.MfaType, ): Promise => { const user = await userModel.getByAuthId( c.env.DB, @@ -564,6 +564,18 @@ export const resetUserMfa = async ( smsPhoneNumberVerified: 0, }, ) + } else { + await userModel.update( + c.env.DB, + user.id, + { + mfaTypes: '', + otpVerified: 0, + otpSecret: '', + smsPhoneNumber: null, + smsPhoneNumberVerified: 0, + }, + ) } return true diff --git a/server/src/utils/identity.ts b/server/src/utils/identity.ts index 390b18b..6a2e344 100644 --- a/server/src/utils/identity.ts +++ b/server/src/utils/identity.ts @@ -107,25 +107,26 @@ export const processPostAuthorize = async ( } } - const requireChangePassword = - step < 6 && - !isSocialLogin && - enablePasswordReset && - authCodeBody.request.policy === Policy.ChangePassword - if (requireChangePassword) { - return { - ...basicInfo, nextPage: IdentityRoute.ChangePassword, + if (step < 6 && !isSocialLogin) { + const requireChangePassword = enablePasswordReset && authCodeBody.request.policy === Policy.ChangePassword + if (requireChangePassword) { + return { + ...basicInfo, nextPage: IdentityRoute.ChangePassword, + } } - } - const requireChangeEmail = - step < 6 && - !isSocialLogin && - enableEmailVerification && - authCodeBody.request.policy === Policy.ChangeEmail - if (requireChangeEmail) { - return { - ...basicInfo, nextPage: IdentityRoute.ChangeEmail, + const requireChangeEmail = enableEmailVerification && authCodeBody.request.policy === Policy.ChangeEmail + if (requireChangeEmail) { + return { + ...basicInfo, nextPage: IdentityRoute.ChangeEmail, + } + } + + const requireResetMfa = authCodeBody.request.policy === Policy.ResetMfa + if (requireResetMfa) { + return { + ...basicInfo, nextPage: IdentityRoute.ResetMfa, + } } } diff --git a/server/src/views/ResetMfa.tsx b/server/src/views/ResetMfa.tsx new file mode 100644 index 0000000..e229313 --- /dev/null +++ b/server/src/views/ResetMfa.tsx @@ -0,0 +1,86 @@ +import { html } from 'hono/html' +import SubmitError from './components/SubmitError' +import { + localeConfig, + routeConfig, + typeConfig, +} from 'configs' +import Layout from 'views/components/Layout' +import { responseScript } from 'views/scripts' +import Title from 'views/components/Title' +import { identityDto } from 'dtos' + +const ResetMfa = ({ + logoUrl, queryDto, locales, redirectUri, +}: { + logoUrl: string; + queryDto: identityDto.GetAuthorizeFollowUpReqDto; + locales: typeConfig.Locale[]; + redirectUri: string; +}) => { + return ( + + +
+ + <SubmitError /> + <p class='w-text'>{localeConfig.resetMfa.desc[queryDto.locale]}</p> + <button + class='button w-full' + type='button' + onclick='handleReset()' + > + {localeConfig.resetMfa.confirm[queryDto.locale]} + </button> + </section> + {html` + <script> + function handleReset () { + fetch('${routeConfig.IdentityRoute.ResetMfa}', { + method: 'POST', + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + code: "${queryDto.code}", + locale: "${queryDto.locale}" + }) + }) + .then((response) => { + ${responseScript.parseRes()} + }) + .then((data) => { + document.getElementById('submit-form').classList.add('hidden'); + document.getElementById('success-message').classList.remove('hidden'); + }) + .catch((error) => { + ${responseScript.handleSubmitError(queryDto.locale)} + }); + return false; + } + </script> + `} + </Layout> + ) +} + +export default ResetMfa diff --git a/server/src/views/index.tsx b/server/src/views/index.tsx index 7e2dd7b..c2eb49c 100644 --- a/server/src/views/index.tsx +++ b/server/src/views/index.tsx @@ -10,6 +10,7 @@ import VerifyEmailView from 'views/VerifyEmail' import AuthCodeExpired from 'views/AuthCodeExpired' import ChangePassword from 'views/ChangePassword' import ChangeEmail from 'views/ChangeEmail' +import ResetMfa from 'views/ResetMfa' export { AuthorizeAccountView, @@ -24,4 +25,5 @@ export { AuthCodeExpired, ChangePassword, ChangeEmail, + ResetMfa, }