refactor(core): extract shared OAuth flow primitives from MCPOAuthProvider#20895
Conversation
|
Hi @SandyTao520, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the modularity and reusability of OAuth authentication logic within the core package. By centralizing common OAuth 2.0 Authorization Code and PKCE flow primitives into a dedicated utility module, it lays the groundwork for consistent authentication implementations across different providers, such as MCP and future A2A agents. This change streamlines the codebase and enhances maintainability without altering any existing functionality. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Size Change: -3.31 kB (-0.01%) Total Size: 26 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that extracts shared OAuth 2.0 flow logic into a new oauth-flow.ts module. This significantly improves code structure and enables reuse, as intended. The new parseTokenEndpointResponse helper is a great addition for deduplicating response handling. I've found one high-severity issue related to type safety in the new shared module that could lead to runtime errors. Please see my detailed comment.
5031897 to
e72e2d5
Compare
…vider Extract generic OAuth 2.0 Authorization Code + PKCE logic into a shared oauth-flow.ts module so both MCP and A2A authentication providers can reuse it. MCPOAuthProvider now delegates to these shared utilities while retaining MCP-specific logic (discovery, dynamic registration, resource parameter handling). Part of #17600
Address review feedback: validate that JSON-parsed response contains a string access_token before treating it as an OAuthTokenResponse. If the JSON doesn't contain the expected field, fall through to form-urlencoded parsing instead of blindly casting.
- Replace unsafe clientId! non-null assertion in MCPOAuthProvider.refreshAccessToken with an explicit validation guard - Add 42 dedicated unit tests for oauth-flow.ts covering all exported functions: generatePKCEParams, getPortFromUrl, buildAuthorizationUrl, startCallbackServer, exchangeCodeForToken, and refreshAccessToken
e72e2d5 to
a8585a6
Compare
The default port for the OAuth callback was changed from 7777 to a random OS-assigned port in #20895, but the documentation still stated that it defaulted to 7777. This commit updates the documentation to reflect the current behavior.
Summary
Extract generic OAuth 2.0 Authorization Code + PKCE primitives from
MCPOAuthProviderinto a sharedoauth-flow.tsmodule, enabling reuse by both MCP and A2A authentication providers. Pure refactor — no behavior change.Details
New file:
packages/core/src/utils/oauth-flow.tsgeneratePKCEParams,startCallbackServer,getPortFromUrl,buildAuthorizationUrl,exchangeCodeForToken,refreshAccessTokenparseTokenEndpointResponse(deduplicates token response parsing that was previously copy-pasted betweenexchangeCodeForTokenandrefreshAccessToken)OAuthFlowConfig,OAuthRefreshConfig,PKCEParams,OAuthAuthorizationResponse,OAuthTokenResponseresource?: stringparameter — callers pass it, the shared functions just append it if presentModified file:
packages/core/src/mcp/oauth-provider.tsoauth-flow.tsbuildResourceParam()private helper consolidates the repeated try/catch pattern for MCP resource parameter buildingOAuthAuthorizationResponseandOAuthTokenResponsefromoauth-flow.tsfor backward compatibilityThis is the foundation for PR 2 (#17600) which will add
OAuth2AuthProviderfor A2A remote agents using these shared primitives.Related Issues
Related to #17600
How to Validate
npm run typecheck— passes with no errorsnpm run lint— passes with no errorsnpm test -w @google/gemini-cli-core -- src/mcp/oauth-provider.test.ts— all 41 existing tests pass unchangednpm test -w @google/gemini-cli-core -- src/tools/mcp-client.test.ts— all 66 tests pass (verifies import compatibility)npm test -w @google/gemini-cli-core— all 5,387 core tests passPre-Merge Checklist