Skip to content

Deduplicate refreshes by storage row - #6361

Merged
jhrozek merged 1 commit into
stacklok:mainfrom
jhrozek:6356-singleflight-storage
Aug 19, 2026
Merged

jhrozek merged 1 commit into
stacklok:mainfrom
jhrozek:6356-singleflight-storage

Conversation

@jhrozek

@jhrozek jhrozek commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Rotating refresh tokens can be redeemed twice when callers resolve to the same storage row but use different session IDs; this can trigger IdP replay detection and revoke the credential family.
  • Key in-process refresh coordination on an opaque storage-row identity, re-read the authoritative row within the flight, and return isolated token copies to each caller.
  • Add regression coverage for aliased rows, late stale callers, independent rows, resolver failures, and opaque identity behavior.

Fixes #6356

Type of change

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

Test plan

  • Unit tests (task test) — run; blocked by the unrelated Docker predefined-address-pool exhaustion in pkg/container/docker/TestCreateNetwork_ConcurrentSameName.
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Changes

File Change
pkg/authserver/storage/types.go Add opaque storage-row identity resolution contract.
pkg/authserver/storage/{memory,redis}.go Derive hashed identities from each backend’s row representation.
pkg/authserver/refresher.go Coordinate by row, re-read authoritatively, and isolate returned results.
pkg/authserver/refresher_test.go Cover concurrent, stale, failure, and result-isolation cases.
pkg/authserver/storage/types_test.go Cover backend row-ID behavior.
docs/arch/11-auth-server-storage.md Document the process-local scope and #4122 boundary.

Does this introduce a user-facing change?

No

Implementation plan

Approved implementation plan
  1. Add a non-empty opaque UpstreamTokenRowID and resolver to UpstreamTokenStorage; storage defines which logical lookups share a row.
  2. Implement a collision-resistant hashed row identity in Memory and Redis without exposing row keys.
  3. Resolve the row ID before singleflight, then re-read the authoritative row inside the bounded flight; return a fresh row directly or refresh the re-read expired snapshot.
  4. Preserve the existing leader session/provider write and rotated-token cleanup path under the storage consistency contract.
  5. Test aliased rows, late stale callers, distinct rows, resolver failures, and isolated result pointers.
  6. Regenerate mocks and run lint, tests, and license checks.

Special notes for reviewers

  • This prevents duplicate refresh-token redemption for one storage row within one process. Cross-replica coordination and CAS-style protection remain out of scope and are tracked by Handle multi-pod refresh token rotation race condition #4122.
  • task test reached the changed auth/storage packages successfully but failed in the Docker network test because the local Docker daemon had exhausted its predefined address pools.

@codecov

codecov Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.04%. Comparing base (72ffb45) to head (7643795).

Files with missing lines Patch % Lines
pkg/authserver/refresher.go 92.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6361      +/-   ##
==========================================
+ Coverage   72.99%   73.04%   +0.04%     
==========================================
  Files         744      744              
  Lines       78611    78648      +37     
==========================================
+ Hits        57385    57449      +64     
+ Misses      17226    17178      -48     
- Partials     4000     4021      +21     

☔ View full report in Codecov by Harness.
📢 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.

@jhrozek
jhrozek force-pushed the 6356-singleflight-storage branch from 5993515 to 67cffe7 Compare August 19, 2026 08:05
Fixes stacklok#6356.

Use an opaque storage-defined row identity as the in-process singleflight key instead of the caller session ID. Re-read the authoritative row inside the flight so late callers do not redeem a stale rotated refresh token, and return independent token copies to each caller.

Memory and Redis derive opaque hashed row identities. Tests cover aliased rows, late stale callers, independent rows, resolver failures, and result isolation. This remains process-local; distributed coordination is tracked by stacklok#4122.
@jhrozek
jhrozek force-pushed the 6356-singleflight-storage branch from 67cffe7 to 7643795 Compare August 19, 2026 08:14
@jhrozek
jhrozek merged commit af68771 into stacklok:main Aug 19, 2026
43 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Singleflight key for upstream refresh must match the storage row, not the session

2 participants