fix 371 trino impersonation

e2e/auth/auth.spec.ts

@@ -48,6 +48,62 @@ test.describe('Authentication', () => {
     await expect(page.getByRole('heading', { name: 'Dashboard' })).toBeVisible();
   });
 
+  test('trinoUser cannot be changed through the update-user endpoint', async ({
+    page,
+    baseURL
+  }) => {
+    const res = await page.request.post('/api/auth/update-user', {
+      headers: { origin: baseURL! },
+      data: { trinoUser: 'someone-else' }
+    });
+    expect(res.status()).toBe(403);
+    expect(await res.json()).toMatchObject({ message: 'trinoUser is not allowed to be set' });
+
+    const session = await (await page.request.get('/api/auth/get-session')).json();
+    expect(session.user.trinoUser).toMatch(/^testuser\d+$/);
+  });
+
+  test('a later login refreshes the Trino user from the OIDC claim', async ({ browser }) => {
+    const context = await browser.newContext({ storageState: { cookies: [], origins: [] } });
+    const page = await context.newPage();
+    const sub = `relogin-${crypto.randomUUID()}`;
+
+    const loginAs = async (preferred_username: string) => {
+      await context.clearCookies();
+      await context.addCookies([
+        {
+          name: 'mock_oidc_user',
+          value: encodeURIComponent(
+            JSON.stringify({
+              sub,
+              name: 'Relogin User',
+              email: `${sub}@example.com`,
+              preferred_username
+            })
+          ),
+          domain: 'localhost',
+          path: '/'
+        }
+      ]);
+      await page.goto('/');
+      await waitForHydration(page);
+      await page.getByRole('button', { name: /sign in with sso/i }).click();
+      await expect(page.getByRole('heading', { name: 'Dashboard' })).toBeVisible();
+      return (await (await page.request.get('/api/auth/get-session')).json()).user;
+    };
+
+    try {
+      const first = await loginAs(`${sub}-old`);
+      expect(first.trinoUser).toBe(`${sub}-old`);
+
+      const second = await loginAs(`${sub}-new`);
+      expect(second.id).toBe(first.id);
+      expect(second.trinoUser).toBe(`${sub}-new`);
+    } finally {
+      await context.close();
+    }
+  });
+
   test('user menu shows sign out option', async ({ page }) => {
     await page.goto('/');
 

e2e/support/mock-oidc-server.ts

@@ -23,10 +23,29 @@ export async function startMockOidc(): Promise<string> {
 
   // Map sub → profile claims so the userinfo endpoint can look them up
   // from the access token instead of relying on shared mutable state.
-  const users = new Map<string, { name: string; email: string; preferred_username: string }>();
+  type Profile = { name: string; email: string; preferred_username: string };
+  const users = new Map<string, Profile>();
+
+  // A test can log in as a fixed user with chosen claims by setting the
+  // `mock_oidc_user` cookie (JSON `{ sub, ...profile }`) before signing in.
+  // The authorise request carries the cookie; the token request only the code.
+  const fixedUsers = new Map<string, { sub: string } & Profile>();
+  server.service.on('beforeAuthorizeRedirect', ({ url }, req) => {
+    const cookie = (req.headers.cookie ?? '').match(/(?:^|;\s*)mock_oidc_user=([^;]+)/)?.[1];
+    const code = url.searchParams.get('code');
+    if (cookie && code) fixedUsers.set(code, JSON.parse(decodeURIComponent(cookie)));
+  });
 
   // Add OIDC profile claims to every issued token
-  server.service.on('beforeTokenSigning', (token) => {
+  server.service.on('beforeTokenSigning', (token, req) => {
+    const fixed = fixedUsers.get((req.body as { code?: string }).code ?? '');
+    if (fixed) {
+      const { sub, ...profile } = fixed;
+      users.set(sub, profile);
+      token.payload.sub = sub;
+      Object.assign(token.payload, profile);
+      return;
+    }
     userCounter++;
     const sub = `mock-user-${String(userCounter).padStart(3, '0')}`;
     const profile = {

e2e/support/mock-trino.ts

@@ -9,6 +9,8 @@ export interface MockTrinoLogEntry {
   event: 'submitted' | 'cancelled';
   sql: string;
   queryId: string;
+  /** `X-Trino-User` of the submit; absent on cancellations. */
+  user?: string;
 }
 
 /**
@@ -20,7 +22,7 @@ export function mockMarker(): string {
 }
 
 /** Statements containing `marker` that reached the mock, and their cancellations. */
-async function mockTrinoLog(marker: string): Promise<MockTrinoLogEntry[]> {
+export async function mockTrinoLog(marker: string): Promise<MockTrinoLogEntry[]> {
   const res = await fetch(`${MOCK_TRINO_URL}/__mock/log?marker=${encodeURIComponent(marker)}`);
   if (!res.ok) {
     throw new Error(`Mock Trino log request failed: ${res.status} ${res.statusText}`);

e2e/support/start-mock-trino.ts

@@ -1,5 +1,5 @@
 import * as http from 'node:http';
-import { MOCK_TRINO_PORT } from './mock-trino';
+import { MOCK_TRINO_PORT, type MockTrinoLogEntry } from './mock-trino';
 
 /** SQL substring → fixture response. First match wins; fallback is a 3-row success. */
 const routes: [string, object][] = [
@@ -134,7 +134,7 @@ interface MockQuery {
 }
 
 const queries = new Map<string, MockQuery>();
-const log: { event: 'submitted' | 'cancelled'; sql: string; queryId: string }[] = [];
+const log: MockTrinoLogEntry[] = [];
 const released = new Set<string>();
 const waiters = new Map<string, (() => void)[]>();
 let queryCounter = 0;
@@ -300,7 +300,12 @@ http
       // Logged on arrival, so tests can act while a gated submit is still pending.
       const id = `q-mock-${++queryCounter}`;
       queries.set(id, { sql: body, pages: [] });
-      log.push({ event: 'submitted', sql: body, queryId: id });
+      log.push({
+        event: 'submitted',
+        sql: body,
+        queryId: id,
+        user: req.headers['x-trino-user'] as string | undefined
+      });
 
       const gate = /GATE:([\w-]+)/.exec(body)?.[1];
       if (gate) await waitForRelease(gate);

e2e/trino/query-api.spec.ts

@@ -1,6 +1,11 @@
 import { test, expect, type APIRequestContext } from '../support/fixtures.js';
 import { randomUUID } from 'node:crypto';
-import { mockMarker, mockTrinoStatements, releaseMockTrino } from '../support/mock-trino';
+import {
+  mockMarker,
+  mockTrinoLog,
+  mockTrinoStatements,
+  releaseMockTrino
+} from '../support/mock-trino';
 
 /**
  * Trino API endpoints end to end: authentication through the server hooks and
@@ -75,6 +80,36 @@ test.describe('Trino API', () => {
       expect(await snapshots(page.request, tabId)).toEqual([]);
     });
 
+    test('runs statements as the OIDC user', async ({ page }) => {
+      const marker = mockMarker();
+      await page.request.post('/api/trino/query', {
+        data: { statements: [`SELECT 1 /* ${marker} */`], tabId: randomUUID() }
+      });
+
+      const session = await (await page.request.get('/api/auth/get-session')).json();
+      expect(session.user.trinoUser).toMatch(/^testuser\d+$/);
+      await expect
+        .poll(async () => (await mockTrinoLog(marker)).map((e) => e.user))
+        .toEqual([session.user.trinoUser]);
+    });
+
+    // The marker goes into a name the endpoint puts in its SQL, so the mock log
+    // can find the metadata statement.
+    for (const [endpoint, query] of [
+      ['catalog', (marker: string) => `level=schemas&catalog=${marker}`],
+      ['completion/metadata', (marker: string) => `level=tables&catalog=tpch&schema=${marker}`]
+    ] as const) {
+      test(`${endpoint} runs metadata queries as the OIDC user`, async ({ page }) => {
+        const marker = mockMarker();
+        const res = await page.request.get(`/api/trino/${endpoint}?${query(marker)}`);
+        expect(res.status()).toBe(200);
+
+        const session = await (await page.request.get('/api/auth/get-session')).json();
+        expect(session.user.trinoUser).toMatch(/^testuser\d+$/);
+        expect((await mockTrinoLog(marker)).map((e) => e.user)).toEqual([session.user.trinoUser]);
+      });
+    }
+
     test('rejects an invalid request', async ({ page }) => {
       const res = await page.request.post('/api/trino/query', {
         data: { statements: ['SELECT 1'], tabId: 'not-a-uuid' }

messages/de.json

@@ -78,6 +78,7 @@
   "trino_connection_test_failed": "Verbindung zu Trino fehlgeschlagen – bitte URL und Zugangsdaten prüfen.",
   "trino_connection_save_failed": "Die Verbindung konnte nicht gespeichert werden. Bitte erneut versuchen.",
   "trino_connection_env_managed": "Die Verbindung wird über Umgebungsvariablen verwaltet.",
+  "trino_user_missing": "Ihrem Konto ist kein Trino-Benutzername zugeordnet. Melden Sie sich ab und erneut an; besteht das Problem weiterhin, wenden Sie sich an Ihre Administration.",
   "trino_cancel_query": "Abbrechen",
   "trino_state_submitting": "Wird eingereicht",
   "trino_state_queued": "Warteschlange",

messages/en.json

@@ -78,6 +78,7 @@
   "trino_connection_test_failed": "Could not connect to Trino — check the URL and credentials.",
   "trino_connection_save_failed": "Could not save the connection. Please try again.",
   "trino_connection_env_managed": "The connection is managed via environment variables.",
+  "trino_user_missing": "Your account has no Trino username. Sign out and sign in again; if the problem persists, contact your administrator.",
   "trino_cancel_query": "Cancel",
   "trino_state_submitting": "Submitting",
   "trino_state_queued": "Queued",

src/lib/server/auth-schema.ts

@@ -12,7 +12,7 @@ export const user = pgTable('user', {
     .defaultNow()
     .$onUpdate(() => /* @__PURE__ */ new Date())
     .notNull(),
-  username: text('username')
+  trinoUser: text('trino_user')
 });
 
 export const session = pgTable(

src/lib/server/auth-utils.test.ts

@@ -1,5 +1,5 @@
-import { describe, it, expect } from 'vitest';
-import { getUserId } from './auth-utils.js';
+import { describe, it, expect, vi } from 'vitest';
+import { findTrinoUser, getTrinoUser, getUserId } from './auth-utils.js';
 
 describe('getUserId', () => {
   it('returns the user ID when user is present', () => {
@@ -17,3 +17,33 @@ describe('getUserId', () => {
     expect(getUserId(locals)).toBe('anonymous');
   });
 });
+
+describe('getTrinoUser', () => {
+  const logger = { warn: vi.fn() };
+
+  it('returns the Trino user of the signed-in user', () => {
+    const locals = { user: { id: 'u1', trinoUser: 'alice' }, logger } as unknown as App.Locals;
+    expect(getTrinoUser(locals)).toBe('alice');
+  });
+
+  it('returns "anonymous" without a signed-in user (OIDC disabled)', () => {
+    const locals = { user: null, logger } as unknown as App.Locals;
+    expect(getTrinoUser(locals)).toBe('anonymous');
+  });
+
+  it('fails instead of falling back to "anonymous" when the Trino user is missing', () => {
+    const locals = { user: { id: 'u1', trinoUser: null }, logger } as unknown as App.Locals;
+    expect(() => getTrinoUser(locals)).toThrow(expect.objectContaining({ status: 500 }));
+    expect(logger.warn).toHaveBeenCalledWith({ user_id: 'u1' }, expect.any(String));
+  });
+});
+
+describe('findTrinoUser', () => {
+  const logger = { warn: vi.fn() };
+
+  it('returns null instead of "anonymous" when the Trino user is missing', () => {
+    const locals = { user: { id: 'u1', trinoUser: null }, logger } as unknown as App.Locals;
+    expect(findTrinoUser(locals)).toBeNull();
+    expect(logger.warn).toHaveBeenCalledWith({ user_id: 'u1' }, expect.any(String));
+  });
+});

src/lib/server/auth-utils.ts

@@ -1,3 +1,22 @@
+import { error } from '@sveltejs/kit';
+
 export function getUserId(locals: App.Locals): string {
   return locals.user?.id ?? 'anonymous';
 }
+
+/**
+ * The user to impersonate in Trino. Without OIDC there is no signed-in user,
+ * so queries run as 'anonymous'. A signed-in user without a Trino user means
+ * sign-up lost the OIDC claim; return null instead of falling back to 'anonymous'.
+ */
+export function findTrinoUser(locals: App.Locals): string | null {
+  if (!locals.user) return 'anonymous';
+  if (locals.user.trinoUser) return locals.user.trinoUser;
+  locals.logger.warn({ user_id: locals.user.id }, 'Signed-in user has no Trino user');
+  return null;
+}
+
+/** Like {@link findTrinoUser}, but fails the request when there is no Trino user. */
+export function getTrinoUser(locals: App.Locals): string {
+  return findTrinoUser(locals) ?? error(500, 'User has no Trino user');
+}

src/lib/server/auth.test.ts

@@ -1,21 +1,39 @@
 import { describe, expect, it } from 'vitest';
-import { parseUserInput } from 'better-auth/db';
+import { parseAdditionalUserInputFromProviderProfile } from 'better-auth/db';
 import { auth } from './auth.js';
 
-describe('user.additionalFields.username', () => {
-  it('rejects a rewrite through the built-in update-user endpoint', () => {
-    expect(() => parseUserInput(auth.options, { username: 'someone-else' }, 'update')).toThrow(
-      /username is not allowed to be set/
-    );
+const hooks = auth.options.databaseHooks!.user!;
+const create = (data: Record<string, unknown>, path: string) =>
+  hooks.create!.before!(data as never, { path } as never);
+const update = (data: Record<string, unknown>, path: string) =>
+  hooks.update!.before!(data as never, { path } as never);
+
+describe('user.additionalFields.trinoUser', () => {
+  it('is taken from the OIDC provider profile', () => {
+    expect(
+      parseAdditionalUserInputFromProviderProfile(auth.options, { trinoUser: 'alice' }, 'create')
+    ).toMatchObject({ trinoUser: 'alice' });
+  });
+
+  it('may be written by the OIDC callback', async () => {
+    await expect(
+      create({ email: 'alice@example.com', trinoUser: 'alice' }, '/oauth2/callback/:providerId')
+    ).resolves.toBeUndefined();
   });
 
-  it('rejects a rewrite smuggled in at sign-up', () => {
-    expect(() => parseUserInput(auth.options, { username: 'someone-else' }, 'create')).toThrow(
-      /username is not allowed to be set/
+  it('rejects a rewrite through the built-in update-user endpoint', async () => {
+    await expect(update({ trinoUser: 'someone-else' }, '/update-user')).rejects.toThrow(
+      /trinoUser is not allowed to be set/
     );
   });
 
-  it('is still declared, so the OIDC claim can populate it', () => {
-    expect(auth.options.user?.additionalFields?.username).toMatchObject({ type: 'string' });
+  it('rejects a rewrite smuggled in at sign-up', async () => {
+    await expect(
+      create({ email: 'eve@example.com', trinoUser: 'someone-else' }, '/sign-up/email')
+    ).rejects.toThrow(/trinoUser is not allowed to be set/);
+  });
+
+  it('leaves other user updates alone', async () => {
+    await expect(update({ emailVerified: true }, '/update-user')).resolves.toBeUndefined();
   });
 });