This commit is contained in:
David 2025-12-18 16:56:35 +01:00
parent 840ad49dcb
commit 7748a49def
5 changed files with 45 additions and 18 deletions

View File

@ -12,6 +12,7 @@ import {
UsePipes, UsePipes,
ValidationPipe, ValidationPipe,
NotFoundException, NotFoundException,
BadRequestException,
ParseUUIDPipe, ParseUUIDPipe,
UseGuards, UseGuards,
Inject, Inject,
@ -108,7 +109,14 @@ export class AdminController {
async getAllUsers(@CurrentUser() user: UserPayload): Promise<UserListResponseDto> { async getAllUsers(@CurrentUser() user: UserPayload): Promise<UserListResponseDto> {
this.logger.log(`[ADMIN: ${user.email}] Fetching ALL users from database`); 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); const userDtos = UserMapper.toDtoArray(users);
this.logger.log(`[ADMIN] Retrieved ${users.length} users from database`); this.logger.log(`[ADMIN] Retrieved ${users.length} users from database`);
@ -189,6 +197,12 @@ export class AdminController {
throw new NotFoundException(`User ${id} not found`); 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 // Apply updates
if (dto.firstName) { if (dto.firstName) {
foundUser.updateFirstName(dto.firstName); foundUser.updateFirstName(dto.firstName);

View File

@ -354,7 +354,7 @@ export class BookingsController {
// ADMIN: Fetch ALL bookings from database // ADMIN: Fetch ALL bookings from database
// Others: Fetch only bookings from their organization // Others: Fetch only bookings from their organization
let bookings: any[]; let bookings: any[];
if (user.role === 'admin') { if (user.role === 'ADMIN') {
this.logger.log(`[ADMIN] Fetching ALL bookings from database`); this.logger.log(`[ADMIN] Fetching ALL bookings from database`);
bookings = await this.bookingRepository.findAll(); bookings = await this.bookingRepository.findAll();
} else { } else {

View File

@ -185,7 +185,7 @@ export class OrganizationsController {
} }
// Authorization: Users can only view their own organization (unless admin) // 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'); throw new ForbiddenException('You can only view your own organization');
} }
@ -340,7 +340,7 @@ export class OrganizationsController {
// Fetch organizations // Fetch organizations
let organizations: Organization[]; let organizations: Organization[];
if (user.role === 'admin') { if (user.role === 'ADMIN') {
// Admins can see all organizations // Admins can see all organizations
organizations = await this.organizationRepository.findAll(); organizations = await this.organizationRepository.findAll();
} else { } else {

View File

@ -13,6 +13,7 @@ import {
UsePipes, UsePipes,
ValidationPipe, ValidationPipe,
NotFoundException, NotFoundException,
BadRequestException,
ParseUUIDPipe, ParseUUIDPipe,
ParseIntPipe, ParseIntPipe,
DefaultValuePipe, DefaultValuePipe,
@ -107,12 +108,12 @@ export class UsersController {
this.logger.log(`[User: ${user.email}] Creating user: ${dto.email} (${dto.role})`); this.logger.log(`[User: ${user.email}] Creating user: ${dto.email} (${dto.role})`);
// Authorization: Only ADMIN can assign ADMIN 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'); throw new ForbiddenException('Only platform administrators can create users with ADMIN role');
} }
// Authorization: Managers can only create users in their own organization // 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'); 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) // 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'); 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`); 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 // 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'); throw new ForbiddenException('Only platform administrators can assign ADMIN role');
} }
// Authorization: Managers can only update users in their own organization // 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'); 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}` `[User: ${currentUser.email}] Listing users: page=${page}, pageSize=${pageSize}, role=${role}`
); );
// ADMIN: Fetch ALL users from database // Fetch users from current user's organization
// MANAGER: Fetch only users from their organization this.logger.log(`[User: ${currentUser.email}] Fetching users from organization: ${currentUser.organizationId}`);
let users: User[]; let users = await this.userRepository.findByOrganization(currentUser.organizationId);
if (currentUser.role === 'admin') {
this.logger.log(`[ADMIN] Fetching ALL users from database`); // Security: Non-admin users cannot see ADMIN users
users = await this.userRepository.findAllActive(); 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 { } else {
this.logger.log(`[MANAGER] Fetching users from organization: ${currentUser.organizationId}`); this.logger.log(`[ADMIN] User ${currentUser.email} can see all users including ADMINs in their organization`);
users = await this.userRepository.findByOrganization(currentUser.organizationId);
} }
// Filter by role if provided // Filter by role if provided

View File

@ -247,7 +247,12 @@ export default function UsersManagementPage() {
className={`text-xs font-semibold rounded-full px-3 py-1 ${getRoleBadgeColor( className={`text-xs font-semibold rounded-full px-3 py-1 ${getRoleBadgeColor(
user.role 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' && <option value="ADMIN">Admin</option>} {currentUser?.role === 'ADMIN' && <option value="ADMIN">Admin</option>}
<option value="MANAGER">Manager</option> <option value="MANAGER">Manager</option>