Add persistent DCRCredentialStore types and memory backend

[tgrunnagle]

DRAFT - not ready for review

Summary

• Why: Phase 3 of the authserver-driven DCR story (Persistent DCRCredentialStore backends (memory + Redis) #4979 / parent Authserver-driven DCR for upstream OAuth 2.1 MCP servers #4976) needs the persisted credential store moved to pkg/authserver/storage/ so both MemoryStorage and the future RedisStorage (sub-issue 2) can implement the same interface. Until that happens, every authserver replica re-registers its own DCR client on boot, no client_secret_expires_at TTL is honored, and the runner-side stub from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 has no production-shaped sibling. This PR ships the storage-level interface, the value type with full RFC 7591 + 7592 fields, and the in-memory backend; the Redis backend and the wiring swap follow as separate PRs.
• What (top of stack — issue Persistent DCRCredentialStore: types, interface, and in-memory backend #5183):
  • New DCRCredentials value type in pkg/authserver/storage/types.go carrying the canonical DCRKey, ClientID/ClientSecret, TokenEndpointAuthMethod, RFC 7592 management fields (RegistrationAccessToken, RegistrationClientURI), AuthorizationEndpoint/TokenEndpoint, CreatedAt, and ClientSecretExpiresAt (added in the review-feedback commit so the Redis backend can satisfy the TTL contract without re-touching the value type).
  • New segregated DCRCredentialStore interface (GetDCRCredentials, StoreDCRCredentials) added to the package's mockgen directive; mocks regenerated.
  • DCRKey and its canonical ScopesHash constructor moved from pkg/authserver/runner/ into pkg/authserver/storage/. The runner keeps a package-local type DCRKey = storage.DCRKey alias and a var scopesHash = storage.ScopesHash binding so existing call sites compile unchanged and the canonical hashing has a single source of truth.
  • MemoryStorage gains a dcrCredentials map[DCRKey]*DCRCredentials guarded by the existing sync.RWMutex; StoreDCRCredentials defensively copies on write and rejects creds with empty Key.Issuer or Key.RedirectURI; GetDCRCredentials defensively copies on read and returns wrapped ErrNotFound on miss. DCR entries are intentionally excluded from cleanupExpired (registrations are long-lived; Redis will use SetEX for the TTL case). Stats exposes the new map's count for parity with the other in-memory maps.
• What (folded-in preparatory commits):
  • Operator CRD surface for upstream DCR (Authserver DCR: expose DCR config in operator CRD (Phase 2, CRD surface) #5040): new DCRUpstreamConfig type on OAuth2UpstreamConfig (discoveryUrl, registrationEndpoint, initialAccessTokenRef, softwareId, softwareStatement), CEL validation enforcing exactly-one-of clientId/dcrConfig and exactly-one-of discoveryUrl/registrationEndpoint, controllerutil conversion that resolves InitialAccessTokenRef to a TOOLHIVE_UPSTREAM_DCR_INITIAL_ACCESS_TOKEN_* env var, regenerated CRD YAML and crd-api.md.
  • Authserver-driven DCR resolver wiring (Authserver DCR: wire resolver into authserver and add structured logs (Phase 2, Steps 2d/2g) #5039 Phase 2 steps 2d/2g): EmbeddedAuthServer owns an in-memory DCRCredentialStore and calls resolveDCRCredentials for any OAuth2 upstream with DCRConfig; resolved ClientSecret is overlaid through a paired applyResolution / applyResolutionToOAuth2Config helper after buildPureOAuth2Config. The caller's RunConfig.Upstreams slice is shallow-copied and the OAuth2 sub-config deep-copied before resolution so the original is not mutated. Structured logs added at the resolver and /oauth/register boundary; per-step error context flows through dcrStepError and is logged once at the boundary.
  • Phase 2 review-feedback fixes against Wire authserver DCR resolver and add structured logs #5044: bound the upstream /register error body at 8 KiB before logging (Wire authserver DCR resolver and add structured logs #5044 F2); restore the documented CIMD precedence in remoteAuthLogContext (Wire authserver DCR resolver and add structured logs #5044 F3, F26); hoist the DCR remediation Warn out of the isTransientNetworkError classifier into markAsUnauthenticated so it fires at most once per state transition and only when a client_id is available (Wire authserver DCR resolver and add structured logs #5044 F5); validate AuthorizationServerConfig in NewHandler and demote the /oauth/register success log from Info to Debug (Wire authserver DCR resolver and add structured logs #5044 F6, F7); URL-sanitiser hardening in sanitizeErrorForLog (strips userinfo + fragment, case-insensitive scheme match, preserves trailing punctuation), lowercased internal dcrStepError types, renamed applyResolution to consumeResolution to communicate one-shot semantics, and assorted doc-comment cleanups (Wire authserver DCR resolver and add structured logs #5044 F1, F4, F11, F12, F13, F18, F19, F20, F21, F22, F23, F24, F25).

Closes #5183

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Specifically:

• pkg/authserver/storage/memory_test.go — round-trip on all DCRCredentials fields including ClientSecretExpiresAt; two distinct DCRKey values do not collide; overwrite semantics replace the prior entry; Get on an absent key returns wrapped ErrNotFound via errors.Is; defensive-copy assertions on both the read and write paths; StoreDCRCredentials rejects empty Key.Issuer and empty Key.RedirectURI; Stats reports the DCRCredentials count.
• pkg/authserver/storage/types_test.go — canonical ScopesHash exercised here as the single source of truth (the runner-side TestScopesHash_* cases were dropped in the review-feedback commit).
• pkg/authserver/runner/dcr_test.go, embeddedauthserver_test.go — full DCR boot path against a mock AS, cache-hit short-circuit issues zero additional HTTP requests, caller's RunConfig.Upstreams slice element is unchanged across both calls, sanitiser test cases for userinfo/fragment/case-insensitive scheme/trailing punctuation, single-emission of the panic-recovery log.
• cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go, controllerutil/authserver_test.go — table-driven CEL/Go validation for DCR + ClientID conflict, both endpoints set, neither endpoint set, valid single-endpoint cases, and InitialAccessTokenRef env-var wiring.
• task gen rerun; pkg/authserver/storage/mocks/mock_storage.go regenerated.

API Compatibility

The CRD changes add new optional fields on OAuth2UpstreamConfig (dcrConfig) and add a CEL validation that requires exactly one of clientId or dcrConfig. Existing manifests that set clientId continue to validate; the new dcrConfig path is additive. clientId is now optional at the CRD level (it was previously required), which is a relaxation, not a break.

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File	Change
pkg/authserver/storage/types.go	Add DCRCredentials, DCRCredentialStore, move DCRKey + ScopesHash here, extend mockgen directive
pkg/authserver/storage/memory.go	Add dcrCredentials map, StoreDCRCredentials/GetDCRCredentials, defensive-copy helpers, exclude from cleanupExpired, surface in Stats
pkg/authserver/storage/memory_test.go	Round-trip / overwrite / not-found / defensive-copy / validation tests
pkg/authserver/storage/mocks/mock_storage.go	Regenerated mock for new interface
pkg/authserver/runner/dcr_store.go	DCRKey becomes a type alias to storage.DCRKey; scopesHash rebound to storage.ScopesHash
pkg/authserver/runner/dcr.go	Resolver structured logs, dcrStepError boundary logging, applyResolution/applyResolutionToOAuth2Config pairing, sanitiser hardening
pkg/authserver/runner/dcr_store_test.go	Drop runner-local ScopesHash tests; the canonical suite lives in the storage package
pkg/authserver/runner/embeddedauthserver.go	Construct in-memory DCR store; resolve DCR before building OAuth2 configs
pkg/authserver/server/handlers/handler.go	Validate AuthorizationServerConfig in NewHandler; simplify issuer()
pkg/authserver/server/handlers/dcr.go	Demote success log to Debug; thread software_id
pkg/authserver/server/registration/dcr.go	Cap software_id length, require printable ASCII
pkg/auth/monitored_token_source.go	Promote upstream/client_id to constructor parameters; emit DCR remediation Warn once per state transition from markAsUnauthenticated
pkg/oauthproto/dcr.go	Bound /register error body read at 8 KiB
pkg/runner/runner.go	Wire upstream/client_id from RemoteAuthConfig (CIMD precedence) into the MonitoredTokenSource
cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go	New DCRUpstreamConfig type, CEL validation, ValidateOAuth2DCRConfig
cmd/thv-operator/pkg/controllerutil/authserver.go	Convert DCRConfig to authserver.DCRUpstreamConfig; resolve InitialAccessTokenRef to env var
deploy/charts/operator-crds/{files,templates}/crds/*.yaml, docs/operator/crd-api.md	Regenerated CRDs and docs

Does this introduce a user-facing change?

Yes. Kubernetes operators can now configure RFC 7591 Dynamic Client Registration on OAuth2UpstreamConfig via a new dcrConfig field (discoveryUrl/registrationEndpoint, initialAccessTokenRef, softwareId, softwareStatement). Until the Redis backend (Phase 3 sub-issue 2) and the wiring swap (sub-issue 3) land, the storage-level DCRCredentialStore introduced here is implemented only by MemoryStorage, so each authserver replica still holds an independent map; cross-replica sharing arrives with the Redis backend.

Special notes for reviewers

• PR scope is wider than the headline issue. The two commits at the top of the stack (94fe0dc7, 17bd2ab5) implement Persistent DCRCredentialStore: types, interface, and in-memory backend #5183 and are the primary review target. The earlier six commits are Phase 2 follow-ups (Authserver DCR: wire resolver into authserver and add structured logs (Phase 2, Steps 2d/2g) #5039, Authserver DCR: expose DCR config in operator CRD (Phase 2, CRD surface) #5040, Wire authserver DCR resolver and add structured logs #5044 review feedback) that landed on the same branch because they touch the same DCR call-graph; reviewers may find it easier to read commit-by-commit than diff-by-diff against main.
• DCRKey consolidation choice. Issue Persistent DCRCredentialStore: types, interface, and in-memory backend #5183 offered two options: move DCRKey to storage/ (option a) or keep it in runner/ with a storage-side alias (option b). This PR takes option (a) — canonical definition in storage/, package-local alias in runner/ — so any future Redis backend hashes keys identically. The runner-side scopesHash is a var binding (var scopesHash = storage.ScopesHash) rather than a re-export, which keeps the indirection visible to callers.
• Runner-side DCRCredentialStore is intentionally NOT removed. Sub-issue 1 lands the new storage-level interface and implementation; the runner-side adapter (Get/Put on *DCRResolution) is left in place as the thin abstraction the wiring PR (sub-issue 3) will swap. Two competing implementations exist on main after this lands — that is expected and called out in the issue.
• ClientSecretExpiresAt on the in-memory backend is informational. The in-memory backend retains the field verbatim and does not act on it; the resolver re-checks expiry on read. The Redis backend will plumb it through SetEX. The interface contract uses SHOULD (not MUST) for TTL handling so backends without a native TTL remain conformant.
• Defensive-copy semantics match UpstreamTokens. StoreDCRCredentials and GetDCRCredentials both do field-by-field copies; DCRCredentials is a flat value type with no nested maps/slices/pointers so a shallow copy is a deep copy here. If a future field introduces nested mutable state, cloneDCRCredentials is the single point to update.
• No new logging of secrets. client_secret, registration_access_token, initial_access_token and refresh tokens are never arguments to slog.* calls; the grep assertion from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 still passes.

Implementation plan

Approved implementation plan (issue #5183)

1. Add DCRCredentials value type to pkg/authserver/storage/types.go adjacent to UpstreamTokens. Mirror the defensive-copy contract documented for UpstreamTokens. Capture RFC 7591 fields plus the RFC 7592 management fields verbatim.
2. Move DCRKey and its ScopesHash constructor from pkg/authserver/runner/ to pkg/authserver/storage/ so any backend hashes keys identically. Keep a package-local type alias in the runner so existing call sites compile unchanged.
3. Add DCRCredentialStore interface (GetDCRCredentials, StoreDCRCredentials) to types.go. Wire it into the existing mockgen directive. Keep it segregated — do NOT add it to the Storage umbrella interface.
4. Extend MemoryStorage with a dcrCredentials map[DCRKey]*DCRCredentials guarded by the existing sync.RWMutex. Defensive copy on read and write; reject empty Key.Issuer/Key.RedirectURI; return wrapped ErrNotFound on miss; exclude from cleanupExpired; surface in Stats. Add the compile-time interface assertion at the bottom of the file.
5. Run task gen to regenerate the DCRCredentialStore mock.
6. Verify with task build, task test, task lint-fix, task license-check. Run the secret-grep assertion from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978.

Out of scope (deferred to sub-issues 2 and 3):

• Redis backend implementation.
• Wiring EmbeddedAuthServer to use the storage-level interface and removing the Phase 2 standalone in-memory stub.

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

[github-actions]

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!

[codecov]

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.71%. Comparing base (1b0e781) to head (cc92472).

Files with missing lines	Patch %	Lines
pkg/authserver/storage/memory.go	93.10%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@                  Coverage Diff                  @@
##           issue_5040_dcr-2d    #5186      +/-   ##
=====================================================
- Coverage              67.74%   67.71%   -0.03%     
=====================================================
  Files                    607      607              
  Lines                  62049    62078      +29     
=====================================================
+ Hits                   42033    42039       +6     
- Misses                 16854    16876      +22     
- Partials                3162     3163       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.