Fix Google sign-in on MCP authorization page - #660
Merged
Merged
Conversation
There was a problem hiding this comment.
Important
The new Google start endpoint depends on callback allowlist configuration that is missing from the Terraform-managed API environments. Keep this prerequisite in the managed configuration before shipping.
Reviewed changes in all nine files across the four commits through e060bf1:
- Google authorization redirect: Replaces One Tap with an opaque, single-use Google state and PKCE flow that returns to the MCP client's original request.
- Sign-in behavior: Reuses Google code exchange without overwriting existing calendar tokens, with localized failure redirects back to email sign-in.
- Contract and coverage: Updates OpenAPI and the agent catalog, removes obsolete One Tap tests, and exercises state binding, expiry, and callback outcomes.
GPT Sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review at e060bf1:
- Managed callback configuration: Added the production and staging API callback URIs to Terraform while retaining the existing web redirect URIs.
- Legacy endpoint compatibility: Restored
POST /oauth/googleas deprecated in the controller and OpenAPI, and cataloged it alongside the new redirect actions. - Callback safety and coverage: Revalidated the stored MCP redirect before issuing a code and added tests for invalid redirects, unavailable configuration, and localized fallback errors.
GPT Sol | 𝕏
|
thomasluizon
added a commit
that referenced
this pull request
Sep 29, 2026
* Bump the nuget-minor-patch group with 3 updates (#658) Bumps AWSSDK.SimpleEmailV2 from 4.0.105 to 4.0.105.1 Bumps coverlet.collector from 10.0.1 to 10.1.0 Bumps FirebaseAdmin from 3.6.0 to 3.7.0 --- updated-dependencies: - dependency-name: AWSSDK.SimpleEmailV2 dependency-version: 4.0.105.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: nuget-minor-patch - dependency-name: coverlet.collector dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget-minor-patch - dependency-name: coverlet.collector dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget-minor-patch - dependency-name: coverlet.collector dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget-minor-patch - dependency-name: coverlet.collector dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget-minor-patch - dependency-name: FirebaseAdmin dependency-version: 3.7.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit eb18808) * Chore(deps): bump the github-actions group with 5 updates (#657) Bumps the github-actions group with 5 updates: | Package | From | To | | --- | --- | --- | | [github/codeql-action/init](https://github.com/github/codeql-action) | `4.38.1` | `4.38.2` | | [github/codeql-action/analyze](https://github.com/github/codeql-action) | `4.38.1` | `4.38.2` | | [pullfrog/pullfrog](https://github.com/pullfrog/pullfrog) | `0.1.83` | `0.1.84` | | [aws-actions/configure-aws-credentials](https://github.com/aws-actions/configure-aws-credentials) | `5.1.1` | `6.3.0` | | [hashicorp/setup-terraform](https://github.com/hashicorp/setup-terraform) | `3.1.2` | `4.0.1` | Updates `github/codeql-action/init` from 4.38.1 to 4.38.2 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@1c5b675...2892aa5) Updates `github/codeql-action/analyze` from 4.38.1 to 4.38.2 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@1c5b675...2892aa5) Updates `pullfrog/pullfrog` from 0.1.83 to 0.1.84 - [Release notes](https://github.com/pullfrog/pullfrog/releases) - [Commits](pullfrog/pullfrog@e9f8115...99c5e78) Updates `aws-actions/configure-aws-credentials` from 5.1.1 to 6.3.0 - [Release notes](https://github.com/aws-actions/configure-aws-credentials/releases) - [Changelog](https://github.com/aws-actions/configure-aws-credentials/blob/main/CHANGELOG.md) - [Commits](aws-actions/configure-aws-credentials@61815dc...e125382) Updates `hashicorp/setup-terraform` from 3.1.2 to 4.0.1 - [Release notes](https://github.com/hashicorp/setup-terraform/releases) - [Changelog](https://github.com/hashicorp/setup-terraform/blob/main/CHANGELOG.md) - [Commits](hashicorp/setup-terraform@v3.1.2...dfe3c3f) --- updated-dependencies: - dependency-name: github/codeql-action/init dependency-version: 4.38.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: github/codeql-action/analyze dependency-version: 4.38.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: pullfrog/pullfrog dependency-version: 0.1.84 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: aws-actions/configure-aws-credentials dependency-version: 6.3.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions - dependency-name: hashicorp/setup-terraform dependency-version: 4.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit a78ba3a) * Fix Google sign-in on MCP authorization page (#660) * Fix MCP Google authorization redirect * Catalog MCP Google redirect actions * Style Google redirect link as button * Preserve calendar tokens during MCP sign-in * Fix MCP Google redirect review findings (cherry picked from commit 1d80602) * Hint audio transcription with account language (#661) (cherry picked from commit 1f8d762) * Bind OAuth Google state to the start of sign-in (#962) (#663) A valid anonymous GET /oauth/authorize used to allocate a pending Google request in OAuthAuthorizationStore even when the visitor never clicked Google. The route carried no rate limit and the store carried no capacity bound, so repeated page views could retain arbitrarily many entries until the five-minute sweep and exhaust the API's memory. The authorize page now hands its already-validated MCP parameters straight to /oauth/google/start, which revalidates the client, the redirect URI and the PKCE method server side before it allocates anything. A page view allocates no server state. The store gains a hard cap on pending Google requests, evicts expired entries on insert, and refuses a new allocation once the cap is reached. GoogleStart turns that refusal into the localized "unavailable" message on the authorize page, where email sign-in stays available. Both browser routes now carry the DistributedRateLimit("auth") policy. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 5dc0a01) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.




Change
Fixes thomasluizon/orbit-tickets#945.
The MCP authorize page now links to
/oauth/google/start, which redirects in the same tab to Google's authorization endpoint with a fresh state and PKCE challenge.OAuthAuthorizationStorekeeps the client ID, client redirect URI, original state, MCP challenge, nonce, Google verifier, and callback URI in a five minute, single-use server record./oauth/google/callbackconsumes that record, signs in throughGoogleCodeAuthCommand, issues the existing MCP authorization code, and redirects to the client. Google cancellation and exchange failures return to the authorize page with an English or Portuguese error while leaving email sign-in available.This shape keeps the existing Google account resolution and MCP token exchange. The MCP flow does not store identity-only Google tokens as calendar credentials. The One Tap script and its obsolete tests are removed.
POST /oauth/googlestays as a deprecated route, so the OpenAPI contract gate keeps passing; a follow-up ticket deletes it. Changes are inOAuthController.cs,OAuthAuthorizationStore.cs,OAuthLoginPage.cs,GoogleCodeAuthCommand.cs, the agent action catalog, the generated OpenAPI document, and their unit tests.Assumptions
OAuthAuthorizationStoreis the intended server-side store for this flow, rather than a new distributed store, because current MCP authorization codes also live there.Accept-Languageheader, rather than a new query parameter, because the authorize page has no locale parameter.openid email profile, rather than calendar scope, because account sign-in only needs identity and the command now preserves existing calendar credentials.POST /oauth/googleendpoint is cataloged under the existing authentication capability, not a new capability for the same action.Manual steps
https://api.useorbit.org/oauth/google/callbackandhttps://api-staging.useorbit.org/oauth/google/callback) are registered on the Google OAuth web client.infra/for both API environment groups (orbit-production-api,orbit-staging-api) and release both API environments throughrelease.yml. ConfirmGoogle__AllowedRedirectUris__1ishttps://api.useorbit.org/oauth/google/callbackin production andGoogle__AllowedRedirectUris__3ishttps://api-staging.useorbit.org/oauth/google/callbackin staging. On each host, verify that the Google button opens account selection and a completed sign-in returns to the MCP client.Test evidence
dotnet test tests/Orbit.Infrastructure.Tests/Orbit.Infrastructure.Tests.csproj --filter FullyQualifiedName~OAuthControllerTests.Authorize_ValidParams_ReturnsHtmlContent -v normalpassed one test while the page still contained One Tap./oauth/google/start?state=and rejectgoogle.accounts.id,dotnet test tests/Orbit.Infrastructure.Tests/Orbit.Infrastructure.Tests.csproj --filter FullyQualifiedName~OAuthControllerTests.Authorize_ValidParams_ReturnsHtmlContent --no-restore -v quietfailed one test before implementation. The same command passed one test after implementation.dotnet test tests/Orbit.Application.Tests/Orbit.Application.Tests.csproj --filter FullyQualifiedName~GoogleCodeAuthCommandHandlerTests.ExistingUser_PreservesOrReplacesRefreshToken --no-restore -v quietpassed two cases. The newMcpIdentityOnlyExchange_DoesNotReplaceCalendarTokenstest failed one case against the previous command behavior, then passed after the MCP mode was added.dotnet build Orbit.slnx --no-restore -v quietfinished with zero errors.dotnet test -v quietpassed 6,951 tests across four projects.node tools/arch-map.mjs, the dash check, and the timeless check passed.be60d5fa: the unchanged OAuth tests passed with the defect present (dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~OAuthControllerTests --no-restore, 51 of 51).RedirectResult, and the deprecated route was absent. Both pass after the fix. The unavailable and nonce cases passed before and after.dotnet build Orbit.slnx --no-restore0 errors. Fulldotnet test Orbit.slnx --no-restorepassed withLANGunset and again withLC_ALL=en_US.UTF-8, 6,959 tests in each run.terraform fmt -checkpassed.terraform validatedid not run to completion in the worktree because it lacks the cached provider plugins.External interface evidence
A real GET to
https://accounts.google.com/.well-known/openid-configurationreturnedauthorization_endpoint: https://accounts.google.com/o/oauth2/v2/auth,response_types_supportedcontainingcode,response_modes_supportedcontainingquery,scopes_supportedcontainingopenid,email, andprofile, andcode_challenge_methods_supportedcontainingS256. Re-runcurl --silent --show-error https://accounts.google.com/.well-known/openid-configurationto inspect the complete live response. A request to that authorization endpoint with an invalid client returned HTTP 302 to Google'ssignin/oauth/errorpage withinvalid_client, confirming the endpoint responds. The callback reads the standard OAuth code, error, and state query parameters specified by the ticket and OAuth 2.0 authorization code response schema. A live successful Google callback cannot be captured until the console redirect URIs are registered.