Repository navigation
fix(oauth): keep a pending login alive across scans and the quarantine exemption - #1739
Merged
Merged
Conversation
…e exemption (#1722) A remote OAuth upstream's browser approval failed at the code exchange with "instance retired" because two background paths tore down the in-flight login: - Every scan (admission, informational rescan, manual) that found the server disconnected called EnsureConnected, which restarts it. A server waiting on a person's login can never connect, so the scan waited 30 s, replaced the instance and retired the flow. The scanner now asks the optional oauthLoginAwareProvider capability (Runtime.ServerAwaitsOAuthLogin: manual flow in flight, or parked in Pending Auth) and scans without reconnecting such a server. - A new server's one-hour inspection exemption was overwritten by the admission scan's 15-minute grant, and the expiry-driven disconnect landed while the operator was still signing in. A grant never shortens an exemption in force, and both expiry paths hold an expired exemption (extending it in 5-minute steps) while a login for the server is in flight. When an instance is still retired mid-login (operator removal, restart or quarantine), the callback waiter now notices through its gate's context, invalidates its state, and the callback page says the sign-in was invalidated instead of showing a success page before a silent exchange failure. RT-02 untouched: nothing here approves, unquarantines or blocks anything.
Deploying mcpproxy-docs with
|
| Latest commit: |
d5590a4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f12b2c58.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-elastic-bell-f543b5.mcpproxy-docs.pages.dev |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 39796957969 --repo smart-mcp-proxy/mcpproxy-go
|
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.
Closes #1722
Adding a remote OAuth upstream and approving it in the browser failed with
instance retired (removed, replaced or quarantined)at the code exchange, although the callback had reached mcpproxy. Two background paths tore down the in-flight login; this PR fixes both and makes the failure visible when it still happens.Cause 2: a scan restarted the pending-auth server
Every scan (admission, the Spec 118 informational rescan, a manual scan) that found the server disconnected called
EnsureConnected, which restarts the server. A server waiting on a person's login can never connect, so the scan waited 30 s, then replaced the instance and retired the flow that owned the PKCE verifier and the callback waiter.internal/security/scanner/service.go: optionaloauthLoginAwareProvidercapability (AwaitingOAuthLogin). BothEnsureConnectedsites stand down for such a server and log why; the scan proceeds with what it can export without a restart (the ingested contracts) or fails closed as before.internal/server/server.go:configServerInfoProvider.AwaitingOAuthLogin→Runtime.ServerAwaitsOAuthLogin: a manual login flow in flight (oauth.IsManualFlowActive) or a live instance parked inPending Auth.Cause 1: the inspection exemption expired under the login
A new server's add grants an hour-long review connection, but the admission scan's
EnsureConnectedimmediately overwrote it with 15 minutes, and the expiry-driven disconnect ran while the operator was still signing in.internal/runtime/supervisor/supervisor.go:RequestInspectionExemptionnever shortens an exemption in force;SetInspectionExemptionHoldinstalls a predicate (the runtime passesoauth.IsManualFlowActive) and both expiry paths (IsInspectionExempted, the cleanup loop viacollectExpiredExemptions) hold an expired exemption while a login is in flight, extending it in 5-minute steps, so it outlives the login by at most that grace.Visibility
internal/upstream/core/connection_oauth.go: the background callback waiter also selects on its gate's context. When the instance is retired mid-login it stops waiting and invalidates its state.internal/oauth/config.go:CallbackServer.InvalidateState(state, reason); the callback that lands afterwards renders Authorization Failed with "This sign-in was invalidated before it could finish: the server's connection was retired (restarted, replaced, removed or quarantined) while the sign-in was in progress" instead of a success page followed by a silent exchange error.RT-02 untouched: nothing here approves, unquarantines or blocks anything. No new dependencies.
Tests (written first, watched fail)
scanner:TestStartScan_DoesNotReconnectServerAwaitingOAuthLogin(failed on main after the 30 s wait withEnsureConnected called 1 time(s)), plus the control that a disconnected server with no login is still reconnected.supervisor: held while a login is in flight (IsInspectionExemptedand the cleanup-loop path), no hold without a predicate, never-shortens.oauth: invalidated state renders the failure page with the reason; unknown state keeps the generic message.core: a retired gate ends the waiter, drops the waiter, and the real loopback callback then returns 400 with the reason.server: the provider predicate follows the manual-flow claim.Docs:
docs/features/oauth-authentication.md(troubleshooting entry),docs/features/security-quarantine.md(informational scan section).