From 02444deb3e5aefbeb4bc610dcdb9d8c918534139 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Wed, 6 May 2026 13:53:42 +0200 Subject: [PATCH] feat(website): sync user with backend on login, use internal user ID for ownership On GitHub login, mapProfileToUser calls POST /users/sync to upsert the user in the backend and stores the returned internal Long ID as internalUserId in the better-auth session (stateless JWE cookie). This replaces the previous use of the GitHub ID for ownership checks on collections and subscriptions. - auth.ts: async mapProfileToUser syncs user, adds internalUserId additional field - authMiddleware.ts: reads internalUserId from session user - backendProxy.ts: forwards internalUserId as userId query param instead of githubId - Collection.ts: ownedBy changed from string to number to match backend Long - PublicUser.ts: new Zod schema for GET /users/{id} response - backendService.ts: adds getUser() to resolve owner names - pages/api/users/[id].ts: new proxy route for public user lookup - collection detail/edit pages: use internalUserId for ownership, display owner name - E2E tests: sync user via POST /users/sync in beforeAll to get internal ID Co-Authored-By: Claude Sonnet 4.6 fix(website): fix lint errors in auth.ts and authMiddleware.ts Co-Authored-By: Claude Sonnet 4.6 refactor(website): rename internalUserId to gsUserId Co-Authored-By: Claude Sonnet 4.6 fix(website): make login fail if backend user sync fails, simplify gsUserId handling Co-Authored-By: Claude Sonnet 4.6 fix(website): make ownerName non-optional, simplify collection detail page Co-Authored-By: Claude Sonnet 4.6 refactor(website): simplify gsUserId assignment in authMiddleware Co-Authored-By: Claude Sonnet 4.6 foo --- website/src/auth.ts | 40 +++++++++++++++---- website/src/backendApi/backendProxy.ts | 8 ++-- website/src/backendApi/backendService.spec.ts | 2 +- website/src/backendApi/backendService.ts | 5 +++ .../collections/detail/CollectionDetail.tsx | 4 +- .../CollectionsOverview.browser.spec.tsx | 4 +- website/src/env.d.ts | 7 ++-- .../layouts/base/header/HamburgerMenu.astro | 2 +- website/src/middleware/authMiddleware.ts | 6 ++- website/src/pages/api/users/[id].ts | 3 ++ .../collections/[organism]/[id]/edit.astro | 2 +- .../collections/[organism]/[id]/index.astro | 27 ++++++++++--- .../pages/collections/[organism]/create.astro | 2 +- .../pages/collections/[organism]/index.astro | 2 +- website/src/pages/subscriptions/create.astro | 2 +- website/src/pages/subscriptions/index.astro | 2 +- website/src/types/Collection.ts | 2 +- website/src/types/PublicUser.ts | 8 ++++ .../collections/collectionDetail.spec.ts | 14 +++++-- .../tests/collections/collectionForm.spec.ts | 15 +++++-- website/tests/helpers/auth.ts | 2 +- 21 files changed, 118 insertions(+), 41 deletions(-) create mode 100644 website/src/pages/api/users/[id].ts create mode 100644 website/src/types/PublicUser.ts diff --git a/website/src/auth.ts b/website/src/auth.ts index 83f1d0a86..11efb03f8 100644 --- a/website/src/auth.ts +++ b/website/src/auth.ts @@ -1,6 +1,9 @@ import { betterAuth } from 'better-auth'; -import { getGitHubClientId, getGitHubClientSecret } from './config'; +import { getBackendHost, getGitHubClientId, getGitHubClientSecret } from './config'; +import { getInstanceLogger } from './logger'; + +const logger = getInstanceLogger('Auth'); export const auth = betterAuth({ // TODO - maybe we can check again if this is read automatically? Should be, according to the docs. @@ -14,8 +17,9 @@ export const auth = betterAuth({ }, user: { additionalFields: { - githubId: { - type: 'string', + gsUserId: { + type: 'number', + required: true, input: false, }, }, @@ -24,11 +28,31 @@ export const auth = betterAuth({ github: { clientId: getGitHubClientId(), clientSecret: getGitHubClientSecret(), - // store the GitHub user ID (i.e. 45882389) in the 'user' object, - // which is easy to access from context later on. - // the 'id' property cannot be overwritten. - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-conversion -- profile.id is typed as string but GitHub returns a number at runtime; String() ensures it's always stored as a string for consistent ownership checks - mapProfileToUser: (profile) => ({ githubId: String(profile.id) }), + mapProfileToUser: async (profile) => { + try { + const response = await fetch(`${getBackendHost()}/users/sync`, { + method: 'POST', + // eslint-disable-next-line @typescript-eslint/naming-convention + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-conversion -- profile.id is typed as string but GitHub returns a number at runtime + githubId: String(profile.id), + name: profile.name || profile.login || '', + email: profile.email ?? null, + }), + }); + + if (response.ok) { + const user = (await response.json()) as { id: number }; + return { gsUserId: String(user.id) }; + } + + throw new Error(`Failed to sync user with backend: HTTP ${response.status}`); + } catch (error) { + logger.error(`Failed to sync user with backend: ${error}`); + throw error; + } + }, }, }, advanced: { diff --git a/website/src/backendApi/backendProxy.ts b/website/src/backendApi/backendProxy.ts index c6ec6923f..31a06d3df 100644 --- a/website/src/backendApi/backendProxy.ts +++ b/website/src/backendApi/backendProxy.ts @@ -15,7 +15,7 @@ const API_PATHNAME_LENGTH = '/api'.length; * in here, instead of in the backend. */ export async function proxyToBackend(context: APIContext): Promise { - const userId = context.locals.user?.githubId; + const userId = context.locals.gsUserId; if (userId === undefined) { return getUnauthorizedResponse(context.request.url); @@ -31,7 +31,7 @@ export async function proxyToBackendNoAuth(context: APIContext): Promise { +async function proxyRequest(request: Request, userId: number | undefined): Promise { const backendUrl = getBackendUrl(request, userId); try { @@ -47,7 +47,7 @@ async function proxyRequest(request: Request, userId: string | undefined): Promi } } -function getBackendUrl(request: Request, userId: string | undefined) { +function getBackendUrl(request: Request, userId: number | undefined) { const backendEndpoint = new URL(request.url).pathname.slice(API_PATHNAME_LENGTH); const backendUrl = new URL(backendEndpoint, getBackendHost()); @@ -56,7 +56,7 @@ function getBackendUrl(request: Request, userId: string | undefined) { }); if (userId !== undefined) { - backendUrl.searchParams.set('userId', userId); + backendUrl.searchParams.set('userId', String(userId)); } return backendUrl; diff --git a/website/src/backendApi/backendService.spec.ts b/website/src/backendApi/backendService.spec.ts index baa26e73c..5d524cd8d 100644 --- a/website/src/backendApi/backendService.spec.ts +++ b/website/src/backendApi/backendService.spec.ts @@ -175,7 +175,7 @@ describe('backendService', () => { const aCollection: Collection = { id: 1, name: 'Test collection', - ownedBy: 'user123', + ownedBy: 123, organism: 'covid', description: 'A test collection', variants: [], diff --git a/website/src/backendApi/backendService.ts b/website/src/backendApi/backendService.ts index 900552612..851d091f8 100644 --- a/website/src/backendApi/backendService.ts +++ b/website/src/backendApi/backendService.ts @@ -4,6 +4,7 @@ import { z, type ZodSchema } from 'zod'; import { UserFacingError } from '../components/ErrorReportInstruction.tsx'; import { collectionSchema, type CollectionRequest, type CollectionUpdate } from '../types/Collection.ts'; import { type ProblemDetail, problemDetailSchema } from '../types/ProblemDetail.ts'; +import { publicUserSchema } from '../types/PublicUser.ts'; import { type SubscriptionPutRequest, type SubscriptionRequest, @@ -166,6 +167,10 @@ export class BackendService extends ApiService { }); } + public async getUser({ id }: { id: number }) { + return this.get({ url: `/users/${id}`, schema: publicUserSchema }); + } + public async getCollections({ organism }: { organism?: string } = {}) { const requestParams = organism !== undefined ? { organism } : undefined; return this.get({ url: '/collections', requestParams, schema: z.array(collectionSchema) }); diff --git a/website/src/components/collections/detail/CollectionDetail.tsx b/website/src/components/collections/detail/CollectionDetail.tsx index 4cc3bbf0f..1c221dad8 100644 --- a/website/src/components/collections/detail/CollectionDetail.tsx +++ b/website/src/components/collections/detail/CollectionDetail.tsx @@ -27,11 +27,13 @@ function CollectionDetailInner({ collection, lapisConfig, isOwner, + ownerName, }: { organism: Organism; collection: Collection; lapisConfig: LapisConfig; isOwner: boolean; + ownerName: string; }) { const organismName = organismConfig[organism].label; const dateFrom30 = dayjs().subtract(30, 'day').format('YYYY-MM-DD'); @@ -56,7 +58,7 @@ function CollectionDetailInner({ {collection.description !== null &&

{collection.description}

}

- {organismName} collection owned by {collection.ownedBy} + {organismName} collection owned by {ownerName}

diff --git a/website/src/components/collections/overview/CollectionsOverview.browser.spec.tsx b/website/src/components/collections/overview/CollectionsOverview.browser.spec.tsx index 4c72eab23..0a3e0b4e2 100644 --- a/website/src/components/collections/overview/CollectionsOverview.browser.spec.tsx +++ b/website/src/components/collections/overview/CollectionsOverview.browser.spec.tsx @@ -12,7 +12,7 @@ const mockCollections = [ { id: 1, name: 'My first collection', - ownedBy: 'user1', + ownedBy: 1, organism: ORGANISM, description: 'A test collection', variants: [ @@ -48,7 +48,7 @@ const mockCollections = [ { id: 2, name: 'Another collection', - ownedBy: 'user1', + ownedBy: 1, organism: ORGANISM, description: null, variants: [], diff --git a/website/src/env.d.ts b/website/src/env.d.ts index c104ca02b..8dbcfb1b9 100644 --- a/website/src/env.d.ts +++ b/website/src/env.d.ts @@ -3,12 +3,13 @@ declare namespace App { // Note: 'import {} from ""' syntax does not work in .d.ts files. - // We derive the User type from the auth config so that additionalFields (e.g. githubId) are included. + // We derive the User type from the auth config so that additionalFields (e.g. gsUserId) are included. type AuthUser = NonNullable>>['user']; interface Locals { - user: AuthUser | null; - session: import('better-auth').Session | null; + user: AuthUser | undefined; + session: import('better-auth').Session | undefined; + gsUserId: number | undefined; } } diff --git a/website/src/layouts/base/header/HamburgerMenu.astro b/website/src/layouts/base/header/HamburgerMenu.astro index b814e76cd..11c378110 100644 --- a/website/src/layouts/base/header/HamburgerMenu.astro +++ b/website/src/layouts/base/header/HamburgerMenu.astro @@ -14,7 +14,7 @@ const { forceLoggedOutState } = Astro.props; const pathogenMegaMenuSections = Object.values(getPathogenMegaMenuSections()); -const showLoggedInState = !forceLoggedOutState && Astro.locals.user !== null; +const showLoggedInState = !forceLoggedOutState && Astro.locals.user !== undefined; const showSubscriptions = isStaging(); --- diff --git a/website/src/middleware/authMiddleware.ts b/website/src/middleware/authMiddleware.ts index 2dbf1db3e..f279c2c33 100644 --- a/website/src/middleware/authMiddleware.ts +++ b/website/src/middleware/authMiddleware.ts @@ -10,9 +10,11 @@ export const authMiddleware = defineMiddleware(async (context, next) => { if (session) { context.locals.user = session.user; context.locals.session = session.session; + context.locals.gsUserId = session.user.gsUserId; } else { - context.locals.user = null; - context.locals.session = null; + context.locals.user = undefined; + context.locals.session = undefined; + context.locals.gsUserId = undefined; } return next(); diff --git a/website/src/pages/api/users/[id].ts b/website/src/pages/api/users/[id].ts new file mode 100644 index 000000000..4aeea0e90 --- /dev/null +++ b/website/src/pages/api/users/[id].ts @@ -0,0 +1,3 @@ +import { proxyToBackendNoAuth } from '../../../backendApi/backendProxy.ts'; + +export const GET = proxyToBackendNoAuth; diff --git a/website/src/pages/collections/[organism]/[id]/edit.astro b/website/src/pages/collections/[organism]/[id]/edit.astro index f981da0d2..cb950f1e8 100644 --- a/website/src/pages/collections/[organism]/[id]/edit.astro +++ b/website/src/pages/collections/[organism]/[id]/edit.astro @@ -24,7 +24,7 @@ if (id === undefined) { } const orgConfig = organismConfig[parsedOrganism.data]; -const currentUserId = Astro.locals.user?.githubId; +const currentUserId = Astro.locals.gsUserId; const config = getDashboardsConfig(); let collection; diff --git a/website/src/pages/collections/[organism]/[id]/index.astro b/website/src/pages/collections/[organism]/[id]/index.astro index 90338e683..139d87721 100644 --- a/website/src/pages/collections/[organism]/[id]/index.astro +++ b/website/src/pages/collections/[organism]/[id]/index.astro @@ -26,8 +26,6 @@ if (id === undefined) { const orgConfig = organismConfig[parsedOrganism.data]; const lapisConfig = getOrganismConfig(parsedOrganism.data).lapis; -const currentUserId = Astro.locals.user?.githubId; - let collection: Collection | undefined; try { @@ -39,11 +37,29 @@ try { logger.error(`Failed to fetch collection ${id}: ${getErrorLogMessage(error)}`); } -const collectionTitle = collection !== undefined ? `#${id} ${collection.name}` : `Collection #${id}`; +if (collection === undefined) { + return Astro.redirect('/404'); +} -if (collection !== undefined && collection.organism !== parsedOrganism.data) { +// It's possible a collection with the ID exists, but the organism in the path is different. +// In that case, we want to treat this as 'not found'. +if (collection.organism !== parsedOrganism.data) { return Astro.redirect('/404'); } + +const collectionTitle = `#${id} ${collection.name}`; + +let ownerName = String(collection.ownedBy); +try { + const owner = await new BackendService(getBackendHost()).getUser({ id: collection.ownedBy }); + ownerName = owner.name; +} catch (error) { + logger.warn(`Failed to fetch owner for collection ${id}: ${getErrorLogMessage(error)}`); +} + +const currentUserId = Astro.locals.gsUserId; +const userIsLoggedIn = currentUserId !== undefined; +const userIsOwner = userIsLoggedIn && currentUserId === collection.ownedBy; --- ) : ( diff --git a/website/src/pages/collections/[organism]/create.astro b/website/src/pages/collections/[organism]/create.astro index 59319a295..7b655475b 100644 --- a/website/src/pages/collections/[organism]/create.astro +++ b/website/src/pages/collections/[organism]/create.astro @@ -28,7 +28,7 @@ const config = getDashboardsConfig(); ]} > { - Astro.locals.user !== null ? ( + Astro.locals.user !== undefined ? ( ) : ( diff --git a/website/src/pages/collections/[organism]/index.astro b/website/src/pages/collections/[organism]/index.astro index bb98bb7ba..f4999df09 100644 --- a/website/src/pages/collections/[organism]/index.astro +++ b/website/src/pages/collections/[organism]/index.astro @@ -19,7 +19,7 @@ if (!parsedOrganism.success) { } const orgConfig = organismConfig[parsedOrganism.data]; -const isLoggedIn = Astro.locals.user !== null; +const isLoggedIn = Astro.locals.user !== undefined; --- - {Astro.locals.user !== null ? : } + {Astro.locals.user !== undefined ? : } diff --git a/website/src/pages/subscriptions/index.astro b/website/src/pages/subscriptions/index.astro index 19c7a7e62..cf9eabca6 100644 --- a/website/src/pages/subscriptions/index.astro +++ b/website/src/pages/subscriptions/index.astro @@ -18,5 +18,5 @@ const breadcrumbs = [ --- - {Astro.locals.user !== null ? : } + {Astro.locals.user !== undefined ? : } diff --git a/website/src/types/Collection.ts b/website/src/types/Collection.ts index 25611a263..eb029b4a3 100644 --- a/website/src/types/Collection.ts +++ b/website/src/types/Collection.ts @@ -33,7 +33,7 @@ const variantSchema = z.discriminatedUnion('type', [queryVariantSchema, filterOb export const collectionSchema = z.object({ id: z.number(), name: z.string(), - ownedBy: z.string(), + ownedBy: z.number(), organism: z.string(), description: z.string().nullable(), variants: z.array(variantSchema), diff --git a/website/src/types/PublicUser.ts b/website/src/types/PublicUser.ts new file mode 100644 index 000000000..305232d8e --- /dev/null +++ b/website/src/types/PublicUser.ts @@ -0,0 +1,8 @@ +import { z } from 'zod'; + +export const publicUserSchema = z.object({ + id: z.number(), + name: z.string(), +}); + +export type PublicUser = z.infer; diff --git a/website/tests/collections/collectionDetail.spec.ts b/website/tests/collections/collectionDetail.spec.ts index 884dbb658..312dbe36c 100644 --- a/website/tests/collections/collectionDetail.spec.ts +++ b/website/tests/collections/collectionDetail.spec.ts @@ -1,9 +1,9 @@ import { expect } from '@playwright/test'; import { test } from '../e2e.fixture.ts'; +import { E2E_GITHUB_ID } from '../helpers/auth.ts'; const BACKEND_URL = process.env.BACKEND_URL ?? 'http://localhost:8080'; -const USER_ID = 'e2e-test'; const ORGANISM = 'covid'; const ORGANISM_LABEL = 'SARS-CoV-2'; @@ -28,6 +28,7 @@ const TEST_COLLECTION = { }; let collectionId: number | undefined; +let userId: number; function getCollectionId(): number { if (collectionId === undefined) throw new Error('collectionId was not set in beforeAll'); @@ -35,7 +36,14 @@ function getCollectionId(): number { } test.beforeAll(async ({ request }) => { - const response = await request.post(`${BACKEND_URL}/collections?userId=${USER_ID}`, { + const syncResponse = await request.post(`${BACKEND_URL}/users/sync`, { + data: { githubId: E2E_GITHUB_ID, name: 'e2e-test', email: null }, + }); + expect(syncResponse.status()).toBe(200); + const userBody = (await syncResponse.json()) as { id: number }; + userId = userBody.id; + + const response = await request.post(`${BACKEND_URL}/collections?userId=${userId}`, { data: TEST_COLLECTION, }); expect(response.status()).toBe(201); @@ -47,7 +55,7 @@ test.afterAll(async ({ request }) => { if (collectionId === undefined) { return; } - const response = await request.delete(`${BACKEND_URL}/collections/${collectionId}?userId=${USER_ID}`); + const response = await request.delete(`${BACKEND_URL}/collections/${collectionId}?userId=${userId}`); expect(response.status()).toBe(204); }); diff --git a/website/tests/collections/collectionForm.spec.ts b/website/tests/collections/collectionForm.spec.ts index 386bdffa0..a66189719 100644 --- a/website/tests/collections/collectionForm.spec.ts +++ b/website/tests/collections/collectionForm.spec.ts @@ -4,7 +4,6 @@ import { test } from '../e2e.fixture.ts'; import { E2E_GITHUB_ID } from '../helpers/auth.ts'; const BACKEND_URL = process.env.BACKEND_URL ?? 'http://localhost:8080'; -const USER_ID = E2E_GITHUB_ID; const ORGANISM = 'covid'; const SEED_COLLECTION = { @@ -28,6 +27,7 @@ const SEED_COLLECTION = { }; let collectionId: number | undefined; +let userId: number; function getCollectionId(): number { if (collectionId === undefined) throw new Error('collectionId was not set in beforeAll'); @@ -35,7 +35,14 @@ function getCollectionId(): number { } test.beforeAll(async ({ request }) => { - const response = await request.post(`${BACKEND_URL}/collections?userId=${USER_ID}`, { + const syncResponse = await request.post(`${BACKEND_URL}/users/sync`, { + data: { githubId: E2E_GITHUB_ID, name: 'e2e-test', email: null }, + }); + expect(syncResponse.status()).toBe(200); + const userBody = (await syncResponse.json()) as { id: number }; + userId = userBody.id; + + const response = await request.post(`${BACKEND_URL}/collections?userId=${userId}`, { data: SEED_COLLECTION, }); expect(response.status()).toBe(201); @@ -47,7 +54,7 @@ test.afterAll(async ({ request }) => { if (collectionId === undefined) { return; } - const response = await request.delete(`${BACKEND_URL}/collections/${collectionId}?userId=${USER_ID}`); + const response = await request.delete(`${BACKEND_URL}/collections/${collectionId}?userId=${userId}`); expect(response.status()).toBe(204); }); @@ -105,7 +112,7 @@ test.describe('New collection page', () => { const url = authenticatedCollectionFormPage.page.url(); const id = /\/collections\/\w+\/(\d+)$/.exec(url)?.[1]; if (id !== undefined) { - await request.delete(`${BACKEND_URL}/collections/${id}?userId=${USER_ID}`); + await request.delete(`${BACKEND_URL}/collections/${id}?userId=${userId}`); } }); }); diff --git a/website/tests/helpers/auth.ts b/website/tests/helpers/auth.ts index 95db5f140..c49f3646c 100644 --- a/website/tests/helpers/auth.ts +++ b/website/tests/helpers/auth.ts @@ -72,7 +72,7 @@ export async function setupAuthCookie(page: Page, name: string): Promise { image: null, createdAt: now.toISOString(), updatedAt: now.toISOString(), - githubId: '1234567', + gsUserId: '1', }, updatedAt: now.getTime(), version: '1',