fix(auth): enforce OAuth policy after 2FA#34
Open
BunsDev wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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