feat(auth): per-IdP basic_user_claim + header shows real CH identity - #5
Merged
Merged
Conversation
Adds a per-IdP `basic_user_claim` to config.json so a `ch_auth:basic` IdP can map to a CH username taken from any JWT claim (e.g. GitHub `nickname`) instead of the default email→preferred_username→sub chain. Why: on a ClickHouse cluster that already has a `<token>` user-directory (Bearer), a static `IDENTIFIED WITH http` user with the SAME name as the token-directory's ephemeral user shadows it (replicated storage sorts before `<token>`), so the Bearer login 403s. Giving each IdP a DISTINCT CH username lets two IdPs (Bearer token-processor + Basic http_authentication_servers) serve the same person side by side — without the single-`<token>`-processor limit (Altinity/ClickHouse#1928). Also fixes the header showing the raw `email` claim instead of the actual CH user: a shared `chUsername()` helper now drives both the Basic credential and the header display (they can no longer disagree), and main() resolves config before first paint so the identity is correct immediately. - net/oauth-config.js: normalizeEntry → `basicUserClaim` (default '') - ui/app.js: `chUsername()` helper; authHeader + app.email() use it; ensureConfig sets app.basicUserClaim - main.js: await ensureConfig() before renderApp when already signed in - tests: oauth-config, app (authHeader + header identity), main fake-app Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef
BorisTyshkevich
added a commit
that referenced
this pull request
Jun 23, 2026
…t, qualified fallback, doc-cache, single tokenize (#26, #27) Five findings from a follow-up manual review; verified live on otel. Correctness: - Hover docs now resolve the hovered word from the string/comment-masked text (host.maskedValue()), so hovering a function/keyword inside a string or comment no longer pops a phantom doc card — consistent with signature help (#1). - Signature help strips the optional-param brackets ClickHouse uses (`name(a, b[, c])`) before splitting on commas, so args render cleanly and the active-arg highlight aligns (was showing `offset[` / `length]` for substring) (#2). - completionContext only flags `qualified` when a real identifier precedes the dot; a bare dot (`.col`, `count().c`) now falls back to normal completion instead of an empty dropdown (#4). - loadEntityDoc returns null on a query FAILURE vs '' for genuinely-no-doc, and entityDoc caches the latter but drops the former — a transient error no longer permanently suppresses a function's hover/footer doc for the session (#8). Performance: - The keystroke path tokenizes the buffer ONCE and shares the token list between the syntax highlighter and the literal mask, instead of two full passes per keystroke. maskFromTokens()/renderTokensInto() consume a token list; the editor memoizes it by (text, refData) so it re-tokenizes when server keyword/func sets arrive after connect (#5). Tests cover each: hover-in-literal, bracketed-param signature split, bare-dot fallback, failed-doc retry, and re-highlight-on-refData-change (the shared-token cache invalidation). All gated layers stay 100/100/100/100 (ui glue within its functions>=95 / branches>=90 floor). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu
BorisTyshkevich
added a commit
that referenced
this pull request
Aug 5, 2026
… retire dashboardTreeRevision app.currentWorkspace/app.mainSurface become signal-backed accessor pairs (peeking getter, notifying setter) so a mutation is its own notification — the #426/#427 bug class (a write site forgetting to bump the invalidation counter) becomes structurally impossible. app.committedWorkspace (ReadonlySignal<StoredWorkspaceV5|null>) and app.treeNavigation (a computed structural key over kind/dashboardId/currentMember) are the two tracked reads app-shell.ts's tab-count/tree/Library effects subscribe through, replacing state.dashboardTreeRevision. app.currentWorkspace's setter is asymmetric (no null): a transitional null publication is a named departure operation owned by a new closure-private surface-retirement coordinator in app.ts (retireToWorkspaceLoading/ -Missing/-Failure/-Login), which batches the publication atomically with disposing any live shell so a surface never repaints against transitional state — five independent review passes each found a different call site where this raced disposal, hence one coordinator with exclusive mutation authority rather than five patches. SurfaceStatePort/DashboardApp/TabsApp/ DashboardTreeApp are narrowed to readonly on currentWorkspace (the fourth, DashboardTreeApp, was documented read-only but not type-readonly before this change). app.reloadDashboardRoute() (a post-commit fold-and-reassign that would double-publish once the aggregate is signal-backed) is deleted; afterLibraryChange's Dashboard branch calls the render-only app.renderCurrentSurface() instead. Tests: tests/unit/surface-lifecycle-arch.test.ts (a static-source scan backing the coordinator's compile/scan-layered "no lifecycle bypass" claim) and tests/unit/surface-accessor-contracts.test.ts (@ts-expect-error fixtures for the asymmetric setter and the four narrowed ports) are new, plus a new app.test.ts describe block covering the issue's Tests #1/#3/#5 and the plan's invariant map (delivery-only no-ops, adversarial-id collision-freedom, failure-path status/null ordering, the four-arm live-shell no-repaint sweep, one-commit-exactly-once settlement, and the mixed-snapshot batch-ordering regression). fake-app.ts's makeApp() installs real per-call signals on the returned object (object spread evaluates an accessor pair into a plain value, so the fake needs the same defineProperty treatment createApp() gets natively) for reactivity parity in fixtures. No persisted/schema change, no user-visible behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz
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.
What
Adds a per-IdP
basic_user_claimoption toconfig.json. For ach_auth:basicIdP, the SPA sends the Basic username from a chosen JWT claim (e.g. GitHubnickname) instead of the defaultemail → preferred_username → subchain.Also fixes the top-bar identity: it showed the raw
emailclaim even when the actual CH user was a different claim. A sharedchUsername()helper now drives both the Basic credential and the header display, and config is resolved before first paint.Why
On a ClickHouse cluster that already has a
<token>user-directory (Bearer/JWT), a staticIDENTIFIED WITH httpuser with the same name as the token-directory's ephemeral user shadows it (thereplicatedaccess storage sorts before<token>), so the Bearer login returns403. Giving each IdP a distinct CH username lets two IdPs coexist for the same person:<token_processor>, CH user = emailhttp_authentication_servers+ ausername_claim=nicknameverifier, CH user = github loginThis sidesteps the single-
<token>-processor limitation (Altinity/ClickHouse#1928) without changing the OSS-CH Basic path (default behavior is unchanged whenbasic_user_claimis unset).Verified live
currentUser()=btyshkevich@altinity.com; GitHub →currentUser()=BorisTyshkevich— both work in the same deployment, header shows the matching identity.Changes
net/oauth-config.js:normalizeEntry→basicUserClaim(default'')ui/app.js:chUsername()helper;authHeader+app.email()use it;ensureConfigsetsapp.basicUserClaimmain.js:await ensureConfig()beforerenderApp()when already signed innpm test→ 361 passing,oauth-config.js+app.jsat 100% lines.🤖 Generated with Claude Code
https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef