Skip to content

Commit c072fbc

Browse files
committed
Merge remote-tracking branch 'upstream/main' into feat/issue-976-logout-token-invalidation
2 parents 87bb1a4 + da23d38 commit c072fbc

27 files changed

Lines changed: 468 additions & 9 deletions
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { ExecutionContext, ForbiddenException } from '@nestjs/common';
2+
import { Reflector } from '@nestjs/core';
3+
import { RolesGuard } from './roles.guard.js';
4+
import { AuthRole } from '../../common/enums/auth-role.enum.js';
5+
6+
describe('RolesGuard', () => {
7+
let guard: RolesGuard;
8+
let reflector: Reflector;
9+
10+
const createContext = (roles?: string[]): ExecutionContext =>
11+
({
12+
getHandler: () => ({}),
13+
getClass: () => ({}),
14+
switchToHttp: () => ({
15+
getRequest: () => ({ user: roles ? { roles } : undefined }),
16+
}),
17+
}) as unknown as ExecutionContext;
18+
19+
beforeEach(() => {
20+
reflector = new Reflector();
21+
guard = new RolesGuard(reflector);
22+
});
23+
24+
it('should allow access when no roles are required', () => {
25+
jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(undefined);
26+
expect(guard.canActivate(createContext())).toBe(true);
27+
});
28+
29+
it('should allow access when user has a required role', () => {
30+
jest
31+
.spyOn(reflector, 'getAllAndOverride')
32+
.mockReturnValue([AuthRole.ADMIN]);
33+
expect(guard.canActivate(createContext([AuthRole.ADMIN]))).toBe(true);
34+
});
35+
36+
it('should throw ForbiddenException with a clear message when user lacks the role', () => {
37+
jest
38+
.spyOn(reflector, 'getAllAndOverride')
39+
.mockReturnValue([AuthRole.ADMIN]);
40+
expect(() => guard.canActivate(createContext([AuthRole.MENTEE]))).toThrow(
41+
ForbiddenException,
42+
);
43+
expect(() => guard.canActivate(createContext([AuthRole.MENTEE]))).toThrow(
44+
/Requires one of the following roles: admin/,
45+
);
46+
});
47+
48+
it('should throw ForbiddenException when there is no authenticated user', () => {
49+
jest
50+
.spyOn(reflector, 'getAllAndOverride')
51+
.mockReturnValue([AuthRole.ADMIN]);
52+
expect(() => guard.canActivate(createContext())).toThrow(
53+
ForbiddenException,
54+
);
55+
});
56+
});

backend/src/auth/guards/roles.guard.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
1+
import {
2+
CanActivate,
3+
ExecutionContext,
4+
ForbiddenException,
5+
Injectable,
6+
} from '@nestjs/common';
27
import { Reflector } from '@nestjs/core';
38
import { AuthRole } from '../../common/enums/auth-role.enum.js';
49
import { ROLES_KEY } from '../decorators/roles.decorator.js';
@@ -20,6 +25,16 @@ export class RolesGuard implements CanActivate {
2025
const { user } = context
2126
.switchToHttp()
2227
.getRequest<{ user?: JwtAccessTokenPayload }>();
23-
return requiredRoles.some((role) => user?.roles?.includes(role));
28+
const hasRequiredRole = requiredRoles.some((role) =>
29+
user?.roles?.includes(role),
30+
);
31+
32+
if (!hasRequiredRole) {
33+
throw new ForbiddenException(
34+
`Requires one of the following roles: ${requiredRoles.join(', ')}`,
35+
);
36+
}
37+
38+
return true;
2439
}
2540
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { MigrationInterface, QueryRunner } from 'typeorm';
2+
3+
export class AddRoleDescription1722000000001 implements MigrationInterface {
4+
name = 'AddRoleDescription1722000000001';
5+
6+
public async up(queryRunner: QueryRunner): Promise<void> {
7+
await queryRunner.query(`
8+
ALTER TABLE "roles" ADD COLUMN "description" varchar
9+
`);
10+
}
11+
12+
public async down(queryRunner: QueryRunner): Promise<void> {
13+
await queryRunner.query(`
14+
ALTER TABLE "roles" DROP COLUMN IF EXISTS "description"
15+
`);
16+
}
17+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { Test, TestingModule } from '@nestjs/testing';
2+
import { AdminRolesController } from './admin-roles.controller.js';
3+
import { UsersService } from './users.service.js';
4+
import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard.js';
5+
import { RolesGuard } from '../auth/guards/roles.guard.js';
6+
import { AuthRole } from '../common/enums/auth-role.enum.js';
7+
8+
describe('AdminRolesController', () => {
9+
let controller: AdminRolesController;
10+
11+
const mockUsersService = {
12+
assignRole: jest.fn(),
13+
revokeRole: jest.fn(),
14+
};
15+
16+
beforeEach(async () => {
17+
const module: TestingModule = await Test.createTestingModule({
18+
controllers: [AdminRolesController],
19+
providers: [{ provide: UsersService, useValue: mockUsersService }],
20+
})
21+
.overrideGuard(JwtAuthGuard)
22+
.useValue({ canActivate: () => true })
23+
.overrideGuard(RolesGuard)
24+
.useValue({ canActivate: () => true })
25+
.compile();
26+
27+
controller = module.get<AdminRolesController>(AdminRolesController);
28+
});
29+
30+
afterEach(() => {
31+
jest.clearAllMocks();
32+
});
33+
34+
describe('assignRole', () => {
35+
it('should delegate to usersService.assignRole', async () => {
36+
mockUsersService.assignRole.mockResolvedValue({ id: 'user-1' });
37+
38+
const result = await controller.assignRole('user-1', {
39+
role: AuthRole.MENTOR,
40+
});
41+
42+
expect(mockUsersService.assignRole).toHaveBeenCalledWith(
43+
'user-1',
44+
AuthRole.MENTOR,
45+
);
46+
expect(result).toEqual({ id: 'user-1' });
47+
});
48+
});
49+
50+
describe('revokeRole', () => {
51+
it('should delegate to usersService.revokeRole', async () => {
52+
mockUsersService.revokeRole.mockResolvedValue({ id: 'user-1' });
53+
54+
const result = await controller.revokeRole('user-1', AuthRole.MENTOR);
55+
56+
expect(mockUsersService.revokeRole).toHaveBeenCalledWith(
57+
'user-1',
58+
AuthRole.MENTOR,
59+
);
60+
expect(result).toEqual({ id: 'user-1' });
61+
});
62+
});
63+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import {
2+
Body,
3+
Controller,
4+
Delete,
5+
HttpCode,
6+
HttpStatus,
7+
Param,
8+
ParseEnumPipe,
9+
Post,
10+
UseGuards,
11+
} from '@nestjs/common';
12+
import { UsersService } from './users.service.js';
13+
import { AssignRoleDto } from './dto/assign-role.dto.js';
14+
import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard.js';
15+
import { RolesGuard } from '../auth/guards/roles.guard.js';
16+
import { Roles } from '../auth/decorators/roles.decorator.js';
17+
import { AuthRole } from '../common/enums/auth-role.enum.js';
18+
19+
@Controller('admin/users/:userId/roles')
20+
@UseGuards(JwtAuthGuard, RolesGuard)
21+
@Roles(AuthRole.ADMIN)
22+
export class AdminRolesController {
23+
constructor(private readonly usersService: UsersService) {}
24+
25+
@Post()
26+
@HttpCode(HttpStatus.CREATED)
27+
async assignRole(
28+
@Param('userId') userId: string,
29+
@Body() dto: AssignRoleDto,
30+
) {
31+
return this.usersService.assignRole(userId, dto.role);
32+
}
33+
34+
@Delete(':role')
35+
@HttpCode(HttpStatus.OK)
36+
async revokeRole(
37+
@Param('userId') userId: string,
38+
@Param('role', new ParseEnumPipe(AuthRole)) role: AuthRole,
39+
) {
40+
return this.usersService.revokeRole(userId, role);
41+
}
42+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { IsEnum } from 'class-validator';
2+
import { AuthRole } from '../../common/enums/auth-role.enum.js';
3+
4+
export class AssignRoleDto {
5+
@IsEnum(AuthRole)
6+
role: AuthRole;
7+
}

backend/src/users/entities/role.entity.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ export class Role {
1616
@Column({ type: 'enum', enum: AuthRole })
1717
name: AuthRole;
1818

19+
@Column({ type: 'varchar', nullable: true })
20+
description: string | null;
21+
1922
@ManyToOne(() => User, (user) => user.roles, { onDelete: 'CASCADE' })
2023
user: User;
2124

backend/src/users/users.module.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { TypeOrmModule } from '@nestjs/typeorm';
33
import { JwtModule } from '@nestjs/jwt';
44
import { ConfigModule } from '@nestjs/config';
55
import { UsersController } from './users.controller.js';
6+
import { AdminRolesController } from './admin-roles.controller.js';
67
import { ProfilesController } from './profiles.controller.js';
78
import { UsersService } from './users.service.js';
89
import { AvatarController } from './avatar.controller.js';
@@ -34,7 +35,12 @@ import { ProfileCompletenessService } from './profile-completeness.service.js';
3435
StorageModule,
3536
AvailabilityModule,
3637
],
37-
controllers: [UsersController, ProfilesController, AvatarController],
38+
controllers: [
39+
UsersController,
40+
AdminRolesController,
41+
ProfilesController,
42+
AvatarController,
43+
],
3844
providers: [
3945
UsersService,
4046
JwtAuthGuard,

backend/src/users/users.service.spec.ts

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ describe('UsersService', () => {
2121
findOne: jest.fn(),
2222
create: jest.fn(),
2323
save: jest.fn(),
24+
increment: jest.fn(),
2425
findAndCount: jest.fn(),
2526
};
2627

2728
const mockRoleRepo = {
2829
create: jest.fn(),
2930
save: jest.fn(),
31+
delete: jest.fn(),
3032
};
3133

3234
const mockMentorProfileRepo = {
@@ -387,6 +389,98 @@ describe('UsersService', () => {
387389
});
388390
});
389391

392+
describe('createUser', () => {
393+
it('should create a user with a default mentee role', async () => {
394+
const createdUser = { ...mockUser, id: 'user-2' };
395+
mockUserRepo.create.mockReturnValue(createdUser);
396+
mockUserRepo.save.mockResolvedValue(createdUser);
397+
mockRoleRepo.create.mockReturnValue({
398+
name: AuthRole.MENTEE,
399+
user: createdUser,
400+
});
401+
mockRoleRepo.save.mockResolvedValue({});
402+
mockUserRepo.findOne.mockResolvedValue({
403+
...createdUser,
404+
roles: [{ name: AuthRole.MENTEE }],
405+
});
406+
407+
const result = await service.createUser('new-wallet');
408+
409+
expect(mockUserRepo.create).toHaveBeenCalledWith({
410+
walletAddress: 'new-wallet',
411+
});
412+
expect(mockRoleRepo.create).toHaveBeenCalledWith({
413+
name: AuthRole.MENTEE,
414+
user: createdUser,
415+
});
416+
expect(result.roles).toEqual([{ name: AuthRole.MENTEE }]);
417+
});
418+
});
419+
420+
describe('assignRole', () => {
421+
it('should assign a new role and bump the token version', async () => {
422+
mockUserRepo.findOne.mockResolvedValue({ ...mockUser, roles: [] });
423+
mockRoleRepo.create.mockReturnValue({
424+
name: AuthRole.ADMIN,
425+
user: mockUser,
426+
});
427+
mockRoleRepo.save.mockResolvedValue({});
428+
mockUserRepo.increment.mockResolvedValue({});
429+
430+
await service.assignRole('user-1', AuthRole.ADMIN);
431+
432+
expect(mockRoleRepo.create).toHaveBeenCalledWith({
433+
name: AuthRole.ADMIN,
434+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
435+
user: expect.objectContaining({ id: 'user-1' }),
436+
});
437+
expect(mockUserRepo.increment).toHaveBeenCalledWith(
438+
{ id: 'user-1' },
439+
'tokenVersion',
440+
1,
441+
);
442+
});
443+
444+
it('should throw ConflictException when user already has the role', async () => {
445+
mockUserRepo.findOne.mockResolvedValue({
446+
...mockUser,
447+
roles: [{ name: AuthRole.ADMIN }],
448+
});
449+
450+
await expect(
451+
service.assignRole('user-1', AuthRole.ADMIN),
452+
).rejects.toThrow(ConflictException);
453+
});
454+
});
455+
456+
describe('revokeRole', () => {
457+
it('should revoke an existing role and bump the token version', async () => {
458+
mockUserRepo.findOne.mockResolvedValue({
459+
...mockUser,
460+
roles: [{ id: 'role-1', name: AuthRole.ADMIN }],
461+
});
462+
mockRoleRepo.delete.mockResolvedValue({});
463+
mockUserRepo.increment.mockResolvedValue({});
464+
465+
await service.revokeRole('user-1', AuthRole.ADMIN);
466+
467+
expect(mockRoleRepo.delete).toHaveBeenCalledWith({ id: 'role-1' });
468+
expect(mockUserRepo.increment).toHaveBeenCalledWith(
469+
{ id: 'user-1' },
470+
'tokenVersion',
471+
1,
472+
);
473+
});
474+
475+
it('should throw NotFoundException when user does not have the role', async () => {
476+
mockUserRepo.findOne.mockResolvedValue({ ...mockUser, roles: [] });
477+
478+
await expect(
479+
service.revokeRole('user-1', AuthRole.ADMIN),
480+
).rejects.toThrow(NotFoundException);
481+
});
482+
});
483+
390484
describe('updateUser', () => {
391485
it('should update user fields and save', async () => {
392486
const existingUser = { ...mockUser, email: undefined };
@@ -428,6 +522,25 @@ describe('UsersService', () => {
428522
});
429523
});
430524

525+
describe('getTokenVersion', () => {
526+
it('should return the token version for an existing user', async () => {
527+
mockUserRepo.findOne.mockResolvedValue({
528+
id: 'user-1',
529+
tokenVersion: 3,
530+
});
531+
532+
const result = await service.getTokenVersion('user-1');
533+
expect(result).toBe(3);
534+
});
535+
536+
it('should return null when the user does not exist', async () => {
537+
mockUserRepo.findOne.mockResolvedValue(null);
538+
539+
const result = await service.getTokenVersion('missing');
540+
expect(result).toBeNull();
541+
});
542+
});
543+
431544
describe('deactivateUser', () => {
432545
it('should set status to deleted and save', async () => {
433546
const existingUser = { ...mockUser, status: UserStatus.ACTIVE };

0 commit comments

Comments
 (0)