From cb4f6a512df065aa5aaa972c85b5383017775961 Mon Sep 17 00:00:00 2001 From: Gabriellvl Date: Sat, 19 Apr 2025 11:01:26 +0200 Subject: [PATCH] fix: includes check + gebruik fetches service laag --- backend/src/interfaces/user.ts | 4 +++ .../auth/checks/assignment-auth-checks.ts | 17 +++++------ .../auth/checks/class-auth-checks.ts | 29 +++++++++---------- .../auth/checks/group-auth-checker.ts | 13 +++++---- backend/src/services/students.ts | 3 +- backend/src/services/teachers.ts | 3 +- backend/tests/controllers/teachers.test.ts | 5 ++-- 7 files changed, 38 insertions(+), 36 deletions(-) diff --git a/backend/src/interfaces/user.ts b/backend/src/interfaces/user.ts index f4413b5e..88252a1e 100644 --- a/backend/src/interfaces/user.ts +++ b/backend/src/interfaces/user.ts @@ -10,6 +10,10 @@ export function mapToUserDTO(user: User): UserDTO { }; } +export function mapToUsername(user: {username: string}): string { + return user.username; +} + export function mapToUser(userData: UserDTO, userInstance: T): T { userInstance.username = userData.username; userInstance.firstName = userData.firstName; diff --git a/backend/src/middleware/auth/checks/assignment-auth-checks.ts b/backend/src/middleware/auth/checks/assignment-auth-checks.ts index a9bbe87f..e9b284c4 100644 --- a/backend/src/middleware/auth/checks/assignment-auth-checks.ts +++ b/backend/src/middleware/auth/checks/assignment-auth-checks.ts @@ -1,7 +1,8 @@ import {authorize} from "./auth-checks"; -import {getAssignment} from "../../../services/assignments"; -import {getClass} from "../../../services/classes"; +import {fetchAssignment, getAssignment} from "../../../services/assignments"; +import {fetchClass, getClass} from "../../../services/classes"; import {getAllGroups} from "../../../services/groups"; +import {mapToUsername} from "../../../interfaces/user"; /** * Expects the path to contain the path parameters 'classId' and 'id' (meaning the ID of the assignment). @@ -12,15 +13,13 @@ import {getAllGroups} from "../../../services/groups"; export const onlyAllowIfHasAccessToAssignment = authorize( async (auth, req) => { const { classid: classId, id: assignmentId } = req.params as { classid: string, id: number }; - const assignment = await getAssignment(classId, assignmentId); - if (assignment === null) { - return false; - } else if (auth.accountType === "teacher") { - const clazz = await getClass(assignment.class); - return auth.username in clazz!.teachers; + const assignment = await fetchAssignment(classId, assignmentId); + if (auth.accountType === "teacher") { + const clazz = await fetchClass(assignment.class); + return clazz.teachers.map(mapToUsername).includes(auth.username); } else { const groups = await getAllGroups(classId, assignmentId, false); - return groups.some(group => auth.username in (group.members as string[])); + return groups.some(group => group.members.map(mapToUsername).includes(auth.username) ); } } ); diff --git a/backend/src/middleware/auth/checks/class-auth-checks.ts b/backend/src/middleware/auth/checks/class-auth-checks.ts index c89a263f..1f8e685b 100644 --- a/backend/src/middleware/auth/checks/class-auth-checks.ts +++ b/backend/src/middleware/auth/checks/class-auth-checks.ts @@ -1,11 +1,12 @@ import {authorize} from "./auth-checks"; import {AuthenticationInfo} from "../authentication-info"; import {AuthenticatedRequest} from "../authenticated-request"; -import {getClass} from "../../../services/classes"; +import {fetchClass, getClass} from "../../../services/classes"; +import {mapToUsername} from "../../../interfaces/user"; async function teaches(teacherUsername: string, classId: string): Promise { - const clazz = await getClass(classId); - return clazz !== null && teacherUsername in clazz.teachers; + const clazz = await fetchClass(classId); + return clazz.teachers.map(mapToUsername).includes(teacherUsername); } /** @@ -20,7 +21,7 @@ export const onlyAllowStudentHimselfAndTeachersOfClass = authorize( } else if (auth.accountType === "teacher") { return teaches(auth.username, req.params.classId); } - return false; + return false; } ); @@ -41,13 +42,11 @@ export const onlyAllowTeacherOfClass = authorize( export const onlyAllowIfInClass = authorize( async (auth: AuthenticationInfo, req: AuthenticatedRequest) => { const classId = req.params.classId ?? req.params.classid ?? req.params.id; - const clazz = await getClass(classId); - if (clazz === null) { - return false; - } else if (auth.accountType === "teacher") { - return auth.username in clazz.teachers; + const clazz = await fetchClass(classId); + if (auth.accountType === "teacher") { + return clazz.teachers.map(mapToUsername).includes(auth.username); } - return auth.username in clazz.students; + return clazz.students.map(mapToUsername).includes(auth.username); } ); @@ -57,13 +56,11 @@ export const onlyAllowIfInClass = authorize( export const onlyAllowOwnClassInBody = authorize( async (auth, req) => { const classId = (req.body as {class: string})?.class; - const clazz = await getClass(classId); + const clazz = await fetchClass(classId); - if (clazz === null) { - return false; - } else if (auth.accountType === "teacher") { - return auth.username in clazz.teachers; + if (auth.accountType === "teacher") { + return clazz.teachers.map(mapToUsername).includes(auth.username); } - return auth.username in clazz.students; + return clazz.students.map(mapToUsername).includes(auth.username); } ); diff --git a/backend/src/middleware/auth/checks/group-auth-checker.ts b/backend/src/middleware/auth/checks/group-auth-checker.ts index f62ab3b9..0f42741e 100644 --- a/backend/src/middleware/auth/checks/group-auth-checker.ts +++ b/backend/src/middleware/auth/checks/group-auth-checker.ts @@ -1,6 +1,7 @@ import {authorize} from "./auth-checks"; -import {getClass} from "../../../services/classes"; -import {getGroup} from "../../../services/groups"; +import {fetchClass, getClass} from "../../../services/classes"; +import {fetchGroup, getGroup} from "../../../services/groups"; +import {mapToUsername} from "../../../interfaces/user"; /** * Expects the path to contain the path parameters 'classid', 'assignmentid' and 'groupid'. @@ -14,11 +15,11 @@ export const onlyAllowIfHasAccessToGroup = authorize( req.params as { classid: string, assignmentid: number, groupid: number }; if (auth.accountType === "teacher") { - const clazz = await getClass(classId); - return auth.username in clazz!.teachers; + const clazz = await fetchClass(classId); + return clazz.teachers.map(mapToUsername).includes(auth.username); } else { // user is student - const group = await getGroup(classId, assignmentId, groupId, false); - return group === null ? false : auth.username in (group.members as string[]); + const group = await fetchGroup(classId, assignmentId, groupId, false); + return clazz.students.map(mapToUsername).includes(auth.username); } } ); diff --git a/backend/src/services/students.ts b/backend/src/services/students.ts index 31dee851..7101960f 100644 --- a/backend/src/services/students.ts +++ b/backend/src/services/students.ts @@ -25,6 +25,7 @@ import { QuestionDTO, QuestionId } from '@dwengo-1/common/interfaces/question'; import { ClassJoinRequestDTO } from '@dwengo-1/common/interfaces/class-join-request'; import { ConflictException } from '../exceptions/conflict-exception.js'; import { Submission } from '../entities/assignments/submission.entity'; +import {mapToUsername} from "../interfaces/user"; export async function getAllStudents(full: boolean): Promise { const studentRepository = getStudentRepository(); @@ -34,7 +35,7 @@ export async function getAllStudents(full: boolean): Promise user.username); + return users.map(mapToUsername); } export async function fetchStudent(username: string): Promise { diff --git a/backend/src/services/teachers.ts b/backend/src/services/teachers.ts index a0e6c7a7..a76f88b7 100644 --- a/backend/src/services/teachers.ts +++ b/backend/src/services/teachers.ts @@ -30,6 +30,7 @@ import { QuestionDTO, QuestionId } from '@dwengo-1/common/interfaces/question'; import { ClassJoinRequestDTO } from '@dwengo-1/common/interfaces/class-join-request'; import { ClassStatus } from '@dwengo-1/common/util/class-join-request'; import { ConflictException } from '../exceptions/conflict-exception.js'; +import {mapToUsername} from "../interfaces/user"; export async function getAllTeachers(full: boolean): Promise { const teacherRepository: TeacherRepository = getTeacherRepository(); @@ -38,7 +39,7 @@ export async function getAllTeachers(full: boolean): Promise user.username); + return users.map(mapToUsername); } export async function fetchTeacher(username: string): Promise { diff --git a/backend/tests/controllers/teachers.test.ts b/backend/tests/controllers/teachers.test.ts index a73a79a5..39e59d6f 100644 --- a/backend/tests/controllers/teachers.test.ts +++ b/backend/tests/controllers/teachers.test.ts @@ -96,7 +96,7 @@ describe('Teacher controllers', () => { }); it('Teacher list', async () => { - req = { query: { full: 'true' } }; + req = { query: { full: 'false' } }; await getAllTeachersHandler(req as Request, res as Response); @@ -104,8 +104,7 @@ describe('Teacher controllers', () => { const result = jsonMock.mock.lastCall?.[0]; - const teacherUsernames = result.teachers.map((s: TeacherDTO) => s.username); - expect(teacherUsernames).toContain('testleerkracht1'); + expect(result.teachers).toContain('testleerkracht1'); expect(result.teachers).toHaveLength(5); });