diff --git a/e2e/auth/auth.spec.ts b/e2e/auth/auth.spec.ts index 0e81b6c2..de54ddcd 100644 --- a/e2e/auth/auth.spec.ts +++ b/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('/'); diff --git a/e2e/support/mock-oidc-server.ts b/e2e/support/mock-oidc-server.ts index 30ba9c26..a3902c21 100644 --- a/e2e/support/mock-oidc-server.ts +++ b/e2e/support/mock-oidc-server.ts @@ -23,10 +23,29 @@ export async function startMockOidc(): Promise { // 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(); + type Profile = { name: string; email: string; preferred_username: string }; + const users = new Map(); + + // 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(); + 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 = { diff --git a/e2e/support/mock-trino.ts b/e2e/support/mock-trino.ts index 5d79ff3c..2734028a 100644 --- a/e2e/support/mock-trino.ts +++ b/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 { +export async function mockTrinoLog(marker: string): Promise { 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}`); diff --git a/e2e/support/start-mock-trino.ts b/e2e/support/start-mock-trino.ts index e11b22b7..83ebff63 100644 --- a/e2e/support/start-mock-trino.ts +++ b/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][] = [ @@ -136,7 +136,7 @@ interface MockQuery { } const queries = new Map(); -const log: { event: 'submitted' | 'cancelled'; sql: string; queryId: string }[] = []; +const log: MockTrinoLogEntry[] = []; const released = new Set(); const waiters = new Map void)[]>(); let queryCounter = 0; @@ -348,7 +348,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); diff --git a/e2e/trino/query-api.spec.ts b/e2e/trino/query-api.spec.ts index 29babe63..1c65c82f 100644 --- a/e2e/trino/query-api.spec.ts +++ b/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' } diff --git a/messages/de.json b/messages/de.json index bb97b15b..bd7844fb 100644 --- a/messages/de.json +++ b/messages/de.json @@ -92,6 +92,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", diff --git a/messages/en.json b/messages/en.json index dcf51ff4..4a25cd94 100644 --- a/messages/en.json +++ b/messages/en.json @@ -92,6 +92,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", diff --git a/src/lib/server/auth-schema.ts b/src/lib/server/auth-schema.ts index 03855c16..4dbd1209 100644 --- a/src/lib/server/auth-schema.ts +++ b/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( diff --git a/src/lib/server/auth-utils.test.ts b/src/lib/server/auth-utils.test.ts index 38f63988..f794eabd 100644 --- a/src/lib/server/auth-utils.test.ts +++ b/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)); + }); +}); diff --git a/src/lib/server/auth-utils.ts b/src/lib/server/auth-utils.ts index 504b39d1..746c575c 100644 --- a/src/lib/server/auth-utils.ts +++ b/src/lib/server/auth-utils.ts @@ -1,4 +1,5 @@ import { error } from '@sveltejs/kit'; +import * as m from '$lib/paraglide/messages.js'; export function getUserId(locals: App.Locals): string { return locals.user?.id ?? 'anonymous'; @@ -12,3 +13,20 @@ export function requireUserId(locals: App.Locals): string { if (!locals.user) throw error(401, 'Authentication required'); return locals.user.id; } + +/** + * 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, m.trino_user_missing()); +} diff --git a/src/lib/server/auth.test.ts b/src/lib/server/auth.test.ts index ac7901f4..b188d8cf 100644 --- a/src/lib/server/auth.test.ts +++ b/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, path: string) => + hooks.create!.before!(data as never, { path } as never); +const update = (data: Record, 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(); }); }); diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index f3b6e65c..8564a0ed 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -1,4 +1,5 @@ import { betterAuth } from 'better-auth'; +import { APIError } from 'better-auth/api'; import { drizzleAdapter } from '@better-auth/drizzle-adapter'; import { genericOAuth, openAPI } from 'better-auth/plugins'; import { sveltekitCookies } from 'better-auth/svelte-kit'; @@ -15,6 +16,18 @@ const env = envModule?.env ?? (process.env as Record const getRequestEvent = appServer?.getRequestEvent ?? (() => undefined as unknown as RequestEvent); const log = loggingModule?.logger.child({ module: 'auth' }); +// trinoUser is the Trino user we impersonate, so only the OIDC callback may +// write it. Marking the field `input: false` would also block that: better-auth +// drops such fields from the provider profile on sign-up (see #371). Database +// hooks see the endpoint's route template, not the request path. +const trinoUserWritePath = '/oauth2/callback/:providerId'; + +function guardTrinoUserWrite(data: Record, context: { path?: string } | null) { + if (!('trinoUser' in data) || context?.path === trinoUserWritePath) return; + log?.warn({ path: context?.path }, 'Rejected trinoUser write outside the OIDC callback'); + throw new APIError('FORBIDDEN', { message: 'trinoUser is not allowed to be set' }); +} + const trinoUserClaim = env.STACKABLE_COCKPIT_TRINO_USER_CLAIM ?? 'preferred_username'; const defaultScopes = ['openid', 'profile', 'email']; @@ -55,13 +68,18 @@ export const auth = betterAuth({ }, user: { additionalFields: { - username: { + trinoUser: { type: 'string', - required: false, - input: false + required: false } } }, + databaseHooks: { + user: { + create: { before: async (user, context) => guardTrinoUserWrite(user, context) }, + update: { before: async (user, context) => guardTrinoUserWrite(user, context) } + } + }, plugins: [ sveltekitCookies(getRequestEvent), openAPI(), @@ -76,12 +94,15 @@ export const auth = betterAuth({ clientSecret: env.STACKABLE_COCKPIT_OIDC_CLIENT_SECRET!, scopes: oidcScopes, pkce: true, + // Refresh the user from the IdP on every login, so changes to the + // Trino user claim reach existing users instead of only new ones. + overrideUserInfo: true, mapProfileToUser: async (profile) => { const fullName = [profile.given_name, profile.family_name] .filter(Boolean) .join(' '); - const trinoUser = profile[trinoUserClaim]; - if (trinoUser) { + const claimedTrinoUser = profile[trinoUserClaim]; + if (claimedTrinoUser) { log?.debug( { trino_user_claim: trinoUserClaim }, 'Resolved Trino user from OIDC claim' @@ -97,7 +118,9 @@ export const auth = betterAuth({ email: profile.email || '', emailVerified: profile.email_verified ?? false, image: profile.picture || null, - username: trinoUser || profile.preferred_username || profile.email || '' + // No `|| ''`: without an email better-auth rejects the sign-in before + // creating the user, so this never stores an empty Trino user. + trinoUser: claimedTrinoUser || profile.preferred_username || profile.email }; } } diff --git a/src/lib/server/migrations/0005_rename_trino_user.sql b/src/lib/server/migrations/0005_rename_trino_user.sql new file mode 100644 index 00000000..f1a4e1ed --- /dev/null +++ b/src/lib/server/migrations/0005_rename_trino_user.sql @@ -0,0 +1 @@ +ALTER TABLE "user" RENAME COLUMN "username" TO "trino_user"; diff --git a/src/lib/server/migrations/meta/0005_snapshot.json b/src/lib/server/migrations/meta/0005_snapshot.json new file mode 100644 index 00000000..438f29a8 --- /dev/null +++ b/src/lib/server/migrations/meta/0005_snapshot.json @@ -0,0 +1,786 @@ +{ + "id": "f85cab9f-93a0-4f90-a025-e93816010a09", + "prevId": "eb516884-ee2b-4d3e-840f-5b5650b7b17f", + "version": "7", + "dialect": "postgresql", + "tables": { + "public.storage_download_manifests": { + "name": "storage_download_manifests", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "connection_id": { + "name": "connection_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "bucket": { + "name": "bucket", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "prefix": { + "name": "prefix", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "entries": { + "name": "entries", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "format": { + "name": "format", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "archive": { + "name": "archive", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "storage_download_manifests_user_expiry_idx": { + "name": "storage_download_manifests_user_expiry_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "storage_download_manifests_expiry_idx": { + "name": "storage_download_manifests_expiry_idx", + "columns": [ + { + "expression": "expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "storage_download_manifests_connection_id_user_storage_connections_id_fk": { + "name": "storage_download_manifests_connection_id_user_storage_connections_id_fk", + "tableFrom": "storage_download_manifests", + "tableTo": "user_storage_connections", + "columnsFrom": ["connection_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.user_bookmarks": { + "name": "user_bookmarks", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "product_id": { + "name": "product_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "environment": { + "name": "environment", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "pinned": { + "name": "pinned", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "user_bookmarks_user_id_idx": { + "name": "user_bookmarks_user_id_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.user_recent_searches": { + "name": "user_recent_searches", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "connection_id": { + "name": "connection_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "buckets": { + "name": "buckets", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'[]'::jsonb" + }, + "query": { + "name": "query", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "use_regex": { + "name": "use_regex", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "exclude_patterns": { + "name": "exclude_patterns", + "type": "text[]", + "primaryKey": false, + "notNull": true, + "default": "'{}'" + }, + "search_path": { + "name": "search_path", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "max_depth": { + "name": "max_depth", + "type": "integer", + "primaryKey": false, + "notNull": false + }, + "search_key": { + "name": "search_key", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "user_recent_searches_connection_idx": { + "name": "user_recent_searches_connection_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "connection_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "user_recent_searches_connection_id_user_storage_connections_id_fk": { + "name": "user_recent_searches_connection_id_user_storage_connections_id_fk", + "tableFrom": "user_recent_searches", + "tableTo": "user_storage_connections", + "columnsFrom": ["connection_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "user_recent_searches_search_key": { + "name": "user_recent_searches_search_key", + "nullsNotDistinct": false, + "columns": ["user_id", "connection_id", "search_key"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.user_storage_connections": { + "name": "user_storage_connections", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "encrypted_payload": { + "name": "encrypted_payload", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "hash": { + "name": "hash", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "additional_buckets": { + "name": "additional_buckets", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'[]'" + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "user_id_idx": { + "name": "user_id_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "user_storage_connections_user_id_name_unique": { + "name": "user_storage_connections_user_id_name_unique", + "nullsNotDistinct": false, + "columns": ["user_id", "name"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.account": { + "name": "account", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "account_id": { + "name": "account_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "provider_id": { + "name": "provider_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "access_token": { + "name": "access_token", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "refresh_token": { + "name": "refresh_token", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "id_token": { + "name": "id_token", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "access_token_expires_at": { + "name": "access_token_expires_at", + "type": "timestamp", + "primaryKey": false, + "notNull": false + }, + "refresh_token_expires_at": { + "name": "refresh_token_expires_at", + "type": "timestamp", + "primaryKey": false, + "notNull": false + }, + "scope": { + "name": "scope", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "password": { + "name": "password", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "account_userId_idx": { + "name": "account_userId_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "account_user_id_user_id_fk": { + "name": "account_user_id_user_id_fk", + "tableFrom": "account", + "tableTo": "user", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.session": { + "name": "session", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "token": { + "name": "token", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "ip_address": { + "name": "ip_address", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "user_agent": { + "name": "user_agent", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "active_storage_connection_id": { + "name": "active_storage_connection_id", + "type": "text", + "primaryKey": false, + "notNull": false + } + }, + "indexes": { + "session_userId_idx": { + "name": "session_userId_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "session_user_id_user_id_fk": { + "name": "session_user_id_user_id_fk", + "tableFrom": "session", + "tableTo": "user", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "session_token_unique": { + "name": "session_token_unique", + "nullsNotDistinct": false, + "columns": ["token"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.user": { + "name": "user", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "email_verified": { + "name": "email_verified", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "image": { + "name": "image", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "trino_user": { + "name": "trino_user", + "type": "text", + "primaryKey": false, + "notNull": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "user_email_unique": { + "name": "user_email_unique", + "nullsNotDistinct": false, + "columns": ["email"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.verification": { + "name": "verification", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "identifier": { + "name": "identifier", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "value": { + "name": "value", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "verification_identifier_idx": { + "name": "verification_identifier_idx", + "columns": [ + { + "expression": "identifier", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + } + }, + "enums": {}, + "schemas": {}, + "sequences": {}, + "roles": {}, + "policies": {}, + "views": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + } +} diff --git a/src/lib/server/migrations/meta/_journal.json b/src/lib/server/migrations/meta/_journal.json index 1f599e9e..b89f7df0 100644 --- a/src/lib/server/migrations/meta/_journal.json +++ b/src/lib/server/migrations/meta/_journal.json @@ -36,6 +36,13 @@ "when": 1793094984249, "tag": "0004_user_bookmarks", "breakpoints": true + }, + { + "idx": 5, + "version": "7", + "when": 1793280111856, + "tag": "0005_rename_trino_user", + "breakpoints": true } ] } diff --git a/src/routes/(app)/api/trino/catalog/+server.ts b/src/routes/(app)/api/trino/catalog/+server.ts index ffca1c5c..e4ac03b9 100644 --- a/src/routes/(app)/api/trino/catalog/+server.ts +++ b/src/routes/(app)/api/trino/catalog/+server.ts @@ -1,6 +1,6 @@ import { json, error } from '@sveltejs/kit'; import { resolveTrinoClient, trinoMetadataQuery } from '$lib/server/trino/client.js'; -import { getUserId } from '$lib/server/auth-utils.js'; +import { getTrinoUser, getUserId } from '$lib/server/auth-utils.js'; import { quoteIdentifier, quoteStringLiteral } from '$lib/editor/identifiers.js'; import type { RequestHandler } from './$types'; @@ -14,7 +14,7 @@ export const GET: RequestHandler = async ({ url, locals }) => { error(400, 'No Trino connection configured'); } - const user = locals.user?.username ?? 'anonymous'; + const user = getTrinoUser(locals); const level = url.searchParams.get('level'); const catalog = url.searchParams.get('catalog'); const schema = url.searchParams.get('schema'); diff --git a/src/routes/(app)/api/trino/catalog/server.test.ts b/src/routes/(app)/api/trino/catalog/server.test.ts index 37956e3d..cb6537ac 100644 --- a/src/routes/(app)/api/trino/catalog/server.test.ts +++ b/src/routes/(app)/api/trino/catalog/server.test.ts @@ -13,7 +13,7 @@ const fakeClient = { fake: true } as unknown as TrinoClient; function mockEvent( params: Record, - user: object | null = { id: 'u1', username: 'alice' } + user: object | null = { id: 'u1', trinoUser: 'alice' } ) { const url = new URL('http://localhost/api/trino/catalog'); for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v); diff --git a/src/routes/(app)/api/trino/completion/metadata/+server.ts b/src/routes/(app)/api/trino/completion/metadata/+server.ts index a0529739..880907be 100644 --- a/src/routes/(app)/api/trino/completion/metadata/+server.ts +++ b/src/routes/(app)/api/trino/completion/metadata/+server.ts @@ -1,7 +1,7 @@ import { json, error } from '@sveltejs/kit'; import { z } from 'zod'; import { resolveTrinoClient, trinoMetadataQuery } from '$lib/server/trino/client.js'; -import { getUserId } from '$lib/server/auth-utils.js'; +import { getTrinoUser, getUserId } from '$lib/server/auth-utils.js'; import { completionEnabled } from '$lib/server/feature-flags.js'; import { quoteIdentifier, quoteStringLiteral } from '$lib/editor/identifiers.js'; import type { RequestHandler } from './$types'; @@ -30,7 +30,7 @@ export const GET: RequestHandler = async ({ url, locals }) => { error(400, 'No Trino connection configured'); } - const user = locals.user?.username ?? 'anonymous'; + const user = getTrinoUser(locals); const raw = Object.fromEntries(url.searchParams.entries()); const parsed = ParamsSchema.safeParse(raw); diff --git a/src/routes/(app)/api/trino/completion/metadata/server.test.ts b/src/routes/(app)/api/trino/completion/metadata/server.test.ts index cce3e18b..7e2cca86 100644 --- a/src/routes/(app)/api/trino/completion/metadata/server.test.ts +++ b/src/routes/(app)/api/trino/completion/metadata/server.test.ts @@ -24,7 +24,7 @@ function mockEvent(params: Record) { for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v); return { url, - locals: { logger: { debug: vi.fn(), info: vi.fn() }, user: { id: 'u1', username: 'alice' } } + locals: { logger: { debug: vi.fn(), info: vi.fn() }, user: { id: 'u1', trinoUser: 'alice' } } } as unknown as Parameters[0]; } diff --git a/src/routes/(app)/api/trino/query/+server.ts b/src/routes/(app)/api/trino/query/+server.ts index 187ca5fa..dea951fd 100644 --- a/src/routes/(app)/api/trino/query/+server.ts +++ b/src/routes/(app)/api/trino/query/+server.ts @@ -1,6 +1,7 @@ import { error, json } from '@sveltejs/kit'; import { z } from 'zod'; -import { getUserId } from '$lib/server/auth-utils.js'; +import { findTrinoUser, getUserId } from '$lib/server/auth-utils.js'; +import * as m from '$lib/paraglide/messages.js'; import { startScript, getQuerySnapshots, @@ -33,7 +34,11 @@ export const POST: RequestHandler = async ({ request, locals }) => { error(400, z.prettifyError(parsed.error)); } - const user = locals.user?.username ?? 'anonymous'; + // Return the error as JSON so the query runner shows it instead of "connection lost". + const user = findTrinoUser(locals); + if (!user) { + return json({ error: m.trino_user_missing() }, { status: 500 }); + } const { statements, tabId, catalog, schema } = parsed.data; // Fire-and-forget — the script orchestrator runs in the background. diff --git a/src/routes/(app)/api/trino/query/server.test.ts b/src/routes/(app)/api/trino/query/server.test.ts index 17ad92e3..8a9df2b1 100644 --- a/src/routes/(app)/api/trino/query/server.test.ts +++ b/src/routes/(app)/api/trino/query/server.test.ts @@ -25,7 +25,7 @@ const TAB_ID = '6f1c2a4e-8b3d-4c5e-9f7a-1b2c3d4e5f60'; const fakeClient = { fake: true } as unknown as TrinoClient; function mockLocals( - user: { id: string; username?: string } | null = { id: 'u1', username: 'alice' } + user: { id: string; trinoUser?: string } | null = { id: 'u1', trinoUser: 'alice' } ) { return { logger: { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, @@ -114,6 +114,16 @@ describe('POST /api/trino/query', () => { }); }); + it('returns a JSON error instead of running as anonymous when the Trino user is missing', async () => { + const res = await POST( + postEvent({ statements: ['SELECT 1'], tabId: TAB_ID }, mockLocals({ id: 'u1' })) + ); + + expect(res.status).toBe(500); + expect(await res.json()).toEqual({ error: expect.any(String) }); + expect(startScript).not.toHaveBeenCalled(); + }); + it('logs but does not fail the request when the script orchestrator rejects', async () => { const err = new Error('boom'); vi.mocked(startScript).mockRejectedValue(err); diff --git a/src/routes/(app)/trino/+page.server.ts b/src/routes/(app)/trino/+page.server.ts index 3affaae5..a9649095 100644 --- a/src/routes/(app)/trino/+page.server.ts +++ b/src/routes/(app)/trino/+page.server.ts @@ -1,7 +1,7 @@ import { fail } from '@sveltejs/kit'; import { superValidate, message, type Infer, type InferIn } from 'sveltekit-superforms'; import { zod4 as zod } from 'sveltekit-superforms/adapters'; -import { getUserId } from '$lib/server/auth-utils.js'; +import { findTrinoUser, getUserId } from '$lib/server/auth-utils.js'; import { getAllQuerySummaries, resetTabQueries } from '$lib/server/trino/queries.js'; import { trinoConfigured, @@ -56,7 +56,14 @@ export const actions: Actions = { } const userId = getUserId(locals); - const user = locals.user?.username ?? 'anonymous'; + const user = findTrinoUser(locals); + if (!user) { + return message( + form, + { type: 'error', message: m.trino_user_missing() } satisfies ConnectionMessage, + { status: 500 } + ); + } const { connectionUrl, authType, authUsername, authPassword } = form.data; const config = { url: connectionUrl,