Merge pull request #65 from jcreek/55-fix-bug-where-one-users-catch-records-are-used-for-everyone

fix(#55): implement user data isolation to prevent cross-user data access

src/app.d.ts

@@ -1,6 +1,7 @@
 import 'vite-plugin-pwa/svelte';
 import 'vite-plugin-pwa/info';
 import 'vite-plugin-pwa/pwa-assets';
+import { SupabaseClient, Session, User } from '@supabase/supabase-js';
 
 // See https://kit.svelte.dev/docs/types#app
 // for information about these interfaces
@@ -11,12 +12,16 @@ declare global {
 	namespace App {
 		// interface Error {}
 		interface Locals {
+			supabase: SupabaseClient;
+			safeGetSession(): Promise<{ session: Session | null; user: User | null }>;
 			userid: string;
 			buildDate: string;
 			periodicUpdates: boolean;
 		}
 
-		// interface PageData {}
+		interface PageData {
+			session: Session | null;
+		}
 		// interface PageState {}
 		// interface Platform {}
 	}

src/lib/repositories/CatchRecordRepository.ts

@@ -9,6 +9,10 @@ class CatchRecordRepository {
 		return CatchRecordModel.find().exec();
 	}
 
+	async findByUserId(userId: string): Promise<CatchRecord[]> {
+		return CatchRecordModel.find({ userId }).exec();
+	}
+
 	async create(data: Partial<CatchRecord>): Promise<CatchRecord> {
 		return CatchRecordModel.create(data);
 	}

src/lib/repositories/CombinedDataRepository.ts

@@ -1,9 +1,10 @@
 import PokedexEntryModel, { type PokedexEntry } from '$lib/models/PokedexEntry';
 import CatchRecordModel, { type CatchRecord } from '$lib/models/CatchRecord';
-import { CombinedData } from '$lib/models/CombinedData';
+import { type CombinedData } from '$lib/models/CombinedData';
 
 class CombinedDataRepository {
 	async findAllCombinedData(
+		userId: string,
 		enableForms: boolean = true,
 		region: string = '',
 		game: string = ''
@@ -12,8 +13,19 @@ class CombinedDataRepository {
 			{
 				$lookup: {
 					from: 'catchrecords',
-					localField: '_id',
-					foreignField: 'pokedexEntryId',
+					let: { entryId: '$_id' },
+					pipeline: [
+						{
+							$match: {
+								$expr: {
+									$and: [
+										{ $eq: ['$pokedexEntryId', '$$entryId'] },
+										{ $eq: ['$userId', userId] }
+									]
+								}
+							}
+						}
+					],
 					as: 'catchRecord'
 				}
 			},
@@ -58,16 +70,14 @@ class CombinedDataRepository {
 
 		const combinedData: CombinedData[] = await PokedexEntryModel.aggregate(pipeline).exec();
 
-		// Filter out entries with no catch records
-		const filteredData = combinedData.filter((data) => data.catchRecord);
-
-		return filteredData.map((data) => ({
+		return combinedData.map((data) => ({
 			pokedexEntry: data as PokedexEntry,
 			catchRecord: data.catchRecord as CatchRecord
 		}));
 	}
 
 	async findCombinedData(
+		userId: string,
 		page: number = 1,
 		limit: number = 20,
 		enableForms: boolean = true,
@@ -79,8 +89,19 @@ class CombinedDataRepository {
 			{
 				$lookup: {
 					from: 'catchrecords',
-					localField: '_id',
-					foreignField: 'pokedexEntryId',
+					let: { entryId: '$_id' },
+					pipeline: [
+						{
+							$match: {
+								$expr: {
+									$and: [
+										{ $eq: ['$pokedexEntryId', '$$entryId'] },
+										{ $eq: ['$userId', userId] }
+									]
+								}
+							}
+						}
+					],
 					as: 'catchRecord'
 				}
 			},
@@ -127,33 +148,71 @@ class CombinedDataRepository {
 
 		const combinedData: CombinedData[] = await PokedexEntryModel.aggregate(pipeline).exec();
 
-		// Filter out entries with no catch records
-		const filteredData = combinedData.filter((data) => data.catchRecord);
-
-		return filteredData.map((data) => ({
+		return combinedData.map((data) => ({
 			pokedexEntry: data as PokedexEntry,
 			catchRecord: data.catchRecord as CatchRecord
 		}));
 	}
 
-	async countCombinedData(enableForms: boolean, region: string, game: string): Promise<number> {
-		const filter: any = {};
+	async countCombinedData(userId: string, enableForms: boolean, region: string, game: string): Promise<number> {
+		const pipeline: any[] = [
+			{
+				$lookup: {
+					from: 'catchrecords',
+					let: { entryId: '$_id' },
+					pipeline: [
+						{
+							$match: {
+								$expr: {
+									$and: [
+										{ $eq: ['$pokedexEntryId', '$$entryId'] },
+										{ $eq: ['$userId', userId] }
+									]
+								}
+							}
+						}
+					],
+					as: 'catchRecord'
+				}
+			},
+			{
+				$unwind: {
+					path: '$catchRecord',
+					preserveNullAndEmptyArrays: true
+				}
+			},
+			{
+				$count: 'total'
+			}
+		];
 
 		if (!enableForms) {
-			filter['boxPlacement.box'] = { $ne: null };
+			pipeline.unshift({
+				$match: {
+					'boxPlacement.box': { $ne: null }
+				}
+			});
 		}
 
 		if (region.length > 0) {
-			filter['regionToCatchIn'] = region;
+			pipeline.unshift({
+				$match: {
+					regionToCatchIn: region
+				}
+			});
 		}
 
 		if (game.length > 0) {
-			filter['gamesToCatchIn'] = { $in: [game] };
+			pipeline.unshift({
+				$match: {
+					gamesToCatchIn: { $in: [game] }
+				}
+			});
 		}
 
 		try {
-			const count = await PokedexEntryModel.countDocuments(filter).exec();
-			return count;
+			const result = await PokedexEntryModel.aggregate(pipeline).exec();
+			return result.length > 0 ? result[0].total : 0;
 		} catch (error) {
 			console.error('Error counting documents:', error);
 			throw error;

src/lib/utils/auth.ts

@@ -0,0 +1,29 @@
+import type { RequestEvent } from '@sveltejs/kit';
+import { error } from '@sveltejs/kit';
+
+/**
+ * Validates the user session and returns the authenticated user ID
+ * Throws a 401 error if the user is not authenticated
+ */
+export async function requireAuth(event: RequestEvent): Promise<string> {
+	const { session, user } = await event.locals.safeGetSession();
+	
+	if (!session || !user) {
+		throw error(401, 'Authentication required');
+	}
+	
+	return user.id;
+}
+
+/**
+ * Optionally validates the user session and returns the user ID if authenticated
+ * Returns null if not authenticated (no error thrown)
+ */
+export async function getOptionalUserId(event: RequestEvent): Promise<string | null> {
+	try {
+		const { session, user } = await event.locals.safeGetSession();
+		return user?.id || null;
+	} catch {
+		return null;
+	}
+}

src/routes/api/catch-records/+server.ts

@@ -1,19 +1,27 @@
-import { json } from '@sveltejs/kit';
+import { json, error } from '@sveltejs/kit';
 import { dbConnect, dbDisconnect } from '$lib/utils/db';
 import { type CatchRecord } from '$lib/models/CatchRecord';
 import CatchRecordRepository from '$lib/repositories/CatchRecordRepository';
+import { requireAuth } from '$lib/utils/auth';
+import type { RequestEvent } from '@sveltejs/kit';
 
-export const GET = async () => {
+export const GET = async (event: RequestEvent) => {
 	let catchData = null as CatchRecord[] | null;
 
 	try {
+		const userId = await requireAuth(event);
 		await dbConnect();
 		const repo = new CatchRecordRepository();
-		catchData = await repo.findAll();
+		catchData = await repo.findByUserId(userId);
 		// order by pokedexEntryId property, ascending
 		catchData = catchData.sort((a, b) => a.pokedexEntryId - b.pokedexEntryId);
 	} catch (error) {
 		console.error(error);
+		if (error.status) {
+			// Re-throw SvelteKit errors (like 401)
+			throw error;
+		}
+		return json({ error: 'Internal Server Error' }, { status: 500 });
 	} finally {
 		dbDisconnect();
 	}
@@ -21,29 +29,40 @@ export const GET = async () => {
 	return json(catchData);
 };
 
-export const PUT = async ({ request }) => {
+export const PUT = async (event: RequestEvent) => {
 	try {
-		const data: Partial<CatchRecord> = await request.json();
+		const userId = await requireAuth(event);
+		const data: Partial<CatchRecord> = await event.request.json();
+		
+		// Ensure the userId is set to the authenticated user
+		data.userId = userId;
+		
 		await dbConnect();
 		const repo = new CatchRecordRepository();
 		const upsertedRecord = await repo.upsert(data);
 		return json(upsertedRecord);
 	} catch (err) {
 		console.error(err);
-		throw error(500, 'Internal Server Error');
+		if (err.status) {
+			throw err;
+		}
+		return json({ error: 'Internal Server Error' }, { status: 500 });
 	} finally {
 		await dbDisconnect();
 	}
 };
 
-export const POST = async ({ request }) => {
+export const POST = async (event: RequestEvent) => {
 	try {
-		const records: Partial<CatchRecord>[] = await request.json();
+		const userId = await requireAuth(event);
+		const records: Partial<CatchRecord>[] = await event.request.json();
 		await dbConnect();
 		const repo = new CatchRecordRepository();
 
 		const insertedRecords = [];
 		for (const record of records) {
+			// Ensure the userId is set to the authenticated user
+			record.userId = userId;
 			const upsertedRecord = await repo.upsert(record);
 			insertedRecords.push(upsertedRecord);
 		}
@@ -51,7 +70,10 @@ export const POST = async ({ request }) => {
 		return json(insertedRecords);
 	} catch (err) {
 		console.error(err);
-		throw error(500, 'Internal Server Error');
+		if (err.status) {
+			throw err;
+		}
+		return json({ error: 'Internal Server Error' }, { status: 500 });
 	} finally {
 		await dbDisconnect();
 	}

src/routes/api/combined-data/+server.ts

@@ -1,8 +1,11 @@
 import { json } from '@sveltejs/kit';
 import { dbConnect, dbDisconnect } from '$lib/utils/db';
 import CombinedDataRepository from '$lib/repositories/CombinedDataRepository';
+import { requireAuth } from '$lib/utils/auth';
+import type { RequestEvent } from '@sveltejs/kit';
 
-export const GET = async ({ url }) => {
+export const GET = async (event: RequestEvent) => {
+	const { url } = event;
 	const page = parseInt(url.searchParams.get('page') || '1', 10);
 	const limit = parseInt(url.searchParams.get('limit') || '20', 10);
 	const enableForms = url.searchParams.get('enableForms') === 'true';
@@ -10,19 +13,25 @@ export const GET = async ({ url }) => {
 	const game = url.searchParams.get('game');
 
 	try {
+		const userId = await requireAuth(event);
 		await dbConnect();
 		const repo = new CombinedDataRepository();
-		const combinedData = await repo.findCombinedData(page, limit, enableForms, region, game);
+		const combinedData = await repo.findCombinedData(userId, page, limit, enableForms, region, game);
 
+		// Return empty array instead of 404 for better UX
 		if (combinedData.length === 0) {
-			return json({ error: 'No combined data found' }, { status: 404 });
+			return json({ combinedData: [], totalPages: 0 });
 		}
-		const totalCount = await repo.countCombinedData(enableForms, region, game);
+		
+		const totalCount = await repo.countCombinedData(userId, enableForms, region, game);
 		const totalPages = Math.ceil(totalCount / limit);
 
 		return json({ combinedData, totalPages });
 	} catch (error) {
 		console.error(error);
+		if (error.status) {
+			throw error;
+		}
 		return json({ error: 'Internal Server Error' }, { status: 500 });
 	} finally {
 		dbDisconnect();

src/routes/api/combined-data/all/+server.ts

@@ -1,23 +1,28 @@
 import { json } from '@sveltejs/kit';
 import { dbConnect, dbDisconnect } from '$lib/utils/db';
 import CombinedDataRepository from '$lib/repositories/CombinedDataRepository';
+import { requireAuth } from '$lib/utils/auth';
+import type { RequestEvent } from '@sveltejs/kit';
 
-export const GET = async ({ url }) => {
+export const GET = async (event: RequestEvent) => {
+	const { url } = event;
 	const enableForms = url.searchParams.get('enableForms') === 'true';
 	const region = url.searchParams.get('region');
 	const game = url.searchParams.get('game');
 
 	try {
+		const userId = await requireAuth(event);
 		await dbConnect();
 		const repo = new CombinedDataRepository();
-		const combinedData = await repo.findAllCombinedData(enableForms, region, game);
+		const combinedData = await repo.findAllCombinedData(userId, enableForms, region, game);
 
-		if (combinedData.length === 0) {
-			return json({ error: 'No combined data found' }, { status: 404 });
-		}
+		// Return empty array instead of 404 for better UX
 		return json(combinedData);
 	} catch (error) {
 		console.error(error);
+		if (error.status) {
+			throw error;
+		}
 		return json({ error: 'Internal Server Error' }, { status: 500 });
 	} finally {
 		dbDisconnect();