Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions website/src/auth.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -14,8 +17,9 @@ export const auth = betterAuth({
},
user: {
additionalFields: {
githubId: {
type: 'string',
gsUserId: {
type: 'number',
required: true,
input: false,
},
},
Expand All @@ -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: {
Expand Down
8 changes: 4 additions & 4 deletions website/src/backendApi/backendProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const API_PATHNAME_LENGTH = '/api'.length;
* in here, instead of in the backend.
*/
export async function proxyToBackend(context: APIContext): Promise<Response> {
const userId = context.locals.user?.githubId;
const userId = context.locals.gsUserId;

if (userId === undefined) {
return getUnauthorizedResponse(context.request.url);
Expand All @@ -31,7 +31,7 @@ export async function proxyToBackendNoAuth(context: APIContext): Promise<Respons
return proxyRequest(context.request, undefined);
}

async function proxyRequest(request: Request, userId: string | undefined): Promise<Response> {
async function proxyRequest(request: Request, userId: number | undefined): Promise<Response> {
const backendUrl = getBackendUrl(request, userId);

try {
Expand All @@ -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());

Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion website/src/backendApi/backendService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down
5 changes: 5 additions & 0 deletions website/src/backendApi/backendService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -56,7 +58,7 @@ function CollectionDetailInner({
</div>
{collection.description !== null && <p className='mt-1 text-gray-500'>{collection.description}</p>}
<p className='mt-1 text-sm text-gray-500'>
{organismName} collection owned by {collection.ownedBy}
{organismName} collection owned by {ownerName}
</p>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const mockCollections = [
{
id: 1,
name: 'My first collection',
ownedBy: 'user1',
ownedBy: 1,
organism: ORGANISM,
description: 'A test collection',
variants: [
Expand Down Expand Up @@ -48,7 +48,7 @@ const mockCollections = [
{
id: 2,
name: 'Another collection',
ownedBy: 'user1',
ownedBy: 1,
organism: ORGANISM,
description: null,
variants: [],
Expand Down
7 changes: 4 additions & 3 deletions website/src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Awaited<ReturnType<(typeof import('./auth').auth)['api']['getSession']>>>['user'];

interface Locals {
user: AuthUser | null;
session: import('better-auth').Session | null;
user: AuthUser | undefined;
session: import('better-auth').Session | undefined;
gsUserId: number | undefined;
}
}

Expand Down
2 changes: 1 addition & 1 deletion website/src/layouts/base/header/HamburgerMenu.astro
Original file line number Diff line number Diff line change
Expand Up @@ -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();
---

Expand Down
6 changes: 4 additions & 2 deletions website/src/middleware/authMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions website/src/pages/api/users/[id].ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { proxyToBackendNoAuth } from '../../../backendApi/backendProxy.ts';

export const GET = proxyToBackendNoAuth;
2 changes: 1 addition & 1 deletion website/src/pages/collections/[organism]/[id]/edit.astro
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
27 changes: 22 additions & 5 deletions website/src/pages/collections/[organism]/[id]/index.astro
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
---

<ContaineredPageLayout
Expand All @@ -61,7 +77,8 @@ if (collection !== undefined && collection.organism !== parsedOrganism.data) {
organism={parsedOrganism.data}
collection={collection}
lapisConfig={lapisConfig}
isOwner={currentUserId !== undefined && currentUserId === collection.ownedBy}
isOwner={userIsOwner}
ownerName={ownerName}
client:load
/>
) : (
Expand Down
2 changes: 1 addition & 1 deletion website/src/pages/collections/[organism]/create.astro
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const config = getDashboardsConfig();
]}
>
{
Astro.locals.user !== null ? (
Astro.locals.user !== undefined ? (
<CollectionCreate client:only='react' organism={parsedOrganism.data} config={config} />
) : (
<NotLoggedIn />
Expand Down
2 changes: 1 addition & 1 deletion website/src/pages/collections/[organism]/index.astro
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (!parsedOrganism.success) {
}

const orgConfig = organismConfig[parsedOrganism.data];
const isLoggedIn = Astro.locals.user !== null;
const isLoggedIn = Astro.locals.user !== undefined;
---

<ContaineredPageLayout
Expand Down
2 changes: 1 addition & 1 deletion website/src/pages/subscriptions/create.astro
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ const config = getDashboardsConfig();
},
]}
>
{Astro.locals.user !== null ? <SubscriptionsCreate config={config} client:only='react' /> : <NotLoggedIn />}
{Astro.locals.user !== undefined ? <SubscriptionsCreate config={config} client:only='react' /> : <NotLoggedIn />}
</SubscriptionPageLayout>
2 changes: 1 addition & 1 deletion website/src/pages/subscriptions/index.astro
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ const breadcrumbs = [
---

<SubscriptionPageLayout title='Subscriptions overview' breadcrumbs={breadcrumbs}>
{Astro.locals.user !== null ? <Subscriptions client:load organismsFromUrl={organisms} /> : <NotLoggedIn />}
{Astro.locals.user !== undefined ? <Subscriptions client:load organismsFromUrl={organisms} /> : <NotLoggedIn />}
</SubscriptionPageLayout>
2 changes: 1 addition & 1 deletion website/src/types/Collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 8 additions & 0 deletions website/src/types/PublicUser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { z } from 'zod';

export const publicUserSchema = z.object({
id: z.number(),
name: z.string(),
});

export type PublicUser = z.infer<typeof publicUserSchema>;
14 changes: 11 additions & 3 deletions website/tests/collections/collectionDetail.spec.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -28,14 +28,22 @@ 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');
return collectionId;
}

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);
Expand All @@ -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);
});

Expand Down
Loading
Loading