From e63694fa6d9ddb95c772dc8fb04fc1310e5443ba Mon Sep 17 00:00:00 2001 From: nourshoreibah Date: Mon, 3 Aug 2026 20:14:54 -0400 Subject: [PATCH] feat(projects): scope GET /dashboard to the caller's projects The dashboard aggregates were admin-only, so a non-admin got a 403 and the frontend had no source for a project's spend or staff count -- which is why every ProjectCard on /dashboard and /projects renders budget_used={0} members={0}. An admin still sees every project. Everyone else now sees the same payload restricted to the projects they are a member of, and a caller with no memberships gets a zeroed payload rather than an error (`where in ()` is not valid SQL, so that case short-circuits before any aggregate query runs). Co-Authored-By: Claude Opus 5 (1M context) --- apps/backend/lambdas/projects/README.md | 2 +- apps/backend/lambdas/projects/handler.ts | 42 +++++++++++++-- apps/backend/lambdas/projects/openapi.yaml | 48 ++++++++++++++++- .../projects/test/dashboard.unit.test.ts | 53 +++++++++++++++++-- .../projects/test/projects.e2e.test.ts | 27 ++++++++-- 5 files changed, 159 insertions(+), 13 deletions(-) diff --git a/apps/backend/lambdas/projects/README.md b/apps/backend/lambdas/projects/README.md index 3d5a7ee9..a6595f30 100644 --- a/apps/backend/lambdas/projects/README.md +++ b/apps/backend/lambdas/projects/README.md @@ -9,7 +9,7 @@ Lambda for managing projects. | Method | Path | Description | |--------|------|-------------| | GET | /health | Health check | -| GET | /dashboard | | +| GET | /dashboard | Spend/staffing aggregates, scoped to the caller's projects (all of them for an admin) | | GET | /projects/{id}/members | | | GET | /projects | | | GET | /projects/{id}/donors | | diff --git a/apps/backend/lambdas/projects/handler.ts b/apps/backend/lambdas/projects/handler.ts index ab5d0d9a..d4b42e13 100644 --- a/apps/backend/lambdas/projects/handler.ts +++ b/apps/backend/lambdas/projects/handler.ts @@ -41,11 +41,38 @@ export const handler = async (event: any): Promise => { // CLI-generated routes will be inserted here // GET /dashboard if ((normalizedPath === '/dashboard' || normalizedPath.endsWith('/dashboard')) && method === 'GET') { - if (!user.isAdmin) { - return json(403, { message: 'Admin access required' }); - } - try { + // Admins see every project. Everyone else sees the ones they are a + // member of, so a non-admin gets the same dashboard scoped to their own + // work rather than a 403 — which is what the project cards on /dashboard + // and /projects need to show real spend and staff counts. + let scopedIds: number[] | null = null; + if (!user.isAdmin) { + const memberships = await db + .selectFrom('branch.project_memberships') + .select('project_id') + .where('user_id', '=', user.userId!) + .execute(); + scopedIds = [...new Set(memberships.map((m) => m.project_id))]; + + // `where in ()` with an empty list is not valid SQL, and there is + // nothing to aggregate anyway. + if (scopedIds.length === 0) { + return json(200, { + summary: { + topExpenseCategory: null, + totalSpent: 0, + totalProjects: 0, + averageSpendPerProject: 0, + }, + projects: [], + expensesByMonth: [], + }); + } + } + const ids = scopedIds; + const scoped = ids !== null; + const [ totalSpentRow, totalProjectsRow, @@ -57,32 +84,39 @@ export const handler = async (event: any): Promise => { ] = await Promise.all([ db.selectFrom('branch.expenditures') .select(db.fn.sum('amount').as('total')) + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .executeTakeFirst(), db.selectFrom('branch.projects') .select(db.fn.count('project_id').as('count')) + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .executeTakeFirst(), db.selectFrom('branch.expenditures') .select(['category', db.fn.sum('amount').as('total')]) .where('category', 'is not', null) + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .groupBy('category') .orderBy(db.fn.sum('amount'), 'desc') .limit(1) .executeTakeFirst(), db.selectFrom('branch.projects') .selectAll() + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .orderBy('project_id', 'asc') .execute(), db.selectFrom('branch.expenditures') .select(['project_id', db.fn.sum('amount').as('total')]) + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .groupBy('project_id') .execute(), db.selectFrom('branch.project_memberships') .select(['project_id', db.fn.count('user_id').as('count')]) + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .groupBy('project_id') .execute(), db.selectFrom('branch.expenditures') .select(['spent_on', 'category', 'amount']) .where('category', 'is not', null) + .$if(scoped, (qb) => qb.where('project_id', 'in', ids!)) .execute(), ]); diff --git a/apps/backend/lambdas/projects/openapi.yaml b/apps/backend/lambdas/projects/openapi.yaml index 8b3253c3..8301b12e 100644 --- a/apps/backend/lambdas/projects/openapi.yaml +++ b/apps/backend/lambdas/projects/openapi.yaml @@ -115,13 +115,57 @@ paths: /dashboard: get: summary: GET /dashboard + description: >- + Spend and staffing aggregates. An admin sees every project; anyone else + sees only the projects they are a member of, and gets an empty dashboard + when they are a member of none. responses: '200': description: OK + content: + application/json: + schema: + type: object + properties: + summary: + type: object + properties: + topExpenseCategory: + type: object + nullable: true + totalSpent: + type: number + totalProjects: + type: integer + averageSpendPerProject: + type: number + projects: + type: array + items: + type: object + properties: + project_id: + type: integer + name: + type: string + total_budget: + type: number + nullable: true + currency: + type: string + nullable: true + spent: + type: number + staff_count: + type: integer + spent_percentage: + type: number + expensesByMonth: + type: array + items: + type: object '401': description: Unauthorized - '403': - description: Forbidden /projects/{id}/donors: get: diff --git a/apps/backend/lambdas/projects/test/dashboard.unit.test.ts b/apps/backend/lambdas/projects/test/dashboard.unit.test.ts index 5caf455c..468ffc3c 100644 --- a/apps/backend/lambdas/projects/test/dashboard.unit.test.ts +++ b/apps/backend/lambdas/projects/test/dashboard.unit.test.ts @@ -47,6 +47,11 @@ function chain(value: any) { ]) { p[m] = jest.fn().mockReturnValue(p); } + // Every builder method above returns `p`, so applying the callback or not + // lands on the same object -- but calling it keeps the `where` spy accurate. + p.$if = jest.fn((condition: boolean, callback: (qb: any) => any) => + condition ? callback(p) : p, + ); p.execute = jest.fn().mockResolvedValue(value as any); p.executeTakeFirst = jest.fn().mockResolvedValue(value as any); return p; @@ -70,11 +75,53 @@ describe('GET /dashboard unit tests', () => { expect(JSON.parse(res.body).message).toBe('Authentication required'); }); - test('403: authenticated non-admin is forbidden', async () => { + test('200: a non-admin gets a dashboard scoped to their memberships', async () => { mockAuthenticateRequest.mockResolvedValue(nonAdminAuthResult as any); + + mockDb.selectFrom = jest.fn(); + // 0) the caller's memberships, which set the scope for everything after + const memberships = chain([{ project_id: 2 }, { project_id: 2 }]); + mockDb.selectFrom.mockReturnValueOnce(memberships); + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '4500.00' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ count: '1' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ category: 'General', total: '3000.00' })); + const projectRows = chain([{ project_id: 2, name: 'P2', total_budget: '300000.00', currency: 'USD' }]); + mockDb.selectFrom.mockReturnValueOnce(projectRows); + mockDb.selectFrom.mockReturnValueOnce(chain([{ project_id: 2, total: '4500.00' }])); + mockDb.selectFrom.mockReturnValueOnce(chain([{ project_id: 2, count: '1' }])); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + + const res = await handler(getEvent()); + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + expect(body.projects).toHaveLength(1); + expect(body.projects[0]).toMatchObject({ project_id: 2, spent: 4500, staff_count: 1 }); + + // Duplicate membership rows must not widen the IN list. + expect(projectRows.where).toHaveBeenCalledWith('project_id', 'in', [2]); + }); + + test('200: a non-admin with no memberships gets an empty dashboard, not a 403', async () => { + mockAuthenticateRequest.mockResolvedValue(nonAdminAuthResult as any); + + mockDb.selectFrom = jest.fn(); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + const res = await handler(getEvent()); - expect(res.statusCode).toBe(403); - expect(JSON.parse(res.body).message).toBe('Admin access required'); + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + expect(body).toEqual({ + summary: { + topExpenseCategory: null, + totalSpent: 0, + totalProjects: 0, + averageSpendPerProject: 0, + }, + projects: [], + expensesByMonth: [], + }); + // Short-circuits before any aggregate query -- `where in ()` is invalid SQL. + expect(mockDb.selectFrom).toHaveBeenCalledTimes(1); }); }); diff --git a/apps/backend/lambdas/projects/test/projects.e2e.test.ts b/apps/backend/lambdas/projects/test/projects.e2e.test.ts index 565e64bb..96779e96 100644 --- a/apps/backend/lambdas/projects/test/projects.e2e.test.ts +++ b/apps/backend/lambdas/projects/test/projects.e2e.test.ts @@ -328,11 +328,32 @@ describe('GET /dashboard (e2e)', () => { expect(res.statusCode).toBe(401); }); - test('403: non-admin is forbidden 🌞', async () => { + test('non-admin sees only the projects they are a member of 🌞', async () => { mockAuthenticateRequest.mockResolvedValue(nonAdminAuthResult); const res = await handler(dashboardEvent()); - expect(res.statusCode).toBe(403); - expect(JSON.parse(res.body).message).toBe('Admin access required'); + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + + // User 3 is seeded into project 2 alone, which has 3000 + 1500 spent. + expect(body.projects.map((p: any) => p.project_id)).toEqual([2]); + expect(body.projects[0]).toMatchObject({ spent: 4500, staff_count: 1 }); + expect(body.summary.totalProjects).toBe(1); + expect(body.summary.totalSpent).toBe(4500); + expect(body.expensesByMonth.every((m: any) => m.amount > 0)).toBe(true); + }); + + test('non-admin with no memberships gets an empty dashboard 🌞', async () => { + mockAuthenticateRequest.mockResolvedValue({ + isAuthenticated: true as const, + user: { cognitoSub: 'nobody-sub', userId: 99999, email: 'nobody@branch.org', isAdmin: false }, + }); + const res = await handler(dashboardEvent()); + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + expect(body.projects).toEqual([]); + expect(body.expensesByMonth).toEqual([]); + expect(body.summary.totalSpent).toBe(0); + expect(body.summary.topExpenseCategory).toBeNull(); }); test('summary aggregates seed totals 🌞', async () => {