Skip to content

Commit

Permalink
Simplify authorize page redirecting conditions (#194)
Browse files Browse the repository at this point in the history
  • Loading branch information
@byn9826
byn9826 authored Nov 23, 2024
1 parent ac0f001 commit f246aff
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 206 deletions.
81 changes: 17 additions & 64 deletions server/src/__tests__/normal/identity-main.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,14 +334,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: true,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll,
})
const { code } = json as { code: string }
const codeStore = JSON.parse(await mockedKV.get(`AC-${code}`) ?? '')
Expand Down Expand Up @@ -674,6 +667,7 @@ describe(
test(
'should get auth code after sign up',
async () => {
process.env.ENABLE_USER_APP_CONSENT = false as unknown as string
const mockFetch = vi.fn(async () => {
return Promise.resolve({ ok: true })
})
Expand All @@ -686,14 +680,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: true,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll,
})
const appRecord = await getApp(db)
const { code } = json as { code: string }
Expand All @@ -715,6 +702,7 @@ describe(
expect(body).toContain(`${routeConfig.IdentityRoute.VerifyEmail}?id=${codeStore.user.authId}&locale=en`)

global.fetch = fetchMock
process.env.ENABLE_USER_APP_CONSENT = true as unknown as string
},
)

Expand Down Expand Up @@ -743,95 +731,74 @@ describe(
'could force otp mfa',
async () => {
global.process.env.OTP_MFA_IS_REQUIRED = true as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = false as unknown as string
const res = await postAuthorizeAccount()
const json = await res.json()
expect(json).toStrictEqual({
code: expect.any(String),
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: true,
requireOtpMfa: true,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeOtpSetup,
})
global.process.env.OTP_MFA_IS_REQUIRED = false as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = true as unknown as string
},
)

test(
'could force email mfa',
async () => {
global.process.env.EMAIL_MFA_IS_REQUIRED = true as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = false as unknown as string
const res = await postAuthorizeAccount()
const json = await res.json()
expect(json).toStrictEqual({
code: expect.any(String),
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: false,
requireEmailMfa: true,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeEmailMfa,
})
global.process.env.EMAIL_MFA_IS_REQUIRED = false as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = true as unknown as string
},
)

test(
'could force sms mfa',
async () => {
global.process.env.SMS_MFA_IS_REQUIRED = true as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = false as unknown as string
const res = await postAuthorizeAccount()
const json = await res.json()
expect(json).toStrictEqual({
code: expect.any(String),
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: true,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeSmsMfa,
})
global.process.env.SMS_MFA_IS_REQUIRED = false as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = true as unknown as string
},
)

test(
'could skip mfa',
async () => {
global.process.env.ENFORCE_ONE_MFA_ENROLLMENT = [] as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = false as unknown as string
const res = await postAuthorizeAccount()
const json = await res.json()
expect(json).toStrictEqual({
code: expect.any(String),
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
global.process.env.ENFORCE_ONE_MFA_ENROLLMENT = ['email', 'otp'] as unknown as string
global.process.env.ENABLE_USER_APP_CONSENT = true as unknown as string
},
)

Expand All @@ -846,14 +813,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: true,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll,
})
global.process.env.ENABLE_USER_APP_CONSENT = true as unknown as string
},
Expand Down Expand Up @@ -1014,14 +974,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: true,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeMfaEnroll,
})
const consent = db.prepare('SELECT * from user_app_consent WHERE "userId" = 1 AND "appId" = 1').get()
expect(consent).toBeTruthy()
Expand Down
66 changes: 2 additions & 64 deletions server/src/__tests__/normal/identity-mfa.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: true,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeEmailMfa,
})

const user = await db.prepare('SELECT * from "user" WHERE id = 1').get() as userModel.Raw
Expand Down Expand Up @@ -318,14 +311,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: true,
requireOtpMfa: true,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeOtpSetup,
})

const user = await db.prepare('SELECT * from "user" WHERE id = 1').get() as userModel.Raw
Expand Down Expand Up @@ -542,14 +528,6 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.OtpMfaCode}-${json.code}`)).toBe('1')
},
Expand Down Expand Up @@ -1516,14 +1494,6 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.SmsMfaCode}-${json.code}`)).toBe('1')

Expand Down Expand Up @@ -1940,14 +1910,6 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.EmailMfaCode}-${json.code}`)).toBe('1')
},
Expand Down Expand Up @@ -2040,14 +2002,6 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.OtpMfaCode}-${json.code}`)).toBe('1')
},
Expand Down Expand Up @@ -2103,14 +2057,6 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.SmsMfaCode}-${json.code}`)).toBe('1')
},
Expand Down Expand Up @@ -2285,14 +2231,6 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: false,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
})
expect(await mockedKV.get(`${adapterConfig.BaseKVKey.EmailMfaCode}-${json.code}`)).toBe('1')
},
Expand Down
18 changes: 2 additions & 16 deletions server/src/__tests__/normal/identity-policy.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: true,
requireEmailMfa: false,
requireSmsMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeConsent,
})
},
)
Expand Down Expand Up @@ -423,14 +416,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: true,
requireEmailMfa: false,
requireSmsMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeConsent,
})
},
)
Expand Down
18 changes: 2 additions & 16 deletions server/src/__tests__/normal/identity-social.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeConsent,
})
}

Expand Down Expand Up @@ -277,14 +270,7 @@ describe(
redirectUri: 'http://localhost:3000/en/dashboard',
state: '123',
scopes: ['profile', 'openid', 'offline_access'],
requireConsent: true,
requireMfaEnroll: false,
requireEmailMfa: false,
requireOtpSetup: false,
requireOtpMfa: false,
requireSmsMfa: false,
requireChangePassword: false,
requireChangeEmail: false,
nextPage: routeConfig.IdentityRoute.AuthorizeConsent,
})
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/handlers/identity/social.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export const getAuthorizeGithub = async (c: Context<typeConfig.Context>) => {
)

const qs = `?state=${detail.state}&code=${detail.code}&locale=${bodyDto.locale}`
const url = detail.requireConsent
const url = detail.nextPage === routeConfig.IdentityRoute.AuthorizeConsent
? `${routeConfig.IdentityRoute.AuthorizeConsent}${qs}&redirect_uri=${detail.redirectUri}`
: `${detail.redirectUri}${qs}`
return c.redirect(url)
Expand Down
Loading

0 comments on commit f246aff

Please sign in to comment.