Repository navigation
fix(oauth2): harden identity linking, tokens and flow security - #133
Merged
Merged
Conversation
- Normalize user and email-account emails and account IDs on write and lookup. - Add table-driven repository and service tests for email normalization. - Harden OAuth2 callback identity resolution, session reuse, token persistence, and redirect handling. - Add handler-facing OAuth2 use case interfaces and expand callback test coverage.
- Store OAuth2 use cases as interfaces instead of pointers to interfaces. - Refactor callback SQLite integration tests into a table-driven test. - Remove redundant provider implementation comments.
…e for more clarity
… code to follow DRY
S256ChallengeOption already hashes the value it receives, so passing the pre-computed challenge sent a double-hashed code_challenge while the token exchange sent the raw verifier. Providers enforcing PKCE (Discord, Google) rejected the exchange with invalid_grant "Code challenge failed.". GeneratePKCEVerifier now only produces the verifier and lets x/oauth2 derive the challenge, and the authorize test asserts the challenge matches the signed verifier cookie.
GitHub now documents code_challenge (S256) on the authorization request and code_verifier on the token exchange for both OAuth apps and GitHub Apps, so enable PKCE ahead of the OAuth 2.1 transition.
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.
Summary
Hardens the OAuth2 plugin to prevent account takeover and fixes token persistence, flow validation, provider configuration, and response handling.
Security
redirect_tois validated against trusted origins and relative paths.Secure,Domain, andSameSiteand are cleared on every terminal path.Added
AccountService.UpdateOAuth2Tokensfor account-scoped token persistence.UserInfo.EmailVerifiedpopulated from provider data.GenericProviderregistration with configurable endpoints, scopes, and field mappings.ProviderConfig.Enabled(*bool), provider HTTP timeout, and email verification field mapping.config.toml.Changed
ProviderConfig.Enabledis a pointer: absent means enabled, explicitfalsedisables it.Init.400to401,404,409,422,502, or500.Removed
RefreshRequest,RefreshResponse,LinkAccountRequest,LinkAccountResponse,RefreshResult, andLinkAccountResulttypes.eventBuswiring,VerifyHMAC,IsProviderEnabled, and handler constructors.Migration
account_idvalues of'','<nil>', or scientific notation.session.tokenfrom the callback JSON must use the session cookie instead.