Skip to content

Commit 94fe0dc

Browse files
committed
Add persistent DCRCredentialStore types and memory backend
Phase 3 sub-issue 1 of #5183. Define the persisted DCRCredentials value type and the storage-level DCRCredentialStore interface in pkg/authserver/storage/, and ship the in-process memory implementation that single-replica deployments and unit tests use. The Redis backend (sub-issue 2) and the wiring change (sub-issue 3) build on this. DCRKey consolidation: chose option (a) from the issue — DCRKey and its ScopesHash constructor move to pkg/authserver/storage/ so any future backend hashes keys identically. The runner package keeps a package-local type alias (type DCRKey = storage.DCRKey) and a var binding for scopesHash so existing call sites compile unchanged; the canonical form has a single source of truth. The runner-side DCRCredentialStore (Get/Put *DCRResolution) is left in place as the thin adapter sub-issue 3 will swap. This sub-issue lands the new storage-level interface, MemoryStorage implementation, and regenerated mock without touching the wire-up. DCR credentials are intentionally excluded from cleanupExpired: RFC 7591 client registrations are long-lived and the authoritative expiry signal is client_secret_expires_at, which the Redis backend will honor as a SetEX TTL.
1 parent 64feb73 commit 94fe0dc

6 files changed

Lines changed: 475 additions & 63 deletions

File tree

pkg/authserver/runner/dcr.go

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ package runner
55

66
import (
77
"context"
8-
"crypto/sha256"
9-
"encoding/hex"
108
"errors"
119
"fmt"
1210
"log/slog"
@@ -15,13 +13,13 @@ import (
1513
"regexp"
1614
"runtime/debug"
1715
"slices"
18-
"sort"
1916
"strings"
2017
"time"
2118

2219
"golang.org/x/sync/singleflight"
2320

2421
"github.com/stacklok/toolhive/pkg/authserver"
22+
"github.com/stacklok/toolhive/pkg/authserver/storage"
2523
"github.com/stacklok/toolhive/pkg/authserver/upstream"
2624
"github.com/stacklok/toolhive/pkg/networking"
2725
"github.com/stacklok/toolhive/pkg/oauthproto"
@@ -229,34 +227,12 @@ func applyResolutionToOAuth2Config(cfg *upstream.OAuth2Config, res *DCRResolutio
229227
cfg.ClientSecret = res.ClientSecret
230228
}
231229

232-
// scopesHash returns the SHA-256 hex digest of the canonical scope set.
233-
//
234-
// Canonicalisation:
235-
// 1. Sort ascending so the digest is order-insensitive — e.g.
236-
// []string{"openid", "profile"} and []string{"profile", "openid"} hash to
237-
// the same value.
238-
// 2. Deduplicate so that []string{"openid"} and []string{"openid", "openid"}
239-
// hash to the same value. An OAuth scope set is a set, not a multiset
240-
// (RFC 6749 §3.3), and without deduplication a caller that accidentally
241-
// duplicated a scope would miss cache entries and trigger redundant
242-
// RFC 7591 registrations.
243-
// 3. Join with newlines (a character not valid in OAuth scope tokens per
244-
// RFC 6749 §3.3) to avoid collision between e.g. ["ab", "c"] and
245-
// ["a", "bc"].
246-
func scopesHash(scopes []string) string {
247-
sorted := slices.Clone(scopes)
248-
sort.Strings(sorted)
249-
sorted = slices.Compact(sorted)
250-
251-
h := sha256.New()
252-
for i, s := range sorted {
253-
if i > 0 {
254-
_, _ = h.Write([]byte("\n"))
255-
}
256-
_, _ = h.Write([]byte(s))
257-
}
258-
return hex.EncodeToString(h.Sum(nil))
259-
}
230+
// scopesHash is a runner-package shorthand for storage.ScopesHash, kept so the
231+
// resolver and its tests can reference the canonical hash function without an
232+
// explicit storage. qualifier on every call site. The canonical implementation
233+
// lives in the storage package next to DCRKey so any future backend hashes
234+
// keys identically.
235+
var scopesHash = storage.ScopesHash
260236

261237
// Step identifiers for structured error logs emitted by the caller of
262238
// resolveDCRCredentials. These values flow through the "step" attribute so

pkg/authserver/runner/dcr_store.go

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,43 +8,23 @@ import (
88
"fmt"
99
"sync"
1010
"time"
11+
12+
"github.com/stacklok/toolhive/pkg/authserver/storage"
1113
)
1214

1315
// dcrStaleAgeThreshold is the age beyond which a cached DCR resolution is
1416
// considered stale and logged as such by higher-level wiring. The store itself
1517
// does not expire or evict entries — RFC 7591 client registrations are
16-
// long-lived and are only purged by explicit RFC 7592 deregistration. This
17-
// threshold is consumed by Step 2g observability logs introduced in the next
18-
// PR in the DCR stack (sub-issue C, #5039); 5042 only defines the constant
19-
// so the consumer can land without a cross-PR cycle.
20-
//
21-
//nolint:unused // consumed by lookupCachedResolution in #5039
18+
// long-lived and are only purged by explicit RFC 7592 deregistration.
2219
const dcrStaleAgeThreshold = 90 * 24 * time.Hour
2320

24-
// DCRKey is the canonical lookup key for a DCR resolution. The tuple is
25-
// designed so a future Redis-backed store can serialise it into a single key
26-
// segment (Phase 3) without redefining the canonical form. ScopesHash rather
27-
// than the raw scope slice is used so the key is comparable and order-
28-
// insensitive.
29-
type DCRKey struct {
30-
// Issuer is *this* auth server's issuer identifier — the local issuer
31-
// of the embedded authorization server that performed the registration,
32-
// NOT the upstream's. The cache is keyed by this value because two
33-
// different local issuers registering against the same upstream are
34-
// distinct OAuth clients and must not share credentials. The upstream's
35-
// issuer is used only for RFC 8414 §3.3 metadata verification inside
36-
// the resolver and is not part of the cache key.
37-
Issuer string
38-
39-
// RedirectURI is the redirect URI registered with the upstream
40-
// authorization server. Lives on the local issuer's origin since it is
41-
// where the upstream sends the user back to us after authentication.
42-
RedirectURI string
43-
44-
// ScopesHash is the SHA-256 hex digest of the sorted scope list.
45-
// See scopesHash in dcr.go for the canonical form.
46-
ScopesHash string
47-
}
21+
// DCRKey is a re-export of storage.DCRKey, kept as a package-local alias so
22+
// existing runner-side callers continue to compile against runner.DCRKey
23+
// while the canonical definition lives in pkg/authserver/storage. The
24+
// canonical form (and its ScopesHash constructor) MUST live in a single place
25+
// so any future Redis backend hashes keys identically to the in-memory
26+
// backend; see storage.DCRKey for the field documentation.
27+
type DCRKey = storage.DCRKey
4828

4929
// DCRCredentialStore caches RFC 7591 Dynamic Client Registration resolutions
5030
// keyed by the (Issuer, RedirectURI, ScopesHash) tuple. Implementations must

pkg/authserver/storage/memory.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ type MemoryStorage struct {
9494
// This enables O(1) lookup during authentication callbacks.
9595
providerIdentities map[string]*ProviderIdentity
9696

97+
// dcrCredentials maps DCRKey -> DCRCredentials for RFC 7591 Dynamic Client
98+
// Registration credentials. Entries are intentionally excluded from the
99+
// periodic cleanupExpired loop: DCR registrations are long-lived and the
100+
// authoritative expiry signal is RFC 7591 client_secret_expires_at, which
101+
// is honored at read time by callers (and by the future Redis backend's
102+
// SetEX TTL). The map is bounded by the operator-configured upstream
103+
// count, so unbounded growth is not a concern.
104+
dcrCredentials map[DCRKey]*DCRCredentials
105+
97106
// cleanupInterval is how often the background cleanup runs
98107
cleanupInterval time.Duration
99108

@@ -129,6 +138,7 @@ func NewMemoryStorage(opts ...MemoryStorageOption) *MemoryStorage {
129138
clientAssertionJWTs: make(map[string]time.Time),
130139
users: make(map[string]*User),
131140
providerIdentities: make(map[string]*ProviderIdentity),
141+
dcrCredentials: make(map[DCRKey]*DCRCredentials),
132142
cleanupInterval: DefaultCleanupInterval,
133143
stopCleanup: make(chan struct{}),
134144
cleanupDone: make(chan struct{}),
@@ -1187,6 +1197,60 @@ func (s *MemoryStorage) GetUserProviderIdentities(_ context.Context, userID stri
11871197
return identities, nil
11881198
}
11891199

1200+
// -----------------------
1201+
// DCR Credentials Storage
1202+
// -----------------------
1203+
1204+
// cloneDCRCredentials returns a field-by-field copy of c, or nil if c is nil.
1205+
// All fields are values (no slices, maps, or pointers), so a shallow copy is
1206+
// sufficient — adding a new reference-typed field requires updating this
1207+
// helper to deep-copy that field.
1208+
func cloneDCRCredentials(c *DCRCredentials) *DCRCredentials {
1209+
if c == nil {
1210+
return nil
1211+
}
1212+
cp := *c
1213+
return &cp
1214+
}
1215+
1216+
// StoreDCRCredentials persists DCR credentials for the given key.
1217+
// The credentials are stored under their own Key field; callers must populate
1218+
// it before calling. A defensive copy is made so subsequent caller mutations
1219+
// do not affect persisted state.
1220+
//
1221+
// Overwrites any existing entry for the same Key. The in-memory backend
1222+
// applies no TTL — DCR registrations are long-lived and bounded by the
1223+
// operator-configured upstream count.
1224+
func (s *MemoryStorage) StoreDCRCredentials(_ context.Context, creds *DCRCredentials) error {
1225+
if creds == nil {
1226+
return fosite.ErrInvalidRequest.WithHint("dcr credentials cannot be nil")
1227+
}
1228+
1229+
s.mu.Lock()
1230+
defer s.mu.Unlock()
1231+
1232+
s.dcrCredentials[creds.Key] = cloneDCRCredentials(creds)
1233+
return nil
1234+
}
1235+
1236+
// GetDCRCredentials retrieves DCR credentials by key.
1237+
// Returns a defensive copy; returns ErrNotFound (wrapped) on miss.
1238+
func (s *MemoryStorage) GetDCRCredentials(_ context.Context, key DCRKey) (*DCRCredentials, error) {
1239+
s.mu.RLock()
1240+
defer s.mu.RUnlock()
1241+
1242+
entry, ok := s.dcrCredentials[key]
1243+
if !ok {
1244+
slog.Debug("dcr credentials not found",
1245+
"issuer", key.Issuer,
1246+
"redirect_uri", key.RedirectURI,
1247+
)
1248+
return nil, fmt.Errorf("%w: %w", ErrNotFound, fosite.ErrNotFound.WithHint("DCR credentials not found"))
1249+
}
1250+
1251+
return cloneDCRCredentials(entry), nil
1252+
}
1253+
11901254
// -----------------------
11911255
// Metrics/Stats (for testing and monitoring)
11921256
// -----------------------
@@ -1234,4 +1298,5 @@ var (
12341298
_ ClientRegistry = (*MemoryStorage)(nil)
12351299
_ UpstreamTokenStorage = (*MemoryStorage)(nil)
12361300
_ UserStorage = (*MemoryStorage)(nil)
1301+
_ DCRCredentialStore = (*MemoryStorage)(nil)
12371302
)

0 commit comments

Comments
 (0)