Clean up & add specification to auth

src/common/auth.test.ts

@@ -1,7 +1,7 @@
 import { describe, it, expect } from "@jest/globals";
-import { JwtPayload, Provider, Role } from "../services/auth/auth-models";
+import { JwtPayload, Provider, Role } from "../services/auth/auth-schemas";
 import Models from "../database/models";
-import { AuthInfo } from "../database/auth-db";
+import { AuthInfo } from "../services/auth/auth-schemas";
 import { getJwtPayloadFromProfile } from "./auth";
 
 const USER_PAYLOAD = {

src/common/auth.ts

@@ -6,10 +6,10 @@ import passport, { AuthenticateOptions, Profile } from "passport";
 
 import Config from "./config";
 
-import { Role, JwtPayload, Provider, ProfileData, RoleOperation } from "../services/auth/auth-models";
+import { Role, JwtPayload, Provider, ProfileData, RoleOperation } from "../services/auth/auth-schemas";
 
 import Models from "../database/models";
-import { AuthInfo } from "../database/auth-db";
+import { AuthInfo } from "../services/auth/auth-schemas";
 import { UpdateQuery } from "mongoose";
 
 type AuthenticateFunction = (strategies: string | string[], options: AuthenticateOptions) => RequestHandler;
@@ -252,7 +252,7 @@ export async function getRoles(id: string): Promise<Role[] | undefined> {
  * @param operation Operation to perform
  * @returns Promise - if valid, then update operation worked. If invalid, then contains why.
  */
-export async function updateRoles(userId: string, role: Role, operation: RoleOperation): Promise<Role[]> {
+export async function updateRoles(userId: string, role: Role, operation: RoleOperation): Promise<Role[] | undefined> {
     let updateQuery: UpdateQuery<AuthInfo>;
 
     // Get filter, representing operation to perform on mongoDB
@@ -274,7 +274,7 @@ export async function updateRoles(userId: string, role: Role, operation: RoleOpe
         if (updatedInfo) {
             return updatedInfo.roles as Role[];
         } else {
-            return Promise.reject("UserNotFound");
+            return undefined;
         }
     } catch (error) {
         return Promise.reject(error);
@@ -342,7 +342,7 @@ export function isAttendee(payload?: JwtPayload): boolean {
  * @param role role that we want to filter for
  * @returns Promise<string[]> - if valid, then contains array of user w/ role. If invalid, then contains "Unknown Error".
  */
-export async function getUsersWithRole(role: string): Promise<string[]> {
+export async function getUsersWithRole(role: Role): Promise<string[]> {
     try {
         //Array of users as MongoDb schema that have role as one of its roles
         const queryResult: AuthInfo[] = await Models.AuthInfo.find({ roles: { $in: [role] } }).select("userId");

src/common/config.ts

@@ -71,7 +71,7 @@ const Config = {
         [Device.IOS, "hackillinois://login/"],
         [Device.ANDROID, "hackillinois://login/"],
         [Device.PUZZLE, "https://runes.hackillinois.org/#/auth/"],
-    ]) as Map<string, string>,
+    ]),
 
     CALLBACK_URLS: {
         GITHUB: `${ROOT_URL}/auth/github/callback/`,

src/common/testTools.ts

@@ -1,6 +1,6 @@
 import request from "supertest";
 
-import { Provider, Role } from "../services/auth/auth-models";
+import { Provider, Role } from "../services/auth/auth-schemas";
 
 // The tester is the user that will be making requests
 // We provide this object so you can do proper testing based on JWT auth

src/database/models.ts

@@ -1,7 +1,7 @@
 import mongoose, { Model } from "mongoose";
 import { getModelForClass } from "@typegoose/typegoose";
 
-import { AuthInfo } from "./auth-db";
+import { AuthInfo } from "../services/auth/auth-schemas";
 import { AttendeeFollowing, AttendeeProfile } from "./attendee-db";
 import { AdmissionDecision } from "./admission-db";
 import { MentorOfficeHours } from "./mentor-db";

src/middleware/specification.ts

@@ -4,7 +4,7 @@ import StatusCode from "status-code-enum";
 import { Response, Request, NextFunction } from "express";
 import { registerPathSpecification } from "../common/openapi";
 import { RouteConfig } from "@asteasolutions/zod-to-openapi";
-import { Role } from "../services/auth/auth-models";
+import { Role } from "../services/auth/auth-schemas";
 import { getAuthenticatedUser } from "../common/auth";
 import { TokenExpiredError } from "jsonwebtoken";
 

src/services/admission/admission-router.ts

@@ -1,7 +1,7 @@
 import { Router, Request, Response } from "express";
 import { strongJwtVerification } from "../../middleware/verify-jwt";
 
-import { JwtPayload, Role } from "../auth/auth-models";
+import { JwtPayload, Role } from "../auth/auth-schemas";
 import { DecisionStatus, DecisionResponse, AdmissionDecision } from "../../database/admission-db";
 import Models from "../../database/models";
 import { hasElevatedPerms } from "../../common/auth";

src/services/auth/auth-router.test.ts

@@ -3,8 +3,9 @@ import { SpiedFunction } from "jest-mock";
 import { RequestHandler } from "express";
 import { StatusCode } from "status-code-enum";
 import {
-    AUTH_ROLE_TO_ROLES,
     TESTER,
+    delAsAdmin,
+    delAsStaff,
     get,
     getAsAttendee,
     getAsStaff,
@@ -15,10 +16,9 @@ import {
 import Config, { Device } from "../../common/config";
 import * as selectAuthMiddleware from "../../middleware/select-auth";
 import { mockGenerateJwtTokenWithWrapper, mockGetJwtPayloadFromProfile } from "../../common/mocks/auth";
-import { JwtPayload, ProfileData, Provider, Role, RoleOperation } from "./auth-models";
+import { JwtPayload, ProfileData, Provider, Role } from "./auth-schemas";
 import Models from "../../database/models";
-import { AuthInfo } from "../../database/auth-db";
-import { ModifyRoleRequest } from "./auth-formats";
+import { AuthInfo } from "./auth-schemas";
 
 const ALL_DEVICES = [Device.WEB, Device.ADMIN, Device.ANDROID, Device.IOS, Device.DEV];
 
@@ -47,12 +47,6 @@ beforeEach(async () => {
 });
 
 describe("GET /auth/dev/", () => {
-    it("errors when a token is not provided", async () => {
-        const response = await getAsUser("/auth/dev/").expect(StatusCode.ClientErrorBadRequest);
-
-        expect(JSON.parse(response.text)).toHaveProperty("error", "NoToken");
-    });
-
     it("returns passed query parameter", async () => {
         const response = await getAsUser("/auth/dev/?token=123").expect(StatusCode.SuccessOK);
 
@@ -81,10 +75,10 @@ describe.each(["github", "google"])("GET /auth/login/%s/", (provider) => {
         });
     });
 
-    it("provides an error when no device is provided", async () => {
+    it("provides an error when an invalid device is provided", async () => {
         const response = await get(`/auth/login/${provider}/?device=abc`).expect(StatusCode.ClientErrorBadRequest);
 
-        expect(JSON.parse(response.text)).toHaveProperty("error", "BadDevice");
+        expect(JSON.parse(response.text)).toHaveProperty("error", "BadRequest");
     });
 
     it("logs in with default device when none is provided", async () => {
@@ -100,7 +94,7 @@ describe.each(["github", "google"])("GET /auth/login/%s/", (provider) => {
     });
 });
 
-describe("GET /auth/:PROVIDER/callback/:DEVICE", () => {
+describe("GET /auth/:provider/callback/:device", () => {
     let mockedGenerateJwtToken: ReturnType<typeof mockGenerateJwtTokenWithWrapper>;
 
     beforeEach(() => {
@@ -111,9 +105,9 @@ describe("GET /auth/:PROVIDER/callback/:DEVICE", () => {
     });
 
     it("provides an error when an invalid device is provided", async () => {
-        const response = await get("/auth/github/callback/abc").expect(StatusCode.ServerErrorInternal);
+        const response = await get("/auth/github/callback/abc").expect(StatusCode.ClientErrorBadRequest);
 
-        expect(JSON.parse(response.text)).toHaveProperty("error", "InternalServerError");
+        expect(JSON.parse(response.text)).toHaveProperty("error", "BadRequest");
     });
 
     it("provides an error when authentication fails", async () => {
@@ -126,24 +120,7 @@ describe("GET /auth/:PROVIDER/callback/:DEVICE", () => {
 
         const response = await get(`/auth/github/callback/${Device.WEB}`).expect(StatusCode.ClientErrorUnauthorized);
 
-        expect(JSON.parse(response.text)).toHaveProperty("error", "FailedAuth");
-    });
-
-    it("provides an error when invalid data is provided", async () => {
-        // Mock select auth to successfully authenticate & return invalid user data
-        mockSelectAuthProvider((req, _res, next) => {
-            req.isAuthenticated = ((): boolean => true) as typeof req.isAuthenticated;
-
-            req.user = {
-                // no content
-            };
-
-            next();
-        });
-
-        const response = await get(`/auth/github/callback/${Device.WEB}`).expect(StatusCode.ClientErrorBadRequest);
-
-        expect(JSON.parse(response.text)).toHaveProperty("error", "InvalidData");
+        expect(JSON.parse(response.text)).toHaveProperty("error", "AuthorizationFailed");
     });
 
     it.each(ALL_DEVICES)("works when authentication passes with device %s", async (device) => {
@@ -183,7 +160,7 @@ describe("GET /auth/:PROVIDER/callback/:DEVICE", () => {
     });
 });
 
-describe("GET /auth/roles/list/:ROLE", () => {
+describe("GET /auth/roles/list/:role", () => {
     it("provides an error for an non-staff user", async () => {
         const response = await getAsAttendee(`/auth/roles/list/USER`).expect(StatusCode.ClientErrorForbidden);
 
@@ -225,12 +202,6 @@ describe("GET /auth/roles/list/:ROLE", () => {
 });
 
 describe("GET /auth/roles/", () => {
-    it("provides an error if the user does not have auth info", async () => {
-        const response = await getAsUser(`/auth/roles/`).expect(StatusCode.ClientErrorNotFound);
-
-        expect(JSON.parse(response.text)).toHaveProperty("error", "UserNotFound");
-    });
-
     it("gets the roles of a user", async () => {
         await Models.AuthInfo.create({
             userId: TESTER.id,
@@ -266,31 +237,13 @@ describe("GET /auth/roles/", () => {
     });
 });
 
-describe("GET /auth/roles/:USERID", () => {
-    it("provides an error if the user does not have auth info", async () => {
-        const response = await getAsStaff(`/auth/roles/123`).expect(StatusCode.ClientErrorNotFound);
-
-        expect(JSON.parse(response.text)).toHaveProperty("error", "UserNotFound");
-    });
-
-    it("provides an error if non-staff user tries to get roles of another user", async () => {
+describe("GET /auth/roles/:id", () => {
+    it("provides an error if non-staff", async () => {
         const response = await getAsAttendee(`/auth/roles/${USER_ATTENDEE.userId}`).expect(StatusCode.ClientErrorForbidden);
 
         expect(JSON.parse(response.text)).toHaveProperty("error", "Forbidden");
     });
 
-    it("gets the roles of themselves if non-staff", async () => {
-        const response = await getAsAttendee(`/auth/roles/${TESTER.id}`).expect(StatusCode.SuccessOK);
-        const json = JSON.parse(response.text);
-
-        const roles = AUTH_ROLE_TO_ROLES[Role.ATTENDEE];
-        expect(json).toMatchObject({
-            id: TESTER.id,
-            roles: expect.arrayContaining(roles),
-        });
-        expect(json?.roles).toHaveLength(roles.length);
-    });
-
     it("gets the roles of another user if staff", async () => {
         const response = await getAsStaff(`/auth/roles/${USER_ATTENDEE.userId}`).expect(StatusCode.SuccessOK);
         const json = JSON.parse(response.text);
@@ -304,55 +257,15 @@ describe("GET /auth/roles/:USERID", () => {
     });
 });
 
-describe("PUT /auth/roles/:OPERATION", () => {
+describe("PUT /auth/roles/:id/:role/", () => {
     it("provides an error if user is not an admin", async () => {
-        const response = await putAsStaff(`/auth/roles/ADD`)
-            .send(
-                JSON.stringify({
-                    id: USER.userId,
-                    role: Role.ATTENDEE,
-                } satisfies ModifyRoleRequest),
-            )
-            .expect(StatusCode.ClientErrorForbidden);
+        const response = await putAsStaff(`/auth/roles/${USER.userId}/${Role.ADMIN}/`).expect(StatusCode.ClientErrorForbidden);
 
         expect(JSON.parse(response.text)).toHaveProperty("error", "Forbidden");
     });
 
-    it("provides an error if operation is invalid", async () => {
-        const response = await putAsAdmin(`/auth/roles/abc`)
-            .send(
-                JSON.stringify({
-                    id: USER.userId,
-                    role: Role.ATTENDEE,
-                } satisfies ModifyRoleRequest),
-            )
-            .expect(StatusCode.ClientErrorBadRequest);
-
-        expect(JSON.parse(response.text)).toHaveProperty("error", "InvalidOperation");
-    });
-
-    it("provides an error if role is invalid", async () => {
-        const response = await putAsAdmin(`/auth/roles/${RoleOperation.ADD}`)
-            .send(
-                JSON.stringify({
-                    id: USER.userId,
-                    role: "42",
-                } satisfies ModifyRoleRequest),
-            )
-            .expect(StatusCode.ClientErrorBadRequest);
-
-        expect(JSON.parse(response.text)).toHaveProperty("error", "InvalidRole");
-    });
-
     it("adds a role if user is admin", async () => {
-        const response = await putAsAdmin(`/auth/roles/${RoleOperation.ADD}`)
-            .send(
-                JSON.stringify({
-                    id: USER.userId,
-                    role: Role.ATTENDEE,
-                } satisfies ModifyRoleRequest),
-            )
-            .expect(StatusCode.SuccessOK);
+        const response = await putAsAdmin(`/auth/roles/${USER.userId}/${Role.ATTENDEE}/`).expect(StatusCode.SuccessOK);
         const json = JSON.parse(response.text);
 
         const newRoles = [...USER.roles, Role.ATTENDEE];
@@ -367,16 +280,17 @@ describe("PUT /auth/roles/:OPERATION", () => {
         expect(stored).toHaveProperty("roles", expect.arrayContaining(newRoles));
         expect(stored?.roles).toHaveLength(newRoles.length);
     });
+});
+
+describe("DELETE /auth/roles/:id/:role/", () => {
+    it("provides an error if user is not an admin", async () => {
+        const response = await delAsStaff(`/auth/roles/${USER.userId}/${Role.ATTENDEE}/`).expect(StatusCode.ClientErrorForbidden);
+
+        expect(JSON.parse(response.text)).toHaveProperty("error", "Forbidden");
+    });
 
     it("removes a role if user is admin", async () => {
-        const response = await putAsAdmin(`/auth/roles/${RoleOperation.REMOVE}`)
-            .send(
-                JSON.stringify({
-                    id: USER.userId,
-                    role: Role.USER,
-                } satisfies ModifyRoleRequest),
-            )
-            .expect(StatusCode.SuccessOK);
+        const response = await delAsAdmin(`/auth/roles/${USER.userId}/${Role.USER}/`).expect(StatusCode.SuccessOK);
         const json = JSON.parse(response.text);
 
         const newRoles: Role[] = [];
@@ -388,6 +302,7 @@ describe("PUT /auth/roles/:OPERATION", () => {
         expect(json?.roles).toHaveLength(newRoles.length);
 
         const stored = await Models.AuthInfo.findOne({ userId: USER.userId });
+        expect(stored).toHaveProperty("roles", expect.arrayContaining(newRoles));
         expect(stored?.roles).toHaveLength(newRoles.length);
     });
 });