Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions backend/src/modules/auth/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1214,8 +1214,9 @@ export class AuthController {
async updateUserRole(
@Param('id') id: string,
@Body() dto: UpdateRoleDto,
@Req() request: Request & { user: { id: string } },
): Promise<User> {
return this.userService.updateRole(id, dto.role);
return this.userService.updateRole(id, dto.role, request.user.id);
}

/**
Expand All @@ -1226,8 +1227,11 @@ export class AuthController {
@Delete('users/:id')
@UseGuards(AdminGuard)
@HttpCode(HttpStatus.NO_CONTENT)
async deleteUser(@Param('id') id: string): Promise<void> {
await this.userService.deleteUser(id);
async deleteUser(
@Param('id') id: string,
@Req() request: Request & { user: { id: string } },
): Promise<void> {
await this.userService.deleteUser(id, request.user.id);
}

/**
Expand Down
19 changes: 19 additions & 0 deletions backend/src/modules/auth/services/user.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ describe('UserService', () => {
'Cannot demote the last local administrator',
);
});

it('should throw BadRequestException when user tries to change their own role', async () => {
await expect(service.updateRole('1', UserRole.USER, '1')).rejects.toThrow(
'Cannot modify your own role',
);
});
});

describe('deleteUser', () => {
Expand Down Expand Up @@ -207,6 +213,19 @@ describe('UserService', () => {
// Note: The actual cascade deletion of refresh tokens is handled by the database
// via the foreign key constraint with ON DELETE CASCADE
});

it('should throw BadRequestException when user tries to delete themselves', async () => {
await expect(service.deleteUser('1', '1')).rejects.toThrow(
'Cannot delete yourself',
);
});

it('should throw BadRequestException when SAML user tries to delete themselves', async () => {
// This confirms the check is provider-agnostic
await expect(service.deleteUser('2', '2')).rejects.toThrow(
'Cannot delete yourself',
);
});
});

describe('countAdministrators', () => {
Expand Down
31 changes: 25 additions & 6 deletions backend/src/modules/auth/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,12 @@ export class UserService {
/**
* Update a user's role
*/
async updateRole(id: string, role: UserRole): Promise<User> {
await this.validateRoleChange(id, role);
async updateRole(
id: string,
role: UserRole,
requestingUserId?: string,
): Promise<User> {
await this.validateRoleChange(id, role, requestingUserId);

const user = await this.findById(id);
user.role = role;
Expand All @@ -61,8 +65,8 @@ export class UserService {
/**
* Delete a user
*/
async deleteUser(id: string): Promise<void> {
await this.validateUserDeletion(id);
async deleteUser(id: string, requestingUserId?: string): Promise<void> {
await this.validateUserDeletion(id, requestingUserId);

const user = await this.findById(id);
await this.userRepository.remove(user);
Expand Down Expand Up @@ -117,7 +121,15 @@ export class UserService {
* Validate that a role change is allowed
* Throws an error if the change would violate last admin protection
*/
async validateRoleChange(userId: string, newRole: UserRole): Promise<void> {
async validateRoleChange(
userId: string,
newRole: UserRole,
requestingUserId?: string,
): Promise<void> {
if (requestingUserId && userId === requestingUserId) {
throw new BadRequestException('Cannot modify your own role');
}

if (newRole === UserRole.USER) {
const isLastLocal = await this.isLastLocalAdministrator(userId);
if (isLastLocal) {
Expand All @@ -132,7 +144,14 @@ export class UserService {
* Validate that a user deletion is allowed
* Throws an error if the deletion would violate last admin protection
*/
async validateUserDeletion(userId: string): Promise<void> {
async validateUserDeletion(
userId: string,
requestingUserId?: string,
): Promise<void> {
if (requestingUserId && userId === requestingUserId) {
throw new BadRequestException('Cannot delete yourself');
}

const isLastLocal = await this.isLastLocalAdministrator(userId);
if (isLastLocal) {
throw new BadRequestException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
[value]="user.role"
(selectionChange)="changeUserRole(user, $event.value)"
[disabled]="!canModifyUser(user)"
[matTooltip]="isLastAdmin(user) ? 'Cannot modify last administrator' : 'Change role'"
[matTooltip]="getModifyTooltip(user)"
class="role-select"
>
<mat-option [value]="UserRole.USER">User</mat-option>
Expand All @@ -76,7 +76,7 @@
color="warn"
(click)="deleteUser(user)"
[disabled]="!canModifyUser(user)"
[matTooltip]="isLastAdmin(user) ? 'Cannot delete last administrator' : 'Delete user'"
[matTooltip]="getDeleteTooltip(user)"
>
<mat-icon>delete</mat-icon>
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class UserManagementComponent implements OnInit {
users = signal<UserResponse[]>([]);
invites = signal<InviteToken[]>([]);
currentUser = signal<UserResponse | null>(null);
loggedInUserId = signal<string | null>(null);
loading = signal(false);
invitesLoading = signal(false);
passwordLoginAllowed = signal(true);
Expand Down Expand Up @@ -107,7 +108,8 @@ export class UserManagementComponent implements OnInit {
this.authService.getCurrentUser().subscribe({
next: (user) => {
if (user) {
// Find the full user details from the users list
this.loggedInUserId.set(user.id);
// Find the full user details from the users list if available
const fullUser = this.users().find((u) => u.id === user.id);
if (fullUser) {
this.currentUser.set(fullUser);
Expand Down Expand Up @@ -279,8 +281,8 @@ export class UserManagementComponent implements OnInit {

canModifyUser(user: UserResponse): boolean {
// Can't modify yourself
const current = this.currentUser();
if (current && current.id === user.id) {
const currentId = this.loggedInUserId();
if (currentId && currentId === user.id) {
return false;
}

Expand Down Expand Up @@ -327,4 +329,26 @@ export class UserManagementComponent implements OnInit {
const hoursUntilExpiry = (expires.getTime() - now.getTime()) / (1000 * 60 * 60);
return hoursUntilExpiry < 24 && hoursUntilExpiry > 0;
}

getModifyTooltip(user: UserResponse): string {
const currentId = this.loggedInUserId();
if (currentId && currentId === user.id) {
return 'Cannot modify your own role';
}
if (this.isLastAdmin(user)) {
return 'Cannot modify last administrator';
}
return 'Change role';
}

getDeleteTooltip(user: UserResponse): string {
const currentId = this.loggedInUserId();
if (currentId && currentId === user.id) {
return 'Cannot delete yourself';
}
if (this.isLastAdmin(user)) {
return 'Cannot delete last administrator';
}
return 'Delete user';
}
}