mirror of
https://github.com/jcreek/LivingDexTracker.git
synced 2026-07-13 11:03:44 +00:00
fix(#55): implement user data isolation to prevent cross-user data access
- Add server-side authentication validation for all protected API endpoints - Create auth utility with requireAuth() function for session validation - Update CombinedDataRepository to filter data by authenticated user ID - Add findByUserId() method to CatchRecordRepository for user-specific queries - Replace client-side userId parameters with server-side session extraction - Use MongoDB aggregation with $lookup and $expr for secure user filtering - Return 401 errors for unauthenticated requests - Fix critical security vulnerability where users could see others' catch records Fixes: User data isolation bug where one user's catch records were used for everyone Security: Prevents unauthorized access to other users' Pokemon tracking data
This commit is contained in:
Vendored
+6
-1
@@ -1,6 +1,7 @@
|
|||||||
import 'vite-plugin-pwa/svelte';
|
import 'vite-plugin-pwa/svelte';
|
||||||
import 'vite-plugin-pwa/info';
|
import 'vite-plugin-pwa/info';
|
||||||
import 'vite-plugin-pwa/pwa-assets';
|
import 'vite-plugin-pwa/pwa-assets';
|
||||||
|
import { SupabaseClient, Session, User } from '@supabase/supabase-js';
|
||||||
|
|
||||||
// See https://kit.svelte.dev/docs/types#app
|
// See https://kit.svelte.dev/docs/types#app
|
||||||
// for information about these interfaces
|
// for information about these interfaces
|
||||||
@@ -11,12 +12,16 @@ declare global {
|
|||||||
namespace App {
|
namespace App {
|
||||||
// interface Error {}
|
// interface Error {}
|
||||||
interface Locals {
|
interface Locals {
|
||||||
|
supabase: SupabaseClient;
|
||||||
|
safeGetSession(): Promise<{ session: Session | null; user: User | null }>;
|
||||||
userid: string;
|
userid: string;
|
||||||
buildDate: string;
|
buildDate: string;
|
||||||
periodicUpdates: boolean;
|
periodicUpdates: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
// interface PageData {}
|
interface PageData {
|
||||||
|
session: Session | null;
|
||||||
|
}
|
||||||
// interface PageState {}
|
// interface PageState {}
|
||||||
// interface Platform {}
|
// interface Platform {}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,6 +9,10 @@ class CatchRecordRepository {
|
|||||||
return CatchRecordModel.find().exec();
|
return CatchRecordModel.find().exec();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async findByUserId(userId: string): Promise<CatchRecord[]> {
|
||||||
|
return CatchRecordModel.find({ userId }).exec();
|
||||||
|
}
|
||||||
|
|
||||||
async create(data: Partial<CatchRecord>): Promise<CatchRecord> {
|
async create(data: Partial<CatchRecord>): Promise<CatchRecord> {
|
||||||
return CatchRecordModel.create(data);
|
return CatchRecordModel.create(data);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,9 +1,10 @@
|
|||||||
import PokedexEntryModel, { type PokedexEntry } from '$lib/models/PokedexEntry';
|
import PokedexEntryModel, { type PokedexEntry } from '$lib/models/PokedexEntry';
|
||||||
import CatchRecordModel, { type CatchRecord } from '$lib/models/CatchRecord';
|
import CatchRecordModel, { type CatchRecord } from '$lib/models/CatchRecord';
|
||||||
import { CombinedData } from '$lib/models/CombinedData';
|
import { type CombinedData } from '$lib/models/CombinedData';
|
||||||
|
|
||||||
class CombinedDataRepository {
|
class CombinedDataRepository {
|
||||||
async findAllCombinedData(
|
async findAllCombinedData(
|
||||||
|
userId: string,
|
||||||
enableForms: boolean = true,
|
enableForms: boolean = true,
|
||||||
region: string = '',
|
region: string = '',
|
||||||
game: string = ''
|
game: string = ''
|
||||||
@@ -12,8 +13,19 @@ class CombinedDataRepository {
|
|||||||
{
|
{
|
||||||
$lookup: {
|
$lookup: {
|
||||||
from: 'catchrecords',
|
from: 'catchrecords',
|
||||||
localField: '_id',
|
let: { entryId: '$_id' },
|
||||||
foreignField: 'pokedexEntryId',
|
pipeline: [
|
||||||
|
{
|
||||||
|
$match: {
|
||||||
|
$expr: {
|
||||||
|
$and: [
|
||||||
|
{ $eq: ['$pokedexEntryId', '$$entryId'] },
|
||||||
|
{ $eq: ['$userId', userId] }
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
as: 'catchRecord'
|
as: 'catchRecord'
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -58,16 +70,14 @@ class CombinedDataRepository {
|
|||||||
|
|
||||||
const combinedData: CombinedData[] = await PokedexEntryModel.aggregate(pipeline).exec();
|
const combinedData: CombinedData[] = await PokedexEntryModel.aggregate(pipeline).exec();
|
||||||
|
|
||||||
// Filter out entries with no catch records
|
return combinedData.map((data) => ({
|
||||||
const filteredData = combinedData.filter((data) => data.catchRecord);
|
|
||||||
|
|
||||||
return filteredData.map((data) => ({
|
|
||||||
pokedexEntry: data as PokedexEntry,
|
pokedexEntry: data as PokedexEntry,
|
||||||
catchRecord: data.catchRecord as CatchRecord
|
catchRecord: data.catchRecord as CatchRecord
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
async findCombinedData(
|
async findCombinedData(
|
||||||
|
userId: string,
|
||||||
page: number = 1,
|
page: number = 1,
|
||||||
limit: number = 20,
|
limit: number = 20,
|
||||||
enableForms: boolean = true,
|
enableForms: boolean = true,
|
||||||
@@ -79,8 +89,19 @@ class CombinedDataRepository {
|
|||||||
{
|
{
|
||||||
$lookup: {
|
$lookup: {
|
||||||
from: 'catchrecords',
|
from: 'catchrecords',
|
||||||
localField: '_id',
|
let: { entryId: '$_id' },
|
||||||
foreignField: 'pokedexEntryId',
|
pipeline: [
|
||||||
|
{
|
||||||
|
$match: {
|
||||||
|
$expr: {
|
||||||
|
$and: [
|
||||||
|
{ $eq: ['$pokedexEntryId', '$$entryId'] },
|
||||||
|
{ $eq: ['$userId', userId] }
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
as: 'catchRecord'
|
as: 'catchRecord'
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -127,33 +148,71 @@ class CombinedDataRepository {
|
|||||||
|
|
||||||
const combinedData: CombinedData[] = await PokedexEntryModel.aggregate(pipeline).exec();
|
const combinedData: CombinedData[] = await PokedexEntryModel.aggregate(pipeline).exec();
|
||||||
|
|
||||||
// Filter out entries with no catch records
|
return combinedData.map((data) => ({
|
||||||
const filteredData = combinedData.filter((data) => data.catchRecord);
|
|
||||||
|
|
||||||
return filteredData.map((data) => ({
|
|
||||||
pokedexEntry: data as PokedexEntry,
|
pokedexEntry: data as PokedexEntry,
|
||||||
catchRecord: data.catchRecord as CatchRecord
|
catchRecord: data.catchRecord as CatchRecord
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
async countCombinedData(enableForms: boolean, region: string, game: string): Promise<number> {
|
async countCombinedData(userId: string, enableForms: boolean, region: string, game: string): Promise<number> {
|
||||||
const filter: any = {};
|
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) {
|
if (!enableForms) {
|
||||||
filter['boxPlacement.box'] = { $ne: null };
|
pipeline.unshift({
|
||||||
|
$match: {
|
||||||
|
'boxPlacement.box': { $ne: null }
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
if (region.length > 0) {
|
if (region.length > 0) {
|
||||||
filter['regionToCatchIn'] = region;
|
pipeline.unshift({
|
||||||
|
$match: {
|
||||||
|
regionToCatchIn: region
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
if (game.length > 0) {
|
if (game.length > 0) {
|
||||||
filter['gamesToCatchIn'] = { $in: [game] };
|
pipeline.unshift({
|
||||||
|
$match: {
|
||||||
|
gamesToCatchIn: { $in: [game] }
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const count = await PokedexEntryModel.countDocuments(filter).exec();
|
const result = await PokedexEntryModel.aggregate(pipeline).exec();
|
||||||
return count;
|
return result.length > 0 ? result[0].total : 0;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error('Error counting documents:', error);
|
console.error('Error counting documents:', error);
|
||||||
throw error;
|
throw error;
|
||||||
|
|||||||
@@ -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;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,19 +1,27 @@
|
|||||||
import { json } from '@sveltejs/kit';
|
import { json, error } from '@sveltejs/kit';
|
||||||
import { dbConnect, dbDisconnect } from '$lib/utils/db';
|
import { dbConnect, dbDisconnect } from '$lib/utils/db';
|
||||||
import { type CatchRecord } from '$lib/models/CatchRecord';
|
import { type CatchRecord } from '$lib/models/CatchRecord';
|
||||||
import CatchRecordRepository from '$lib/repositories/CatchRecordRepository';
|
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;
|
let catchData = null as CatchRecord[] | null;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
const userId = await requireAuth(event);
|
||||||
await dbConnect();
|
await dbConnect();
|
||||||
const repo = new CatchRecordRepository();
|
const repo = new CatchRecordRepository();
|
||||||
catchData = await repo.findAll();
|
catchData = await repo.findByUserId(userId);
|
||||||
// order by pokedexEntryId property, ascending
|
// order by pokedexEntryId property, ascending
|
||||||
catchData = catchData.sort((a, b) => a.pokedexEntryId - b.pokedexEntryId);
|
catchData = catchData.sort((a, b) => a.pokedexEntryId - b.pokedexEntryId);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error(error);
|
console.error(error);
|
||||||
|
if (error.status) {
|
||||||
|
// Re-throw SvelteKit errors (like 401)
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
return json({ error: 'Internal Server Error' }, { status: 500 });
|
||||||
} finally {
|
} finally {
|
||||||
dbDisconnect();
|
dbDisconnect();
|
||||||
}
|
}
|
||||||
@@ -21,29 +29,40 @@ export const GET = async () => {
|
|||||||
return json(catchData);
|
return json(catchData);
|
||||||
};
|
};
|
||||||
|
|
||||||
export const PUT = async ({ request }) => {
|
export const PUT = async (event: RequestEvent) => {
|
||||||
try {
|
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();
|
await dbConnect();
|
||||||
const repo = new CatchRecordRepository();
|
const repo = new CatchRecordRepository();
|
||||||
const upsertedRecord = await repo.upsert(data);
|
const upsertedRecord = await repo.upsert(data);
|
||||||
return json(upsertedRecord);
|
return json(upsertedRecord);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error(err);
|
console.error(err);
|
||||||
throw error(500, 'Internal Server Error');
|
if (err.status) {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
return json({ error: 'Internal Server Error' }, { status: 500 });
|
||||||
} finally {
|
} finally {
|
||||||
await dbDisconnect();
|
await dbDisconnect();
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
export const POST = async ({ request }) => {
|
export const POST = async (event: RequestEvent) => {
|
||||||
try {
|
try {
|
||||||
const records: Partial<CatchRecord>[] = await request.json();
|
const userId = await requireAuth(event);
|
||||||
|
const records: Partial<CatchRecord>[] = await event.request.json();
|
||||||
await dbConnect();
|
await dbConnect();
|
||||||
const repo = new CatchRecordRepository();
|
const repo = new CatchRecordRepository();
|
||||||
|
|
||||||
const insertedRecords = [];
|
const insertedRecords = [];
|
||||||
for (const record of records) {
|
for (const record of records) {
|
||||||
|
// Ensure the userId is set to the authenticated user
|
||||||
|
record.userId = userId;
|
||||||
const upsertedRecord = await repo.upsert(record);
|
const upsertedRecord = await repo.upsert(record);
|
||||||
insertedRecords.push(upsertedRecord);
|
insertedRecords.push(upsertedRecord);
|
||||||
}
|
}
|
||||||
@@ -51,7 +70,10 @@ export const POST = async ({ request }) => {
|
|||||||
return json(insertedRecords);
|
return json(insertedRecords);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error(err);
|
console.error(err);
|
||||||
throw error(500, 'Internal Server Error');
|
if (err.status) {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
return json({ error: 'Internal Server Error' }, { status: 500 });
|
||||||
} finally {
|
} finally {
|
||||||
await dbDisconnect();
|
await dbDisconnect();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,8 +1,11 @@
|
|||||||
import { json } from '@sveltejs/kit';
|
import { json } from '@sveltejs/kit';
|
||||||
import { dbConnect, dbDisconnect } from '$lib/utils/db';
|
import { dbConnect, dbDisconnect } from '$lib/utils/db';
|
||||||
import CombinedDataRepository from '$lib/repositories/CombinedDataRepository';
|
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 page = parseInt(url.searchParams.get('page') || '1', 10);
|
||||||
const limit = parseInt(url.searchParams.get('limit') || '20', 10);
|
const limit = parseInt(url.searchParams.get('limit') || '20', 10);
|
||||||
const enableForms = url.searchParams.get('enableForms') === 'true';
|
const enableForms = url.searchParams.get('enableForms') === 'true';
|
||||||
@@ -10,19 +13,25 @@ export const GET = async ({ url }) => {
|
|||||||
const game = url.searchParams.get('game');
|
const game = url.searchParams.get('game');
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
const userId = await requireAuth(event);
|
||||||
await dbConnect();
|
await dbConnect();
|
||||||
const repo = new CombinedDataRepository();
|
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) {
|
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);
|
const totalPages = Math.ceil(totalCount / limit);
|
||||||
|
|
||||||
return json({ combinedData, totalPages });
|
return json({ combinedData, totalPages });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error(error);
|
console.error(error);
|
||||||
|
if (error.status) {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
return json({ error: 'Internal Server Error' }, { status: 500 });
|
return json({ error: 'Internal Server Error' }, { status: 500 });
|
||||||
} finally {
|
} finally {
|
||||||
dbDisconnect();
|
dbDisconnect();
|
||||||
|
|||||||
@@ -1,23 +1,28 @@
|
|||||||
import { json } from '@sveltejs/kit';
|
import { json } from '@sveltejs/kit';
|
||||||
import { dbConnect, dbDisconnect } from '$lib/utils/db';
|
import { dbConnect, dbDisconnect } from '$lib/utils/db';
|
||||||
import CombinedDataRepository from '$lib/repositories/CombinedDataRepository';
|
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 enableForms = url.searchParams.get('enableForms') === 'true';
|
||||||
const region = url.searchParams.get('region');
|
const region = url.searchParams.get('region');
|
||||||
const game = url.searchParams.get('game');
|
const game = url.searchParams.get('game');
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
const userId = await requireAuth(event);
|
||||||
await dbConnect();
|
await dbConnect();
|
||||||
const repo = new CombinedDataRepository();
|
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 empty array instead of 404 for better UX
|
||||||
return json({ error: 'No combined data found' }, { status: 404 });
|
|
||||||
}
|
|
||||||
return json(combinedData);
|
return json(combinedData);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error(error);
|
console.error(error);
|
||||||
|
if (error.status) {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
return json({ error: 'Internal Server Error' }, { status: 500 });
|
return json({ error: 'Internal Server Error' }, { status: 500 });
|
||||||
} finally {
|
} finally {
|
||||||
dbDisconnect();
|
dbDisconnect();
|
||||||
|
|||||||
Reference in New Issue
Block a user