Skip to content

Commit 4f25ed6

Browse files
authored
#3881 - Validate user account for all routes (#3985)
# Validate user account for all routes ## New global guard and decorator - [x] New guard `RequiresUserAccountGuard` has been introduced globally to ensure that routes are authorized with the user token which belongs to valid SIMS user. There are exceptional routes like public routes and routes used that setup the user itself are skipped from this validation. - [x] New decorator `@RequiresUserAccount()` is introduced to get the metadata context for the guard. ## Student page container - [x] Student page container updated to NOT render restriction and SIN banners for pages which does not require a valid student account. ## E2E Tests - [x] The existing method to mock the student info from token `mockUserLoginInfo()` does not have a way to restore the mock, if the mock needs to be restored for other tests in same suite. Hence refactored the code to use `jest.spyOn()` to mock the userService method implementation and also created a reset mock method to reset the mock as required in the test suite. Here is an example. **Mock applied** ![image](https://github.com/user-attachments/assets/9f0277d8-27c9-4c1a-98d0-8fd5fc5583a8) **Mock Reset** ![image](https://github.com/user-attachments/assets/0e9115b3-d456-43eb-aace-d282d68d5297) - [x] Created new Auth E2E tests ![image](https://github.com/user-attachments/assets/b3e48542-c314-4ee7-81e3-6807a6ea32f6) ## Volar extension - [x] Updated the workspace file with deprecated vue extension by replacing with recommended extension. ![image](https://github.com/user-attachments/assets/1ee3de6f-b315-4d87-acd2-32f58dce0891)
1 parent 0d57f6a commit 4f25ed6

27 files changed

+274
-39
lines changed

sims.code-workspace

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
],
2828
"extensions": {
2929
"recommendations": [
30-
"vue.vscode-typescript-vue-plugin",
30+
"Vue.volar",
3131
"dbaeumer.vscode-eslint",
3232
"esbenp.prettier-vscode",
3333
"streetsidesoftware.code-spell-checker",

sources/packages/backend/apps/api/src/auth/_tests_/e2e/auth.e2e-spec.ts

+45-3
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@ import { AuthTestController } from "../../../testHelpers/controllers/auth-test/a
1919
import { DiscoveryModule } from "@golevelup/nestjs-discovery";
2020
import { DataSource } from "typeorm";
2121
import { JwtService } from "@nestjs/jwt";
22-
import { INVALID_BETA_USER } from "../../../constants";
23-
import { BEARER_AUTH_TYPE } from "../../../testHelpers";
22+
import { INVALID_BETA_USER, MISSING_USER_ACCOUNT } from "../../../constants";
23+
import {
24+
BEARER_AUTH_TYPE,
25+
mockUserLoginInfo,
26+
resetMockUserLoginInfo,
27+
} from "../../../testHelpers";
2428
import * as dayjs from "dayjs";
29+
import { Student, User } from "@sims/sims-db";
2530

2631
describe("Authentication (e2e)", () => {
2732
// Nest application to be shared for all e2e tests
@@ -38,6 +43,7 @@ describe("Authentication (e2e)", () => {
3843
let configService: ConfigService;
3944
let db: E2EDataSources;
4045
let studentDecodedToken: any;
46+
let moduleFixture: TestingModule;
4147

4248
beforeAll(async () => {
4349
await KeycloakConfig.load();
@@ -57,7 +63,7 @@ describe("Authentication (e2e)", () => {
5763
);
5864
aestAccessToken = aestToken.access_token;
5965

60-
const moduleFixture: TestingModule = await Test.createTestingModule({
66+
moduleFixture = await Test.createTestingModule({
6167
imports: [AppModule, createZeebeModuleMock(), DiscoveryModule],
6268
// AuthTestController is used only for e2e tests and could be
6369
// changed as needed to implement more test scenarios.
@@ -80,6 +86,8 @@ describe("Authentication (e2e)", () => {
8086
jest
8187
.spyOn(configService, "allowBetaUsersOnly", "get")
8288
.mockReturnValue(false);
89+
// Reset mock user login info.
90+
await resetMockUserLoginInfo(moduleFixture);
8391
});
8492

8593
it("Load publicKey from Keycloak", async () => {
@@ -254,6 +262,40 @@ describe("Authentication (e2e)", () => {
254262
expect(resp.body.email).toBeTruthy();
255263
});
256264
});
265+
266+
it(
267+
"Should return a HttpStatus FORBIDDEN(403) when there is no user account associated to the user token " +
268+
"to a default route that requires a user account.",
269+
async () => {
270+
await mockUserLoginInfo(moduleFixture, {
271+
id: null,
272+
user: { id: null, isActive: null } as User,
273+
} as Student);
274+
return request(app.getHttpServer())
275+
.get("/auth-test/default-requires-user-route")
276+
.auth(studentAccessToken, { type: "bearer" })
277+
.expect(HttpStatus.FORBIDDEN)
278+
.expect({
279+
message: "No user account has been associated to the user token.",
280+
errorType: MISSING_USER_ACCOUNT,
281+
});
282+
},
283+
);
284+
285+
it(
286+
"Should return a HttpStatus OK(200) when there is no user account associated to the user token " +
287+
"to a route that does not requires a user account.",
288+
async () => {
289+
await mockUserLoginInfo(moduleFixture, {
290+
id: null,
291+
user: { id: null, isActive: null } as User,
292+
} as Student);
293+
return request(app.getHttpServer())
294+
.get("/auth-test/user-not-required-route")
295+
.auth(studentAccessToken, { type: "bearer" })
296+
.expect(HttpStatus.OK);
297+
},
298+
);
257299
});
258300

259301
afterAll(async () => {

sources/packages/backend/apps/api/src/auth/auth.module.ts

+5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
RequiresStudentAccountGuard,
2525
InstitutionBCPublicGuard,
2626
InstitutionStudentDataAccessGuard,
27+
RequiresUserAccountGuard,
2728
} from "./guards";
2829
import { RolesGuard } from "./guards/roles.guard";
2930
import { ConfigModule } from "@sims/utilities/config";
@@ -99,6 +100,10 @@ const jwtModule = JwtModule.register({
99100
provide: APP_GUARD,
100101
useClass: InstitutionStudentDataAccessGuard,
101102
},
103+
{
104+
provide: APP_GUARD,
105+
useClass: RequiresUserAccountGuard,
106+
},
102107
],
103108
exports: [jwtModule, KeycloakService],
104109
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { Reflector } from "@nestjs/core";
2+
/**
3+
* Specifies when a user account must be already created in order to access a route.
4+
*/
5+
export const RequiresUserAccount = Reflector.createDecorator<boolean>();

sources/packages/backend/apps/api/src/auth/decorators/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ export * from "./allow-inactive-user.decorator";
1010
export * from "./groups.decorator";
1111
export * from "./student/check-sin-status.decorator";
1212
export * from "./student/requires-student-account.decorator";
13+
export * from "./common.decorator";

sources/packages/backend/apps/api/src/auth/guards/groups.guard.ts

+21-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
1-
import { Injectable, CanActivate, ExecutionContext } from "@nestjs/common";
1+
import {
2+
Injectable,
3+
CanActivate,
4+
ExecutionContext,
5+
ForbiddenException,
6+
} from "@nestjs/common";
27
import { Reflector } from "@nestjs/core";
38
import { GROUPS_KEY } from "../decorators";
49
import { AuthorizedParties, UserGroups, IUserToken } from "..";
10+
import { MISSING_GROUP_ACCESS } from "../../constants";
11+
import { ApiProcessError } from "../../types";
512

613
/**
714
* Allow the authorization based on groups making use
@@ -34,8 +41,20 @@ export class GroupsGuard implements CanActivate {
3441
// Check if the user has any of the groups required to have access to the resource.
3542
// UserGroups 'aest/user/x' and 'aest/user' are valid, as they start with 'aest/user'
3643
// we are here looking for matches, not the exact string.
37-
return requiredGroups.some((group: string) =>
44+
const hasGroupAccess = requiredGroups.some((group: string) =>
3845
userToken.groups?.some((userGroup) => userGroup.startsWith(group)),
3946
);
47+
48+
// Throw custom error to be handled by the consumer.
49+
if (!hasGroupAccess) {
50+
throw new ForbiddenException(
51+
new ApiProcessError(
52+
"Required group permission is missing for the user.",
53+
MISSING_GROUP_ACCESS,
54+
),
55+
);
56+
}
57+
58+
return true;
4059
}
4160
}

sources/packages/backend/apps/api/src/auth/guards/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ export * from "./active-user.guard";
88
export * from "./groups.guard";
99
export * from "./student/sin-validation.guard";
1010
export * from "./student/requires-student-account.guard";
11+
export * from "./requires-user-account.guard";
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import {
2+
Injectable,
3+
CanActivate,
4+
ExecutionContext,
5+
ForbiddenException,
6+
} from "@nestjs/common";
7+
import { Reflector } from "@nestjs/core";
8+
import { IS_PUBLIC_KEY, RequiresUserAccount } from "../decorators";
9+
import { IUserToken } from "apps/api/src/auth/userToken.interface";
10+
import { MISSING_USER_ACCOUNT } from "../../constants";
11+
import { ApiProcessError } from "../../types";
12+
13+
/**
14+
* Validates that a user account must be already created in order to access a route.
15+
* Public routes and routes that do not require a user account are skipped.
16+
*/
17+
@Injectable()
18+
export class RequiresUserAccountGuard implements CanActivate {
19+
constructor(private readonly reflector: Reflector) {}
20+
21+
canActivate(context: ExecutionContext): boolean {
22+
// Check if the route is public.
23+
const isPublic = this.reflector.getAllAndOverride<boolean>(IS_PUBLIC_KEY, [
24+
context.getHandler(),
25+
context.getClass(),
26+
]);
27+
// If the route is public, no validation is required.
28+
if (isPublic) {
29+
return true;
30+
}
31+
32+
const requiresUserAccount = this.reflector.getAllAndOverride(
33+
RequiresUserAccount,
34+
[context.getHandler(), context.getClass()],
35+
);
36+
37+
if (requiresUserAccount === false) {
38+
return true;
39+
}
40+
41+
const { user } = context.switchToHttp().getRequest();
42+
const userToken = user as IUserToken;
43+
44+
if (!userToken?.userId) {
45+
throw new ForbiddenException(
46+
new ApiProcessError(
47+
"No user account has been associated to the user token.",
48+
MISSING_USER_ACCOUNT,
49+
),
50+
);
51+
}
52+
53+
return true;
54+
}
55+
}

sources/packages/backend/apps/api/src/constants/error-code.constants.ts

+3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ export const OFFERING_START_DATE_ERROR = "OFFERING_START_DATE_ERROR";
22
export const INVALID_STUDY_DATES = "INVALID_STUDY_DATES";
33
export const OFFERING_INTENSITY_MISMATCH = "OFFERING_INTENSITY_MISMATCH";
44
export const MISSING_STUDENT_ACCOUNT = "MISSING_STUDENT_ACCOUNT";
5+
export const MISSING_USER_ACCOUNT = "MISSING_USER_ACCOUNT";
6+
export const MISSING_GROUP_ACCESS = "MISSING_GROUP_ACCESS";
7+
export const MISSING_USER_INFO = "MISSING_USER_INFO";
58
export const INVALID_APPLICATION_NUMBER = "INVALID_APPLICATION_NUMBER";
69
/**
710
* An application can have only one pending appeal at a time.

sources/packages/backend/apps/api/src/route-controllers/assessment/_tests_/e2e/assessment.students.controller.getAssessmentAwardDetails.e2e-spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe("AssessmentStudentsController(e2e)-getAssessmentAwardDetails", () => {
4242
db = createE2EDataSources(dataSource);
4343
// Shared student to be used for tests that would not need further student customization.
4444
sharedStudent = await saveFakeStudent(db.dataSource);
45-
mockUserLoginInfo(module, sharedStudent);
45+
await mockUserLoginInfo(module, sharedStudent);
4646
// Valid MSFAA Number that will be part of the expected returned values.
4747
sharedMSFAANumber = await db.msfaaNumber.save(
4848
createFakeMSFAANumber(

sources/packages/backend/apps/api/src/route-controllers/audit/audit.controller.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { Controller, Post, Req, Body } from "@nestjs/common";
22
import { ApiTags } from "@nestjs/swagger";
33
import { AuthorizedParties, IUserToken } from "../../auth";
4-
import { AllowAuthorizedParty, UserToken } from "../../auth/decorators";
4+
import {
5+
AllowAuthorizedParty,
6+
RequiresUserAccount,
7+
UserToken,
8+
} from "../../auth/decorators";
59
import BaseController from "../BaseController";
610
import { AuditService } from "../../services";
711
import { Request } from "express";
@@ -13,6 +17,7 @@ import { AuditAPIInDTO } from "./models/audit.dto";
1317
AuthorizedParties.supportingUsers,
1418
AuthorizedParties.aest,
1519
)
20+
@RequiresUserAccount(false)
1621
@Controller("audit")
1722
@ApiTags("audit")
1823
export class AuditController extends BaseController {

sources/packages/backend/apps/api/src/route-controllers/dynamic-form/dynamic-form.controller.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { Controller, Get, Param } from "@nestjs/common";
22
import BaseController from "../BaseController";
33
import { FormService } from "../../services";
4-
import { AllowAuthorizedParty } from "../../auth/decorators";
4+
import {
5+
AllowAuthorizedParty,
6+
RequiresUserAccount,
7+
} from "../../auth/decorators";
58
import { AuthorizedParties } from "../../auth/authorized-parties.enum";
69
import { ApiTags } from "@nestjs/swagger";
710
import { FormNameParamAPIInDTO } from "./models/form.dto";
@@ -12,6 +15,7 @@ import { FormNameParamAPIInDTO } from "./models/form.dto";
1215
AuthorizedParties.supportingUsers,
1316
AuthorizedParties.aest,
1417
)
18+
@RequiresUserAccount(false)
1519
@Controller("dynamic-form")
1620
@ApiTags("dynamic-form")
1721
export class DynamicFormController extends BaseController {

sources/packages/backend/apps/api/src/route-controllers/institution-user/institution-user.institutions.controller.ts

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
AllowAuthorizedParty,
1515
AllowInactiveUser,
1616
IsInstitutionAdmin,
17+
RequiresUserAccount,
1718
UserToken,
1819
} from "../../auth/decorators";
1920
import { InstitutionService, UserService, BCeIDService } from "../../services";
@@ -197,6 +198,7 @@ export class InstitutionUserInstitutionsController extends BaseController {
197198
* @returns information to support the institution login process and
198199
* the decisions that need happen to complete the process.
199200
*/
201+
@RequiresUserAccount(false)
200202
@AllowInactiveUser()
201203
@Get("status")
202204
async getInstitutionUserStatus(

sources/packages/backend/apps/api/src/route-controllers/institution/institution.institutions.controller.ts

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { AuthorizedParties } from "../../auth/authorized-parties.enum";
1111
import {
1212
AllowAuthorizedParty,
1313
IsInstitutionAdmin,
14+
RequiresUserAccount,
1415
UserToken,
1516
} from "../../auth/decorators";
1617
import { InstitutionService, UserService } from "../../services";
@@ -58,6 +59,7 @@ export class InstitutionInstitutionsController extends BaseController {
5859
@ApiUnprocessableEntityResponse({
5960
description: "Institution user already exist",
6061
})
62+
@RequiresUserAccount(false)
6163
@Post()
6264
async createInstitutionWithAssociatedUser(
6365
@Body() payload: CreateInstitutionAPIInDTO,
@@ -139,6 +141,7 @@ export class InstitutionInstitutionsController extends BaseController {
139141
* list (key/value pair) schema.
140142
* @returns institutions types in an option list (key/value pair) schema.
141143
*/
144+
@RequiresUserAccount(false)
142145
@Get("type/options-list")
143146
async getInstitutionTypeOptions(): Promise<OptionItemAPIOutDTO[]> {
144147
return this.institutionControllerService.getInstitutionTypeOptions();

sources/packages/backend/apps/api/src/route-controllers/student-account-applications/student-account-application.students.controller.ts

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { AuthorizedParties } from "../../auth/authorized-parties.enum";
1717
import {
1818
AllowAuthorizedParty,
1919
RequiresStudentAccount,
20+
RequiresUserAccount,
2021
UserToken,
2122
} from "../../auth/decorators";
2223
import BaseController from "../BaseController";
@@ -66,6 +67,7 @@ export class StudentAccountApplicationStudentsController extends BaseController
6667
"There is already a student account application in progress or the user is already present.",
6768
})
6869
@Post()
70+
@RequiresUserAccount(false)
6971
@RequiresStudentAccount(false)
7072
async create(
7173
@UserToken() userToken: IUserToken,
@@ -110,6 +112,7 @@ export class StudentAccountApplicationStudentsController extends BaseController
110112
* @returns true if there is a pending student account application
111113
* to be assessed by the Ministry, otherwise, false.
112114
*/
115+
@RequiresUserAccount(false)
113116
@RequiresStudentAccount(false)
114117
@Get("has-pending-account-application")
115118
async hasPendingAccountApplication(

sources/packages/backend/apps/api/src/route-controllers/student/student.students.controller.ts

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { AuthorizedParties } from "../../auth/authorized-parties.enum";
2727
import {
2828
AllowAuthorizedParty,
2929
RequiresStudentAccount,
30+
RequiresUserAccount,
3031
UserToken,
3132
} from "../../auth/decorators";
3233
import {
@@ -108,6 +109,7 @@ export class StudentStudentsController extends BaseController {
108109
@ApiForbiddenResponse({
109110
description: "User is not allowed to create a student account.",
110111
})
112+
@RequiresUserAccount(false)
111113
@RequiresStudentAccount(false)
112114
async create(
113115
@UserToken() studentUserToken: StudentUserToken,

sources/packages/backend/apps/api/src/route-controllers/supporting-user/supporting-user.supporting-users.controller.ts

+3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
} from "@nestjs/swagger";
4646
import BaseController from "../BaseController";
4747
import { WorkflowClientService } from "@sims/services";
48+
import { RequiresUserAccount } from "../../auth/decorators";
4849

4950
@AllowAuthorizedParty(AuthorizedParties.supportingUsers)
5051
@Controller("supporting-user")
@@ -70,6 +71,7 @@ export class SupportingUserSupportingUsersController extends BaseController {
7071
* Application.
7172
* @returns application details.
7273
*/
74+
@RequiresUserAccount(false)
7375
@Post(":supportingUserType/application")
7476
@HttpCode(HttpStatus.OK)
7577
@ApiUnprocessableEntityResponse({
@@ -130,6 +132,7 @@ export class SupportingUserSupportingUsersController extends BaseController {
130132
* the supporting data (e.g. parent/partner).
131133
* @param payload data used for validation and to execute the update.
132134
*/
135+
@RequiresUserAccount(false)
133136
@Patch(":supportingUserType")
134137
@ApiUnprocessableEntityResponse({
135138
description:

0 commit comments

Comments
 (0)