From 7748a49def491f9310f4732c2453c76546be400d Mon Sep 17 00:00:00 2001 From: David Date: Thu, 18 Dec 2025 16:56:35 +0100 Subject: [PATCH] fix --- .../controllers/admin.controller.ts | 16 ++++++++- .../controllers/bookings.controller.ts | 2 +- .../controllers/organizations.controller.ts | 4 +-- .../controllers/users.controller.ts | 34 ++++++++++++------- .../app/dashboard/settings/users/page.tsx | 7 +++- 5 files changed, 45 insertions(+), 18 deletions(-) diff --git a/apps/backend/src/application/controllers/admin.controller.ts b/apps/backend/src/application/controllers/admin.controller.ts index cefa202..8bde9fa 100644 --- a/apps/backend/src/application/controllers/admin.controller.ts +++ b/apps/backend/src/application/controllers/admin.controller.ts @@ -12,6 +12,7 @@ import { UsePipes, ValidationPipe, NotFoundException, + BadRequestException, ParseUUIDPipe, UseGuards, Inject, @@ -108,7 +109,14 @@ export class AdminController { async getAllUsers(@CurrentUser() user: UserPayload): Promise { this.logger.log(`[ADMIN: ${user.email}] Fetching ALL users from database`); - const users = await this.userRepository.findAll(); + let users = await this.userRepository.findAll(); + + // Security: Non-admin users (MANAGER and below) cannot see ADMIN users + if (user.role !== 'ADMIN') { + users = users.filter(u => u.role !== 'ADMIN'); + this.logger.log(`[SECURITY] Non-admin user ${user.email} - filtered out ADMIN users`); + } + const userDtos = UserMapper.toDtoArray(users); this.logger.log(`[ADMIN] Retrieved ${users.length} users from database`); @@ -189,6 +197,12 @@ export class AdminController { throw new NotFoundException(`User ${id} not found`); } + // Security: Prevent users from changing their own role + if (dto.role && id === user.id) { + this.logger.warn(`[SECURITY] User ${user.email} attempted to change their own role`); + throw new BadRequestException('You cannot change your own role'); + } + // Apply updates if (dto.firstName) { foundUser.updateFirstName(dto.firstName); diff --git a/apps/backend/src/application/controllers/bookings.controller.ts b/apps/backend/src/application/controllers/bookings.controller.ts index e9e933f..5da7b5c 100644 --- a/apps/backend/src/application/controllers/bookings.controller.ts +++ b/apps/backend/src/application/controllers/bookings.controller.ts @@ -354,7 +354,7 @@ export class BookingsController { // ADMIN: Fetch ALL bookings from database // Others: Fetch only bookings from their organization let bookings: any[]; - if (user.role === 'admin') { + if (user.role === 'ADMIN') { this.logger.log(`[ADMIN] Fetching ALL bookings from database`); bookings = await this.bookingRepository.findAll(); } else { diff --git a/apps/backend/src/application/controllers/organizations.controller.ts b/apps/backend/src/application/controllers/organizations.controller.ts index 49359aa..821ca37 100644 --- a/apps/backend/src/application/controllers/organizations.controller.ts +++ b/apps/backend/src/application/controllers/organizations.controller.ts @@ -185,7 +185,7 @@ export class OrganizationsController { } // Authorization: Users can only view their own organization (unless admin) - if (user.role !== 'admin' && organization.id !== user.organizationId) { + if (user.role !== 'ADMIN' && organization.id !== user.organizationId) { throw new ForbiddenException('You can only view your own organization'); } @@ -340,7 +340,7 @@ export class OrganizationsController { // Fetch organizations let organizations: Organization[]; - if (user.role === 'admin') { + if (user.role === 'ADMIN') { // Admins can see all organizations organizations = await this.organizationRepository.findAll(); } else { diff --git a/apps/backend/src/application/controllers/users.controller.ts b/apps/backend/src/application/controllers/users.controller.ts index 4abd859..e6c46d4 100644 --- a/apps/backend/src/application/controllers/users.controller.ts +++ b/apps/backend/src/application/controllers/users.controller.ts @@ -13,6 +13,7 @@ import { UsePipes, ValidationPipe, NotFoundException, + BadRequestException, ParseUUIDPipe, ParseIntPipe, DefaultValuePipe, @@ -107,12 +108,12 @@ export class UsersController { this.logger.log(`[User: ${user.email}] Creating user: ${dto.email} (${dto.role})`); // Authorization: Only ADMIN can assign ADMIN role - if (dto.role === 'ADMIN' && user.role !== 'admin') { + if (dto.role === 'ADMIN' && user.role !== 'ADMIN') { throw new ForbiddenException('Only platform administrators can create users with ADMIN role'); } // Authorization: Managers can only create users in their own organization - if (user.role === 'manager' && dto.organizationId !== user.organizationId) { + if (user.role === 'MANAGER' && dto.organizationId !== user.organizationId) { throw new ForbiddenException('You can only create users in your own organization'); } @@ -194,7 +195,7 @@ export class UsersController { } // Authorization: Can only view users in same organization (unless admin) - if (currentUser.role !== 'admin' && user.organizationId !== currentUser.organizationId) { + if (currentUser.role !== 'ADMIN' && user.organizationId !== currentUser.organizationId) { throw new ForbiddenException('You can only view users in your organization'); } @@ -239,13 +240,19 @@ export class UsersController { throw new NotFoundException(`User ${id} not found`); } + // Security: Prevent users from changing their own role + if (dto.role && id === currentUser.id) { + this.logger.warn(`[SECURITY] User ${currentUser.email} attempted to change their own role`); + throw new BadRequestException('You cannot change your own role'); + } + // Authorization: Only ADMIN can assign ADMIN role - if (dto.role === 'ADMIN' && currentUser.role !== 'admin') { + if (dto.role === 'ADMIN' && currentUser.role !== 'ADMIN') { throw new ForbiddenException('Only platform administrators can assign ADMIN role'); } // Authorization: Managers can only update users in their own organization - if (currentUser.role === 'manager' && user.organizationId !== currentUser.organizationId) { + if (currentUser.role === 'MANAGER' && user.organizationId !== currentUser.organizationId) { throw new ForbiddenException('You can only update users in your own organization'); } @@ -363,15 +370,16 @@ export class UsersController { `[User: ${currentUser.email}] Listing users: page=${page}, pageSize=${pageSize}, role=${role}` ); - // ADMIN: Fetch ALL users from database - // MANAGER: Fetch only users from their organization - let users: User[]; - if (currentUser.role === 'admin') { - this.logger.log(`[ADMIN] Fetching ALL users from database`); - users = await this.userRepository.findAllActive(); + // Fetch users from current user's organization + this.logger.log(`[User: ${currentUser.email}] Fetching users from organization: ${currentUser.organizationId}`); + let users = await this.userRepository.findByOrganization(currentUser.organizationId); + + // Security: Non-admin users cannot see ADMIN users + if (currentUser.role !== 'ADMIN') { + users = users.filter(u => u.role !== DomainUserRole.ADMIN); + this.logger.log(`[SECURITY] Non-admin user ${currentUser.email} - filtered out ADMIN users`); } else { - this.logger.log(`[MANAGER] Fetching users from organization: ${currentUser.organizationId}`); - users = await this.userRepository.findByOrganization(currentUser.organizationId); + this.logger.log(`[ADMIN] User ${currentUser.email} can see all users including ADMINs in their organization`); } // Filter by role if provided diff --git a/apps/frontend/app/dashboard/settings/users/page.tsx b/apps/frontend/app/dashboard/settings/users/page.tsx index efa126c..1241fa9 100644 --- a/apps/frontend/app/dashboard/settings/users/page.tsx +++ b/apps/frontend/app/dashboard/settings/users/page.tsx @@ -247,7 +247,12 @@ export default function UsersManagementPage() { className={`text-xs font-semibold rounded-full px-3 py-1 ${getRoleBadgeColor( user.role )}`} - disabled={changeRoleMutation.isPending || (user.role === 'ADMIN' && currentUser?.role !== 'ADMIN')} + disabled={ + changeRoleMutation.isPending || + (user.role === 'ADMIN' && currentUser?.role !== 'ADMIN') || + user.id === currentUser?.id + } + title={user.id === currentUser?.id ? 'You cannot change your own role' : ''} > {currentUser?.role === 'ADMIN' && }