Repository navigation
fix: fix Web Console hanging when SSO token refresh fails - #603
Open
glasstiger wants to merge 46 commits into
Open
glasstiger wants to merge 46 commits into
glasstiger wants to merge 46 commits into
Conversation
When an OIDC token refresh failed at the transport level (token endpoint unreachable, or a non-JSON response body), refreshTokenMethod() rejected inside the setInterval callback of refreshAuthToken(), before the static refreshTokenPending flag was reset. The flag was left stuck true, the outer promise never settled, and every subsequent query deadlocked waiting on it — the console silently froze with no 401 and no logout. Wrap the refresh in try/catch/finally so refreshTokenPending is always reset. On failure the stale token is kept in place and the next request receives a 401, which drives the existing re-auth flow — matching the behaviour when there is no refresh token at all. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
glasstiger
added
bug
# Conflicts: # src/utils/questdb/client.test.ts
Web Console deploy preview
|
glasstiger
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The OIDC refresh-failure E2E test reuses the fake token fixtures used throughout oidc.spec.js. Ignore the findings in 0f4d4ac and mark the lines with gitleaks:allow so the squash commit on main is not flagged either.
glasstiger
changed the title
The legacy grid mounts only after the first result arrives, so asserting .qg-viewport right after reloading with useNewGrid=0 always timed out. Assert it after the first query instead. Also release the old query's page before the new result loads that page. The previous scroll away and back could refetch the page, hiding the stale write, so the test only sometimes failed without the generation guard.
When the server reads the user's groups from the ID token (acl.oidc.groups.encoded.in.token), the console authenticates with the ID token, yet it timed the refresh by the token response's expires_in, which describes the access token. Time it by the ID token's exp claim instead, and fall back to expires_in only when the claim is unreadable. If the identity provider did not return the ID token the server expects, at sign-in or on refresh, the console sent "Bearer undefined", and the resulting 401s ended the session with no explanation. Show a "Missing ID token" error instead, and never install a missing bearer. The OAuth callback params stayed in the URL after a failed sign-in, so a reload resubmitted the used code, and its invalid_grant error replaced the original one. Restore the URL whatever the outcome, which also keeps a shared query link across a failed sign-in. A token endpoint that could not be reached, returned a response that is not JSON, or did not respond left the sign-in on a blank screen. Show an error instead, and give up after 10 seconds, the same timeout as the refresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A token response may omit expires_in, which OAuth only recommends. An ID token bearer expires at its exp claim regardless, so it no longer needs one. Without any known expiry, sign in anyway, but never refresh the token, proactively or after a 401: such a provider typically issues no refresh tokens either, so the token is used until the server rejects it, and the user then signs in again. A catalog request's 10s deadline kept running while the client waited for the token refresh that follows a 401, so Query Activity and table details could become unavailable even though the refresh succeeded. The client now reports every refresh wait, and the catalog deadline gives the refresh its own bounded time before restarting the SQL timeout. Modal dialogs hide the rest of the page from assistive technology, except live regions, which hid the SSO refresh warning from screen readers. Mount the notifications in a live region, as later react-toastify versions do. The refresh warning promised a retry, which does not happen while a timed-out refresh request is still pending. State only the consequence. The enterprise reload test returned expires_in: 0, which is not a valid lifetime. Use 300 seconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Settle execution-bound loading notifications and make cancellation idempotent. Stop scripts safely across authentication teardown, preserving cleanup and suppressing stale results. Add regressions for detached session isolation, script auth loss, catalog switching during shared refresh, and telemetry auth-gate ordering and refreshed SSO authorization.
glasstiger
added a commit
to questdb/documentation
that referenced
this pull request
Oct 14, 2026
… renewal and Java token providers Align the OIDC, RBAC and client pages with questdb/questdb-enterprise#1267, questdb/questdb#7799, questdb/ui#603 and questdb/java-questdb-client#102. - Issuer validation (QuestDB Enterprise 4.0.2): acl.oidc.issuer and acl.oidc.issuer.validation.enabled, a new "The expected issuer" section with where the issuer comes from, the issuers of Entra ID v1.0 and v2.0 tokens, the common/organizations templates, issuers that depend on the host name of the request, the opt-out and its advisory, the InvalidIssuer and MissingRequiredClaim("iss") log lines, and the upgrade notes. acl.oidc.host needs acl.oidc.issuer unless validation is disabled. PGWire tokens need the expected issuer too. - HTTP sessions: a request with a credential that QuestDB rejects, or cannot check because the provider is unreachable, gets 401 and ends its session. Sessions end with the password or REST token that opened them, or when the user is disabled or dropped, and on a replica when they are next used. acl.oidc.cache.ttl reuses only the check of a password that QuestDB verifies itself, even with OIDC disabled. - Provider outages: with the User Info endpoint, Web Console users are logged out once acl.oidc.cache.ttl has passed; with tokens validated locally, once the ID token expires; ROPC logins need the token endpoint in both modes. - Web Console: it renews the ID token by its exp claim, keeps its token while it cannot reach the provider, and without a refresh token logs the user out when the token of the login expires. Remove the stale "logged out about an hour after the login" text from the OIDC page and the Entra ID guide; Entra ID limits the refresh tokens of single-page applications to 24 hours. Its password login sends a REST token that survives a password change, so drop the user's REST tokens to log the user out. - Java client: "Rotating tokens" with token_provider=azure, the questdb-client-azure artifact, RefreshingTokenProvider and how token failures are handled, "Connection health", the token provider keys in the connect string reference, and a Java example in the Entra ID guide. - With the User Info endpoint, QuestDB treats the access token as opaque. - Changelog entries for the above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fence headless commits and refresh-engine result persistence against ended authentication sessions, and tear down refresh work during layout cleanup. Skip loading notification settlement when an execution never published loading, and add regression coverage for session races and fast queries.
Keep schema compatibility attempts within their originating auth session. Commit headless notebook metadata and snapshots together, rolling back session-invalidated writes and publishing search updates only after commit. Keep loading notifications out of retained history and settle them through a query-ID index that follows rekeys and namespace moves. Add regression coverage for all three review findings. Validation: typecheck, lint, 2813 unit tests across 145 files, and production build passed.
Contributor
Author
Code Review — APPROVEDReview outcome: APPROVED. This level-3 PR review found no confirmed introduced defects or blocking findings. The validation limits below remain applicable. IssuesNo confirmed introduced defects found in PR #603 at The level-3 review covered all 13 review domains, including the independent adversarial pass, plus a wider follow-up on unchanged callers. No broken out-of-diff consumer was established. False-positivesNo draft findings were submitted, so none required dismissal. The supplemental caller check classified unfenced raw-client requests and store-owned telemetry continuations as preexisting behavior, not regressions introduced by this PR. SummaryReview verdict: APPROVED, based on source review and passing local quality gates.
Validation limits:
The PR combines authentication, notebook persistence, notification cleanup, grid pagination, and test-fixture changes. No concrete regression was established, but the passing local gates do not replace the outstanding integration checks. Findings: 0 confirmed, 0 draft findings dropped; 0 in-diff / 0 out-of-diff. No source files were changed during the review. |
Contributor
Author
Level-3 review — ApprovedReviewed PR #603 at Review notesNo introduced functional defects were confirmed.
False-positivesBoth candidates describe real existing behavior, but neither was introduced by this PR.
These remain separate follow-up issues. Their dismissal here concerns regression attribution, not safety. Seven targeted probes—including baseline-Client comparisons—corroborated the findings. SummaryOutcome: Approved, with the non-blocking PR-description correction above. The review completed all 13 domain passes, per-finding verification, and a wider follow-up on unchanged callers. No new functional regression was established across authentication, query execution, notebook persistence, notifications, catalog deadlines, or grid pagination.
Public CI and context-path tests passed for the reviewed SHA. Enterprise E2E dispatch was skipped; live IdP/EE behavior and screen-reader announcements remain unverified. Local Cypress suites were not run, and the server-fixture range was not independently audited as server code. Verification totals: 3 candidates checked; 1 minor metadata issue confirmed, 2 dismissed as preexisting. Location split: 1 in-diff/gitlink metadata finding, 0 introduced out-of-diff defects. No source files were changed during the review. |
Address the PR review findings across token renewal, script outcomes, notebook persistence, session teardown, accessibility, and auth types. Add regression coverage and document the complete PR scope.\n\nValidated with typecheck, lint, production build, 2,838 unit tests, and 38 stubbed auth browser tests.
Preserve never-started notebook run outcomes, reject stale headless results before serialization, and fix dialog and login focus handling. Update persistent refresh warnings at token expiry, add regression coverage, and correct the PR description shortcut and fixture count.
Restore SSO refresh capability across provider remounts, recover schema after local renewal failures, and preserve no-send outcomes for queued headless reads. Settle script loading independently of abort targets, defer accepted snapshot preparation until after session invalidation, and align headless cancellation metadata. Add 14 regression tests. Validated with typecheck, lint, build, and 2,887 unit tests across 151 files.
Keep transient refresh failures recoverable after SQL 401 responses, preserve password form focus, and restore shortcut focus when closing notifications. Simplify loading notification settlement to use only the query ID and extract the shared button base without tooltip dependencies. Add unit and browser regression coverage.
Prevent obsolete initial code exchanges from replacing recovered logins, coordinate recovery across mounted schema panels, and report validation-only notebook runs as never executed. Add regression coverage for all three review findings.
Contributor
Author
Level-3 code reviewIssuesNo confirmed introduced defects found in PR #603, reviewed at The level-3 review completed all 13 independent passes and a widened follow-up on unchanged callers. No broken out-of-diff contract was established. False-positivesNo draft findings were submitted for per-finding verification. Two exploratory concerns were resolved during the caller follow-up:
SummaryVerdict: approve, based on source review and passing local quality gates—not a completed CI or live-integration sign-off.
Validation limits at review completion:
No introduced regression was confirmed across authentication, query execution, notebook persistence, notifications, catalog recovery, or grid replacement. Totals: 0 confirmed findings; 0 draft findings requiring verification. 0 in-diff / 0 out-of-diff defects. No source files were changed. |
Resolve all 32 review findings covering refresh coordination, session isolation, keyboard access, schema recovery, notebook persistence, and regression coverage. Remove the stale PR draft and unrelated server bump.
Address the Moderate and Minor review findings for PR 603, including toast accessibility, CSV upload races, interruption history, session fencing, schema recovery, settings feedback, and non-renewing telemetry. Consolidate shared helpers and extend regression coverage. Validated typecheck, lint, production build, 3,031 unit tests, and 64 stubbed Chrome browser-auth tests. Live Enterprise OIDC and manual screen-reader validation remain separate checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review follow-up pushed in
9da50a589c36e7bf1d8719f8db919f0e6636b3fd.Summary
Fix the Web Console hanging indefinitely when an SSO token refresh fails, and
make query execution and notebook persistence respect the authentication session
that started the work.
Refreshing is shared across SQL consumers, cancellable and bounded. Queries
normally wait up to 10 seconds; a rotating refresh request can continue until
its separate hard deadline (at most 60 seconds). A server-side 401 recovery
waits for that request rather than cancelling a potentially usable rotated
refresh token.
Transient transport, timeout, HTTP 429 and 5xx failures back off without forcing
logout. Once the bearer expires, SQL is not sent until renewal succeeds. The
console reports that explicitly, rather than reporting a database outage or
telling an agent that a never-sent write may have committed. An authoritative
OAuth rejection still returns the user to sign-in.
Complete scope
optional expiry handling; ID-token bearer expiry; refresh-token rotation;
missing required ID-token errors; callback URL restoration; tab-local session
generations; stable provider callbacks and subscription cleanup.
for SQL, validation, import and preferences requests; bounded refresh waits;
one safe retry after a rejected bearer; session-aware result/error fencing;
explicit session-ended and not-sent outcomes.
live queries, cancel delayed telemetry polls, and reset active/pending query
confirmations (including confirmations already waiting for cancellation).
excluded from notification history; settled/superseded loading notifications
are cleaned up; the notification Stop button only cancels its own execution;
scripts preserve confirmed outcomes and report partial completion on logout.
Script teardown is shared, without an extra settlement dispatch per statement.
separate layout-phase detachment from passive destruction; flush frames
accepted before logout while discarding late responses; atomically save
accepted headless metadata/snapshots; recover with metadata-only persistence
when a large snapshot aborts the root transaction. Byte capping runs before
acquiring the buffer write lock. Search updates publish only after root
commit.
refresh waits and SQL requests their own deadline budgets. Catalog code
receives the refresh budget from the client rather than importing coordinator
internals.
an older page cannot replace the current result; expose recycled row indices
to the stale-page regression test and document result-specific layout restore.
of queued requests, preserving concurrency limits and ordering.
its heading and transfer focus into the recovered console; preserve Retry
focus while busy; focus/describe sign-in errors; keep toast announcements
available outside the SQL gate and during dialogs; use live theme tokens; warn
both before and after token expiry; prevent incidental click dismissal;
restore a dismissed warning on subsequent failures; provide expiry
information, a sign-in action and Alt+N keyboard access. Renewal failures use
consistent copy.
types; extend unit and stubbed browser regressions; allowlist fake test tokens
in gitleaks; update the QuestDB E2E submodule (26 commits, including
permission-aware table visibility).
.github/workflows/enterprise_e2e.yml, an opt-in Azuredispatch using
vars.ENTERPRISE_E2E_ENABLEDandsecrets.ENT_DISPATCH_PAT.Fork PRs do not receive that secret. Private Enterprise source, binaries and
live fixtures remain in the private repository. Dispatch success is not a
passing integration result: check
enterprise-e2e-web-consolefor the testedSHA. Setup and local execution are documented in
e2e/README.md.Review follow-up coverage
Validation
Validation of earlier PR head
908eba9dd43af4f7080599b4cb1f7760e2d65a67(before the local six-issue follow-up):
yarn typecheck— passed.yarn lint— passed, including color-token checks.yarn build— passed (existing bundle-size advisory remains).yarn test:unit— 2,925 tests passed in 153 files.yarn cypress run --spec 'e2e/tests/auth/*.spec.js' --browser electron—57 passed against the production preview with stubbed HTTP responses:
47 auth, 6 notification and 4 unauthorized-recovery tests.
Six-issue follow-up validated locally on 2026-10-16 and now published in
842d2545e84cd134c01fb719ed0b0ad71e3a3f81(included in current PR head75eae8bb9ccb3526cc3270eb38eb32a2f521ded1):yarn typecheck,yarn lintandyarn build— passed.yarn test:unit— 2,941 tests passed in 154 files.yarn cypress run --spec 'e2e/tests/auth/*.spec.js' --browser electron—60 passed against the production preview with stubbed HTTP responses:
50 auth, 6 notification and 4 unauthorized-recovery tests.
CI selector follow-up in
75eae8bb9ccb3526cc3270eb38eb32a2f521ded1:name,
Close notification, instead of the obsoletecloseselector.yarn cypress run --spec e2e/tests/console/editor.spec.js --browser electroncompleted with 122 tests passing and 1 pre-existing pending test. The local
macOS Electron process then segfaulted during shutdown, so this was not a
clean command exit. A focused rerun of the previously failing test passed
with a clean exit against the real local QuestDB server.
and context-path tests.
Their final results are not claimed by the local validation above.
The earlier 2,838-unit/38-auth figures described an older validation checkpoint,
not the current published head or the local follow-up. The full console E2E
suite and live Enterprise OIDC lane were not run locally. The Enterprise
rotation assertion was extended locally but remains unverified in the private
lane; the stubbed auth suite is not a substitute for live IdP/Enterprise tests.