Skip to content

Auth: storage/read timeouts surface as 401 Unauthorized #637

Closed

Description

@rbuergi

During silo degradation, ApiToken validation reads timed out (8s) and every authenticated call answered 401 — indistinguishable from an invalid token. Cost hours of misdiagnosis + a forced re-auth. 'Cannot validate (retry/503)' must be distinct from 'invalid (401)'.

From the 2026-07-24 plugin-install retrospective.

🤖 Generated with Claude Code

Activity

  1. rbuergi commented on Aug 8, 2026

    @rbuergi
    ContributorAuthor

    Status audit against current main (2026-08-08) — still OPEN for the client-visible half; the server-side half improved.

    What changed since filing (mitigation, not the asked fix): validation moved off the cross-silo mesh read onto a direct IStorageAdapter.Read with absence as a terminal negative (ApiTokenService.cs:342-374), and the handler logs hash prefix + elapsed ms with an explicit 'never a silent 401' note (ApiTokenAuthenticationHandler.cs:45-53) — an operator can now tell timeout from invalid in the logs, and the 8s window is much less likely.

    What remains (the issue's actual ask): a read TIMEOUT still collapses into the same null as 'no such token' (ApiTokenService.cs:397-406) and answers 401 'Invalid or expired API token' (ApiTokenAuthenticationHandler.cs:43-55) — identical to a forged token. Same shape on UserContextMiddleware.cs:315-325, SignalRConnectionRegistry.cs:99, GrpcConnectionRegistry.cs:71/215: infrastructure faults degrade to Anonymous. No 503/Retry-After path exists anywhere in auth, and no test asserts a retryable outcome. Clients (and their retry logic) still cannot distinguish 'cannot validate right now' from 'invalid'.

  2. rbuergi commented on Aug 8, 2026

    @rbuergi
    ContributorAuthor

    Closing — both legs of this issue are now fixed, and the remainder is tracked separately as #974 rather than left implicit here.

    The API-token leg was fixed by #947: tri-state TokenValidationStatus at the ApiTokenService seam, a 503 + Retry-After via HandleChallengeAsync, and the matching changes in UserContextMiddleware's bearer branch, the SignalR and gRPC registries, ApiTokenNodeType and ValidateTokenResponse.IsUnavailable.

    The identity/authorization leg was fixed by #970, and it was the worse half — it is what a browser user hits:

    • OnboardingMiddleware.FindUserByEmail fell back to null on a storage timeout, and null means "not onboarded" — so a stall redirected a signed-in user to the sign-up form. The old log said it outright: "falling back to null (will redirect to /onboarding)".
    • LoadUserRoles defaulted to an empty role set on timeout/fault — silent privilege loss surfacing as "Access denied" everywhere.
    • ApiTokenAuthenticationHandler had a bare catch { } doing the same to API callers.

    The fix decides IdentityReadOutcome<T> (Resolved vs Unavailable) inside IdentityRead.Bounded — the one place a budget is applied — so the distinction is made where the condition is known, never reconstructed upstream by sniffing a message string. No widened timeout, no retry, no catch-and-default: the 20 s budget is unchanged, only what it reports changed.

    Two things worth recording for whoever touches this next:

    Removing a catch moves the failure, it doesn't remove it. After deleting the swallow, a sweep found a third instance of the same defect inside the fix itself — LoadDbRolesAsync resolved the hub and workspace outside the classified chain, so a hub mid-disposal throwing from GetWorkspace() would have escaped as a 500. It is now inside an Observable.Defer with explicit classification. Sweep after removing a catch; don't just patch the reported instance.

    The tests were verified to have teeth, not assumed to. Reverting Bounded's timeout/fault legs to the old Resolved(null) turns the two classification tests red; restoring the handler's swallow turns the 503-challenge test red. The remaining tests stay green, correctly — they pin the definitive legs and the happy path, which those reverts don't touch.

    Still outstanding, now tracked in #974: three further sites collapse an availability failure into a definitive negative — AccessControlPipeline (permission-fold fault → "Access denied"), MeshOperations.FetchNode (read timeout → "Not found") and UserIdentityCache (cold cache → "user unknown"). Each needs a tri-state carried through the permission fold or a new ErrorType on the bus, which is why they were deliberately left out of #970 rather than half-fixed.

  3. added a commit that references this issue on Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions