Skip to content

Commit

Permalink
Support resend change email code (#192)
Browse files Browse the repository at this point in the history
  • Loading branch information
byn9826 authored Nov 19, 2024
1 parent 9cd92ee commit 425eca5
Show file tree
Hide file tree
Showing 15 changed files with 220 additions and 19 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@
- OTP MFA
- SMS MFA
- MFA Enrollment
- <b>Policy</b>
- <b>Policy</b> [How to trigger a different policy](https://auth.valuemelody.com/q_a.html#how-to-trigger-a-different-policy)
- sign_in_or_sign_up
- change_password
- change_email
- <b>Mailer Option</b>:
- SendGrid
- Mailgun
Expand All @@ -74,6 +75,7 @@
- OTP MFA attempts
- SMS MFA attempts
- Email MFA attempts
- Change Email attempts
- <b>Logging</b>:
- Email Logs
- SMS Logs
Expand Down
6 changes: 6 additions & 0 deletions admin-panel/app/[lang]/dashboard/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,12 @@ const Page = () => {
{configs.EMAIL_MFA_EMAIL_THRESHOLD}
</Table.Cell>
</Table.Row>
<Table.Row>
<Table.Cell>CHANGE_EMAIL_EMAIL_THRESHOLD</Table.Cell>
<Table.Cell>
{configs.CHANGE_EMAIL_EMAIL_THRESHOLD}
</Table.Cell>
</Table.Row>
<Table.Row>
<Table.Cell>ENFORCE_ONE_MFA_ENROLLMENT</Table.Cell>
<Table.Cell>
Expand Down
4 changes: 4 additions & 0 deletions docs/auth-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ npm run prod:deploy
- **Default:** 10
- **Description:** Maximum number of Email MFA email requests allowed per 30 minutes for a single account based on ip address. 0 means no restriction.

### CHANGE_EMAIL_EMAIL_THRESHOLD
- **Default:** 5
- **Description:** Maximum number of Change Email verification code requests allowed per 30 minutes for a single account. 0 means no restriction.

### ENFORCE_ONE_MFA_ENROLLMENT
- **Default:** ['otp', 'email']
- **Description:** Enforce one MFA type from the list. Available options are ‘email’, ‘otp’, and ‘sms’. This setting is only effective if OTP_MFA_IS_REQUIRED, SMS_MFA_IS_REQUIRED, and EMAIL_MFA_IS_REQUIRED are all set to false. An empty list means no MFA type will be enforced. You must enable email functionality for the email MFA option to work.
Expand Down
102 changes: 93 additions & 9 deletions server/src/__tests__/normal/identity-policy.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,23 @@ import {
} from 'vitest'
import { Database } from 'better-sqlite3'
import { JSDOM } from 'jsdom'
import { Context } from 'hono'
import app from 'index'
import {
migrate, mock,
mockedKV,
} from 'tests/mock'
import {
adapterConfig, routeConfig,
adapterConfig, localeConfig, routeConfig,
typeConfig,
} from 'configs'
import {
prepareFollowUpBody, prepareFollowUpParams,
insertUsers, postSignInRequest, getApp,
postAuthorizeBody,
} from 'tests/identity'
import { jwtService } from 'services'
import { cryptoUtil } from 'utils'

let db: Database

Expand Down Expand Up @@ -254,6 +259,21 @@ describe(
describe(
'post /change-email-code',
() => {
const sendEmailCode = async (code: string) => {
return app.request(
routeConfig.IdentityRoute.ChangeEmailCode,
{
method: 'POST',
body: JSON.stringify({
code,
email: 'test_new@email.com',
locale: 'en',
}),
},
mock(db),
)
}

test(
'could send code',
async () => {
Expand All @@ -263,22 +283,86 @@ describe(
)
const body = await prepareFollowUpBody(db)

const res = await app.request(
routeConfig.IdentityRoute.ChangeEmailCode,
const res = await sendEmailCode(body.code)
const json = await res.json()
expect(json).toStrictEqual({ success: true })

expect((await mockedKV.get(`${adapterConfig.BaseKVKey.ChangeEmailCode}-1-test_new@email.com`) ?? '').length).toBe(8)
},
)

test(
'should stop after reach threshold',
async () => {
global.process.env.CHANGE_EMAIL_EMAIL_THRESHOLD = 2 as unknown as string

await insertUsers(
db,
false,
)
const body = await prepareFollowUpBody(db)

const res = await sendEmailCode(body.code)
const json = await res.json()
expect(json).toStrictEqual({ success: true })
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.ChangeEmailAttempts}-test@email.com`)).toBe('1')

const res1 = await sendEmailCode(body.code)
const json1 = await res1.json()
expect(json1).toStrictEqual({ success: true })
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.ChangeEmailAttempts}-test@email.com`)).toBe('2')

const res2 = await sendEmailCode(body.code)
expect(res2.status).toBe(400)

global.process.env.CHANGE_EMAIL_EMAIL_THRESHOLD = 0 as unknown as string
const res3 = await sendEmailCode(body.code)
expect(res3.status).toBe(200)
const json3 = await res3.json()
expect(json3).toStrictEqual({ success: true })

global.process.env.CHANGE_EMAIL_EMAIL_THRESHOLD = 5 as unknown as string
},
)

test(
'should throw error for social account',
async () => {
global.process.env.GOOGLE_AUTH_CLIENT_ID = '123'
const publicKey = await mockedKV.get(adapterConfig.BaseKVKey.JwtPublicSecret)
const jwk = await cryptoUtil.secretToJwk(publicKey ?? '')
const c = { env: { KV: mockedKV } } as unknown as Context<typeConfig.Context>
const credential = await jwtService.signWithKid(
c,
{
iss: 'https://accounts.google.com',
email: 'test@gmail.com',
sub: 'gid123',
email_verified: true,
given_name: 'first',
family_name: 'last',
kid: jwk.kid,
},
)

const appRecord = await getApp(db)
const tokenRes = await app.request(
routeConfig.IdentityRoute.AuthorizeGoogle,
{
method: 'POST',
body: JSON.stringify({
code: body.code,
email: 'test_new@email.com',
locale: 'en',
...(await postAuthorizeBody(appRecord)),
credential,
}),
},
mock(db),
)
const json = await res.json()
expect(json).toStrictEqual({ success: true })
const tokenJson = await tokenRes.json() as { code: string }

expect((await mockedKV.get(`${adapterConfig.BaseKVKey.ChangeEmailCode}-1-test_new@email.com`) ?? '').length).toBe(8)
const res = await sendEmailCode(tokenJson.code)
expect(res.status).toBe(400)
expect(await res.text()).toBe(localeConfig.Error.SocialAccountNotSupported)
global.process.env.GOOGLE_AUTH_CLIENT_ID = ''
},
)
},
Expand Down
1 change: 1 addition & 0 deletions server/src/__tests__/normal/other.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ describe(
ENABLE_EMAIL_VERIFICATION: true,
EMAIL_MFA_IS_REQUIRED: false,
EMAIL_MFA_EMAIL_THRESHOLD: 10,
CHANGE_EMAIL_EMAIL_THRESHOLD: 5,
OTP_MFA_IS_REQUIRED: false,
SMS_MFA_IS_REQUIRED: false,
SMS_MFA_MESSAGE_THRESHOLD: 5,
Expand Down
1 change: 1 addition & 0 deletions server/src/configs/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export enum BaseKVKey {
EmailMfaEmailAttempts = 'EMEA',
PasswordResetAttempts = 'PRA',
ChangeEmailCode = 'CEC',
ChangeEmailAttempts = 'CEA',
}

export const getKVKey = (
Expand Down
10 changes: 10 additions & 0 deletions server/src/configs/locale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ export enum Error {
EmailTaken = 'The email address is already in use.',
WrongRedirectUri = 'Invalid redirect_uri',
NoUser = 'No user found',
SocialAccountNotSupported = 'This function is unavailable for social login accounts.',
AccountLocked = 'Account temporarily locked due to excessive login failures',
OtpMfaLocked = 'Too many failed OTP verification attempts. Please try again after 30 minutes.',
SmsMfaLocked = 'Too many SMS verification attempts. Please try again after 30 minutes.',
EmailMfaLocked = 'Too many Email verification attempts. Please try again after 30 minutes.',
PasswordResetLocked = 'Too many password reset email requests. Please try again tomorrow.',
ChangeEmailLocked = 'Too many password change email requests. Please try again after 30 minutes.',
UserDisabled = 'This account has been disabled',
EmailAlreadyVerified = 'Email already verified',
OtpAlreadySet = 'OTP authentication already set',
Expand Down Expand Up @@ -444,6 +446,14 @@ export const changeEmail = Object.freeze({
en: 'Verification Code',
fr: 'Code de vérification',
},
resend: {
en: 'Resend a new code',
fr: 'Renvoyer un nouveau code',
},
resent: {
en: 'New code sent.',
fr: 'Nouveau code envoyé.',
},
})

export const verifyEmail = Object.freeze({
Expand Down
1 change: 1 addition & 0 deletions server/src/configs/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export type Bindings = {
ENABLE_EMAIL_VERIFICATION: boolean;
EMAIL_MFA_IS_REQUIRED: boolean;
EMAIL_MFA_EMAIL_THRESHOLD: number;
CHANGE_EMAIL_EMAIL_THRESHOLD: number;
OTP_MFA_IS_REQUIRED: boolean;
GOOGLE_AUTH_CLIENT_ID: string;
ENFORCE_ONE_MFA_ENROLLMENT: userModel.MfaType[];
Expand Down
10 changes: 5 additions & 5 deletions server/src/handlers/identity/other.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ export const postResetCode = async (c: Context<typeConfig.Context>) => {
if (!email) throw new errorConfig.Forbidden()

const ip = requestUtil.getRequestIP(c)
const resetAttempts = await kvService.getPasswordResetAttemptsByIP(
c.env.KV,
email,
ip,
)
const { PASSWORD_RESET_EMAIL_THRESHOLD: resetThreshold } = env(c)

if (resetThreshold) {
const resetAttempts = await kvService.getPasswordResetAttemptsByIP(
c.env.KV,
email,
ip,
)
if (resetAttempts >= resetThreshold) throw new errorConfig.Forbidden(localeConfig.Error.PasswordResetLocked)

await kvService.setPasswordResetAttemptsByIP(
Expand Down
29 changes: 29 additions & 0 deletions server/src/handlers/identity/policy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ import { validateUtil } from 'utils'
import {
ChangeEmail, ChangePassword,
} from 'views'
import { userModel } from 'models'

const checkAccount = (user: userModel.Record) => {
if (!user.email || user.socialAccountId) {
throw new errorConfig.Forbidden(localeConfig.Error.SocialAccountNotSupported)
}
}

export const getChangePassword = async (c: Context<typeConfig.Context>) => {
const queryDto = await identityDto.parseGetAuthorizeFollowUpReq(c)
Expand All @@ -23,6 +30,7 @@ export const getChangePassword = async (c: Context<typeConfig.Context>) => {
queryDto.code,
)
if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${queryDto.locale}`)
checkAccount(authInfo.user)

const {
COMPANY_LOGO_URL: logoUrl,
Expand All @@ -49,6 +57,7 @@ export const postChangePassword = async (c: Context<typeConfig.Context>) => {
bodyDto.code,
)
if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${bodyDto.locale}`)
checkAccount(authInfo.user)

await userService.changeUserPassword(
c,
Expand All @@ -67,6 +76,7 @@ export const getChangeEmail = async (c: Context<typeConfig.Context>) => {
queryDto.code,
)
if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${queryDto.locale}`)
checkAccount(authInfo.user)

const {
COMPANY_LOGO_URL: logoUrl,
Expand All @@ -93,6 +103,7 @@ export const postChangeEmail = async (c: Context<typeConfig.Context>) => {
bodyDto.code,
)
if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${bodyDto.locale}`)
checkAccount(authInfo.user)

const isCorrectCode = await kvService.verifyChangeEmailCode(
c.env.KV,
Expand Down Expand Up @@ -123,6 +134,24 @@ export const postVerificationCode = async (c: Context<typeConfig.Context>) => {
bodyDto.code,
)
if (!authInfo) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${bodyDto.locale}`)
checkAccount(authInfo.user)

const { CHANGE_EMAIL_EMAIL_THRESHOLD: emailThreshold } = env(c)

if (emailThreshold) {
const emailAttempts = await kvService.getChangeEmailAttempts(
c.env.KV,
authInfo.user.email ?? '',
)

if (emailAttempts >= emailThreshold) throw new errorConfig.Forbidden(localeConfig.Error.ChangeEmailLocked)

await kvService.setChangeEmailAttempts(
c.env.KV,
authInfo.user.email ?? '',
emailAttempts + 1,
)
}

const code = await emailService.sendChangeEmailVerificationCode(
c,
Expand Down
1 change: 1 addition & 0 deletions server/src/handlers/other.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const getSystemInfo = async (c: Context<typeConfig.Context>) => {
ENABLE_EMAIL_VERIFICATION: environment.ENABLE_EMAIL_VERIFICATION,
EMAIL_MFA_IS_REQUIRED: environment.EMAIL_MFA_IS_REQUIRED,
EMAIL_MFA_EMAIL_THRESHOLD: environment.EMAIL_MFA_EMAIL_THRESHOLD,
CHANGE_EMAIL_EMAIL_THRESHOLD: environment.CHANGE_EMAIL_EMAIL_THRESHOLD,
OTP_MFA_IS_REQUIRED: environment.OTP_MFA_IS_REQUIRED,
SMS_MFA_IS_REQUIRED: environment.SMS_MFA_IS_REQUIRED,
SMS_MFA_MESSAGE_THRESHOLD: environment.SMS_MFA_MESSAGE_THRESHOLD,
Expand Down
28 changes: 28 additions & 0 deletions server/src/services/kv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -603,3 +603,31 @@ export const verifyChangeEmailCode = async (
if (isValid) await kv.delete(key)
return isValid
}

export const getChangeEmailAttempts = async (
kv: KVNamespace,
email: string,
) => {
const key = adapterConfig.getKVKey(
adapterConfig.BaseKVKey.ChangeEmailAttempts,
email,
)
const stored = await kv.get(key)
return stored ? Number(stored) : 0
}

export const setChangeEmailAttempts = async (
kv: KVNamespace,
email: string,
count: number,
) => {
const key = adapterConfig.getKVKey(
adapterConfig.BaseKVKey.ChangeEmailAttempts,
email,
)
await kv.put(
key,
String(count),
{ expirationTtl: 1800 },
)
}
4 changes: 0 additions & 4 deletions server/src/services/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,6 @@ export const changeUserEmail = async (
user: userModel.Record,
bodyDto: identityDto.PostChangeEmailReqDto,
): Promise<true> => {
if (!user.email || user.socialAccountId) {
throw new errorConfig.NotFound(localeConfig.Error.NoUser)
}

const isSame = user.email === bodyDto.email
if (isSame) {
throw new errorConfig.Forbidden(localeConfig.Error.RequireDifferentEmail)
Expand Down
Loading

0 comments on commit 425eca5

Please sign in to comment.