Persistent DCRCredentialStore: types, interface, and in-memory backend

[tgrunnagle]

Description

Sub-task 1 of 3 for Phase 3 of the DCR story (#4976, parent #4979). Define the persisted DCRCredentials value type and the storage-level DCRCredentialStore interface in pkg/authserver/storage/, and ship the in-process memory implementation that both unit tests and single-replica deployments use. This sub-issue is the foundation the Redis backend (sub-issue 2) and the wiring change (sub-issue 3) build on.

Context

#4978 (Phase 2) defined the narrow runner-side DCRCredentialStore interface — Get(ctx, DCRKey) (*DCRResolution, bool, error) and Put(ctx, DCRKey, *DCRResolution) error — together with the DCRKey{Issuer, RedirectURI, ScopesHash} canonical cache key, and shipped an in-process in-memory stub. Phase 3 promotes the interface and value type into pkg/authserver/storage/ so the same interface is implemented by both MemoryStorage and RedisStorage. Full RFC 7592 fields (RegistrationAccessToken, RegistrationClientURI) are already captured by Phase 2 and round-trip through this PR's value type.

This sub-issue introduces no new protocol behavior. It is a structural refactor of the Phase 2 abstraction onto the existing pkg/authserver/storage/ shape, plus the in-memory implementation that the Redis backend (sub-issue 2) parallels.

Dependencies: ##4978
Blocks: sub-issue 2 (Redis), sub-issue 3 (wiring)

Acceptance Criteria

DCRCredentials value type — pkg/authserver/storage/types.go

• New exported type DCRCredentials is added with the following fields (order may follow the local file convention, but all fields must be present):
  • Key DCRKey — canonical cache key (mirrors the DCRKey type from pkg/authserver/runner/dcr_store.go defined in Authserver DCR integration (Phase 2, Steps 2a-2g) #4978; Phase 3 consumes the same canonical form — do NOT redefine the hashing).
  • ProviderName string — debug / audit only (the upstream UpstreamRunConfig.Name that triggered the registration; never used as a primary key).
  • ClientID string
  • ClientSecret string — //nolint:gosec // G117 comment consistent with UpstreamTokens.AccessToken.
  • TokenEndpointAuthMethod string
  • RegistrationAccessToken string (RFC 7592 — //nolint:gosec as above)
  • RegistrationClientURI string (RFC 7592)
  • AuthorizationEndpoint string
  • TokenEndpoint string
  • CreatedAt time.Time
• Doc comment on DCRCredentials states that consumers receive a defensive copy (the type is effectively immutable from the caller's perspective).
• The existing DCRKey type is re-exported from (or imported into) pkg/authserver/storage/ in a way that keeps the canonical hashing helper in a single place. Acceptable options: (a) move DCRKey + its ScopesHash constructor from pkg/authserver/runner/ into pkg/authserver/storage/ and update the runner to depend on it; (b) leave DCRKey in the runner and reference it via type alias in storage. Pick one and document the choice in the commit message. Do NOT duplicate the hashing logic.

DCRCredentialStore interface — pkg/authserver/storage/types.go

• DCRCredentialStore interface is declared in pkg/authserver/storage/types.go with:
  • GetDCRCredentials(ctx context.Context, key DCRKey) (*DCRCredentials, error) returning ErrNotFound (already defined in types.go:36) when the key is absent.
  • StoreDCRCredentials(ctx context.Context, creds *DCRCredentials) error.
  • Optionally DeleteDCRCredentials(ctx context.Context, key DCRKey) error if needed for tests; otherwise omit.
• The interface is added to the //go:generate mockgen directive at the top of types.go so task gen produces a mock in pkg/authserver/storage/mocks/.
• The runner-side DCRCredentialStore interface from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 is either (a) replaced by this storage-level interface everywhere in pkg/authserver/runner/, or (b) kept as a thinner adapter that delegates to the storage-level one. Pick the cleaner of the two — no duplicated abstraction. (Note: full removal of the Phase 2 standalone in-memory stub is sub-issue 3's job; this sub-issue lands the new interface and the new implementation, leaving the wire-up swap to the wiring PR.)

In-memory backend — pkg/authserver/storage/memory.go

• MemoryStorage gains a dcrCredentials map[DCRKey]*DCRCredentials field guarded by the existing sync.RWMutex.
• NewMemoryStorage initializes the map.
• StoreDCRCredentials performs a defensive copy of the input before storing (matching the StoreUpstreamTokens pattern at memory.go:693-734).
• GetDCRCredentials returns a defensive copy; returns ErrNotFound (wrapped per the existing convention: fmt.Errorf("%w: ...", ErrNotFound, ...)) on miss.
• Compile-time interface-compliance assertion is added at the bottom of the file: _ DCRCredentialStore = (*MemoryStorage)(nil).
• DCR credentials are intentionally excluded from the periodic cleanupExpired loop (they are long-lived; the Redis backend applies TTL via SetEX instead of in-process cleanup).

Regenerate mocks

• task gen is rerun; the new mock for DCRCredentialStore appears in pkg/authserver/storage/mocks/mock_storage.go and compiles.

Cross-cutting

• task build, task test, task lint-fix, and task license-check all pass.
• No secrets appear in any log record (grep assertion from Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 applies unchanged: client_secret, registration_access_token, initial_access_token, refresh tokens are never arguments to slog.* calls).
• Code reviewed and approved.

Technical Approach

Recommended Implementation

1. DCRCredentials value type — add to pkg/authserver/storage/types.go adjacent to UpstreamTokens. Mirror the defensive-copy semantics documented for UpstreamTokens. Capture RegistrationAccessToken / RegistrationClientURI verbatim from Phase 2 — they're already captured by resolveDCRCredentials and live on DCRResolution.

2. Consolidate DCRKey — decide in this PR whether DCRKey lives in pkg/authserver/storage/ or pkg/authserver/runner/. Recommended: move it to storage/ and have the runner import it, since the storage backends need to hash keys identically and the runner already depends on storage/ for createStorage. If you move it, rewrite the runner's dcr_store.go to import the storage-level version and delete the local copy.

3. DCRCredentialStore interface — add to pkg/authserver/storage/types.go. Keep the surface minimal: Get + Store, nothing else. Wire it into the mockgen directive at the top of the file.

4. Memory backend — extend MemoryStorage with a dcrCredentials map guarded by the existing sync.RWMutex. Copy-before-mutate on both read and write (see StoreUpstreamTokens at memory.go:693-734 and GetUpstreamTokens at memory.go:739-781 as the reference). DO NOT add the map to cleanupExpired — DCR entries are long-lived; the Redis backend uses TTL for the rare case the provider expires the secret.

5. Mocks — task gen regenerates pkg/authserver/storage/mocks/mock_storage.go. Integration consumers in pkg/authserver/runner/ use the regenerated mock directly — no custom fakes.

6. Verify — 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.

Patterns & Frameworks

• Plain testing + github.com/stretchr/testify per .claude/rules/testing.md — NOT Ginkgo. require.NoError over t.Fatal. Table-driven tests for the memory backend round-trip.
• Copy-before-mutate per .claude/rules/go-style.md §"Copy Before Mutating Caller Input". Apply on both read and write paths, just like the existing StoreUpstreamTokens / GetUpstreamTokens pair in memory.go.
• Write durable first per .claude/rules/go-style.md §"Write to Durable Storage Before Updating In-Memory State". The memory backend has a single in-process map; the rule is trivially satisfied here but the invariant must be preserved if a write-through cache is added later.
• SPDX 2-line header on any newly-created Go file. task license-fix catches omissions.
• Always drive builds/tests/lint through task (task build, task test, task lint-fix, task license-check, task gen). Never go test ./... or golangci-lint run directly.
• Commit-message style: imperative mood, capitalized subject, no trailing period, 50-char limit, no Conventional Commits prefixes (CLAUDE.md).

Code Pointers

• pkg/authserver/storage/types.go:54-81 — UpstreamTokens is the closest precedent for a storage value type with sensitive fields (//nolint:gosec // G117 pattern). Mirror that shape for DCRCredentials.
• pkg/authserver/storage/types.go:19 — //go:generate mockgen directive; add DCRCredentialStore to the -source list (or extend the comma-separated interface list) so task gen produces a mock.
• pkg/authserver/storage/types.go:339-383 — Storage superset. DO NOT add DCR methods to Storage; keep DCRCredentialStore as a parallel, segregated interface (follows the same interface-segregation pattern already used for UpstreamTokenStorage / ClientRegistry / UserStorage).
• pkg/authserver/storage/memory.go:54-105 — MemoryStorage struct + constructor; extend with the dcrCredentials map.
• pkg/authserver/storage/memory.go:693-781 — StoreUpstreamTokens / GetUpstreamTokens are the definitive in-tree reference for defensive copy + ErrNotFound wrapping + ErrExpired semantics. Mirror them for the DCR methods (minus the expiry branch — DCR entries don't track access-token-style expiry in-memory).
• pkg/authserver/storage/memory.go:1174-1181 — compile-time interface assertion block; add _ DCRCredentialStore = (*MemoryStorage)(nil).
• pkg/authserver/runner/dcr_store.go — defined by Authserver DCR integration (Phase 2, Steps 2a-2g) #4978. In this PR, either consolidate into pkg/authserver/storage/ or leave as a thin alias. Don't keep two competing implementations.
• pkg/auth/oauth/dynamic_registration.go:142 — ClientSecretExpiresAt int64 field on the existing DCR response struct; this field will eventually drive the Redis-TTL plumbing (sub-issue 2), so propagate it through DCRCredentials (already allowed by Authserver DCR integration (Phase 2, Steps 2a-2g) #4978's capture of full RFC 7591 fields).

Component Interfaces

// pkg/authserver/storage/types.go (additions)

// DCRCredentials is the persisted form of a DCR registration result.
// All fields are populated from the RFC 7591 DCR response. The RFC 7592
// management fields (RegistrationAccessToken, RegistrationClientURI) are
// preserved so future rotation / management flows can use them.
//
// Callers receive a defensive copy from the store; mutations do not
// affect persisted state.
type DCRCredentials struct {
    // Key is the canonical cache key: (Issuer, RedirectURI, ScopesHash).
    Key DCRKey

    // ProviderName is the upstream's UpstreamRunConfig.Name. Debug / audit
    // only — never used as a primary key. Two upstreams with different
    // ProviderName but identical Key share one credential record.
    ProviderName string

    ClientID                string
    ClientSecret            string //nolint:gosec // G117: field legitimately holds sensitive data
    TokenEndpointAuthMethod string

    // RegistrationAccessToken and RegistrationClientURI are RFC 7592 fields
    // captured for future management operations (rotation, deletion).
    RegistrationAccessToken string //nolint:gosec // G117: field legitimately holds sensitive data
    RegistrationClientURI   string

    AuthorizationEndpoint string
    TokenEndpoint         string

    // CreatedAt supports the cache-hit stale-age log from #4978.
    CreatedAt time.Time
}

// DCRCredentialStore is a narrow, segregated interface for dynamic-client-
// registration credential persistence. Both MemoryStorage and RedisStorage
// implement it; an authserver backed by Redis shares DCR credentials
// across replicas and restarts.
//
// Cross-replica limitation: sharing DCR credentials does NOT imply cross-
// replica session / token delivery. Callers that need that must still route
// through the proxy runner and (if applicable) pin sessions to a replica.
type DCRCredentialStore interface {
    // GetDCRCredentials returns the credentials for the given key.
    // Returns ErrNotFound if no entry exists for the key.
    // The returned value is a defensive copy.
    GetDCRCredentials(ctx context.Context, key DCRKey) (*DCRCredentials, error)

    // StoreDCRCredentials persists the credentials. Implementations MUST
    // honor DCR client_secret_expires_at as a TTL when the source DCR
    // response provided it (propagated through the DCRResolution /
    // DCRCredentials value). When absent or zero, no TTL is applied.
    StoreDCRCredentials(ctx context.Context, creds *DCRCredentials) error
}

Testing Strategy

Unit Tests

• pkg/authserver/storage/memory_test.go (new or extend existing): StoreDCRCredentials + GetDCRCredentials round-trip (all fields preserved); two distinct DCRKey values do not collide; overwrite semantics — a second Store with the same Key replaces the first; Get on an absent key returns ErrNotFound via errors.Is; returned value is a defensive copy (mutating the returned pointer does not affect subsequent Get calls).

Edge Cases

• ErrNotFound plumbed through correctly (errors.Is(err, storage.ErrNotFound) holds).
• Mocks for the new interface compile (task gen then task build) — catches signature-drift between the runner and storage packages.

Out of Scope

• Redis backend — see sub-issue 2.
• Wiring EmbeddedAuthServer to use the storage-level interface and removing the Phase 2 standalone in-memory stub — see sub-issue 3.
• All other items listed in the parent issue's "Out of Scope" section (auto re-register, RFC 7592 rotation, cross-replica session/token delivery, OTEL metrics, CLI persistence, provider-specific quirks, delete API).

References

• Parent issue: Persistent DCRCredentialStore backends (memory + Redis) #4979
• Parent story: Authserver-driven DCR for upstream OAuth 2.1 MCP servers #4976 — "Authserver-driven DCR for upstream OAuth 2.1 MCP servers".
• Upstream dependency: #Authserver DCR integration (Phase 2, Steps 2a-2g) #4978 (Phase 2 defines DCRCredentialStore + DCRKey + DCRResolution and captures RFC 7592 fields required here).
• .claude/rules/go-style.md, .claude/rules/testing.md, .claude/rules/security.md, .claude/rules/pr-creation.md, CLAUDE.md.
• RFC 7591 §3.2.1, RFC 7592.