fix(gateway): coalesce concurrent native OAuth refresh requests

[Doud-FR]

Summary

• Coalesce overlapping native OAuth refresh requests that present the same old rotating refresh token.
• Reuse the first successfully rotated session for concurrent requests and short-lived retries.
• Add a short negative cache to absorb retry storms for definitively rejected refresh tokens.
• Run synchronous identity-provider refresh operations outside the ASGI event loop.
• Bound and garbage-collect the in-process replay cache and per-token locks.
• Add regression tests for sequential retries, concurrent refreshes, rejected-token storms, and lock lifecycle races.

Root cause

Hermes Desktop can issue multiple overlapping requests to /auth/native/refresh while each caller still holds the same old refresh token.

With rotating refresh tokens:

1. The first request successfully rotates the token.
2. Concurrent requests replay the now-consumed old token.
3. The identity provider rejects those requests.
4. The desktop retries and can create a refresh storm, eventually receiving HTTP 429 responses.

Fix

The gateway now derives a SHA-256 cache key from the client IP and old refresh token, without storing the raw token.

Requests sharing that key use a per-token single-flight lock:

• one request reaches the identity provider;
• concurrent requests reuse the resulting rotated Session;
• successful results are cached for 30 seconds;
• definitive rejections are cached for 5 seconds;
• transient provider outages are not cached;
• cache entries are limited to 256 and garbage-collected;
• active and waiting lock references are counted to prevent a second lock from being created for the same token.

Provider refresh calls are synchronous and may perform network I/O, so they are executed with run_in_threadpool() instead of blocking the ASGI event loop.

Validation

Native OAuth flow

Command:

uv run python -m pytest -q tests/hermes_cli/test_dashboard_auth_native_flow.py

Result: 25 passed, 5 warnings

Related authentication suites

Command:

uv run python -m pytest -q tests/hermes_cli/test_dashboard_auth_401_reauth.py tests/hermes_cli/test_dashboard_auth_middleware.py tests/hermes_cli/test_dashboard_auth_native_flow.py

Result: 109 passed, 5 warnings

Lint

Command:

uv run ruff check hermes_cli/dashboard_auth/routes.py tests/hermes_cli/test_dashboard_auth_native_flow.py

Result: All checks passed

Pre-existing test-order issue

Running test_dashboard_auth_gate.py before test_dashboard_auth_password_login.py produces three password-login failures.

The exact same failures were reproduced in a clean detached worktree at origin/main, so they are pre-existing and unrelated to this change.

[Copilot]

Pull request overview

This PR hardens the dashboard gateway’s native OAuth refresh endpoint against concurrent refresh-token rotation storms by introducing a per-token “single-flight” refresh path with a short-lived replay cache and running synchronous provider refreshes off the ASGI event loop.

Changes:

• Added an in-process replay cache + per-token locks to coalesce overlapping /auth/native/refresh calls that present the same old rotating refresh token.
• Executed provider refresh operations in a threadpool to avoid blocking the ASGI event loop.
• Added regression tests covering sequential retries, concurrent refresh coalescing, negative-cache behavior for definitively rejected tokens, and lock lifecycle races.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
hermes_cli/dashboard_auth/routes.py	Implements single-flight refresh coalescing + bounded replay/negative caching and moves provider refresh work into a threadpool.
tests/hermes_cli/test_dashboard_auth_native_flow.py	Adds test coverage for refresh coalescing, retries, negative caching, and lock lifecycle behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[teknium1]

Thanks for addressing a real native-refresh race. Current main still calls provider.refresh_session() independently in hermes_cli/dashboard_auth/routes.py:915-932, so the single-flight direction is needed.

Problems

• hermes_cli/dashboard_auth/routes.py:134 derives the replay key without provider_hint, while the existing refresh contract makes that hint order providers (hermes_cli/dashboard_auth/middleware.py:547-565; see tests/hermes_cli/test_dashboard_auth_401_reauth.py:211-224). A cache hit can therefore return a session chosen under a different provider ordering. Include the hint in the key, or preserve equivalent ordering before returning cached results, and cover two providers sharing an opaque token value.
• hermes_cli/dashboard_auth/routes.py:76 and :128 use Tuple, but the import remains from typing import Any, Deque, Dict. Please import Tuple or use built-in tuple[...] annotations.

This is an automated hermes-sweeper review.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

hermes_cli/dashboard_auth/routes.py:140

• The _refresh_native_session_sync docstring doesn’t match the actual return semantics: the function can return (None, None) for a definitive rejection (including negative-cache hits), and (None, <provider>) only when at least one provider was unreachable. Clarifying this helps callers/tests interpret unreachable correctly.

    """Single-flight native refresh for one old rotating refresh token.

    Returns ``(session, None)`` on success/cache hit, or
    ``(None, unavailable_provider)`` when no provider refreshed it.
    """

[Doud-FR]

Follow-up: I also addressed Copilot's suppressed low-confidence observation in f8b2079c7.

The _refresh_native_session_sync docstring now documents all three return outcomes accurately:

• (session, None) for a successful refresh or positive cache hit;
• (None, None) when every provider definitively rejects the token, including a negative-cache hit;
• (None, provider_name) when no provider succeeds and at least one provider is unreachable.

This is documentation-only and does not change runtime behavior.

Validation:

• Ruff check: passed
• git diff --check: passed

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/hermes_cli/test_dashboard_auth_native_flow.py:695

• This test still uses a fixed sleep to "give time" before asserting refresh_calls == 2. The assertion is already deterministic here because second_entered guarantees the second provider call has started and release_second is not set yet, so the third call cannot advance. Removing the sleep avoids unnecessary timing sensitivity in CI.

            third = pool.submit(auth_routes._refresh_native_session_sync, *args)
            assert wait_for_lock_users(2) is original_lock

            time.sleep(0.05)
            assert provider.refresh_calls == 2

hermes_cli/dashboard_auth/routes.py:145

• provider_hint is incorporated into the replay-cache key without validation. Since provider comes from the request body, a buggy/malicious client can bypass coalescing/negative-caching by varying an unknown provider value while reusing the same old refresh token (undermining the retry-storm mitigation). Normalize provider_hint to "" unless it matches a registered session provider before deriving cache_key.

    cache_key = _native_refresh_cache_key(refresh_token, provider_hint, client_ip)

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(gateway): coalesce concurrent native OAuth refresh requests

1. _native_refresh_client_ip is byte-for-byte identical to the existing _client_ip (hermes_cli/dashboard_auth/routes.py:108) — both return request.client.host if request.client else "". Pure duplication; reuse _client_ip and drop the new helper.
2. Single-flight correctness is solid (user-counted per-token locks, waiter keeps the lock registered, deterministic barrier tests). One assumption worth documenting: the replay cache and lock table are in-process, so coalescing only holds within a single ASGI worker — if the dashboard is ever run with more than one worker, concurrent refreshes across workers can still replay a consumed refresh token at the IdP.
3. Unknown provider_hint canonicalization is good for the cache key, but a hint that is a display name or alias (rather than an exact provider name) is silently canonicalized away — it no longer fragments the cache, but it also silently loses the intended provider-ordering preference. Consider a debug log when a non-empty hint is canonicalized to "" so a typo'd provider name is visible.
4. The negative-cache path is correctly limited to definitive rejections only (a transient ProviderError is never cached), which keeps real outages immediately retryable. Nice detail: the success TTL (30s) vs failure TTL (5s) split is well reasoned.

[oliver-mee]

Field confirmation for this one, in case it helps it get looked at: I hit exactly the failure your root-cause section describes, on a self-hosted Authelia 4.39.20 with rotating refresh tokens and reuse detection.

After a wake, four requests left for /auth/native/refresh inside the same second, all carrying the same old refresh token. The provider revoked the whole chain rather than just rejecting the replays, so every refresh after that returned "The refresh token has not been found" and the desktop dropped to a sign-in prompt. The 429s you mention showed up too, as Rate Limit Exceeded on the provider's token endpoint. Full evidence, including the provider's refresh-token table showing the newest two rows as revoked=1, is in #55712.

Two things you may want to link, since I could not find either reference in the PR:

• Remote dashboard session expires due to rotating refresh-token replay #55712 is the pre-existing issue for this bug, filed on 30 June. It reported the cookie path rather than the native one, but it is the same replay race, and a Fixes #55712 here would give this PR a filed bug behind it.
• The triage comment on that issue notes that fix(dashboard-auth): coalesce concurrent refresh requests to prevent RT reuse detection #55717 coalesces refreshes but leaves the synchronous provider refresh on the ASGI event loop. Your run_in_threadpool() change looks like it closes that gap, which is worth calling out explicitly, because from the outside the two PRs read as duplicates when they actually cover different paths (middleware.py cookie versus routes.py native).

I can reproduce this on demand by letting the machine sleep past the access-token lifetime, so if it would help to have this branch tested against a rotating-RT provider with reuse detection, I am happy to run it and report back.

[Enough1122]

AI code review — automated follow-up, for reference.

Appreciate the field confirmation on Authelia 4.39.20 — a real rotating-refresh-token + reuse-detection reproduction is exactly the evidence the coalescing fix targets. No comments from the review side.