-
Notifications
You must be signed in to change notification settings - Fork 7
OAuth refactor #188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
OAuth refactor #188
Conversation
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
Introduce auth contracts, deterministic dummy provider, and unit tests to exercise the adapter flow.
Introduce SQLite-backed token store with hashing/encryption, SessionManager lifecycle helpers, AuthService issuer flow, and supporting tests/exports.
Validate access tokens through SessionManager/ProviderAdapter, keep legacy path, and add middleware tests covering happy and failure paths.
Make token_getter mandatory and string-based to decouple SDK from FastMCP, update middleware tests accordingly, and adapt server wiring to supply get_access_token().token.
Add explanatory comments across auth tests and clarify where sessions are first created in AuthService; no behavioral changes.
…andle scopes Introduce a new test to ensure that session scopes are preserved during session reloads. Update the SqliteTokenStore to include a 'scopes' column in the sessions table, implement logic to add this column if it doesn't exist, and adjust session storage and retrieval methods to handle scopes correctly.
Enforces data integrity at the database level - each session_id should identify exactly one session. This catches potential bugs early rather than silently allowing duplicate session_ids.
Adds the ability to look up a session by its refresh token hash, enabling future implementation of OAuth token refresh flows (RFC 6749 §6). Includes tests for: happy path, non-existent token, session without refresh token, and expired session handling.
…gic to ensure expired sessions are correctly identified and managed.
…state - Generate separate PKCE pair for Google provider (independent of MCP client's PKCE) - Store provider_code_verifier in StateRecord for Google token exchange - Store client_state to return original state to MCP client in redirect - Remove PKCE verification from exchange_token() since MCP framework verifies it upstream - Add database migration for provider_code_verifier and client_state columns - Fix state handling: return client's original state in redirect, not Google's state This fixes the 'Missing code verifier' error when exchanging authorization code with Google.
If the MCP client did not supply an original state, omit the state parameter in redirects instead of falling back to the internal MXCP/IdP state.
Redirect provider callback errors back to the stored client redirect_uri when state is available, and add an in-process ASGI regression test for the callback route.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
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.
This is refactoring the implementation of our OAuth issuer mode.
Note
Overhauls authentication to issuer-mode with a new, testable architecture and safer defaults.
ProviderAdaptercontracts,AuthService,SessionManager, and storage types; removes legacyExternalOAuthHandler/GeneralOAuthAuthorizationServerand old persistence layergithub,google,atlassian) and addsdummy; improves PKCE handling, token/error parsing, and userinfo normalizationAuthenticationMiddlewareto validate MXCP sessions, optionally refresh user info, and avoid sensitive loggingscopenow required; addsauth_encryption_keyto persistence configcryptographyfor token storage encryptionWritten by Cursor Bugbot for commit 90ce0b5. This will update automatically on new commits. Configure here.