diff --git a/backend/src/controllers/assignments.ts b/backend/src/controllers/assignments.ts index 1520fc10..d92def19 100644 --- a/backend/src/controllers/assignments.ts +++ b/backend/src/controllers/assignments.ts @@ -3,7 +3,7 @@ import { createAssignment, getAllAssignments, getAssignment, getAssignmentsSubmi import { AssignmentDTO } from '@dwengo-1/common/interfaces/assignment'; // Typescript is annoying with parameter forwarding from class.ts -interface AssignmentParams { +export interface AssignmentParams { classid: string; id: string; } diff --git a/backend/src/middleware/auth/authenticated-request.d.ts b/backend/src/middleware/auth/authenticated-request.d.ts index 9737fa7e..c0049dc7 100644 --- a/backend/src/middleware/auth/authenticated-request.d.ts +++ b/backend/src/middleware/auth/authenticated-request.d.ts @@ -1,8 +1,15 @@ import { Request } from 'express'; import { JwtPayload } from 'jsonwebtoken'; import { AuthenticationInfo } from './authentication-info.js'; +import * as core from "express-serve-static-core"; -export interface AuthenticatedRequest extends Request { +export interface AuthenticatedRequest< + P = core.ParamsDictionary, + ResBody = unknown, + ReqBody = unknown, + ReqQuery = core.Query, + Locals extends Record = Record, +> extends Request { // Properties are optional since the user is not necessarily authenticated. jwtPayload?: JwtPayload; auth?: AuthenticationInfo; diff --git a/backend/src/middleware/auth/checks/assignment-auth-checks.ts b/backend/src/middleware/auth/checks/assignment-auth-checks.ts new file mode 100644 index 00000000..a9bbe87f --- /dev/null +++ b/backend/src/middleware/auth/checks/assignment-auth-checks.ts @@ -0,0 +1,26 @@ +import {authorize} from "./auth-checks"; +import {getAssignment} from "../../../services/assignments"; +import {getClass} from "../../../services/classes"; +import {getAllGroups} from "../../../services/groups"; + +/** + * Expects the path to contain the path parameters 'classId' and 'id' (meaning the ID of the assignment). + * Only allows requests from users who are + * - either teachers of the class the assignment was posted in, + * - or students in a group of the assignment. + */ +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; + } else { + const groups = await getAllGroups(classId, assignmentId, false); + return groups.some(group => auth.username in (group.members as string[])); + } + } +); diff --git a/backend/src/middleware/auth/checks/auth-checks.ts b/backend/src/middleware/auth/checks/auth-checks.ts index ee70c109..d14e8dee 100644 --- a/backend/src/middleware/auth/checks/auth-checks.ts +++ b/backend/src/middleware/auth/checks/auth-checks.ts @@ -3,6 +3,7 @@ import {AuthenticatedRequest} from "../authenticated-request"; import * as express from "express"; import {UnauthorizedException} from "../../../exceptions/unauthorized-exception"; import {ForbiddenException} from "../../../exceptions/forbidden-exception"; +import {RequestHandler} from "express"; /** * Middleware which rejects unauthenticated users (with HTTP 401) and authenticated users which do not fulfill @@ -10,10 +11,10 @@ import {ForbiddenException} from "../../../exceptions/forbidden-exception"; * @param accessCondition Predicate over the current AuthenticationInfo. Access is only granted when this evaluates * to true. */ -export function authorize( - accessCondition: (auth: AuthenticationInfo, req: AuthenticatedRequest) => boolean | Promise -) { - return async (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction): Promise => { +export function authorize>( + accessCondition: (auth: AuthenticationInfo, req: AuthenticatedRequest) => boolean | Promise +): RequestHandler { + return async (req: AuthenticatedRequest, _res: express.Response, next: express.NextFunction): Promise => { if (!req.auth) { throw new UnauthorizedException(); } else if (!await accessCondition(req.auth, req)) { diff --git a/backend/src/middleware/auth/checks/class-auth-checks.ts b/backend/src/middleware/auth/checks/class-auth-checks.ts index f51cf740..c89a263f 100644 --- a/backend/src/middleware/auth/checks/class-auth-checks.ts +++ b/backend/src/middleware/auth/checks/class-auth-checks.ts @@ -3,9 +3,9 @@ import {AuthenticationInfo} from "../authentication-info"; import {AuthenticatedRequest} from "../authenticated-request"; import {getClass} from "../../../services/classes"; -async function teaches(teacherUsername: string, classId: string) { +async function teaches(teacherUsername: string, classId: string): Promise { const clazz = await getClass(classId); - return clazz != null && teacherUsername in clazz.teachers; + return clazz !== null && teacherUsername in clazz.teachers; } /** @@ -19,9 +19,9 @@ export const onlyAllowStudentHimselfAndTeachersOfClass = authorize( return true; } else if (auth.accountType === "teacher") { return teaches(auth.username, req.params.classId); - } else { - return false; } + return false; + } ); @@ -38,21 +38,32 @@ export const onlyAllowTeacherOfClass = authorize( * Only let the request pass through if the class id in it refers to a class the current user is in (as a student * or teacher) */ -function createOnlyAllowIfInClass(onlyTeacher: boolean) { - return 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 (onlyTeacher || auth.accountType === "teacher") { - return auth.username in clazz.teachers; - } else { - return auth.username in clazz.students; - } +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; } - ); -} + return auth.username in clazz.students; + } +); -export const onlyAllowIfInClass = createOnlyAllowIfInClass(false); -export const onlyAllowIfTeacherInClass = createOnlyAllowIfInClass(true); +/** + * Only allows the request to pass if the 'class' property in its body is a class the current user is a member of. + */ +export const onlyAllowOwnClassInBody = authorize( + async (auth, req) => { + const classId = (req.body as {class: string})?.class; + const clazz = await getClass(classId); + + if (clazz === null) { + return false; + } else if (auth.accountType === "teacher") { + return auth.username in clazz.teachers; + } + return auth.username in clazz.students; + } +); diff --git a/backend/src/middleware/auth/checks/group-auth-checker.ts b/backend/src/middleware/auth/checks/group-auth-checker.ts new file mode 100644 index 00000000..f62ab3b9 --- /dev/null +++ b/backend/src/middleware/auth/checks/group-auth-checker.ts @@ -0,0 +1,24 @@ +import {authorize} from "./auth-checks"; +import {getClass} from "../../../services/classes"; +import {getGroup} from "../../../services/groups"; + +/** + * Expects the path to contain the path parameters 'classid', 'assignmentid' and 'groupid'. + * Only allows requests from users who are + * - either teachers of the class the assignment for the group was posted in, + * - or students in the group + */ +export const onlyAllowIfHasAccessToGroup = authorize( + async (auth, req) => { + const { classid: classId, assignmentid: assignmentId, groupid: groupId } = + req.params as { classid: string, assignmentid: number, groupid: number }; + + if (auth.accountType === "teacher") { + const clazz = await getClass(classId); + return auth.username in clazz!.teachers; + } else { // user is student + const group = await getGroup(classId, assignmentId, groupId, false); + return group === null ? false : auth.username in (group.members as string[]); + } + } +); diff --git a/backend/src/routes/assignments.ts b/backend/src/routes/assignments.ts index 3652dcc6..4db12769 100644 --- a/backend/src/routes/assignments.ts +++ b/backend/src/routes/assignments.ts @@ -6,20 +6,23 @@ import { getAssignmentsSubmissionsHandler, } from '../controllers/assignments.js'; import groupRouter from './groups.js'; +import {adminOnly, teachersOnly} from "../middleware/auth/checks/auth-checks"; +import {onlyAllowOwnClassInBody} from "../middleware/auth/checks/class-auth-checks"; +import {onlyAllowIfHasAccessToAssignment} from "../middleware/auth/checks/assignment-auth-checks"; const router = express.Router({ mergeParams: true }); // Root endpoint used to search objects -router.get('/', getAllAssignmentsHandler); +router.get('/', adminOnly, getAllAssignmentsHandler); -router.post('/', createAssignmentHandler); +router.post('/', teachersOnly, onlyAllowOwnClassInBody, createAssignmentHandler); // Information about an assignment with id 'id' -router.get('/:id', getAssignmentHandler); +router.get('/:id', onlyAllowIfHasAccessToAssignment, getAssignmentHandler); -router.get('/:id/submissions', getAssignmentsSubmissionsHandler); +router.get('/:id/submissions', onlyAllowIfHasAccessToAssignment, getAssignmentsSubmissionsHandler); -router.get('/:id/questions', (_req, res) => { +router.get('/:id/questions', onlyAllowIfHasAccessToAssignment, (_req, res) => { res.json({ questions: ['0'], }); diff --git a/backend/src/routes/classes.ts b/backend/src/routes/classes.ts index f2dd7686..36ba82e1 100644 --- a/backend/src/routes/classes.ts +++ b/backend/src/routes/classes.ts @@ -8,7 +8,7 @@ import { } from '../controllers/classes.js'; import assignmentRouter from './assignments.js'; import {adminOnly, teachersOnly} from "../middleware/auth/checks/auth-checks"; -import {onlyAllowIfInClass, onlyAllowIfTeacherInClass} from "../middleware/auth/checks/class-auth-checks"; +import {onlyAllowIfInClass} from "../middleware/auth/checks/class-auth-checks"; const router = express.Router(); @@ -20,7 +20,7 @@ router.post('/', teachersOnly, createClassHandler); // Information about an class with id 'id' router.get('/:id', onlyAllowIfInClass, getClassHandler); -router.get('/:id/teacher-invitations', onlyAllowIfTeacherInClass, getTeacherInvitationsHandler); +router.get('/:id/teacher-invitations', teachersOnly, onlyAllowIfInClass, getTeacherInvitationsHandler); router.get('/:id/students', onlyAllowIfInClass, getClassStudentsHandler); diff --git a/backend/src/routes/groups.ts b/backend/src/routes/groups.ts index 1486edce..e8e83fb2 100644 --- a/backend/src/routes/groups.ts +++ b/backend/src/routes/groups.ts @@ -1,5 +1,6 @@ import express from 'express'; import { createGroupHandler, getAllGroupsHandler, getGroupHandler, getGroupSubmissionsHandler } from '../controllers/groups.js'; +import {onlyAllowIfHasAccessToGroup} from "../middleware/auth/checks/group-auth-checker"; const router = express.Router({ mergeParams: true }); @@ -9,12 +10,12 @@ router.get('/', getAllGroupsHandler); router.post('/', createGroupHandler); // Information about a group (members, ... [TODO DOC]) -router.get('/:groupid', getGroupHandler); +router.get('/:groupid', onlyAllowIfHasAccessToGroup, getGroupHandler); -router.get('/:groupid/submissions', getGroupSubmissionsHandler); +router.get('/:groupid/submissions', onlyAllowIfHasAccessToGroup, getGroupSubmissionsHandler); // The list of questions a group has made -router.get('/:id/questions', (_req, res) => { +router.get('/:groupid/questions', onlyAllowIfHasAccessToGroup, (_req, res) => { res.json({ questions: ['0'], }); diff --git a/backend/src/routes/students.ts b/backend/src/routes/students.ts index e378e6ea..04d260b4 100644 --- a/backend/src/routes/students.ts +++ b/backend/src/routes/students.ts @@ -20,7 +20,7 @@ const router = express.Router(); router.get('/', adminOnly, getAllStudentsHandler); // Users will be created automatically when some resource is created for them. Therefore, this endpoint -// can only be used by an administrator. +// Can only be used by an administrator. router.post('/', adminOnly, createStudentHandler); router.delete('/:username', onlyAllowUserHimself, deleteStudentHandler); diff --git a/backend/src/services/students.ts b/backend/src/services/students.ts index 6158a0f8..ed951a5a 100644 --- a/backend/src/services/students.ts +++ b/backend/src/services/students.ts @@ -52,7 +52,7 @@ export async function getStudent(username: string): Promise { return mapToStudentDTO(user); } -export async function createStudent(userData: StudentDTO, allowUpdate: boolean = false): Promise { +export async function createStudent(userData: StudentDTO, allowUpdate = false): Promise { const studentRepository = getStudentRepository(); const newStudent = mapToStudent(userData);