feat: support client_secret_post token endpoint auth method#2910
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🦋 Changeset detectedLatest commit: eb0a517 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
8f321aa to
01915bf
Compare
cb6a46a to
e6f5011
Compare
01915bf to
060efd1
Compare
060efd1 to
45d1135
Compare
🚀 Preview Environment (PR #2910)Preview URL: https://pr-2910.dev.getgram.ai
Gram Preview Bot |
45d1135 to
bb3692f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bb3692f to
f5e6963
Compare
There was a problem hiding this comment.
Do we not need to update this?
There was a problem hiding this comment.
This is slated for removal - robot snuck it by me, but this is a silly implicit pathway for registering clients. I can scope in its removal here
There was a problem hiding this comment.
Scoped the implicit auto_register pathway out of createRemoteSessionClient in 04dba8a (AGE-2408). registerClientViaDCR stays since registerRemoteSessionIssuer still uses it explicitly.
04dba8a to
2601e74
Compare
b84822f to
7f9fa2d
Compare
…sion_clients Adds a `token_endpoint_auth_method` attribute to the remote_session_clients management API (createRemoteSessionClient, cloneClientFromOAuthProxyProvider, updateRemoteSessionClient payloads + the shared response type). Goa Enum constrains writes to `client_secret_basic` / `client_secret_post`; the column stays nullable so existing rows continue to resolve to client_secret_basic at runtime. Introduces `ClientAuthMethodBasic` / `ClientAuthMethodPost` constants and `resolveClientAuthMethod` for the runtime branching that lands next. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nge + refresh
ChallengeManager.exchangeCode and refreshAccessToken now branch on the
stored remote_session_client.token_endpoint_auth_method:
- client_secret_basic (default, including NULL): Authorization: Basic
<id:secret> header — unchanged behaviour.
- client_secret_post: client_id + client_secret in the form body, no
Authorization header. Needed for upstreams that reject Basic (e.g.
PostHog).
- No stored secret: public client, PKCE-only — unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- clienthandlers_test.go: round-trip the new attribute through Create
(with client_secret_post + with NULL) and through Update (switch
method on an existing client).
- tokenservice_authmethod_test.go: drive validateAndRefresh against
an httptest token endpoint and assert that client_secret_post puts
the secret in the form body (no Authorization header) and
client_secret_basic uses Authorization: Basic (no body secret).
Covers exchangeCode by construction since both call sites share
applyTokenEndpointAuth.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mise run gen:sdk after server design + Goa regen for AGE-2387. Adds the new attribute to RemoteSessionClient, CreateRemoteSessionClientForm, UpdateRemoteSessionClientForm, and CloneClientFromOAuthProxyProviderForm in the public OpenAPI spec and the TypeScript SDK models. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…in remote session negotiation
…request constructor applyTokenEndpointAuth had to re-encode the request body to inject client_secret for client_secret_post, since the form mutation had to happen after the request was already built. Fold both steps into newTokenEndpointRequest so form-side credentials land before encoding and header-side credentials land after construction — no body rewrite, no body NopCloser dance. exchangeCode and refreshAccessToken each shed three lines of header boilerplate plus the trailing applyTokenEndpointAuth call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the implicit auto-DCR pathway from createRemoteSessionClient. client_id is now required on the form; callers obtain it out-of-band from the upstream issuer. The explicit registerRemoteSessionIssuer method (and the underlying registerClientViaDCR helper) stays — it still has live dashboard callers and is a separately invoked pathway. Refs AGE-2408. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…URIs Rename the client we self-identify as during outbound RFC 7591 DCR from "Gram" to "Speakeasy" (proxy-register, external-oauth, the remotesessions DCR fallback). Add /mcp/remote_login_callback to the redirect_uris in /oauth/proxy-register so a single proxy-registered client works for both the OAuth-proxy flow (/oauth/callback) and the remote-session flow (/mcp/remote_login_callback). Unblocks moving the dashboard off registerRemoteSessionIssuer onto proxy-register + createRemoteSessionClient. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nt_auth_method verbatim The handler previously accepted the upstream issuer's advertised scopes_supported / token_endpoint_auth_methods_supported sets and translated them into the outbound DCR request's scope (space-joined) and token_endpoint_auth_method (via pickProxyAuthMethod, falling back to client_secret_basic). That was the wrong layer: advertisement is metadata, not a request, and "we'll pick basic if you don't say" silently overrides upstreams (PostHog) that would have chosen client_secret_post themselves. New contract: optional scope + token_endpoint_auth_method on the request body; forwarded verbatim into the DCR request when set, omitted otherwise. RFC 7591 §3.2.1's default (basic) is the upstream's problem. Drop pickProxyAuthMethod. Add omitempty on DCRRequest.TokenEndpointAuthMethod so the empty-string zero value drops from the wire. Update the OAuth Proxy wizard caller in the dashboard to stop sending the advertised sets and drop the now-unused discoveredAuthMethods helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ead of registerRemoteSessionIssuer
Both dashboard callers that used to register a remote_session_client as
a side effect of an issuer call now do it in two explicit steps:
1. POST /oauth/proxy-register to mint the upstream client_id /
client_secret via Gram's CORS-busting DCR proxy.
2. createRemoteSessionClient with the returned credentials.
Add lib/proxyRegisterUpstreamClient.ts as the shared helper; refactor
the OAuth Proxy wizard's registerClient service to delegate to it so
the fetch + decode lives in one place. Plumb authedFetch into
onboardExternalMcpToUserSessions (catalog-import flow) and into
createMigrationServices (wire-user-session-issuer wizard). Update the
useExternalMcpReleaseWorkflow fixture to assert the new two-call shape.
The registerRemoteSessionIssuer Goa method now has no callers; it's
deleted in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lpers The /rpc/remoteSessionIssuers.register method has no remaining callers after the dashboard moved to /oauth/proxy-register + createRemoteSessionClient. Delete the Goa method, the handler, the four tests, the RegisterRemoteSessionIssuerForm type, and the now-dead RFC 7591 helpers in clienthandlers.go (registerClientViaDCR, dcrParams, dcrHTTPTimeout, dcrMaxBodyBytes, rfc7591Request, rfc7591Response). Regenerate Goa + SDK; the SDK loses the registerRemoteSessionIssuer operation, react-query hook, and form/operation types. Closes AGE-2408. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7f9fa2d to
eb0a517
Compare
Summary
Some upstream OAuth providers (PostHog, others) reject
Authorization: Basiccredentials at the token endpoint and requireclient_secret_post. This PR teachesremote_session_clientsto carry per-client auth-method preference and threads the choice through both the initial code exchange and the silent refresh.token_endpoint_auth_methodattribute onRemoteSessionClient(Create / Update / Clone forms + response).Enum("client_secret_basic", "client_secret_post")constrains writes; NULL on the column resolves toclient_secret_basicat runtime viaResolveTokenEndpointAuthMethod.applyTokenEndpointAuthhelper intokenservice.goowns the basic-vs-post branch; bothexchangeCodeandrefreshAccessTokencall it.server/internal/remotesessions/types.go(following theoauth/types.gopattern — design declares the strings independently so the design package has no dependency on internal packages).Stacked on #2909 (the migration ships in its own PR per project rules). After #2909 merges this PR will auto-retarget to
main.Followup tracked in Linear AGE-2408 — strip the now-unused
auto_register/ outbound RFC 7591 DCR path.Refs Linear AGE-2387.
Test plan
mise test:server ./internal/remotesessions/...green (new tests:TestCreateRemoteSessionClient_Manual_WithAuthMethodPost,TestCreateRemoteSessionClient_Manual_AuthMethodOmittedStaysNil,TestUpdateRemoteSessionClient_SwitchAuthMethod,TestResolveAccessToken_RefreshUsesClientSecretPost,TestResolveAccessToken_RefreshUsesClientSecretBasic).mise lint:serverclean.remote_session_clientsrow withtoken_endpoint_auth_method=client_secret_post, attempt login + refresh against PostHog, confirmclient_secretlands in the form body and noAuthorization: Basicheader is sent.Authorization: Basic.🤖 Generated with Claude Code