Skip to content
Merged
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
22 changes: 20 additions & 2 deletions apps/backend/src/middleware/auth.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
import type { Context, Next } from 'hono';
import { getAuth } from '../lib/auth.js';
import { getLegacyExternalIdByEmail } from '../models/queries/users.queries.js';
import { AuthenticationError } from '../utils/errors.js';

export interface AuthContext {
/** Legacy auth.users.external_id (UUID) — used by domain queries (friends, circles, etc.) */
userId: string;
/** Better Auth user.id (opaque string) — used by auth."user" queries (self-profile, preferences) */
betterAuthId: string;
email: string;
}

/**
* Middleware to authenticate requests using Better Auth sessions.
* Validates the session cookie via Better Auth's API and stores the
* full session in context so handlers don't need a second lookup.
*
* Resolves both the Better Auth user ID and the legacy auth.users
* external_id, since domain queries (friends, encounters, etc.)
* reference auth.users.external_id which is UUID-typed.
*/
export async function authMiddleware(c: Context, next: Next) {
const session = await getAuth().api.getSession({
Expand All @@ -21,9 +29,19 @@ export async function authMiddleware(c: Context, next: Next) {
throw new AuthenticationError('Unauthorized');
}

// Set user info to context (matching existing AuthContext shape)
// Resolve the legacy auth.users.external_id from the email.
// Better Auth user IDs are opaque strings, but domain tables
// (friends, encounters, etc.) join via auth.users.external_id (UUID).
const db = c.get('db');
const [legacyUser] = await getLegacyExternalIdByEmail.run({ email: session.user.email }, db);

if (!legacyUser) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance: Extra DB Round-Trip on Every Authenticated Request

getLegacyExternalIdByEmail is called inside authMiddleware, which runs on every protected route. This adds a synchronous DB lookup to every single authenticated request.

Two mitigations to consider:

  1. Add an index on auth.users(email) if one doesn't exist — keeps the query fast even at scale.
  2. Longer-term: Consider caching the email → external_id mapping in the session context or resolving it at registration time (e.g. store the legacy external_id directly on the Better Auth user record), so the per-request overhead disappears entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed both items:

  1. Index: auth.users(email) already has a UNIQUE constraint and an explicit index (idx_users_email) from the initial schema migration — no additional indexing needed.
  2. as any cast: Replaced with {} as unknown as pg.Pool in b1130c5.

Agree on the longer-term approach of resolving the legacy UUID at registration time to eliminate the per-request lookup.

throw new AuthenticationError('User account not fully provisioned');
}

c.set('user', {
userId: session.user.id,
userId: legacyUser.external_id,
betterAuthId: session.user.id,
email: session.user.email,
});

Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/middleware/onboarding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export async function onboardingMiddleware(c: Context, next: Next) {
throw new AuthenticationError('Unauthorized');
}

const result = await hasSelfProfile.run({ userExternalId: authUser.userId }, db);
const result = await hasSelfProfile.run({ userExternalId: authUser.betterAuthId }, db);

if (!result[0]?.has_self_profile) {
logger.info({ userId: authUser.userId }, 'User has not completed onboarding');
Expand Down
28 changes: 28 additions & 0 deletions apps/backend/src/models/queries/users.queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,34 @@ const setUserSelfProfileIR: any = {"usedParamSet":{"userExternalId":true,"friend
export const setUserSelfProfile = new PreparedQuery<ISetUserSelfProfileParams,ISetUserSelfProfileResult>(setUserSelfProfileIR);


/** 'GetLegacyExternalIdByEmail' parameters type */
export interface IGetLegacyExternalIdByEmailParams {
email?: string | null | void;
}

/** 'GetLegacyExternalIdByEmail' return type */
export interface IGetLegacyExternalIdByEmailResult {
/** Public UUID for API exposure (always use this in APIs) */
external_id: string;
}

/** 'GetLegacyExternalIdByEmail' query type */
export interface IGetLegacyExternalIdByEmailQuery {
params: IGetLegacyExternalIdByEmailParams;
result: IGetLegacyExternalIdByEmailResult;
}

const getLegacyExternalIdByEmailIR: any = {"usedParamSet":{"email":true},"params":[{"name":"email","required":false,"transform":{"type":"scalar"},"locs":[{"a":49,"b":54}]}],"statement":"SELECT external_id FROM auth.users WHERE email = :email"};

/**
* Query generated from SQL:
* ```
* SELECT external_id FROM auth.users WHERE email = :email
* ```
*/
export const getLegacyExternalIdByEmail = new PreparedQuery<IGetLegacyExternalIdByEmailParams,IGetLegacyExternalIdByEmailResult>(getLegacyExternalIdByEmailIR);


/** 'CreateLegacyUserForBetterAuth' parameters type */
export interface ICreateLegacyUserForBetterAuthParams {
email?: string | null | void;
Expand Down
3 changes: 3 additions & 0 deletions apps/backend/src/models/queries/users.sql
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ WHERE ba_u.id = :userExternalId
AND c.deleted_at IS NULL
RETURNING ba_u.id as external_id, c.external_id as self_profile_external_id;

/* @name GetLegacyExternalIdByEmail */
SELECT external_id FROM auth.users WHERE email = :email;

/* @name CreateLegacyUserForBetterAuth */
INSERT INTO auth.users (email, password_hash)
VALUES (:email, '')
Expand Down
6 changes: 3 additions & 3 deletions apps/backend/src/routes/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ app.get('/me', authMiddleware, async (c) => {

const response: UserWithPreferencesResponse = {
user: {
externalId: authUser.userId,
externalId: authUser.betterAuthId,
email: authUser.email,
selfProfileId: selfProfileExternalId ?? undefined,
hasCompletedOnboarding: selfProfileExternalId !== null,
Expand Down Expand Up @@ -103,7 +103,7 @@ app.patch('/preferences', authMiddleware, async (c) => {
}

// Get current preferences
const users = await getUserWithPreferences.run({ externalId: authUser.userId }, db);
const users = await getUserWithPreferences.run({ externalId: authUser.betterAuthId }, db);

if (users.length === 0) {
throw new UserNotFoundError();
Expand All @@ -124,7 +124,7 @@ app.patch('/preferences', authMiddleware, async (c) => {
// Update in database
const result = await updateUserPreferences.run(
{
externalId: authUser.userId,
externalId: authUser.betterAuthId,
preferences: toJson(newPreferences),
},
db,
Expand Down
15 changes: 9 additions & 6 deletions apps/backend/src/routes/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ app.get('/me/self-profile', async (c) => {
const db = c.get('db');

const authUser = getAuthUser(c);
const result = await getUserSelfProfile.run({ userExternalId: authUser.userId }, db);
const result = await getUserSelfProfile.run({ userExternalId: authUser.betterAuthId }, db);
const selfProfileExternalId = result[0]?.self_profile_external_id ?? null;

return c.json({ selfProfileId: selfProfileExternalId });
Expand All @@ -149,7 +149,7 @@ app.put('/me/self-profile', async (c) => {

const result = await setUserSelfProfile.run(
{
userExternalId: authUser.userId,
userExternalId: authUser.betterAuthId,
friendExternalId: validated.friendId,
},
db,
Expand Down Expand Up @@ -186,19 +186,22 @@ app.post('/me/self-profile', async (c) => {
}

// Check if user already has a self-profile
const existingResult = await getUserSelfProfile.run({ userExternalId: authUser.userId }, db);
const existingResult = await getUserSelfProfile.run(
{ userExternalId: authUser.betterAuthId },
db,
);
if (existingResult[0]?.self_profile_external_id) {
throw new ValidationError('Self-profile already exists');
}

// Create the friend
// Create the friend (uses legacy auth.users.external_id)
const friendsService = new FriendsService(db, logger);
const newFriend = await friendsService.createFriend(authUser.userId, validated);

// Set it as the self-profile
// Set it as the self-profile (uses Better Auth user.id)
const setResult = await setUserSelfProfile.run(
{
userExternalId: authUser.userId,
userExternalId: authUser.betterAuthId,
friendExternalId: newFriend.id,
},
db,
Expand Down
58 changes: 50 additions & 8 deletions apps/backend/tests/middleware/auth.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { Hono } from 'hono';
import type pg from 'pg';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { authMiddleware, getAuthUser } from '../../src/middleware/auth.js';
import type { AppContext } from '../../src/types/context.js';
import { isAppError } from '../../src/utils/errors.js';

const mockGetSession = vi.fn();
const mockGetLegacyExternalId = vi.fn();

// Mock the Better Auth module
vi.mock('../../src/lib/auth.ts', () => ({
Expand All @@ -14,14 +17,25 @@ vi.mock('../../src/lib/auth.ts', () => ({
}),
}));

// Mock the legacy external ID lookup
vi.mock('../../src/models/queries/users.queries.ts', () => ({
getLegacyExternalIdByEmail: {
run: (...args: unknown[]) => mockGetLegacyExternalId(...args),
},
}));

describe('authMiddleware', () => {
let app: Hono;
let app: Hono<AppContext>;

beforeEach(() => {
app = new Hono();
app = new Hono<AppContext>();
vi.clearAllMocks();

// Setup test route with auth middleware
// Setup db context and test route with auth middleware
app.use('/protected/*', async (c, next) => {
c.set('db', {} as unknown as pg.Pool); // mock db pool
return next();
});
app.use('/protected/*', authMiddleware);
app.get('/protected/resource', (c) => {
const user = getAuthUser(c);
Expand All @@ -43,6 +57,7 @@ describe('authMiddleware', () => {
user: { id: 'user-123', email: 'test@example.com', name: 'Test' },
session: { id: 'session-1', userId: 'user-123', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([{ external_id: 'legacy-uuid-123' }]);

const res = await app.request('/protected/resource');

Expand All @@ -51,7 +66,7 @@ describe('authMiddleware', () => {
const body = await res.json();
expect(body).toEqual({
message: 'success',
user: { userId: 'user-123', email: 'test@example.com' },
user: { userId: 'legacy-uuid-123', betterAuthId: 'user-123', email: 'test@example.com' },
});
});

Expand All @@ -60,10 +75,15 @@ describe('authMiddleware', () => {
user: { id: 'user-456', email: 'another@example.com', name: 'Another' },
session: { id: 'session-2', userId: 'user-456', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([{ external_id: 'legacy-uuid-456' }]);

const res = await app.request('/protected/resource');
const body = (await res.json()) as { user: unknown };
expect(body.user).toEqual({ userId: 'user-456', email: 'another@example.com' });
expect(body.user).toEqual({
userId: 'legacy-uuid-456',
betterAuthId: 'user-456',
email: 'another@example.com',
});
});
});

Expand All @@ -85,6 +105,20 @@ describe('authMiddleware', () => {

expect(res.status).toBe(401);
});

it('should return 401 when legacy user does not exist', async () => {
mockGetSession.mockResolvedValue({
user: { id: 'user-new', email: 'new@example.com', name: 'New' },
session: { id: 's1', userId: 'user-new', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([]);

const res = await app.request('/protected/resource');

expect(res.status).toBe(401);
const body = await res.json();
expect(body).toEqual({ error: 'User account not fully provisioned' });
});
});

describe('getAuthUser helper', () => {
Expand All @@ -93,10 +127,15 @@ describe('authMiddleware', () => {
user: { id: 'user-999', email: 'helper@example.com', name: 'Helper' },
session: { id: 'session-3', userId: 'user-999', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([{ external_id: 'legacy-uuid-999' }]);

const res = await app.request('/protected/resource');
const body = (await res.json()) as { user: unknown };
expect(body.user).toEqual({ userId: 'user-999', email: 'helper@example.com' });
expect(body.user).toEqual({
userId: 'legacy-uuid-999',
betterAuthId: 'user-999',
email: 'helper@example.com',
});
});

it('should return correct user for different authenticated requests', async () => {
Expand All @@ -108,18 +147,20 @@ describe('authMiddleware', () => {
user: user1,
session: { id: 's1', userId: 'user-1', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([{ external_id: 'legacy-uuid-1' }]);
const res1 = await app.request('/protected/resource');
const body1 = (await res1.json()) as { user: { userId: string } };
expect(body1.user.userId).toBe('user-1');
expect(body1.user.userId).toBe('legacy-uuid-1');

// Second request
mockGetSession.mockResolvedValue({
user: user2,
session: { id: 's2', userId: 'user-2', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([{ external_id: 'legacy-uuid-2' }]);
const res2 = await app.request('/protected/resource');
const body2 = (await res2.json()) as { user: { userId: string } };
expect(body2.user.userId).toBe('user-2');
expect(body2.user.userId).toBe('legacy-uuid-2');
});
});

Expand All @@ -129,6 +170,7 @@ describe('authMiddleware', () => {
user: { id: 'user-chain', email: 'chain@example.com', name: 'Chain' },
session: { id: 's1', userId: 'user-chain', token: 'tok' },
});
mockGetLegacyExternalId.mockResolvedValue([{ external_id: 'legacy-uuid-chain' }]);

const res = await app.request('/protected/resource');

Expand Down
Loading