Skip to content

fix 371 trino impersonation - #397

Merged
Bohreromir merged 12 commits into
mainfrom
worktree-fix-371-trino-impersonation
Oct 29, 2026
Merged

Bohreromir merged 12 commits into
mainfrom
worktree-fix-371-trino-impersonation

Conversation

@Bohreromir

@Bohreromir Bohreromir commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

I hope this handles all cases well again

  • AI Review

closes #371

input: false on user.additionalFields.username also made better-auth drop
the field from the OIDC provider profile, so new users had no username and
Trino ran their queries as 'anonymous'. Allow the write only from the OIDC
callback via database hooks instead, which still rejects rewrites through
/update-user (#330).
…ame (#371)

Fall back to 'anonymous' only when nobody is signed in (OIDC disabled). A
signed-in user without a username now gets a logged error and a 500, so a
lost username claim can no longer go unnoticed.
The mock Trino now records X-Trino-User, so the query API test can check the
impersonated user matches the signed-in user.
#371)

- Set overrideUserInfo on the OIDC provider, so a changed Trino user
  claim reaches existing users on their next login, not only new ones.
- Drop the `|| ''` fallback for the username: better-auth rejects a
  sign-in without email before creating the user, so an empty username
  is never stored.
- Split findTrinoUser (returns null) from getTrinoUser (fails with 500).
  The connection-test action now returns a localised form error
  (trino_user_missing) instead of the error page.
- Mock OIDC server: a test can sign in as a fixed user with chosen
  claims via the mock_oidc_user cookie.
- E2E: a later login updates the username from the claim.
@Bohreromir Bohreromir self-assigned this Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 67.3% 9108 / 13532
🔵 Statements 66.28% 12270 / 18511
🔵 Functions 68.14% 2941 / 4316
🔵 Branches 57.32% 5150 / 8984
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/lib/server/auth-schema.ts 36.84% 100% 0% 43.75% 13, 26-35, 46-59, 72-75, 78-81, 83-88, 90-95
src/lib/server/auth-utils.ts 100% 100% 100% 100%
src/lib/server/auth.ts 66.66% 43.75% 44.44% 76% 11, 12, 13, 16, 101-124
src/routes/(app)/api/trino/catalog/+server.ts 100% 95% 100% 100%
src/routes/(app)/api/trino/completion/metadata/+server.ts 100% 96.29% 100% 100%
src/routes/(app)/api/trino/query/+server.ts 100% 100% 100% 100%
src/routes/(app)/trino/+page.server.ts 0% 0% 0% 0% 24-104
Generated in workflow #1204 for commit 323a14e by the Vitest Coverage Report Action

The field holds the Trino user we impersonate, not a login name. Rename the
column to trino_user in the initial migration; there is no release yet, so no
new migration is needed.
@Bohreromir
Bohreromir force-pushed the worktree-fix-371-trino-impersonation branch from 8b6e575 to 2adba17 Compare October 21, 2026 14:54
@Bohreromir
Bohreromir marked this pull request as ready for review October 22, 2026 12:09
@Bohreromir Bohreromir moved this to Development: Waiting for Review in Stackable Engineering Oct 22, 2026
Return the query route error as JSON so the query runner shows it
instead of "connection lost", log the missing Trino user at warn level,
and tell affected users to sign in again.
@dervoeti dervoeti moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Oct 28, 2026
Comment thread src/lib/server/auth-utils.ts Resolved
…o-impersonation

# Conflicts:
#	src/lib/server/auth-utils.ts
…ing Trino user error

Revert the in-place edit of 0000_initial_schema so databases that already
applied it get the rename via 0005.
@Bohreromir
Bohreromir requested a review from dervoeti October 29, 2026 13:47
@Bohreromir
Bohreromir added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 3681a3b Oct 29, 2026
12 checks passed
@Bohreromir
Bohreromir deleted the worktree-fix-371-trino-impersonation branch October 29, 2026 14:25
@Bohreromir Bohreromir moved this from Development: In Review to Development: Done in Stackable Engineering Oct 29, 2026
Bohreromir added a commit that referenced this pull request Oct 29, 2026
better-auth 1.7 routes generic OAuth callbacks through the shared
/callback/:id endpoint, so the trinoUser write guard from #397 rejected
every OIDC sign-in. Take the path from the callbackOAuth endpoint.
@lfrancke lfrancke moved this from Development: Done to Done in Stackable Engineering Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Broken Trino impersonation

2 participants