Repository navigation
authserver/storage: DCRCredentialStore has no way to update an existing record #6673
Description
Problem
DCRCredentialStore (pkg/authserver/storage/types.go) exposes exactly two operations:
type DCRCredentialStore interface {
GetDCRCredentials(ctx context.Context, key DCRKey) (*DCRCredentials, error)
StoreDCRCredentialsIfAbsent(ctx context.Context, creds *DCRCredentials) (*DCRCredentials, error)
}StoreDCRCredentialsIfAbsent is create-only by design: it claims creds.Key and returns the concurrent winner's value if the key is already taken. There is no way to rewrite an existing record in place.
This is a real gap for a storage decorator that needs to rewrite a DCR credential's persisted representation without changing its RFC 7591 identity or values — for example one that transparently transforms stored bytes at rest (re-encoding, compression, a checksum, or similar). Such a decorator can Get a record fine, but has no supported write path to persist a modified copy back: calling StoreDCRCredentialsIfAbsent on an existing key is a no-op that silently discards the caller's changes and hands back the old value instead.
The doc comment on DCRCredentialStore already anticipates callers reaching past the interface for this kind of thing ("a Redis SCAN, an admin-tool dump, or a cross-replica reconciliation path can identify a record's logical cache slot ... from the value alone"), but enumeration is not the same problem as rewriting — a caller that finds a record still has nothing to call to persist a changed copy of it.
Proposed change
Add a CAS-style update, symmetric with the existing IfAbsent naming:
// UpdateDCRCredentialsIfPresent replaces the record at creds.Key with creds,
// iff a record currently exists at that key. Returns ErrNotFound (wrapped) if
// no entry exists. Implementations MUST defensively copy on input, mirroring
// the Store/Get contract, and MUST honor the same ClientSecretExpiresAt TTL
// handling documented on StoreDCRCredentialsIfAbsent.
UpdateDCRCredentialsIfPresent(ctx context.Context, creds *DCRCredentials) (*DCRCredentials, error)Design notes:
- Presence-gated, not blind overwrite — an update racing a delete (if one is ever added) or a not-yet-created record should fail rather than silently create, keeping the interface's existing create/update split explicit (mirrors why
StoreDCRCredentialsIfAbsentdoesn't silently update). - Same defensive-copy and TTL contract as the existing methods, so backends that already implement
DCRCredentialStore(in-memory, Redis) get a small, consistent addition rather than a new set of rules. - Both shipped backends need it:
MemoryStorage(trivial map overwrite) andRedisStorage(aSETthat preserves the TTL-derivation contract already used forStoreDCRCredentialsIfAbsent). - Not proposing any change to
StoreDCRCredentialsIfAbsent's create semantics — this is purely additive.
Alternatives considered
- Caller reconstructs and re-derives the backend's own encoding/TTL logic via a raw store-specific write, bypassing the interface. Rejected: it duplicates backend-internal marshaling logic outside the package and drifts silently the next time that logic changes.
- Leave it to
StoreDCRCredentialsIfAbsentwith a delete-then-create. Rejected: not atomic, and momentarily removes a record other readers may be relying on.
Activity
authserver/storage: add UpdateDCRCredentialsIfPresent (#6674)