Rename pkg/oauth to pkg/oauthproto and relocate DCR primitives

[jhrozek]

Description

Phase 1 of the DCR story (#4976). Rename the existing pkg/oauth leaf package to pkg/oauthproto, move the RFC 7591 Dynamic Client Registration primitives out of pkg/auth/oauth/dynamic_registration.go into pkg/oauthproto/dcr.go, and relocate the handful of helpers (IsLocalhost, UserAgent) they depend on so that pkg/oauthproto/ stays a true leaf package (no pkg/networking import). This unblocks Phase 2: once the rename lands, pkg/authserver/ can import the DCR client without creating an import cycle.

Context

Today, DCR primitives live in pkg/auth/oauth/dynamic_registration.go, which transitively imports pkg/networking. The authserver cannot import pkg/auth/oauth without creating a cycle, so there is no path for pkg/authserver/ to reuse the DCR client. This task solves that by preserving the pkg/oauth leaf-package invariant under a new name (pkg/oauthproto) and moving only the pure protocol primitives into it. FetchAuthorizationServerMetadata, DCRUpstreamConfig, and the authserver wiring are deliberately deferred to Phase 2 (#4978).

Key design points carried into this task:

• pkg/oauthproto/ must remain a leaf package. IsLocalhost is therefore relocated into pkg/oauthproto/ (as a private isLoopbackHost), and pkg/networking imports it back out rather than the other way around. This keeps the pkg/oauthproto dep graph pointing at net/url / errors / fosite only.
• RegisterClientDynamically takes *http.Client directly (nil means "build a default one"). The private registerClientDynamicallyWithClient helper and the networking.HTTPClient interface indirection are dropped in this move.
• The CLI-specific error string in handleHTTPResponse (referencing --remote-auth-client-id / --remote-auth-client-secret) is generalized before the move, because the protocol package must not leak CLI assumptions.
• The CLI-side NewDynamicClientRegistrationRequest(scopes, callbackPort) helper — which hardcodes http://localhost:<port>/callback — stays out of the protocol package and is hosted in pkg/auth/discovery/, its sole caller.
• UserAgent = "ToolHive/1.0" constant moves out of pkg/auth/oauth/oidc.go:24 into pkg/oauthproto/constants.go so the moved DCR code and any future leaf-package HTTP builders can share it.

The existing pkg/oauth/doc.go documents the leaf-package invariant that the rename preserves.

Dependencies: #4976 (parent story only — no technical task dependency)
Blocks: #4978 (authserver DCR integration cannot import pkg/oauthproto until this rename lands)

Acceptance Criteria

• pkg/oauth/ directory is renamed to pkg/oauthproto/; all 17 call sites that import github.com/stacklok/toolhive/pkg/oauth are updated to import github.com/stacklok/toolhive/pkg/oauthproto (the 8 that already alias as oauthproto can drop the alias).
• pkg/auth/oauth/dynamic_registration.go is deleted; its content (types, constructors, RegisterClientDynamically, validation helpers, handleHTTPResponse, ToolHiveMCPClientName, and ScopeList JSON codec) lives in pkg/oauthproto/dcr.go.
• pkg/auth/oauth/dynamic_registration_test.go is deleted; tests that cover the moved functions live in pkg/oauthproto/dcr_test.go and pass from the new location.
• grep -r "pkg/auth/oauth/dynamic_registration" returns no hits across the repository (including Go files, docs, and planning artifacts).
• RegisterClientDynamically signature is func(ctx context.Context, registrationEndpoint string, request *DynamicClientRegistrationRequest, client *http.Client) (*DynamicClientRegistrationResponse, error) — takes *http.Client directly (nil for default); the private registerClientDynamicallyWithClient helper is removed; the networking.HTTPClient interface is no longer referenced from DCR code.
• IsLocalhost is relocated out of pkg/networking/utilities.go:99-106 and hosted privately inside pkg/oauthproto/ (e.g., isLoopbackHost). pkg/networking imports it back through the exported wrapper so existing callers outside this task's scope are unaffected. pkg/oauthproto/ has no github.com/stacklok/toolhive/pkg/networking import.
• UserAgent constant is moved from pkg/auth/oauth/oidc.go:24 into pkg/oauthproto/constants.go and all references to it resolve to the new location.
• The CLI-specific hint string in handleHTTPResponse (currently suggesting --remote-auth-client-id / --remote-auth-client-secret flags) is rewritten to a protocol-neutral message before the function moves into pkg/oauthproto/dcr.go.
• pkg/auth/discovery/ hosts NewDynamicClientRegistrationRequest(scopes []string, callbackPort int) *oauthproto.DynamicClientRegistrationRequest (the sole caller is registerDynamicClient at pkg/auth/discovery/discovery.go:~720); the factory no longer lives in pkg/auth/oauth/.
• pkg/oauthproto/doc.go is updated to reflect the expanded (still leaf-package) surface area.
• go build ./... succeeds with no import cycles.
• task test passes; all moved tests run from pkg/oauthproto/ and exercise the same behavior they covered in pkg/auth/oauth/.
• task lint-fix reports no findings.
• task license-check is clean. Any new files written in pkg/oauthproto/ use the 2-line SPDX header style documented in .claude/rules/go-style.md; existing 13-line Apache header blocks on moved files are preserved unless the file is being rewritten in full.
• Code reviewed and approved.

Technical Approach

Recommended Implementation

Do the rename and the move in a single PR where possible (the plan treats Phase 1 as one unit, and the diff should stay comfortably under the 400-LOC / 10-file cap). Order matters — do it in roughly this sequence so intermediate go build failures are contained:

1. Rename the directory pkg/oauth/ -> pkg/oauthproto/ and update the package oauth line at the top of every file to package oauthproto. Fix the package doc in doc.go.
2. Update call-site imports across the 17 files that import pkg/oauth. The 8 sites already using the oauthproto "github.com/stacklok/toolhive/pkg/oauth" alias can drop the alias. Use one commit per logical area (upstream, auth/oauth, auth/discovery, server/handlers, etc.) if that makes review easier, but keep the PR singular.
3. Move UserAgent from pkg/auth/oauth/oidc.go:24 into pkg/oauthproto/constants.go. Update the three referencing call sites (grep for UserAgent inside pkg/auth/oauth/ first).
4. Relocate IsLocalhost into pkg/oauthproto/ as private isLoopbackHost. Change pkg/networking/utilities.go so the package-level IsLocalhost wraps the new private helper via an exported pass-through (func IsLocalhost(host string) bool { return oauthproto.IsLoopbackHost(host) } — or export it from pkg/oauthproto if the call surface warrants it). Verify pkg/oauthproto still has no pkg/networking import.
5. Generalize handleHTTPResponse error string in pkg/auth/oauth/dynamic_registration.go before moving the file — replace the CLI-flag-specific hint ("Please configure OAuth client credentials using --remote-auth-client-id…") with a protocol-neutral message (e.g., "the provider does not support RFC 7591 Dynamic Client Registration; configure client credentials out of band"). Land this as part of the same PR; keeping it as a separate prep commit inside the PR is fine.
6. Move the DCR content from pkg/auth/oauth/dynamic_registration.go to pkg/oauthproto/dcr.go. Keep ToolHiveMCPClientName, DynamicClientRegistrationRequest, DynamicClientRegistrationResponse, ScopeList (with its MarshalJSON / UnmarshalJSON — preserve verbatim), validateRegistrationEndpoint, validateAndSetDefaults, createHTTPRequest, handleHTTPResponse, and RegisterClientDynamically. Drop getHTTPClient(networking.HTTPClient) and registerClientDynamicallyWithClient. Delete the original file.
7. Collapse the client-injection seam on RegisterClientDynamically — its new signature takes *http.Client (nil means build the default 30s/10s/10s client inline). Tests that previously used a mock networking.HTTPClient swap to httptest.NewServer (already the idiom in the existing dynamic_registration_test.go) plus a real http.Client pointed at the test server.
8. Host the CLI factory — create NewDynamicClientRegistrationRequest(scopes, callbackPort) inside pkg/auth/discovery/ (the sole caller). Update registerDynamicClient at pkg/auth/discovery/discovery.go:~720 to call the local factory and oauthproto.RegisterClientDynamically directly. The CLI factory retains the http://localhost:<port>/callback redirect URI assumption that does not belong in the protocol package.
9. Move the tests. pkg/auth/oauth/dynamic_registration_test.go → pkg/oauthproto/dcr_test.go with package oauthproto_test (external test package to mirror the public API shape). Keep the existing httptest.NewServer structure. Drop any test that exercised the removed registerClientDynamicallyWithClient entry point; rewrite the equivalent case to use httptest.NewServer and the real *http.Client parameter.
10. Update pkg/oauthproto/doc.go so the package comment mentions both the pre-existing protocol surface (RFC 8414 types, redirect policy, well-known paths) and the new DCR surface (RFC 7591 request/response types, RegisterClientDynamically, ScopeList, UserAgent, ToolHiveMCPClientName). Reassert the leaf-package invariant in the doc comment.
11. Verify. Run task build, task test, task lint-fix, and task license-check. Run the explicit grep checks listed in Acceptance Criteria. Confirm pkg/oauthproto has no pkg/networking import (a quick grep -r "pkg/networking" pkg/oauthproto sanity check).

Patterns & Frameworks

• Plain testing + github.com/stretchr/testify for unit tests. The existing unit tests in pkg/oauth/, pkg/auth/*, and pkg/authserver/* do not use Ginkgo — match that convention. Use require.NoError over t.Fatal; table-driven tests where input permutations matter.
• httptest.NewServer for HTTP mocking — already the idiom in pkg/auth/oauth/dynamic_registration_test.go:1-80. Register teardown with t.Cleanup(server.Close), not defer, when the test uses t.Parallel().
• Standard-library only for URL handling in pkg/oauthproto/ (net/url, errors). fosite is already pulled in via pkg/oauthproto/redirect.go's redirect-URI helpers and stays.
• SPDX headers per .claude/rules/go-style.md: 2-line style on new files written in pkg/oauthproto/; leave existing 13-line Apache headers on moved files alone unless the diff rewrites the whole file. Run task license-fix if task license-check complains.
• Always drive builds/tests/lint through task (task build, task test, task lint-fix, task license-check) per CLAUDE.md. Never go test ./... or golangci-lint run directly.
• Commit-message style: imperative mood, capitalized subject, no trailing period, 50-char limit, no conventional-commit prefixes (CLAUDE.md).

Code Pointers

• pkg/oauth/ — current leaf package. Rename in place to pkg/oauthproto/; update the package line in every .go file. Preserve the existing 13-line Apache headers on these files.
• pkg/oauth/doc.go — rewrite the package comment to describe the expanded scope (types + constants + DCR client). Reassert the "leaf package, no pkg/networking dep" invariant.
• pkg/auth/oauth/dynamic_registration.go (352 LOC) — canonical content to move. ToolHiveMCPClientName (L23), DynamicClientRegistrationRequest (L26-36), NewDynamicClientRegistrationRequest (L39-53 — stays in pkg/auth/discovery/, does not move to pkg/oauthproto/), ScopeList + codec (L70-132 — preserve verbatim), DynamicClientRegistrationResponse (L135-153), RegisterClientDynamically (L156-162 — new signature takes *http.Client), validators (L165-211), createHTTPRequest (L214-237), getHTTPClient (L240-252 — delete, fold the default-client creation into RegisterClientDynamically), handleHTTPResponse (L255-307 — generalize the error string at L275-278 before moving), registerClientDynamicallyWithClient (L310-351 — delete; inline its body into RegisterClientDynamically).
• pkg/auth/oauth/dynamic_registration_test.go — tests moved to pkg/oauthproto/dcr_test.go. Change package from oauth_test to oauthproto_test. Convert any mock-networking.HTTPClient tests to httptest.NewServer + real *http.Client.
• pkg/auth/oauth/oidc.go:24 — UserAgent = "ToolHive/1.0" moves to pkg/oauthproto/constants.go. Update three in-tree references (grep first).
• pkg/auth/discovery/discovery.go:720 — registerDynamicClient updated to call a locally-hosted NewDynamicClientRegistrationRequest and oauthproto.RegisterClientDynamically. Add the factory in this package (same file or a new dcr_request.go).
• pkg/networking/utilities.go:99-106 — IsLocalhost relocated into pkg/oauthproto/ as isLoopbackHost (or an exported IsLoopbackHost if callers outside pkg/networking need it). The pkg/networking.IsLocalhost symbol remains as a thin wrapper to avoid touching the existing external callers (6+ sites across the repo).
• pkg/auth/discovery/discovery.go:745-812 — template (FetchResourceMetadata) for the future FetchAuthorizationServerMetadata in Authserver DCR integration (Phase 2, Steps 2a-2g) #4978; reference only, not changed here.

Component Interfaces

// pkg/oauthproto/dcr.go

// ToolHiveMCPClientName is advertised in dynamic client registration requests.
const ToolHiveMCPClientName = "ToolHive MCP Client"

// DynamicClientRegistrationRequest (RFC 7591). Shape unchanged from pkg/auth/oauth.
type DynamicClientRegistrationRequest struct {
    RedirectURIs            []string  `json:"redirect_uris"`
    ClientName              string    `json:"client_name,omitempty"`
    TokenEndpointAuthMethod string    `json:"token_endpoint_auth_method,omitempty"`
    GrantTypes              []string  `json:"grant_types,omitempty"`
    ResponseTypes           []string  `json:"response_types,omitempty"`
    Scopes                  ScopeList `json:"scope,omitempty"`
}

// DynamicClientRegistrationResponse (RFC 7591 + RFC 7592 fields). Shape unchanged.
type DynamicClientRegistrationResponse struct {
    ClientID                string    `json:"client_id"`
    ClientSecret            string    `json:"client_secret,omitempty"` //nolint:gosec
    ClientIDIssuedAt        int64     `json:"client_id_issued_at,omitempty"`
    ClientSecretExpiresAt   int64     `json:"client_secret_expires_at,omitempty"`
    RegistrationAccessToken string    `json:"registration_access_token,omitempty"`
    RegistrationClientURI   string    `json:"registration_client_uri,omitempty"`
    ClientName              string    `json:"client_name,omitempty"`
    RedirectURIs            []string  `json:"redirect_uris,omitempty"`
    TokenEndpointAuthMethod string    `json:"token_endpoint_auth_method,omitempty"`
    GrantTypes              []string  `json:"grant_types,omitempty"`
    ResponseTypes           []string  `json:"response_types,omitempty"`
    Scopes                  ScopeList `json:"scope,omitempty"`
}

// RegisterClientDynamically performs RFC 7591 Dynamic Client Registration.
// If client is nil, a default http.Client with 30s timeout / 10s TLS handshake
// / 10s response header timeout is used.
func RegisterClientDynamically(
    ctx context.Context,
    registrationEndpoint string,
    request *DynamicClientRegistrationRequest,
    client *http.Client,
) (*DynamicClientRegistrationResponse, error)

// ScopeList handles the RFC 7591 scope-field ambiguity (string vs. array on the
// wire, []string in Go). Marshal/Unmarshal preserved verbatim from the previous
// location in pkg/auth/oauth.
type ScopeList []string

// pkg/oauthproto/constants.go
// (additions to the existing file)

// UserAgent is sent on all HTTP requests originating from this package.
const UserAgent = "ToolHive/1.0"

// pkg/oauthproto/locality.go (or folded into an existing file)

// isLoopbackHost reports whether host is a loopback hostname.
// Kept private to preserve the leaf-package invariant — pkg/networking exposes
// the public IsLocalhost wrapper for legacy callers.
func isLoopbackHost(host string) bool

// pkg/auth/discovery/dcr_request.go (or adjacent to the sole caller)

// NewDynamicClientRegistrationRequest constructs a DCR request for the CLI flow,
// which always uses an http://localhost:<port>/callback redirect URI (loopback
// public client per RFC 8252).
func NewDynamicClientRegistrationRequest(
    scopes []string,
    callbackPort int,
) *oauthproto.DynamicClientRegistrationRequest

Testing Strategy

Unit Tests (in pkg/oauthproto/dcr_test.go, package oauthproto_test):

• RegisterClientDynamically happy path against an httptest.NewServer returning a 201 JSON body with client_id, client_secret, and echoed request fields.
• RegisterClientDynamically with client == nil builds the default *http.Client (no panic; request lands on the test server — bind to a loopback host so validateRegistrationEndpoint accepts the HTTP URL).
• RegisterClientDynamically with a caller-supplied *http.Client (pointed at httptest.NewServer) demonstrates the non-default client path.
• validateRegistrationEndpoint rejects non-HTTPS non-loopback URLs, accepts localhost HTTP URLs, and rejects malformed URLs.
• validateAndSetDefaults rejects nil request, empty RedirectURIs, and scope values containing spaces; fills in defaults for ClientName, GrantTypes, ResponseTypes, TokenEndpointAuthMethod when omitted.
• handleHTTPResponse maps 404/405/501 to the new protocol-neutral "provider does not support DCR" error (verify the CLI-flag hint is absent from the error string — this is the regression guard for the generalization step).
• handleHTTPResponse rejects non-JSON content types, caps body size via io.LimitReader, and requires client_id on success.
• ScopeList.UnmarshalJSON covers the four shapes documented in the type's doc comment: space-delimited string, JSON array, null, empty/whitespace. MarshalJSON round-trips.
• isLoopbackHost covers localhost, localhost:8080, 127.0.0.1, 127.0.0.1:1, [::1], [::1]:80, plus negative cases (e.g., example.com, 127.0.0.1.example.com).

Integration Tests

• No new integration tests in this task — Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 extends pkg/authserver/integration_test.go to cover the DCR path end-to-end once the authserver wiring exists.

Edge Cases

• pkg/oauthproto builds standalone with go build ./pkg/oauthproto/... (no pkg/networking transitive dep — verify via go list -deps ./pkg/oauthproto/... that the networking path is absent).
• pkg/auth/discovery/ still compiles after the factory move; the CLI flow at pkg/auth/discovery/discovery.go:720 produces an identical request body as before (diff the on-wire JSON to confirm no behavioral drift in the move).
• The 8 call sites that already alias pkg/oauth as oauthproto drop the alias cleanly — no dangling aliases left behind.

Out of Scope

• FetchAuthorizationServerMetadata and the three-path RFC 8414 discovery fallback (deferred to Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 Phase 2).
• DCRUpstreamConfig, resolveDCRCredentials, DCRResolution, DCRCredentialStore interface / in-memory stub, and authserver wiring (all Phase 2 — Authserver DCR integration (Phase 2, Steps 2a-2g) #4978).
• Persistent DCRCredentialStore backends (memory + Redis) — Phase 3, Persistent DCRCredentialStore backends (memory + Redis) #4979.
• Log-level upgrade at pkg/authserver/server/handlers/dcr.go:98, stale-cache warn log, and invalid_client remediation log at pkg/auth/monitored_token_source.go:361 — Phase 2 (Authserver DCR integration (Phase 2, Steps 2a-2g) #4978).
• Removing networking.HTTPClient from the rest of the codebase (still used by e.g. pkg/auth/oauth/oidc.go:42). This task only drops it from DCR code; a wider cleanup is out of scope.
• CLI persistence of registration_client_uri in pkg/auth/remote/config.go (deferred follow-up).
• Any change to the authserver /register handler semantics at pkg/authserver/server/handlers/dcr.go beyond what is unavoidable for the rename.

References

• Parent story: Authserver-driven DCR for upstream OAuth 2.1 MCP servers #4976 — "Authserver-driven DCR for upstream OAuth 2.1 MCP servers".
• Downstream: Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 (blocked until this lands), Persistent DCRCredentialStore backends (memory + Redis) #4979.
• .claude/rules/go-style.md — SPDX headers, constructor validation, drain-response-body conventions.
• .claude/rules/testing.md — testify over t.Fatal, t.Cleanup over defer, no Ginkgo in this area.
• CLAUDE.md — always use task (never go test / golangci-lint directly); 400 LOC / 10 files PR cap.
• RFC 7591 (Dynamic Client Registration); RFC 8252 (loopback redirect URIs for public clients).

[tgrunnagle]

Before closing and after #5036 : the authserver still defines its own DCRRequest/DCRResponse in pkg/authserver/server/registration/dcr.go alongside the new types in pkg/oauthproto. Reconciling Scope string vs ScopeList is the blocking work, so it doesn't belong in #5036.

See #5371