From 858cf31d38aba5baa8a0acec4c623ba496e380b4 Mon Sep 17 00:00:00 2001 From: Eric Gullickson <16152721+ericgullickson@users.noreply.github.com> Date: Thu, 6 Nov 2025 14:07:16 -0600 Subject: [PATCH] Admin settings fixed --- .../src/core/plugins/admin-guard.plugin.ts | 17 +- .../features/admin/api/admin.controller.ts | 25 +++ .../admin/tests/unit/admin.guard.test.ts | 181 ++++++++---------- 3 files changed, 119 insertions(+), 104 deletions(-) diff --git a/backend/src/core/plugins/admin-guard.plugin.ts b/backend/src/core/plugins/admin-guard.plugin.ts index 22989a6..066fa64 100644 --- a/backend/src/core/plugins/admin-guard.plugin.ts +++ b/backend/src/core/plugins/admin-guard.plugin.ts @@ -3,7 +3,7 @@ * @ai-context Checks if authenticated user is an admin and enforces access control */ -import { FastifyPluginAsync, FastifyRequest, FastifyReply } from 'fastify'; +import { FastifyPluginAsync, FastifyRequest, FastifyReply, FastifyInstance } from 'fastify'; import fp from 'fastify-plugin'; import { Pool } from 'pg'; import { logger } from '../logging/logger'; @@ -23,8 +23,21 @@ export function setAdminGuardPool(pool: Pool): void { const adminGuardPlugin: FastifyPluginAsync = async (fastify) => { // Decorate with requireAdmin function that enforces admin authorization - fastify.decorate('requireAdmin', async function(request: FastifyRequest, reply: FastifyReply) { + fastify.decorate('requireAdmin', async function(this: FastifyInstance, request: FastifyRequest, reply: FastifyReply) { try { + if (typeof this.authenticate !== 'function') { + logger.error('Admin guard: authenticate handler missing'); + return reply.code(500).send({ + error: 'Internal server error', + message: 'Authentication handler missing' + }); + } + + await this.authenticate(request, reply); + if (reply.sent) { + return; + } + // Ensure user is authenticated first if (!request.userContext?.userId) { logger.warn('Admin guard: user context missing'); diff --git a/backend/src/features/admin/api/admin.controller.ts b/backend/src/features/admin/api/admin.controller.ts index 76d6a10..1eb657d 100644 --- a/backend/src/features/admin/api/admin.controller.ts +++ b/backend/src/features/admin/api/admin.controller.ts @@ -35,11 +35,15 @@ export class AdminController { const userId = request.userContext?.userId; const userEmail = this.resolveUserEmail(request); + console.log('[DEBUG] Admin verify - userId:', userId); + console.log('[DEBUG] Admin verify - userEmail:', userEmail); + if (userEmail && request.userContext) { request.userContext.email = userEmail.toLowerCase(); } if (!userId && !userEmail) { + console.log('[DEBUG] Admin verify - No userId or userEmail, returning 401'); return reply.code(401).send({ error: 'Unauthorized', message: 'User context missing' @@ -50,15 +54,26 @@ export class AdminController { ? await this.adminService.getAdminByAuth0Sub(userId) : null; + console.log('[DEBUG] Admin verify - adminRecord by auth0Sub:', adminRecord ? 'FOUND' : 'NOT FOUND'); + // Fallback: attempt to resolve admin by email for legacy records if (!adminRecord && userEmail) { const emailMatch = await this.adminService.getAdminByEmail(userEmail.toLowerCase()); + console.log('[DEBUG] Admin verify - emailMatch:', emailMatch ? 'FOUND' : 'NOT FOUND'); + if (emailMatch) { + console.log('[DEBUG] Admin verify - emailMatch.auth0Sub:', emailMatch.auth0Sub); + console.log('[DEBUG] Admin verify - emailMatch.revokedAt:', emailMatch.revokedAt); + } + if (emailMatch && !emailMatch.revokedAt) { // If the stored auth0Sub differs, link it to the authenticated user if (userId && emailMatch.auth0Sub !== userId) { + console.log('[DEBUG] Admin verify - Calling linkAdminAuth0Sub to update auth0Sub'); adminRecord = await this.adminService.linkAdminAuth0Sub(userEmail, userId); + console.log('[DEBUG] Admin verify - adminRecord after link:', adminRecord ? 'SUCCESS' : 'FAILED'); } else { + console.log('[DEBUG] Admin verify - Using emailMatch as adminRecord'); adminRecord = emailMatch; } } @@ -70,6 +85,7 @@ export class AdminController { request.userContext.adminRecord = adminRecord; } + console.log('[DEBUG] Admin verify - Returning isAdmin: true'); // User is an active admin return reply.code(200).send({ isAdmin: true, @@ -86,12 +102,14 @@ export class AdminController { request.userContext.adminRecord = undefined; } + console.log('[DEBUG] Admin verify - Returning isAdmin: false'); // User is not an admin return reply.code(200).send({ isAdmin: false, adminRecord: null }); } catch (error) { + console.log('[DEBUG] Admin verify - Error caught:', error instanceof Error ? error.message : 'Unknown error'); logger.error('Error verifying admin access', { error: error instanceof Error ? error.message : 'Unknown error', userId: request.userContext?.userId?.substring(0, 8) + '...' @@ -381,6 +399,9 @@ export class AdminController { } private resolveUserEmail(request: FastifyRequest): string | undefined { + console.log('[DEBUG] resolveUserEmail - request.userContext:', JSON.stringify(request.userContext, null, 2)); + console.log('[DEBUG] resolveUserEmail - request.user:', JSON.stringify((request as any).user, null, 2)); + const candidates: Array = [ request.userContext?.email, (request as any).user?.email, @@ -389,11 +410,15 @@ export class AdminController { (request as any).user?.preferred_username, ]; + console.log('[DEBUG] resolveUserEmail - candidates:', candidates); + for (const value of candidates) { if (typeof value === 'string' && value.includes('@')) { + console.log('[DEBUG] resolveUserEmail - found email:', value); return value.trim(); } } + console.log('[DEBUG] resolveUserEmail - no email found'); return undefined; } } diff --git a/backend/src/features/admin/tests/unit/admin.guard.test.ts b/backend/src/features/admin/tests/unit/admin.guard.test.ts index 1c62253..2ae43db 100644 --- a/backend/src/features/admin/tests/unit/admin.guard.test.ts +++ b/backend/src/features/admin/tests/unit/admin.guard.test.ts @@ -1,123 +1,100 @@ -/** - * @ai-summary Admin guard plugin unit tests - * @ai-context Tests authorization logic for admin-only routes - */ - -import { FastifyRequest, FastifyReply } from 'fastify'; +import Fastify, { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify'; import { Pool } from 'pg'; -import { setAdminGuardPool } from '../../../../core/plugins/admin-guard.plugin'; +import adminGuardPlugin, { setAdminGuardPool } from '../../../../core/plugins/admin-guard.plugin'; -describe('Admin Guard', () => { - let mockPool: Pool; - let mockRequest: Partial; - let mockReply: Partial; +const createReply = (): Partial & { payload?: unknown; statusCode?: number } => { + return { + sent: false, + code: jest.fn(function(this: any, status: number) { + this.statusCode = status; + return this; + }), + send: jest.fn(function(this: any, payload: unknown) { + this.payload = payload; + this.sent = true; + return this; + }), + }; +}; - beforeEach(() => { - // Mock database pool - mockPool = { - query: jest.fn(), - } as unknown as Pool; +describe('admin guard plugin', () => { + let fastify: FastifyInstance; + let authenticateMock: jest.Mock; + let mockPool: { query: jest.Mock }; - // Mock reply methods - mockReply = { - code: jest.fn().mockReturnThis(), - send: jest.fn().mockReturnThis(), - }; - }); - - describe('Authorization checks', () => { - it('should reject request without user context', async () => { - mockRequest = { - userContext: undefined, - }; - - const requireAdmin = jest.fn(); - // Test would call requireAdmin and verify 401 response - }); - - it('should reject non-admin users', async () => { - mockRequest = { - userContext: { - userId: 'auth0|123456', - isAdmin: false, - }, - }; - - // Test database query returns no admin record - (mockPool.query as jest.Mock).mockResolvedValue({ rows: [] }); - - // Test would call requireAdmin and verify 403 response - }); - - it('should accept active admin users', async () => { - mockRequest = { - userContext: { - userId: 'auth0|123456', - isAdmin: false, - }, - }; - - const adminRecord = { - auth0_sub: 'auth0|123456', + beforeEach(async () => { + fastify = Fastify(); + authenticateMock = jest.fn(async (request: FastifyRequest) => { + request.userContext = { + userId: 'auth0|admin', email: 'admin@motovaultpro.com', - role: 'admin', - revoked_at: null, + isAdmin: false, }; - - (mockPool.query as jest.Mock).mockResolvedValue({ rows: [adminRecord] }); - - // Test would call requireAdmin and verify isAdmin set to true }); + fastify.decorate('authenticate', authenticateMock); - it('should reject revoked admin users', async () => { - mockRequest = { - userContext: { - userId: 'auth0|123456', - isAdmin: false, - }, - }; + await fastify.register(adminGuardPlugin); - // Test database query returns no rows (admin is revoked) - (mockPool.query as jest.Mock).mockResolvedValue({ rows: [] }); + mockPool = { + query: jest.fn().mockResolvedValue({ + rows: [{ + auth0_sub: 'auth0|admin', + email: 'admin@motovaultpro.com', + role: 'admin', + revoked_at: null, + }], + }), + }; - // Test would call requireAdmin and verify 403 response - }); + setAdminGuardPool(mockPool as unknown as Pool); + }); - it('should handle database errors gracefully', async () => { - mockRequest = { - userContext: { - userId: 'auth0|123456', - isAdmin: false, - }, - }; + afterEach(async () => { + await fastify.close(); + setAdminGuardPool(null as unknown as Pool); + jest.clearAllMocks(); + }); - const dbError = new Error('Database connection failed'); - (mockPool.query as jest.Mock).mockRejectedValue(dbError); + it('authenticates the request before enforcing admin access', async () => { + const request = {} as FastifyRequest; + const reply = createReply(); - // Test would call requireAdmin and verify 500 response + await fastify.requireAdmin(request, reply as FastifyReply); + + expect(authenticateMock).toHaveBeenCalledTimes(1); + expect(mockPool.query).toHaveBeenCalledTimes(1); + expect(request.userContext?.isAdmin).toBe(true); + expect(reply.code).not.toHaveBeenCalled(); + expect(reply.send).not.toHaveBeenCalled(); + }); + + it('rejects non-admin users with 403', async () => { + mockPool.query.mockResolvedValue({ rows: [] }); + const request = {} as FastifyRequest; + const reply = createReply(); + + await fastify.requireAdmin(request, reply as FastifyReply); + + expect(authenticateMock).toHaveBeenCalledTimes(1); + expect(mockPool.query).toHaveBeenCalledTimes(1); + expect(reply.code).toHaveBeenCalledWith(403); + expect(reply.send).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Admin access required', }); }); - describe('Pool management', () => { - it('should set and use database pool for queries', () => { - const testPool = {} as Pool; - setAdminGuardPool(testPool); + it('responds with 500 when database pool is not initialized', async () => { + setAdminGuardPool(null as unknown as Pool); + const request = {} as FastifyRequest; + const reply = createReply(); - // Pool should be available for guards to use - }); + await fastify.requireAdmin(request, reply as FastifyReply); - it('should handle missing pool gracefully', async () => { - // Reset pool to null - setAdminGuardPool(null as any); - - mockRequest = { - userContext: { - userId: 'auth0|123456', - isAdmin: false, - }, - }; - - // Test would call requireAdmin and verify 500 response for missing pool + expect(reply.code).toHaveBeenCalledWith(500); + expect(reply.send).toHaveBeenCalledWith({ + error: 'Internal server error', + message: 'Admin check unavailable', }); }); });