Migrate CLI OAuth flow to pkg/auth/dcr resolver

[tgrunnagle]

Description

Sub-issue 4b of #5145. With sub-issue 4a (PR #5198) the new pkg/auth/dcr package is in place and EmbeddedAuthServer consumes it. The CLI OAuth flow at pkg/auth/discovery/discovery.go::PerformOAuthFlow still calls oauthproto.RegisterClientDynamically directly, so it does not yet inherit the review-property behaviours added during #5042 (S256 PKCE gating, RFC 7591 §3.2.1 expiry-driven refetch, bearer-token transport with redirect refusal, panic recovery, singleflight deduplication). This sub-issue migrates that call site to the shared resolver and locks in the invariant with a CI grep guard.

After 4b lands, all four acceptance criteria of the parent issue are met and #5145 closes.

Context

pkg/auth/dcr.ResolveCredentials today takes embedded-authserver-shaped types — *authserver.OAuth2UpstreamRunConfig and *upstream.OAuth2Config — because in 4a we deliberately kept the move-only scope rather than designing a profile-neutral input with only one consumer in hand. 4b is when the second consumer arrives and the right shape becomes obvious.

The CLI side speaks OAuthFlowConfig (pkg/auth/discovery/discovery.go:501) and persists DCR results into OAuthFlowResult.ClientID / ClientSecret (pkg/auth/discovery/discovery.go:517). The downstream remote handler reads those back via pkg/auth/remote/handler.go's CachedClientID / CachedClientSecretRef / CachedRegTokenRef fields. That is the CLI's existing persistence model and the migration must preserve it.

Dependencies: sub-issue 4a (PR #5198) — provides the pkg/auth/dcr package and the EmbeddedAuthServer consumer.
Blocks: closure of #5145.

Acceptance Criteria

Profile-neutral resolver input — pkg/auth/dcr

• pkg/auth/dcr exposes a profile-neutral input type (or a small adapter pair) so both EmbeddedAuthServer and the CLI flow can call ResolveCredentials without coupling the package to either consumer's domain types. The current *authserver.OAuth2UpstreamRunConfig / *upstream.OAuth2Config parameters are either replaced or wrapped.
• The package doc comment in pkg/auth/dcr/resolver.go is updated to drop the "target state" caveat added in 4a — the package is now genuinely profile-agnostic.
• Existing EmbeddedAuthServer call sites continue to work (either unchanged via an adapter, or migrated to the new shape in the same PR).

CLI flow migration — pkg/auth/discovery

• pkg/auth/discovery/discovery.go::PerformOAuthFlow no longer calls oauthproto.RegisterClientDynamically directly. The DCR step routes through dcr.ResolveCredentials.
• handleDynamicRegistration (or its replacement) populates OAuthFlowResult.ClientID / ClientSecret from the dcr.Resolution returned by the resolver.
• CLI-specific concerns stay at the call site: loopback redirect URI per RFC 8252 §7.3 (the existing NewDynamicClientRegistrationRequest(config.Scopes, config.CallbackPort) shape), and the existing CLI-flag fallback error message ("configure with --remote-auth-client-id / --remote-auth-client-secret") when the provider does not advertise a registration endpoint.

Credential persistence — pkg/auth/remote/handler.go

• CachedClientID / CachedClientSecretRef / CachedRegTokenRef continue to work as the CLI's persistent credential store across runs. Either: (a) wrap them as a dcr.CredentialStore adapter so the resolver short-circuits on cache hit, or (b) treat them as a read-only fallback after dcr.ResolveCredentials returns, with the resolver running against dcr.NewInMemoryStore() for the duration of a single CLI invocation.
• The chosen approach is documented in the commit message and in a comment at the wiring site.

Inherited behaviours — verified by tests

• CLI flow inherits S256 PKCE gating: when the upstream advertises only plain PKCE, the CLI surfaces a clear error from the resolver rather than silently downgrading.
• CLI flow inherits RFC 7591 §3.2.1 expiry-driven refetch: when a cached registration is expired, the next PerformOAuthFlow call re-registers.
• CLI flow inherits bearer-token transport redirect refusal: registration POSTs do not follow redirects.
• CLI flow inherits panic recovery around the registration body.
• CLI flow inherits singleflight deduplication: two concurrent PerformOAuthFlow invocations with the same (issuer, scopes, redirectURI) tuple result in exactly one upstream /register call.

Invariant guard

• CI fails if oauthproto.RegisterClientDynamically is referenced outside pkg/auth/dcr/ or pkg/oauthproto/ itself. Implementation: a grep step in the existing lint/check pipeline (or a Taskfile target invoked from CI), not a new linter. The parent issue calls this out: "No direct calls to oauthproto.RegisterClientDynamically outside pkg/auth/dcr (search-grep enforces this)."

Cross-cutting

• task build, task test, task lint-fix, and task license-check all pass.
• The PR closes Consolidate the two RFC 7591 DCR client implementations into pkg/auth/dcr #5145.
• Code reviewed and approved.

Technical Approach

Recommended implementation

1. Design the neutral input. Walk both call sites (embedded authserver and CLI) and identify the minimal field set the resolver actually reads: registration endpoint (or discovery URL), requested scopes, redirect URI, optional initial access token, optional explicit endpoint overrides, and an HTTP client. Promote that into a dcr.Request (or similar) struct. Convert the existing OAuth2UpstreamRunConfig / upstream.OAuth2Config consumers to construct a dcr.Request at the call site; the resolver no longer imports either domain type.

2. Write the CLI adapter. pkg/auth/discovery/discovery.go constructs a dcr.Request from OAuthFlowConfig (issuer, scopes, callback port → loopback redirect URI per RFC 8252 §7.3) and either:

   • (a) passes a dcr.CredentialStore backed by OAuthFlowResult / pkg/auth/remote/handler.go's Cached* fields. This is the cleaner long-term shape because cache hits stay inside the resolver. It does require designing a small Get / Put adapter over the secretProvider interface used by the remote handler.
   • (b) passes dcr.NewInMemoryStore() and continues to populate OAuthFlowResult from the returned dcr.Resolution. Simpler, ships faster, but forfeits cross-invocation cache hits at the resolver layer (the remote handler still reads CachedClientID directly, so cross-invocation reuse still works in practice — just outside the resolver).

   Recommend (a) if the secret-provider plumbing is straightforward; otherwise (b) is acceptable provided the commit message documents the trade-off.

3. Replace the discovery.go DCR call. Delete the direct oauthproto.RegisterClientDynamically call at pkg/auth/discovery/discovery.go:747. Translate the existing CLI-flag fallback error message ("this provider does not support Dynamic Client Registration (DCR). Please configure ...") into a check on the resolver's returned error, since the resolver also synthesises a registration endpoint per RFC 8414 in some cases.

4. Add the CI grep guard. Either a new task check-dcr-isolation Taskfile target invoked from the existing CI lint/check job, or a one-line grep in the existing job. The check fails if RegisterClientDynamically matches anywhere outside pkg/auth/dcr/ and pkg/oauthproto/.

5. Drop the 4a doc caveat. With a real second consumer in hand, edit pkg/auth/dcr/resolver.go's package doc comment to remove the "target state" framing introduced in 4a's review-feedback commit.

Code pointers

• pkg/auth/discovery/discovery.go:500-535 — OAuthFlowConfig, OAuthFlowResult, shouldDynamicallyRegisterClient. The migration site is centred here.
• pkg/auth/discovery/discovery.go:589-613 — handleDynamicRegistration. This is the function that currently calls oauthproto.RegisterClientDynamically (line 747 is inside its helper).
• pkg/auth/discovery/discovery.go:744-754 — the direct oauthproto.RegisterClientDynamically call to remove.
• pkg/auth/dcr/resolver.go:386 — ResolveCredentials signature. Refactor target.
• pkg/auth/dcr/resolver.go:140 — Resolution struct. CLI consumer reads ClientID / ClientSecret off this.
• pkg/auth/dcr/resolver.go:208,253,287 — NeedsDCR / ConsumeResolution / ApplyResolutionToOAuth2Config. Profile-coupled API to either neutralise or split.
• pkg/auth/remote/handler.go:252-258 — where CachedClientID / CachedClientSecretRef are read back. The persistence contract this PR must preserve.

Patterns & frameworks

• Plain testing + github.com/stretchr/testify per .claude/rules/testing.md — NOT Ginkgo.
• Always drive builds/tests/lint through task commands.
• Commit-message style: imperative mood, capitalized subject, no trailing period, 50-char limit, no Conventional Commits prefixes.
• PR-size cap: 400 LOC / 10 files (excluding tests/docs/generated). If 4b exceeds the cap after the input refactor, split into a (4b-i: neutralise resolver inputs) + (4b-ii: migrate CLI + add guard) pair.

Testing Strategy

Unit tests

• pkg/auth/dcr — resolver behaviour tests under the new neutral input type. Existing test cases carry over with minimal surface change.
• pkg/auth/discovery — PerformOAuthFlow against a mock authorization server that advertises /register. Assert that one OAuthFlowResult.ClientID is populated and that the resolver was the code path that produced it (e.g. via a fake dcr.CredentialStore whose Put call is observed).

Integration tests

• S256 gating: mock AS advertises only plain PKCE; PerformOAuthFlow returns a resolver-shaped error.
• Expiry refetch: cache an expired registration; next PerformOAuthFlow invocation re-registers.
• Redirect refusal: mock AS responds to /register with 302 to an evil host; resolver refuses and the CLI surfaces the error.
• Singleflight: two goroutines call PerformOAuthFlow concurrently with the same (issuer, scopes, redirectURI); the mock AS records exactly one /register request.
• Persistence preserved: register once, then construct a fresh PerformOAuthFlow against the same Cached* fields; assert the second call short-circuits (zero /register requests).

Edge cases

• Provider does not advertise a registration_endpoint and synthesis is unavailable: CLI surfaces the existing flag-fallback message verbatim.
• Provider advertises a registration endpoint but returns 404/405/501: CLI surfaces a clear error (the protocol-neutral message lives in pkg/oauthproto.handleHTTPResponse; the resolver surfaces it through).

Out of Scope

• RFC 7592 client management / rotation (Implement Client Secret Expiry and Renewal for Dynamic Client Registration #3631).
• SSRF hardening on operator-supplied DCR URLs (Harden DCR resolver against SSRF when operator trust boundary changes #5135).
• The Notify When DCR Is Unsupported for Remote MCP Server UX work (Notify When DCR Is Unsupported for Remote MCP Server #2680).
• Any changes to pkg/oauthproto wire-shape types.
• Refactoring pkg/auth/remote/handler.go's caching beyond what is needed to satisfy the persistence-preservation acceptance criterion.

References

• Parent issue: Consolidate the two RFC 7591 DCR client implementations into pkg/auth/dcr #5145.
• Sub-issue 4a: PR Extract DCR resolver into pkg/auth/dcr #5198 — extracted pkg/auth/dcr and migrated EmbeddedAuthServer.
• Originating PR: Add authserver DCR credential store and resolver #5042 — defined the review-property behaviours that this PR propagates to the CLI flow.
• .claude/rules/go-style.md, .claude/rules/testing.md, .claude/rules/pr-creation.md, CLAUDE.md.
• RFC 7591 §3.2.1 (registration expiry), RFC 7636 / OAuth 2.1 (S256 PKCE), RFC 8252 §7.3 (loopback redirect for native CLI clients).