-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR closes a policy enforcement gap where Better-Auth’s 2FA flow defers final session issuance to /two-factor/* endpoints, allowing OAuth/social sign-ins to bypass workspace SSO hard-binding and auth-method restrictions for 2FA-enabled accounts. It does this by normalizing hook path handling, treating 2FA verification endpoints as session-creating, and persisting the first-factor provider across the 2FA challenge so the completion endpoint can be evaluated against the same OAuth policy.
Changes:
- Normalize Better-Auth hook
ctx.pathvalues so policy checks work with both@opencoven-feedback/...-prefixed route ids and raw auth endpoint paths. - Extend the post-session cleanup logic to include
/two-factor/*session completion endpoints and carry the first-factor provider through 2FA via a short-lived signed cookie. - Add regression tests to ensure deferred OAuth callbacks remember the provider and that
/two-factor/verify-totpenforces the same OAuth policy.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/web/src/lib/server/auth/hooks.ts | Normalizes auth paths, adds 2FA completion handling to post-session policy enforcement, and remembers/consumes the first-factor OAuth provider via a signed cookie. |
| apps/web/src/lib/server/auth/tests/hooks-callback-cleanup.test.ts | Adds tests covering the deferred-2FA provider remember/consume flow and enforcement on /two-factor/verify-totp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a98a34c to
10c9efa
Compare
Motivation
/two-factor/*endpoints, which were not included in the post-callback policy cleanup, allowing OAuth/social sign-ins to bypass workspace SSO and auth-method restrictions for 2FA-enabled accounts.Description
normalizeAuthPathso policy code matches both@opencoven-feedback/...prefixed route ids and raw auth endpoint paths.TWO_FACTOR_SESSION_COMPLETION_PATHSso they are considered by post-session policy logic.quackback.2fa_policy_provider, 10 minutes) viarememberTwoFactorPolicyProviderandconsumeTwoFactorPolicyProviderso the/two-factor/*completion can be evaluated against the same OAuth policy as the original callback.handleCallbackPolicyCleanupto remember the provider when a callback defers final session issuance and to recover & apply the existing SSO hard-binding / auth-method revocation logic on the two-factor verification endpoints; make token revocation conditional and normalize redirect targets/paths.apps/web/src/lib/server/auth/__tests__/hooks-callback-cleanup.test.tswith tests that assert the provider is remembered on deferred callbacks and that/two-factor/verify-totpenforces the same OAuth policy.Testing
apps/web/src/lib/server/auth/__tests__/hooks-callback-cleanup.test.ts(tests committed with the change).git diff --checkpassed locally.bun --checkandbun run test), but test/runtime validation could not be executed in this environment becausebun install --frozen-lockfilefailed fetching dependencies (npm registry 403s) andvitest/other test runner deps were not available, so the test suite was not run here.Codex Task