From 08eae0cd890d9fab1e52b41ad67126dbc412a862 Mon Sep 17 00:00:00 2001 From: thom Date: Mon, 30 Sep 2024 11:41:24 -0700 Subject: [PATCH 01/25] feat: adding role to session, will be needed for lots of features --- src/backend/auth/adapter.ts | 10 +++++++- src/pages/api/auth/[...nextauth].ts | 40 ++++++++++++++++++++++++++++- src/pages/index.tsx | 23 +++++++++++++---- tsconfig.json | 3 ++- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index b89a97bb..7b949d4c 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -6,14 +6,22 @@ import { } from "../lib"; import { InsertObject, Selectable } from "kysely"; +// Extend AdapterUser type +interface CustomAdapterUser extends AdapterUser { + profile?: { + role: string; + }; +} + const mapStoredUserToAdapterUser = ( user: Selectable> -): AdapterUser => ({ +): CustomAdapterUser => ({ id: user.user_id, email: user.email, emailVerified: user.email_verified_at, name: `${user.first_name} ${user.last_name}`, image: user.image_url, + profile: { role: user.role }, // Add the role to the profile }); const mapStoredSessionToAdapterSession = ( diff --git a/src/pages/api/auth/[...nextauth].ts b/src/pages/api/auth/[...nextauth].ts index c1e062d1..30af0040 100644 --- a/src/pages/api/auth/[...nextauth].ts +++ b/src/pages/api/auth/[...nextauth].ts @@ -2,6 +2,20 @@ import { getNextAuthOptions } from "@/backend/auth/options"; import { createContext } from "@/backend/context"; import { NextApiHandler } from "next"; import NextAuth from "next-auth"; +import type { Session, User } from "next-auth"; + +interface UserWithRole extends User { + profile: { + role: string; + }; +} + +// Extend the Session type to include the role property +export interface ExtendedSession extends Session { + user: Session["user"] & { + role: string; + }; +} const handler: NextApiHandler = async (req, res) => { const { db } = await createContext({ @@ -9,8 +23,32 @@ const handler: NextApiHandler = async (req, res) => { res, }); + const authOptions = getNextAuthOptions(db); + + // Modify only the session callback + const modifiedAuthOptions = { + ...authOptions, + callbacks: { + ...authOptions.callbacks, + session({ + session, + user, + }: { + session: Session; + user: User; + }): ExtendedSession { + const extendedSession = session as ExtendedSession; + if (extendedSession.user) { + const userWithRole = user as UserWithRole; + extendedSession.user.role = userWithRole.profile.role; + } + return extendedSession; + }, + }, + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-call - return NextAuth(getNextAuthOptions(db))(req, res); + return NextAuth(modifiedAuthOptions)(req, res); }; export default handler; diff --git a/src/pages/index.tsx b/src/pages/index.tsx index bbba146d..ad5d5947 100644 --- a/src/pages/index.tsx +++ b/src/pages/index.tsx @@ -2,16 +2,29 @@ import type { NextPage } from "next"; import { useSession } from "next-auth/react"; import { useRouter } from "next/router"; import React, { useEffect } from "react"; +import { ExtendedSession } from "@/pages/api/auth/[...nextauth]"; const Home: NextPage = () => { const router = useRouter(); - const { status } = useSession(); + const { data: session, status } = useSession() as { + data: ExtendedSession | null; + status: "loading" | "authenticated" | "unauthenticated"; + }; useEffect(() => { - status === "authenticated" - ? void router.push("/students") - : void router.push("/signInPage"); - }, [status, router]); + if (status === "authenticated" && session) { + switch (session.user.role) { + case "para": + void router.push("/benchmarks"); + break; + default: + void router.push("/students"); + break; + } + } else { + void router.push("/signInPage"); + } + }, [status, router, session]); return <>Redirecting...; }; diff --git a/tsconfig.json b/tsconfig.json index ce18cd10..2200f4c1 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -17,7 +17,8 @@ "baseUrl": ".", "paths": { "@/*": ["src/*"] - } + }, + "sourceMap": true }, "include": ["next-env.d.ts", "references.d.ts", "**/*.ts", "**/*.tsx"], "exclude": ["node_modules"] From 69acc50801288745908cf30bbf41bb76669c2b63 Mon Sep 17 00:00:00 2001 From: thom Date: Mon, 30 Sep 2024 15:17:14 -0700 Subject: [PATCH 02/25] fix: refactor types into src/types/auth.ts --- src/pages/api/auth/[...nextauth].ts | 17 ++--------------- src/pages/index.tsx | 2 +- src/types/auth.ts | 13 +++++++++++++ 3 files changed, 16 insertions(+), 16 deletions(-) create mode 100644 src/types/auth.ts diff --git a/src/pages/api/auth/[...nextauth].ts b/src/pages/api/auth/[...nextauth].ts index 30af0040..f900e890 100644 --- a/src/pages/api/auth/[...nextauth].ts +++ b/src/pages/api/auth/[...nextauth].ts @@ -1,21 +1,8 @@ import { getNextAuthOptions } from "@/backend/auth/options"; import { createContext } from "@/backend/context"; import { NextApiHandler } from "next"; -import NextAuth from "next-auth"; -import type { Session, User } from "next-auth"; - -interface UserWithRole extends User { - profile: { - role: string; - }; -} - -// Extend the Session type to include the role property -export interface ExtendedSession extends Session { - user: Session["user"] & { - role: string; - }; -} +import NextAuth, { Session, User } from "next-auth"; +import { ExtendedSession, UserWithRole } from "@/types/auth"; const handler: NextApiHandler = async (req, res) => { const { db } = await createContext({ diff --git a/src/pages/index.tsx b/src/pages/index.tsx index ad5d5947..67c134f7 100644 --- a/src/pages/index.tsx +++ b/src/pages/index.tsx @@ -2,7 +2,7 @@ import type { NextPage } from "next"; import { useSession } from "next-auth/react"; import { useRouter } from "next/router"; import React, { useEffect } from "react"; -import { ExtendedSession } from "@/pages/api/auth/[...nextauth]"; +import { ExtendedSession } from "@/types/auth"; const Home: NextPage = () => { const router = useRouter(); diff --git a/src/types/auth.ts b/src/types/auth.ts new file mode 100644 index 00000000..0e4c3bd5 --- /dev/null +++ b/src/types/auth.ts @@ -0,0 +1,13 @@ +import { User, Session } from "next-auth"; + +export interface UserWithRole extends User { + profile: { + role: string; + }; +} + +export interface ExtendedSession extends Session { + user: Session["user"] & { + role: string; + }; +} From c4607be4e3a9276b3a0798a478f95fb4d84aa596 Mon Sep 17 00:00:00 2001 From: thom Date: Mon, 30 Sep 2024 16:32:07 -0700 Subject: [PATCH 03/25] fix: move type to auth.ts types --- src/backend/auth/adapter.ts | 8 +------- src/types/auth.ts | 7 +++++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index 7b949d4c..2c80d3fc 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -5,13 +5,7 @@ import { ZapatosTableNameToKyselySchema, } from "../lib"; import { InsertObject, Selectable } from "kysely"; - -// Extend AdapterUser type -interface CustomAdapterUser extends AdapterUser { - profile?: { - role: string; - }; -} +import { CustomAdapterUser } from "@/types/auth"; const mapStoredUserToAdapterUser = ( user: Selectable> diff --git a/src/types/auth.ts b/src/types/auth.ts index 0e4c3bd5..1213d820 100644 --- a/src/types/auth.ts +++ b/src/types/auth.ts @@ -1,4 +1,5 @@ import { User, Session } from "next-auth"; +import { AdapterUser } from "next-auth/adapters"; export interface UserWithRole extends User { profile: { @@ -11,3 +12,9 @@ export interface ExtendedSession extends Session { role: string; }; } + +export interface CustomAdapterUser extends AdapterUser { + profile?: { + role: string; + }; +} From be24b11a7029e2dbf41d95d7cf37417e801bcf6d Mon Sep 17 00:00:00 2001 From: thom Date: Mon, 7 Oct 2024 12:30:54 -0700 Subject: [PATCH 04/25] fix: refactor trpc auth guard name + add two more --- src/backend/routers/admin.ts | 4 +- src/backend/routers/case_manager.ts | 22 ++++----- src/backend/routers/file.ts | 12 ++--- src/backend/routers/iep.ts | 36 +++++++------- src/backend/routers/para.ts | 10 ++-- src/backend/routers/public.ts | 4 +- src/backend/routers/student.ts | 14 +++--- src/backend/routers/user.ts | 6 +-- src/backend/trpc.ts | 76 +++++++++++++++++++++++++++-- 9 files changed, 126 insertions(+), 58 deletions(-) diff --git a/src/backend/routers/admin.ts b/src/backend/routers/admin.ts index 5c5099a8..80a2eb98 100644 --- a/src/backend/routers/admin.ts +++ b/src/backend/routers/admin.ts @@ -1,8 +1,8 @@ import { sql } from "kysely"; -import { adminProcedure, router } from "../trpc"; +import { hasAdmin, router } from "../trpc"; export const admin = router({ - getPostgresInfo: adminProcedure.query(async (req) => { + getPostgresInfo: hasAdmin.query(async (req) => { const result = await sql<{ version: string }>`SELECT version()`.execute( req.ctx.db ); diff --git a/src/backend/routers/case_manager.ts b/src/backend/routers/case_manager.ts index aaa5d91a..7d7091c4 100644 --- a/src/backend/routers/case_manager.ts +++ b/src/backend/routers/case_manager.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { authenticatedProcedure, router } from "../trpc"; +import { hasAuthenticated, router } from "../trpc"; import { createPara, assignParaToCaseManager, @@ -10,7 +10,7 @@ export const case_manager = router({ /** * Get all students assigned to the current user */ - getMyStudents: authenticatedProcedure.query(async (req) => { + getMyStudents: hasAuthenticated.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db @@ -22,7 +22,7 @@ export const case_manager = router({ return result; }), - getMyStudentsAndIepInfo: authenticatedProcedure.query(async (req) => { + getMyStudentsAndIepInfo: hasAuthenticated.query(async (req) => { const { userId } = req.ctx.auth; const studentData = await req.ctx.db @@ -50,7 +50,7 @@ export const case_manager = router({ * it doesn't already exist. Throws an error if the student is already * assigned to another CM. */ - addStudent: authenticatedProcedure + addStudent: hasAuthenticated .input( z.object({ first_name: z.string(), @@ -72,7 +72,7 @@ export const case_manager = router({ /** * Edits the given student in the CM's roster. Throws an error if the student was not found in the db. */ - editStudent: authenticatedProcedure + editStudent: hasAuthenticated .input( z.object({ student_id: z.string(), @@ -115,7 +115,7 @@ export const case_manager = router({ /** * Removes the case manager associated with this student. */ - removeStudent: authenticatedProcedure + removeStudent: hasAuthenticated .input( z.object({ student_id: z.string(), @@ -131,7 +131,7 @@ export const case_manager = router({ .execute(); }), - getMyParas: authenticatedProcedure.query(async (req) => { + getMyParas: hasAuthenticated.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db @@ -152,7 +152,7 @@ export const case_manager = router({ * Handles creation of para and assignment to user, attempts to send * email but does not await email success */ - addStaff: authenticatedProcedure + addStaff: hasAuthenticated .input( z.object({ first_name: z.string(), @@ -180,7 +180,7 @@ export const case_manager = router({ /** * Deprecated: use addStaff instead */ - addPara: authenticatedProcedure + addPara: hasAuthenticated .input( z.object({ para_id: z.string(), @@ -195,7 +195,7 @@ export const case_manager = router({ return; }), - editPara: authenticatedProcedure + editPara: hasAuthenticated .input( z.object({ para_id: z.string(), @@ -236,7 +236,7 @@ export const case_manager = router({ .executeTakeFirstOrThrow(); }), - removePara: authenticatedProcedure + removePara: hasAuthenticated .input( z.object({ para_id: z.string(), diff --git a/src/backend/routers/file.ts b/src/backend/routers/file.ts index cd99aed8..1206a89e 100644 --- a/src/backend/routers/file.ts +++ b/src/backend/routers/file.ts @@ -5,12 +5,12 @@ import { GetObjectCommand, } from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; -import { authenticatedProcedure, router } from "../trpc"; +import { hasAuthenticated, router } from "../trpc"; import { randomUUID } from "crypto"; import { deleteFile } from "../lib/files"; export const file = router({ - getMyFiles: authenticatedProcedure.query(async (req) => { + getMyFiles: hasAuthenticated.query(async (req) => { return req.ctx.db .selectFrom("file") .selectAll() @@ -18,7 +18,7 @@ export const file = router({ .execute(); }), - getPresignedUrlForFileDownload: authenticatedProcedure + getPresignedUrlForFileDownload: hasAuthenticated .input( z.object({ file_id: z.string().uuid(), @@ -50,7 +50,7 @@ export const file = router({ }; }), - getPresignedUrlForFileUpload: authenticatedProcedure + getPresignedUrlForFileUpload: hasAuthenticated .input( z.object({ type: z.string(), @@ -71,7 +71,7 @@ export const file = router({ return { url, key }; }), - finishFileUpload: authenticatedProcedure + finishFileUpload: hasAuthenticated .input( z.object({ filename: z.string(), @@ -99,7 +99,7 @@ export const file = router({ return file; }), - deleteFile: authenticatedProcedure + deleteFile: hasAuthenticated .input( z.object({ file_id: z.string().uuid(), diff --git a/src/backend/routers/iep.ts b/src/backend/routers/iep.ts index 0a7ae50a..2f5a716a 100644 --- a/src/backend/routers/iep.ts +++ b/src/backend/routers/iep.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { authenticatedProcedure, router } from "../trpc"; +import { hasAuthenticated, router } from "../trpc"; import { jsonArrayFrom } from "kysely/helpers/postgres"; import { deleteFile } from "../lib/files"; import { substituteTransactionOnContext } from "../lib/utils/context"; @@ -7,7 +7,7 @@ import { TRPCError } from "@trpc/server"; // TODO: define .output() schemas for all procedures export const iep = router({ - addGoal: authenticatedProcedure + addGoal: hasAuthenticated .input( z.object({ iep_id: z.string(), @@ -31,7 +31,7 @@ export const iep = router({ return result; }), - editGoal: authenticatedProcedure + editGoal: hasAuthenticated .input( z.object({ goal_id: z.string(), @@ -70,7 +70,7 @@ export const iep = router({ return result; }), - addSubgoal: authenticatedProcedure + addSubgoal: hasAuthenticated .input( z.object({ // current_level not included, should be calculated as trial data is collected @@ -123,7 +123,7 @@ export const iep = router({ return result; }), - addTask: authenticatedProcedure + addTask: hasAuthenticated .input( z.object({ subgoal_id: z.string(), @@ -148,7 +148,7 @@ export const iep = router({ return result; }), - assignTaskToParas: authenticatedProcedure + assignTaskToParas: hasAuthenticated .input( z.object({ subgoal_id: z.string().uuid(), @@ -175,7 +175,7 @@ export const iep = router({ return result; }), //Temporary function to easily assign tasks to self for testing - tempAddTaskToSelf: authenticatedProcedure + tempAddTaskToSelf: hasAuthenticated .input( z.object({ subgoal_id: z.string(), @@ -217,7 +217,7 @@ export const iep = router({ return result; }), - addTrialData: authenticatedProcedure + addTrialData: hasAuthenticated .input( z.object({ task_id: z.string(), @@ -246,7 +246,7 @@ export const iep = router({ return result; }), - updateTrialData: authenticatedProcedure + updateTrialData: hasAuthenticated .input( z.object({ trial_data_id: z.string(), @@ -271,7 +271,7 @@ export const iep = router({ .execute(); }), - getGoals: authenticatedProcedure + getGoals: hasAuthenticated .input( z.object({ iep_id: z.string(), @@ -289,7 +289,7 @@ export const iep = router({ return result; }), - getGoal: authenticatedProcedure + getGoal: hasAuthenticated .input( z.object({ goal_id: z.string(), @@ -307,7 +307,7 @@ export const iep = router({ return result; }), - getSubgoals: authenticatedProcedure + getSubgoals: hasAuthenticated .input( z.object({ goal_id: z.string(), @@ -325,7 +325,7 @@ export const iep = router({ return result; }), - getSubgoal: authenticatedProcedure + getSubgoal: hasAuthenticated .input( z.object({ subgoal_id: z.string(), @@ -342,7 +342,7 @@ export const iep = router({ return result; }), - getSubgoalsByAssignee: authenticatedProcedure + getSubgoalsByAssignee: hasAuthenticated .input( z.object({ assignee_id: z.string(), @@ -361,7 +361,7 @@ export const iep = router({ return result; }), - getSubgoalAndTrialData: authenticatedProcedure + getSubgoalAndTrialData: hasAuthenticated .input( z.object({ task_id: z.string(), @@ -424,7 +424,7 @@ export const iep = router({ return result; }), - markAsSeen: authenticatedProcedure + markAsSeen: hasAuthenticated .input( z.object({ task_id: z.string(), @@ -442,7 +442,7 @@ export const iep = router({ .execute(); }), - attachFileToTrialData: authenticatedProcedure + attachFileToTrialData: hasAuthenticated .input( z.object({ trial_data_id: z.string(), @@ -461,7 +461,7 @@ export const iep = router({ .execute(); }), - removeFileFromTrialDataAndDelete: authenticatedProcedure + removeFileFromTrialDataAndDelete: hasAuthenticated .input( z.object({ trial_data_id: z.string(), diff --git a/src/backend/routers/para.ts b/src/backend/routers/para.ts index 2a5ac7e0..44acccee 100644 --- a/src/backend/routers/para.ts +++ b/src/backend/routers/para.ts @@ -1,9 +1,9 @@ import { z } from "zod"; -import { authenticatedProcedure, router } from "../trpc"; +import { hasAuthenticated, router } from "../trpc"; import { createPara } from "../lib/db_helpers/case_manager"; export const para = router({ - getParaById: authenticatedProcedure + getParaById: hasAuthenticated .input(z.object({ user_id: z.string().uuid() })) .query(async (req) => { const { user_id } = req.input; @@ -17,7 +17,7 @@ export const para = router({ return result; }), - getParaByEmail: authenticatedProcedure + getParaByEmail: hasAuthenticated .input(z.object({ email: z.string() })) .query(async (req) => { const { email } = req.input; @@ -34,7 +34,7 @@ export const para = router({ /** * Deprecated: use case_manager.addStaff instead */ - createPara: authenticatedProcedure + createPara: hasAuthenticated .input( z.object({ first_name: z.string(), @@ -61,7 +61,7 @@ export const para = router({ // TODO elsewhere: add "email_verified_at" timestamp when para first signs in with their email address (entered into db by cm) }), - getMyTasks: authenticatedProcedure.query(async (req) => { + getMyTasks: hasAuthenticated.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db diff --git a/src/backend/routers/public.ts b/src/backend/routers/public.ts index ae5de6b7..d7ab270d 100644 --- a/src/backend/routers/public.ts +++ b/src/backend/routers/public.ts @@ -1,7 +1,7 @@ -import { publicProcedure, router } from "../trpc"; +import { noAuth, router } from "../trpc"; export const publicRouter = router({ - healthCheck: publicProcedure.query(() => { + healthCheck: noAuth.query(() => { return "Ok"; }), }); diff --git a/src/backend/routers/student.ts b/src/backend/routers/student.ts index 263a09d4..9170475d 100644 --- a/src/backend/routers/student.ts +++ b/src/backend/routers/student.ts @@ -1,9 +1,9 @@ import { z } from "zod"; -import { authenticatedProcedure, router } from "../trpc"; +import { hasAuthenticated, router } from "../trpc"; // TODO: define .output() schemas for all procedures export const student = router({ - getStudentById: authenticatedProcedure + getStudentById: hasAuthenticated .input(z.object({ student_id: z.string().uuid() })) .query(async (req) => { const { student_id } = req.input; @@ -17,7 +17,7 @@ export const student = router({ return result; }), - getStudentByTaskId: authenticatedProcedure + getStudentByTaskId: hasAuthenticated .input(z.object({ task_id: z.string().uuid() })) .query(async (req) => { const { task_id } = req.input; @@ -38,7 +38,7 @@ export const student = router({ /** * Adds a new IEP for the given student. */ - addIep: authenticatedProcedure + addIep: hasAuthenticated .input( z.object({ student_id: z.string(), @@ -67,7 +67,7 @@ export const student = router({ /** * Adds a new IEP for the given student. */ - editIep: authenticatedProcedure + editIep: hasAuthenticated .input( z.object({ student_id: z.string(), @@ -104,7 +104,7 @@ export const student = router({ /** * Returns all the IEPs associated with the given student. */ - getIeps: authenticatedProcedure + getIeps: hasAuthenticated .input( z.object({ student_id: z.string(), @@ -130,7 +130,7 @@ export const student = router({ * per the MVP that there will only be one IEP per student, * but this should be revisited after the MVP. */ - getActiveStudentIep: authenticatedProcedure + getActiveStudentIep: hasAuthenticated .input( z.object({ student_id: z.string().uuid(), diff --git a/src/backend/routers/user.ts b/src/backend/routers/user.ts index aa031aeb..0c0bfdb3 100644 --- a/src/backend/routers/user.ts +++ b/src/backend/routers/user.ts @@ -1,7 +1,7 @@ -import { authenticatedProcedure, router } from "../trpc"; +import { hasAuthenticated, router } from "../trpc"; export const user = router({ - getMe: authenticatedProcedure.query(async (req) => { + getMe: hasAuthenticated.query(async (req) => { const { userId } = req.ctx.auth; const user = await req.ctx.db @@ -23,7 +23,7 @@ export const user = router({ /** * @returns Whether the current user is a case manager */ - isCaseManager: authenticatedProcedure.query(async (req) => { + isCaseManager: hasAuthenticated.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db diff --git a/src/backend/trpc.ts b/src/backend/trpc.ts index 58391158..a149497d 100644 --- a/src/backend/trpc.ts +++ b/src/backend/trpc.ts @@ -2,6 +2,28 @@ import { TRPCError, initTRPC } from "@trpc/server"; import { createContext } from "./context"; import superjson from "superjson"; +// Role-based access control type +type RoleLevel = { + user: 0; + para: 1; + case_manager: 2; + admin: 3; +}; + +const ROLE_LEVELS: RoleLevel = { + user: 0, + para: 1, + case_manager: 2, + admin: 3, +}; + +type Role = keyof RoleLevel; + +// Function to compare roles +function hasMinimumRole(userRole: Role, requiredRole: Role): boolean { + return ROLE_LEVELS[userRole] >= ROLE_LEVELS[requiredRole]; +} + // initialize tRPC exactly once per application: export const t = initTRPC.context().create({ // SuperJSON allows us to transparently use, e.g., standard Date/Map/Sets @@ -22,8 +44,43 @@ const isAuthenticated = t.middleware(({ next, ctx }) => { }); }); +const atLeastPara = t.middleware(({ next, ctx }) => { + if ( + ctx.auth.type !== "session" || + !hasMinimumRole(ctx.auth.role as Role, "para") + ) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + } + + return next({ + ctx: { + ...ctx, + auth: ctx.auth, + }, + }); +}); + +const atLeastCaseManager = t.middleware(({ next, ctx }) => { + if ( + ctx.auth.type !== "session" || + !hasMinimumRole(ctx.auth.role as Role, "case_manager") + ) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + } + + return next({ + ctx: { + ...ctx, + auth: ctx.auth, + }, + }); +}); + const isAdmin = t.middleware(({ next, ctx }) => { - if (ctx.auth.type !== "session" || ctx.auth.role !== "admin") { + if ( + ctx.auth.type !== "session" || + !hasMinimumRole(ctx.auth.role as Role, "admin") + ) { throw new TRPCError({ code: "UNAUTHORIZED" }); } @@ -37,6 +94,17 @@ const isAdmin = t.middleware(({ next, ctx }) => { // Define and export the tRPC router export const router = t.router; -export const publicProcedure = t.procedure; -export const authenticatedProcedure = t.procedure.use(isAuthenticated); -export const adminProcedure = t.procedure.use(isAuthenticated).use(isAdmin); + +// Define and export the tRPC procedures that can be used as auth guards inside routes +export const noAuth = t.procedure; // Can be used for public routes +export const hasAuthenticated = t.procedure // for routes that require authentication only, no specific role + .use(isAuthenticated); +export const hasPara = t.procedure // for routes that require at least para role + .use(isAuthenticated) + .use(atLeastPara); +export const hasCaseManager = t.procedure // for routes that require at least case manager role + .use(isAuthenticated) + .use(atLeastCaseManager); +export const hasAdmin = t.procedure // for routes that require admin role + .use(isAuthenticated) + .use(isAdmin); From 473ba16ecb75d9998c961027c8b6690056337a5b Mon Sep 17 00:00:00 2001 From: thom Date: Mon, 7 Oct 2024 12:58:17 -0700 Subject: [PATCH 05/25] feat: migration to build out roles in db + one rename of auth guard --- src/backend/db/migrations/2_rbac_roles.sql | 14 ++++++++++++++ src/backend/routers/student.ts | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 src/backend/db/migrations/2_rbac_roles.sql diff --git a/src/backend/db/migrations/2_rbac_roles.sql b/src/backend/db/migrations/2_rbac_roles.sql new file mode 100644 index 00000000..fe03bcba --- /dev/null +++ b/src/backend/db/migrations/2_rbac_roles.sql @@ -0,0 +1,14 @@ +-- Step 1: Drop the existing check constraint if it exists +ALTER TABLE "public"."user" DROP CONSTRAINT IF EXISTS user_role_check; + +-- Step 3: Update existing roles +UPDATE "public"."user" SET role = 'case_manager' WHERE role = 'admin'; +UPDATE "public"."user" SET role = 'para' WHERE role = 'staff'; + +-- Step 2: Add the new check constraint with the updated roles +ALTER TABLE "public"."user" ADD CONSTRAINT user_role_check +CHECK (role = ANY (ARRAY['user'::text, 'para'::text, 'case_manager'::text, 'admin'::text])); + + +-- Step 4: Add a comment to the table explaining the role values +COMMENT ON COLUMN "public"."user".role IS 'User role: user, para, case_manager, or admin'; diff --git a/src/backend/routers/student.ts b/src/backend/routers/student.ts index 9170475d..dc9e6bd4 100644 --- a/src/backend/routers/student.ts +++ b/src/backend/routers/student.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasAuthenticated, router } from "../trpc"; +import { hasAuthenticated, hasCaseManager, router } from "../trpc"; // TODO: define .output() schemas for all procedures export const student = router({ @@ -130,7 +130,7 @@ export const student = router({ * per the MVP that there will only be one IEP per student, * but this should be revisited after the MVP. */ - getActiveStudentIep: hasAuthenticated + getActiveStudentIep: hasCaseManager .input( z.object({ student_id: z.string().uuid(), From 46e7330feef97ef016d835dd07eea818989e0e46 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Mon, 7 Oct 2024 15:40:41 -0700 Subject: [PATCH 06/25] Add UserType enum and refactor hasMinimumRole --- src/backend/auth/adapter.ts | 4 ++- src/backend/context.ts | 5 ++-- src/backend/db/lib/seed.ts | 3 +- src/backend/db/zapatos/schema.d.ts | 10 +++++++ src/backend/lib/db_helpers/case_manager.ts | 5 ++-- src/backend/routers/admin.test.ts | 5 ++-- src/backend/routers/case_manager.test.ts | 29 ++++++++++--------- src/backend/routers/iep.test.ts | 7 +++-- src/backend/routers/para.test.ts | 17 ++++++----- src/backend/routers/student.test.ts | 15 +++++----- src/backend/routers/user.test.ts | 7 +++-- src/backend/tests/fixtures/get-test-server.ts | 3 +- src/backend/tests/seed.ts | 7 +++-- src/backend/trpc.ts | 26 +++++++---------- src/client/lib/protected-page.tsx | 5 ++-- src/pages/index.tsx | 3 +- src/types/auth.ts | 7 +++-- src/types/global.ts | 6 ++++ 18 files changed, 96 insertions(+), 68 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index 2c80d3fc..72cac16e 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -6,6 +6,7 @@ import { } from "../lib"; import { InsertObject, Selectable } from "kysely"; import { CustomAdapterUser } from "@/types/auth"; +import { UserType } from "@/types/global"; const mapStoredUserToAdapterUser = ( user: Selectable> @@ -44,7 +45,8 @@ export const createPersistedAuthAdapter = ( // First created user is an admin // todo: this should be pulled from an invite or something else instead of defaulting to a para - currently devs signing in are being assigned as paras - const role = Number(numOfUsers.count) === 0 ? "admin" : "staff"; + const role = + Number(numOfUsers.count) === 0 ? UserType.Admin : UserType.Para; const [first_name, last_name] = user.name?.split(" ") ?? [ user.email?.split("@")[0], diff --git a/src/backend/context.ts b/src/backend/context.ts index 8b996a08..ec0008d1 100644 --- a/src/backend/context.ts +++ b/src/backend/context.ts @@ -4,8 +4,9 @@ import { S3Client } from "@aws-sdk/client-s3"; import { Session, getServerSession } from "next-auth"; import { Env } from "./lib/types"; import { getNextAuthOptions } from "./auth/options"; +import { UserType } from "@/types/global"; -type Auth = +export type Auth = | { type: "none"; } @@ -13,7 +14,7 @@ type Auth = type: "session"; session: Session; userId: string; - role: string; + role: UserType; }; export type tRPCContext = ReturnType & { diff --git a/src/backend/db/lib/seed.ts b/src/backend/db/lib/seed.ts index 8dcb5dc2..63ebed31 100644 --- a/src/backend/db/lib/seed.ts +++ b/src/backend/db/lib/seed.ts @@ -1,5 +1,6 @@ import { logger } from "@/backend/lib"; import { getDb } from "@/backend/db/lib/get-db"; +import { UserType } from "@/types/global"; export const seedfile = async (databaseUrl: string) => { const { db } = getDb(databaseUrl); @@ -49,7 +50,7 @@ export const seedfile = async (databaseUrl: string) => { first_name: "Helen", last_name: "Parr", email: "elastic@example.com", - role: "staff", + role: UserType.Para, }) .returning("user_id") .executeTakeFirstOrThrow(); diff --git a/src/backend/db/zapatos/schema.d.ts b/src/backend/db/zapatos/schema.d.ts index d9c952e3..aa9f5c9f 100644 --- a/src/backend/db/zapatos/schema.d.ts +++ b/src/backend/db/zapatos/schema.d.ts @@ -2605,6 +2605,8 @@ declare module 'zapatos/schema' { last_name: string; /** * **user.role** + * + * User role: user, para, case_manager, or admin * - `text` in database * - `NOT NULL`, no default */ @@ -2649,6 +2651,8 @@ declare module 'zapatos/schema' { last_name: string; /** * **user.role** + * + * User role: user, para, case_manager, or admin * - `text` in database * - `NOT NULL`, no default */ @@ -2693,6 +2697,8 @@ declare module 'zapatos/schema' { last_name?: string | db.Parameter | db.SQLFragment | db.ParentColumn | db.SQLFragment | db.SQLFragment | db.ParentColumn>; /** * **user.role** + * + * User role: user, para, case_manager, or admin * - `text` in database * - `NOT NULL`, no default */ @@ -2737,6 +2743,8 @@ declare module 'zapatos/schema' { last_name: string | db.Parameter | db.SQLFragment; /** * **user.role** + * + * User role: user, para, case_manager, or admin * - `text` in database * - `NOT NULL`, no default */ @@ -2781,6 +2789,8 @@ declare module 'zapatos/schema' { last_name?: string | db.Parameter | db.SQLFragment | db.SQLFragment | db.SQLFragment>; /** * **user.role** + * + * User role: user, para, case_manager, or admin * - `text` in database * - `NOT NULL`, no default */ diff --git a/src/backend/lib/db_helpers/case_manager.ts b/src/backend/lib/db_helpers/case_manager.ts index ef78be9f..ad918db8 100644 --- a/src/backend/lib/db_helpers/case_manager.ts +++ b/src/backend/lib/db_helpers/case_manager.ts @@ -2,6 +2,7 @@ import { Env } from "@/backend/lib/types"; import { KyselyDatabaseInstance } from "@/backend/lib"; import { getTransporter } from "@/backend/lib/nodemailer"; import { user } from "zapatos/schema"; +import { UserType } from "@/types/global"; interface paraInputProps { first_name: string; @@ -11,7 +12,7 @@ interface paraInputProps { /** * Checks for the existence of a user with the given email, if - * they do not exist, create the user with the role of "staff", + * they do not exist, create the user with the role of "para", * initiate email sending without awaiting result */ export async function createPara( @@ -37,7 +38,7 @@ export async function createPara( first_name, last_name, email: email.toLowerCase(), - role: "staff", + role: UserType.Para, }) .returningAll() .executeTakeFirst(); diff --git a/src/backend/routers/admin.test.ts b/src/backend/routers/admin.test.ts index 8a07cb0d..d287e3bf 100644 --- a/src/backend/routers/admin.test.ts +++ b/src/backend/routers/admin.test.ts @@ -1,15 +1,16 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; +import { UserType } from "@/types/global"; test("getPostgresInfo", async (t) => { - const { trpc } = await getTestServer(t, { authenticateAs: "admin" }); + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Admin }); const postgresInfo = await trpc.admin.getPostgresInfo.query(); t.true(postgresInfo.includes("PostgreSQL")); }); test("getPostgresInfo (throws if not admin)", async (t) => { - const { trpc } = await getTestServer(t, { authenticateAs: "para" }); + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); await t.throwsAsync(async () => { await trpc.admin.getPostgresInfo.query(); diff --git a/src/backend/routers/case_manager.test.ts b/src/backend/routers/case_manager.test.ts index 0de75f9c..daf42366 100644 --- a/src/backend/routers/case_manager.test.ts +++ b/src/backend/routers/case_manager.test.ts @@ -5,10 +5,11 @@ import { STUDENT_ASSIGNED_TO_YOU_ERR, STUDENT_ALREADY_ASSIGNED_ERR, } from "@/backend/lib/db_helpers/case_manager"; +import { UserType } from "@/types/global"; test("getMyStudents", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const { student_id } = await db @@ -30,7 +31,7 @@ test("getMyStudents", async (t) => { test("getMyStudentsAndIepInfo - student does not have IEP", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const student = await db @@ -54,7 +55,7 @@ test("getMyStudentsAndIepInfo - student does not have IEP", async (t) => { test("getMyStudentsAndIepInfo - student has IEP", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await trpc.case_manager.addStudent.mutate({ @@ -83,7 +84,7 @@ test("getMyStudentsAndIepInfo - student has IEP", async (t) => { test("addStudent - student doesn't exist in db", async (t) => { const { trpc, db } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const myStudentsBefore = await trpc.case_manager.getMyStudents.query(); @@ -109,7 +110,7 @@ test("addStudent - student doesn't exist in db", async (t) => { test("addStudent - student exists in db but is unassigned", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const before = await trpc.case_manager.getMyStudents.query(); @@ -144,7 +145,7 @@ test("addStudent - student exists in db but is unassigned", async (t) => { test("addStudent - student exists in db and is already assigned to user", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const studentsBefore = await trpc.case_manager.getMyStudents.query(); @@ -175,7 +176,7 @@ test("addStudent - student exists in db and is already assigned to user", async test("addStudent - student exists in db but is assigned to another case manager", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const studentsBefore = await trpc.case_manager.getMyStudents.query(); @@ -187,7 +188,7 @@ test("addStudent - student exists in db but is assigned to another case manager" .values({ first_name: "Fake", last_name: "CM", - role: "admin", + role: UserType.Admin, email: "fakecm@test.com", }) .returningAll() @@ -249,7 +250,7 @@ test("addStudent - student exists in db but is assigned to another case manager" test("addStudent - invalid email", async (t) => { const { trpc } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const err = await t.throwsAsync( @@ -279,7 +280,7 @@ test("addStudent - invalid email", async (t) => { test("removeStudent", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const { student_id } = await db @@ -307,7 +308,7 @@ test("removeStudent", async (t) => { test("getMyParas", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); let myParas = await trpc.case_manager.getMyParas.query(); @@ -327,7 +328,7 @@ test("getMyParas", async (t) => { test("addStaff", async (t) => { const { trpc } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const parasBeforeAdd = await trpc.case_manager.getMyParas.query(); @@ -352,7 +353,7 @@ test("addStaff", async (t) => { test("addPara", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); let myParas = await trpc.case_manager.getMyParas.query(); @@ -368,7 +369,7 @@ test("addPara", async (t) => { test("removePara", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await db diff --git a/src/backend/routers/iep.test.ts b/src/backend/routers/iep.test.ts index f04b21c9..8a8e0b55 100644 --- a/src/backend/routers/iep.test.ts +++ b/src/backend/routers/iep.test.ts @@ -1,10 +1,11 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; +import { UserType } from "@/types/global"; // TODO: Write more tests test("basic flow - add/get goals, subgoals, tasks", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const para_id = seed.para.user_id; @@ -90,7 +91,7 @@ test("basic flow - add/get goals, subgoals, tasks", async (t) => { test("add benchmark - check full schema", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const iep = await trpc.student.addIep.mutate({ @@ -138,7 +139,7 @@ test("add benchmark - check full schema", async (t) => { test("edit goal", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await trpc.case_manager.addStudent.mutate({ diff --git a/src/backend/routers/para.test.ts b/src/backend/routers/para.test.ts index ecb07a97..97d0a1ae 100644 --- a/src/backend/routers/para.test.ts +++ b/src/backend/routers/para.test.ts @@ -1,9 +1,10 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; +import { UserType } from "@/types/global"; test("getParaById", async (t) => { const { trpc, db } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const { user_id } = await db @@ -12,7 +13,7 @@ test("getParaById", async (t) => { first_name: "Foo", last_name: "Bar", email: "foo.bar@email.com", - role: "staff", + role: UserType.Para, }) .returningAll() .executeTakeFirstOrThrow(); @@ -23,7 +24,7 @@ test("getParaById", async (t) => { test("getParaByEmail", async (t) => { const { trpc, db } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const { email } = await db @@ -32,7 +33,7 @@ test("getParaByEmail", async (t) => { first_name: "Foo", last_name: "Bar", email: "foo.bar@email.com", - role: "staff", + role: UserType.Para, }) .returningAll() .executeTakeFirstOrThrow(); @@ -43,7 +44,7 @@ test("getParaByEmail", async (t) => { test("createPara", async (t) => { const { trpc, db, nodemailerMock } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await trpc.para.createPara.mutate({ @@ -69,7 +70,7 @@ test("createPara", async (t) => { test("paras are deduped by email", async (t) => { const { trpc, db } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); t.falsy(await trpc.para.getParaByEmail.query({ email: "foo.bar@email.com" })); @@ -97,7 +98,7 @@ test("paras are deduped by email", async (t) => { test("createPara - invalid email", async (t) => { const { trpc } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await t.throwsAsync( @@ -111,7 +112,7 @@ test("createPara - invalid email", async (t) => { test("getMyTasks", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const FIRST_NAME = "Foo"; diff --git a/src/backend/routers/student.test.ts b/src/backend/routers/student.test.ts index bf049d81..57cf2ecf 100644 --- a/src/backend/routers/student.test.ts +++ b/src/backend/routers/student.test.ts @@ -1,10 +1,11 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; import { parseISO } from "date-fns"; +import { UserType } from "@/types/global"; test("getStudentById", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const { student_id } = await db @@ -27,7 +28,7 @@ test("getStudentById", async (t) => { // Improve this test test("doNotAddDuplicateEmails", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await db @@ -60,7 +61,7 @@ test("doNotAddDuplicateEmails", async (t) => { test("addIep and getIep", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const start_date = new Date("2023-01-01"); @@ -84,7 +85,7 @@ test("addIep and getIep", async (t) => { test("editIep", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); // * must add student this way to populate the assigned_case_manager_id @@ -136,7 +137,7 @@ test("editIep", async (t) => { test("getActiveStudentIep - return only one iep object", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const start_date = new Date("2023-01-01"); @@ -160,7 +161,7 @@ test("getActiveStudentIep - return only one iep object", async (t) => { test("checkAddedIEPEndDates", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const start_date = new Date("2023-01-01"); const end_date = new Date("2022-01-01"); @@ -182,7 +183,7 @@ test("checkAddedIEPEndDates", async (t) => { test("checkEditedIEPEndDates", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); await trpc.case_manager.addStudent.mutate({ diff --git a/src/backend/routers/user.test.ts b/src/backend/routers/user.test.ts index b9cedf51..f20bb312 100644 --- a/src/backend/routers/user.test.ts +++ b/src/backend/routers/user.test.ts @@ -1,9 +1,10 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; +import { UserType } from "@/types/global"; test("getMe", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "para", + authenticateAs: UserType.Para, }); const me = await trpc.user.getMe.query(); @@ -20,7 +21,7 @@ test("getMe (throws if missing auth)", async (t) => { test("isCaseManager (user is case_manager)", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); // Assign a para to the case manager @@ -38,7 +39,7 @@ test("isCaseManager (user is case_manager)", async (t) => { test("isCaseManager (user is para)", async (t) => { const { trpc, db, seed } = await getTestServer(t, { - authenticateAs: "para", + authenticateAs: UserType.Para, }); // A user is not a case manager by default i.e. when paras_assigned_to_case_manager is empty diff --git a/src/backend/tests/fixtures/get-test-server.ts b/src/backend/tests/fixtures/get-test-server.ts index a53e8331..d2226aac 100644 --- a/src/backend/tests/fixtures/get-test-server.ts +++ b/src/backend/tests/fixtures/get-test-server.ts @@ -11,9 +11,10 @@ import ms from "ms"; import builtNextJsFixture from "../../../../.nsm"; import { getTestMinio } from "./get-test-minio"; import superjson from "superjson"; +import { UserType } from "@/types/global"; export interface GetTestServerOptions { - authenticateAs?: "case_manager" | "para" | "admin"; + authenticateAs?: UserType.CaseManager | UserType.Para | UserType.Admin; } export const getTestServer = async ( diff --git a/src/backend/tests/seed.ts b/src/backend/tests/seed.ts index abbdf6d3..d549716f 100644 --- a/src/backend/tests/seed.ts +++ b/src/backend/tests/seed.ts @@ -1,4 +1,5 @@ import { KyselyDatabaseInstance } from "@/backend/lib"; +import { UserType } from "@/types/global"; export type SeedResult = Awaited>; @@ -9,7 +10,7 @@ export const seed = async (db: KyselyDatabaseInstance) => { first_name: "Admin", last_name: "One", email: "admin1@example.com", - role: "admin", + role: UserType.Admin, }) .returningAll() .executeTakeFirstOrThrow(); @@ -20,7 +21,7 @@ export const seed = async (db: KyselyDatabaseInstance) => { first_name: "CaseManager", last_name: "One", email: "case_manager1@example.com", - role: "staff", + role: UserType.CaseManager, }) .returningAll() .executeTakeFirstOrThrow(); @@ -31,7 +32,7 @@ export const seed = async (db: KyselyDatabaseInstance) => { first_name: "Para", last_name: "One", email: "para1@example.com", - role: "staff", + role: UserType.Para, }) .returningAll() .executeTakeFirstOrThrow(); diff --git a/src/backend/trpc.ts b/src/backend/trpc.ts index a149497d..a7dbd780 100644 --- a/src/backend/trpc.ts +++ b/src/backend/trpc.ts @@ -1,6 +1,7 @@ import { TRPCError, initTRPC } from "@trpc/server"; -import { createContext } from "./context"; +import { Auth, createContext } from "./context"; import superjson from "superjson"; +import { UserType } from "@/types/global"; // Role-based access control type type RoleLevel = { @@ -20,8 +21,12 @@ const ROLE_LEVELS: RoleLevel = { type Role = keyof RoleLevel; // Function to compare roles -function hasMinimumRole(userRole: Role, requiredRole: Role): boolean { - return ROLE_LEVELS[userRole] >= ROLE_LEVELS[requiredRole]; +function hasMinimumRole(auth: Auth, requiredRole: Role): boolean { + const { type } = auth; + + return ( + type === "session" && ROLE_LEVELS[auth.role] >= ROLE_LEVELS[requiredRole] + ); } // initialize tRPC exactly once per application: @@ -45,10 +50,7 @@ const isAuthenticated = t.middleware(({ next, ctx }) => { }); const atLeastPara = t.middleware(({ next, ctx }) => { - if ( - ctx.auth.type !== "session" || - !hasMinimumRole(ctx.auth.role as Role, "para") - ) { + if (!hasMinimumRole(ctx.auth, UserType.Para)) { throw new TRPCError({ code: "UNAUTHORIZED" }); } @@ -61,10 +63,7 @@ const atLeastPara = t.middleware(({ next, ctx }) => { }); const atLeastCaseManager = t.middleware(({ next, ctx }) => { - if ( - ctx.auth.type !== "session" || - !hasMinimumRole(ctx.auth.role as Role, "case_manager") - ) { + if (!hasMinimumRole(ctx.auth, UserType.CaseManager)) { throw new TRPCError({ code: "UNAUTHORIZED" }); } @@ -77,10 +76,7 @@ const atLeastCaseManager = t.middleware(({ next, ctx }) => { }); const isAdmin = t.middleware(({ next, ctx }) => { - if ( - ctx.auth.type !== "session" || - !hasMinimumRole(ctx.auth.role as Role, "admin") - ) { + if (!hasMinimumRole(ctx.auth, UserType.Admin)) { throw new TRPCError({ code: "UNAUTHORIZED" }); } diff --git a/src/client/lib/protected-page.tsx b/src/client/lib/protected-page.tsx index 49a8eb09..8b50b4ea 100644 --- a/src/client/lib/protected-page.tsx +++ b/src/client/lib/protected-page.tsx @@ -1,6 +1,7 @@ import { trpc } from "./trpc"; import { useEffect } from "react"; import { useRouter } from "next/router"; +import { UserType } from "@/types/global"; export const requiresAdminAuth = (WrappedPage: React.ComponentType) => @@ -10,7 +11,7 @@ export const requiresAdminAuth = const { data: me, error } = trpc.user.getMe.useQuery(); useEffect(() => { - if ((me && me.role !== "admin") || error) { + if ((me && me.role !== UserType.Admin) || error) { void router.push("/"); } }, [me, error, router]); @@ -19,7 +20,7 @@ export const requiresAdminAuth = return "Loading..."; } - if (me?.role === "admin") { + if (me?.role === UserType.Admin) { return ; } }; diff --git a/src/pages/index.tsx b/src/pages/index.tsx index 67c134f7..1963cb15 100644 --- a/src/pages/index.tsx +++ b/src/pages/index.tsx @@ -3,6 +3,7 @@ import { useSession } from "next-auth/react"; import { useRouter } from "next/router"; import React, { useEffect } from "react"; import { ExtendedSession } from "@/types/auth"; +import { UserType } from "@/types/global"; const Home: NextPage = () => { const router = useRouter(); @@ -14,7 +15,7 @@ const Home: NextPage = () => { useEffect(() => { if (status === "authenticated" && session) { switch (session.user.role) { - case "para": + case UserType.Para: void router.push("/benchmarks"); break; default: diff --git a/src/types/auth.ts b/src/types/auth.ts index 1213d820..b6a3a702 100644 --- a/src/types/auth.ts +++ b/src/types/auth.ts @@ -1,20 +1,21 @@ +import { UserType } from "@/types/global"; import { User, Session } from "next-auth"; import { AdapterUser } from "next-auth/adapters"; export interface UserWithRole extends User { profile: { - role: string; + role: UserType; }; } export interface ExtendedSession extends Session { user: Session["user"] & { - role: string; + role: UserType; }; } export interface CustomAdapterUser extends AdapterUser { profile?: { - role: string; + role: UserType; }; } diff --git a/src/types/global.ts b/src/types/global.ts index 155eaf34..905d82ae 100644 --- a/src/types/global.ts +++ b/src/types/global.ts @@ -4,3 +4,9 @@ export type Goal = SelectableForTable<"goal">; export type Subgoal = SelectableForTable<"subgoal">; export type ChangeEvent = React.ChangeEvent; export type FormEvent = React.FormEvent; +export enum UserType { + User = "user", + Para = "para", + CaseManager = "case_manager", + Admin = "admin", +} From 9c4040b65d14215c20e15da8356d3719138a1816 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Mon, 7 Oct 2024 16:01:41 -0700 Subject: [PATCH 07/25] Do not display staff link on frontend unless user role is case manager or admin --- src/backend/auth/adapter.ts | 2 +- src/backend/context.ts | 2 +- src/backend/trpc.ts | 4 +--- src/components/navbar/NavBar.tsx | 12 ++++++++++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index 72cac16e..1d383c77 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -16,7 +16,7 @@ const mapStoredUserToAdapterUser = ( emailVerified: user.email_verified_at, name: `${user.first_name} ${user.last_name}`, image: user.image_url, - profile: { role: user.role }, // Add the role to the profile + profile: { role: user.role as UserType }, // Add the role to the profile }); const mapStoredSessionToAdapterSession = ( diff --git a/src/backend/context.ts b/src/backend/context.ts index ec0008d1..68e170d2 100644 --- a/src/backend/context.ts +++ b/src/backend/context.ts @@ -53,7 +53,7 @@ export const createContext = async ( type: "session", session, userId: user.user_id, - role: user.role, + role: user.role as UserType, }; } diff --git a/src/backend/trpc.ts b/src/backend/trpc.ts index a7dbd780..13b0f46b 100644 --- a/src/backend/trpc.ts +++ b/src/backend/trpc.ts @@ -18,10 +18,8 @@ const ROLE_LEVELS: RoleLevel = { admin: 3, }; -type Role = keyof RoleLevel; - // Function to compare roles -function hasMinimumRole(auth: Auth, requiredRole: Role): boolean { +function hasMinimumRole(auth: Auth, requiredRole: UserType): boolean { const { type } = auth; return ( diff --git a/src/components/navbar/NavBar.tsx b/src/components/navbar/NavBar.tsx index aef88302..2281e3b7 100644 --- a/src/components/navbar/NavBar.tsx +++ b/src/components/navbar/NavBar.tsx @@ -21,6 +21,8 @@ import * as React from "react"; import { MouseEventHandler } from "react"; import $navbar from "./Navbar.module.css"; import BreadcrumbsNav from "../design_system/breadcrumbs/Breadcrumbs"; +import { ExtendedSession } from "@/types/auth"; +import { UserType } from "@/types/global"; interface NavItemProps { href?: string; @@ -34,7 +36,10 @@ export default function NavBar() { const desktop = useMediaQuery("(min-width: 992px)"); const router = useRouter(); - const { status } = useSession(); + const { status, data: session } = useSession() as { + data: ExtendedSession | null; + status: "loading" | "authenticated" | "unauthenticated"; + }; const handleDrawerToggle = () => { setMobileOpen(!mobileOpen); @@ -93,7 +98,10 @@ export default function NavBar() { } text="Assigned" /> } text="Students" /> - } text="Staff" /> + {(session?.user.role === UserType.CaseManager || + session?.user.role === UserType.Admin) && ( + } text="Staff" /> + )} } text="Settings" /> } From c76a507762ab4cb356ea7265f462d688ed87d099 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Tue, 8 Oct 2024 18:36:25 -0700 Subject: [PATCH 08/25] Fix typescript types --- src/backend/auth/adapter.ts | 2 +- src/backend/routers/file.test.ts | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index 1d383c77..5ea7ab71 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -1,4 +1,4 @@ -import { Adapter, AdapterSession, AdapterUser } from "next-auth/adapters"; +import { Adapter, AdapterSession } from "next-auth/adapters"; import { KyselyDatabaseInstance, KyselySchema, diff --git a/src/backend/routers/file.test.ts b/src/backend/routers/file.test.ts index 81a89a8f..c4e4fc8a 100644 --- a/src/backend/routers/file.test.ts +++ b/src/backend/routers/file.test.ts @@ -2,9 +2,12 @@ import test from "ava"; import axios from "axios"; import fs from "node:fs/promises"; import { getTestServer } from "@/backend/tests"; +import { UserType } from "@/types/global"; test("can upload files", async (t) => { - const { trpc, db } = await getTestServer(t, { authenticateAs: "para" }); + const { trpc, db } = await getTestServer(t, { + authenticateAs: UserType.Para, + }); const { url, key } = await trpc.file.getPresignedUrlForFileUpload.mutate({ type: "image/png", @@ -27,7 +30,7 @@ test("can upload files", async (t) => { }); test("finishFileUpload throws if file already exists", async (t) => { - const { trpc } = await getTestServer(t, { authenticateAs: "para" }); + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); const { url, key } = await trpc.file.getPresignedUrlForFileUpload.mutate({ type: "image/png", @@ -50,7 +53,7 @@ test("finishFileUpload throws if file already exists", async (t) => { }); test("finishFileUpload throws if invalid key is provided", async (t) => { - const { trpc } = await getTestServer(t, { authenticateAs: "para" }); + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); await t.throwsAsync(async () => { await trpc.file.finishFileUpload.mutate({ @@ -61,7 +64,7 @@ test("finishFileUpload throws if invalid key is provided", async (t) => { }); test("can download files", async (t) => { - const { trpc } = await getTestServer(t, { authenticateAs: "para" }); + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); const { url, key } = await trpc.file.getPresignedUrlForFileUpload.mutate({ type: "image/png", From 75ee0f4e8dd65d83682f9e47b51cb9356b933190 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Tue, 8 Oct 2024 19:14:24 -0700 Subject: [PATCH 09/25] Move UserType enum to auth types, only let Paras see Assigned link --- src/backend/auth/adapter.ts | 3 +-- src/backend/context.ts | 2 +- src/backend/db/lib/seed.ts | 2 +- src/backend/lib/db_helpers/case_manager.ts | 2 +- src/backend/routers/admin.test.ts | 2 +- src/backend/routers/case_manager.test.ts | 2 +- src/backend/routers/file.test.ts | 2 +- src/backend/routers/iep.test.ts | 2 +- src/backend/routers/para.test.ts | 2 +- src/backend/routers/student.test.ts | 2 +- src/backend/routers/user.test.ts | 2 +- src/backend/tests/fixtures/get-test-server.ts | 2 +- src/backend/tests/seed.ts | 2 +- src/backend/trpc.ts | 2 +- src/client/lib/protected-page.tsx | 2 +- src/components/navbar/NavBar.tsx | 7 ++++--- src/pages/index.tsx | 3 +-- src/types/auth.ts | 8 +++++++- src/types/global.ts | 6 ------ 19 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index 5ea7ab71..d6b15ad9 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -5,8 +5,7 @@ import { ZapatosTableNameToKyselySchema, } from "../lib"; import { InsertObject, Selectable } from "kysely"; -import { CustomAdapterUser } from "@/types/auth"; -import { UserType } from "@/types/global"; +import { CustomAdapterUser, UserType } from "@/types/auth"; const mapStoredUserToAdapterUser = ( user: Selectable> diff --git a/src/backend/context.ts b/src/backend/context.ts index 68e170d2..72023fb3 100644 --- a/src/backend/context.ts +++ b/src/backend/context.ts @@ -4,7 +4,7 @@ import { S3Client } from "@aws-sdk/client-s3"; import { Session, getServerSession } from "next-auth"; import { Env } from "./lib/types"; import { getNextAuthOptions } from "./auth/options"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; export type Auth = | { diff --git a/src/backend/db/lib/seed.ts b/src/backend/db/lib/seed.ts index 63ebed31..c3d94aae 100644 --- a/src/backend/db/lib/seed.ts +++ b/src/backend/db/lib/seed.ts @@ -1,6 +1,6 @@ import { logger } from "@/backend/lib"; import { getDb } from "@/backend/db/lib/get-db"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; export const seedfile = async (databaseUrl: string) => { const { db } = getDb(databaseUrl); diff --git a/src/backend/lib/db_helpers/case_manager.ts b/src/backend/lib/db_helpers/case_manager.ts index ad918db8..176abee9 100644 --- a/src/backend/lib/db_helpers/case_manager.ts +++ b/src/backend/lib/db_helpers/case_manager.ts @@ -2,7 +2,7 @@ import { Env } from "@/backend/lib/types"; import { KyselyDatabaseInstance } from "@/backend/lib"; import { getTransporter } from "@/backend/lib/nodemailer"; import { user } from "zapatos/schema"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; interface paraInputProps { first_name: string; diff --git a/src/backend/routers/admin.test.ts b/src/backend/routers/admin.test.ts index d287e3bf..05a94a55 100644 --- a/src/backend/routers/admin.test.ts +++ b/src/backend/routers/admin.test.ts @@ -1,6 +1,6 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; test("getPostgresInfo", async (t) => { const { trpc } = await getTestServer(t, { authenticateAs: UserType.Admin }); diff --git a/src/backend/routers/case_manager.test.ts b/src/backend/routers/case_manager.test.ts index daf42366..438782b3 100644 --- a/src/backend/routers/case_manager.test.ts +++ b/src/backend/routers/case_manager.test.ts @@ -5,7 +5,7 @@ import { STUDENT_ASSIGNED_TO_YOU_ERR, STUDENT_ALREADY_ASSIGNED_ERR, } from "@/backend/lib/db_helpers/case_manager"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; test("getMyStudents", async (t) => { const { trpc, db, seed } = await getTestServer(t, { diff --git a/src/backend/routers/file.test.ts b/src/backend/routers/file.test.ts index c4e4fc8a..ce228c33 100644 --- a/src/backend/routers/file.test.ts +++ b/src/backend/routers/file.test.ts @@ -2,7 +2,7 @@ import test from "ava"; import axios from "axios"; import fs from "node:fs/promises"; import { getTestServer } from "@/backend/tests"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; test("can upload files", async (t) => { const { trpc, db } = await getTestServer(t, { diff --git a/src/backend/routers/iep.test.ts b/src/backend/routers/iep.test.ts index 8a8e0b55..f8df0797 100644 --- a/src/backend/routers/iep.test.ts +++ b/src/backend/routers/iep.test.ts @@ -1,6 +1,6 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; // TODO: Write more tests test("basic flow - add/get goals, subgoals, tasks", async (t) => { diff --git a/src/backend/routers/para.test.ts b/src/backend/routers/para.test.ts index 97d0a1ae..af7db797 100644 --- a/src/backend/routers/para.test.ts +++ b/src/backend/routers/para.test.ts @@ -1,6 +1,6 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; test("getParaById", async (t) => { const { trpc, db } = await getTestServer(t, { diff --git a/src/backend/routers/student.test.ts b/src/backend/routers/student.test.ts index 57cf2ecf..3f876619 100644 --- a/src/backend/routers/student.test.ts +++ b/src/backend/routers/student.test.ts @@ -1,7 +1,7 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; import { parseISO } from "date-fns"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; test("getStudentById", async (t) => { const { trpc, db, seed } = await getTestServer(t, { diff --git a/src/backend/routers/user.test.ts b/src/backend/routers/user.test.ts index f20bb312..2b845320 100644 --- a/src/backend/routers/user.test.ts +++ b/src/backend/routers/user.test.ts @@ -1,6 +1,6 @@ import test from "ava"; import { getTestServer } from "@/backend/tests"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; test("getMe", async (t) => { const { trpc, seed } = await getTestServer(t, { diff --git a/src/backend/tests/fixtures/get-test-server.ts b/src/backend/tests/fixtures/get-test-server.ts index d2226aac..51a4f0d2 100644 --- a/src/backend/tests/fixtures/get-test-server.ts +++ b/src/backend/tests/fixtures/get-test-server.ts @@ -11,7 +11,7 @@ import ms from "ms"; import builtNextJsFixture from "../../../../.nsm"; import { getTestMinio } from "./get-test-minio"; import superjson from "superjson"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; export interface GetTestServerOptions { authenticateAs?: UserType.CaseManager | UserType.Para | UserType.Admin; diff --git a/src/backend/tests/seed.ts b/src/backend/tests/seed.ts index d549716f..84dc8053 100644 --- a/src/backend/tests/seed.ts +++ b/src/backend/tests/seed.ts @@ -1,5 +1,5 @@ import { KyselyDatabaseInstance } from "@/backend/lib"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; export type SeedResult = Awaited>; diff --git a/src/backend/trpc.ts b/src/backend/trpc.ts index 13b0f46b..6eabeed8 100644 --- a/src/backend/trpc.ts +++ b/src/backend/trpc.ts @@ -1,7 +1,7 @@ import { TRPCError, initTRPC } from "@trpc/server"; import { Auth, createContext } from "./context"; import superjson from "superjson"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; // Role-based access control type type RoleLevel = { diff --git a/src/client/lib/protected-page.tsx b/src/client/lib/protected-page.tsx index 8b50b4ea..9b8c81a7 100644 --- a/src/client/lib/protected-page.tsx +++ b/src/client/lib/protected-page.tsx @@ -1,7 +1,7 @@ import { trpc } from "./trpc"; import { useEffect } from "react"; import { useRouter } from "next/router"; -import { UserType } from "@/types/global"; +import { UserType } from "@/types/auth"; export const requiresAdminAuth = (WrappedPage: React.ComponentType) => diff --git a/src/components/navbar/NavBar.tsx b/src/components/navbar/NavBar.tsx index 2281e3b7..ceb969e8 100644 --- a/src/components/navbar/NavBar.tsx +++ b/src/components/navbar/NavBar.tsx @@ -21,8 +21,7 @@ import * as React from "react"; import { MouseEventHandler } from "react"; import $navbar from "./Navbar.module.css"; import BreadcrumbsNav from "../design_system/breadcrumbs/Breadcrumbs"; -import { ExtendedSession } from "@/types/auth"; -import { UserType } from "@/types/global"; +import { ExtendedSession, UserType } from "@/types/auth"; interface NavItemProps { href?: string; @@ -96,7 +95,9 @@ export default function NavBar() { const drawer = (
- } text="Assigned" /> + {session?.user.role === UserType.Para && ( + } text="Assigned" /> + )} } text="Students" /> {(session?.user.role === UserType.CaseManager || session?.user.role === UserType.Admin) && ( diff --git a/src/pages/index.tsx b/src/pages/index.tsx index 1963cb15..affbd7a6 100644 --- a/src/pages/index.tsx +++ b/src/pages/index.tsx @@ -2,8 +2,7 @@ import type { NextPage } from "next"; import { useSession } from "next-auth/react"; import { useRouter } from "next/router"; import React, { useEffect } from "react"; -import { ExtendedSession } from "@/types/auth"; -import { UserType } from "@/types/global"; +import { ExtendedSession, UserType } from "@/types/auth"; const Home: NextPage = () => { const router = useRouter(); diff --git a/src/types/auth.ts b/src/types/auth.ts index b6a3a702..8d805845 100644 --- a/src/types/auth.ts +++ b/src/types/auth.ts @@ -1,4 +1,3 @@ -import { UserType } from "@/types/global"; import { User, Session } from "next-auth"; import { AdapterUser } from "next-auth/adapters"; @@ -19,3 +18,10 @@ export interface CustomAdapterUser extends AdapterUser { role: UserType; }; } + +export enum UserType { + User = "user", + Para = "para", + CaseManager = "case_manager", + Admin = "admin", +} diff --git a/src/types/global.ts b/src/types/global.ts index 905d82ae..155eaf34 100644 --- a/src/types/global.ts +++ b/src/types/global.ts @@ -4,9 +4,3 @@ export type Goal = SelectableForTable<"goal">; export type Subgoal = SelectableForTable<"subgoal">; export type ChangeEvent = React.ChangeEvent; export type FormEvent = React.FormEvent; -export enum UserType { - User = "user", - Para = "para", - CaseManager = "case_manager", - Admin = "admin", -} From 774ab085e9833b0e13e324201a63d63a1361f94f Mon Sep 17 00:00:00 2001 From: thom Date: Thu, 10 Oct 2024 13:15:04 -0700 Subject: [PATCH 10/25] feat: navitems are role-based, sorry page added, front-end error handling of 401 --- src/components/navbar/NavBar.tsx | 106 ++++++++++++++++++++++++------- src/pages/_app.tsx | 21 +++--- src/pages/index.tsx | 3 + src/pages/sorry.tsx | 14 ++++ 4 files changed, 113 insertions(+), 31 deletions(-) create mode 100644 src/pages/sorry.tsx diff --git a/src/components/navbar/NavBar.tsx b/src/components/navbar/NavBar.tsx index ceb969e8..4bc04c20 100644 --- a/src/components/navbar/NavBar.tsx +++ b/src/components/navbar/NavBar.tsx @@ -91,27 +91,87 @@ export default function NavBar() { ); }; - // Navigation Links - const drawer = ( -
- - {session?.user.role === UserType.Para && ( - } text="Assigned" /> - )} - } text="Students" /> - {(session?.user.role === UserType.CaseManager || - session?.user.role === UserType.Admin) && ( - } text="Staff" /> - )} - } text="Settings" /> - } - text="Logout" - onClick={() => signOut({ callbackUrl: "/" })} - /> - -
- ); + const drawer = () => { + switch (session?.user.role) { + case UserType.Admin: + return ( + + } + text="Assigned" + /> + } + text="Students" + /> + } text="Staff" /> + } + text="Settings" + /> + } + text="Logout" + onClick={() => signOut({ callbackUrl: "/" })} + /> + + ); + case UserType.CaseManager: + return ( + + } + text="Students" + /> + } text="Staff" /> + } + text="Settings" + /> + } + text="Logout" + onClick={() => signOut({ callbackUrl: "/" })} + /> + + ); + case UserType.Para: + return ( + + } + text="Assigned" + /> + } + text="Settings" + /> + } + text="Logout" + onClick={() => signOut({ callbackUrl: "/" })} + /> + + ); + default: + return ( + + } + text="Logout" + onClick={() => signOut({ callbackUrl: "/" })} + /> + + ); + } + }; return ( <> @@ -122,7 +182,7 @@ export default function NavBar() { {/* Sidebar for screens & breadcrumbs > md size */} {logo} - {drawer} + {drawer()} @@ -156,7 +216,7 @@ export default function NavBar() { } /> - {drawer} + {drawer()} )} diff --git a/src/pages/_app.tsx b/src/pages/_app.tsx index 57a411e9..c5dadc3a 100644 --- a/src/pages/_app.tsx +++ b/src/pages/_app.tsx @@ -17,6 +17,7 @@ import { AdapterDateFns } from "@mui/x-date-pickers/AdapterDateFns"; import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles"; import { compassTheme as theme } from "@/theme"; import { FontProvider } from "@/components/font-provider"; +import { useRouter } from "next/router"; interface CustomPageProps { session: Session; @@ -36,6 +37,7 @@ export default function App({ Component, pageProps, }: AppProps) { + const router = useRouter(); const [errorMessage, setErrorMessage] = useState(""); const [queryClient] = useState( @@ -44,15 +46,18 @@ export default function App({ queryCache: new QueryCache({ onError: (error) => { if (error instanceof Error) { - const errorMessages: { [key: string]: string } = { - BAD_REQUEST: "400: Bad request, please try again", - UNAUTHORIZED: "401: Unauthorized Error", - NOT_FOUND: "404: Page not found", - }; + if (error.message === "UNAUTHORIZED") { + void router.push("/sorry"); + } else { + const errorMessages: { [key: string]: string } = { + BAD_REQUEST: "400: Bad request, please try again", + NOT_FOUND: "404: Page not found", + }; - const defaultMessage = "An error occured. Please try again"; - const msg = errorMessages[error.message] || defaultMessage; - setErrorMessage(msg); + const defaultMessage = "An error occurred. Please try again"; + const msg = errorMessages[error.message] || defaultMessage; + setErrorMessage(msg); + } } }, }), diff --git a/src/pages/index.tsx b/src/pages/index.tsx index affbd7a6..77bf1f86 100644 --- a/src/pages/index.tsx +++ b/src/pages/index.tsx @@ -14,6 +14,9 @@ const Home: NextPage = () => { useEffect(() => { if (status === "authenticated" && session) { switch (session.user.role) { + case UserType.User: + void router.push("/sorry"); + break; case UserType.Para: void router.push("/benchmarks"); break; diff --git a/src/pages/sorry.tsx b/src/pages/sorry.tsx new file mode 100644 index 00000000..a3daf14a --- /dev/null +++ b/src/pages/sorry.tsx @@ -0,0 +1,14 @@ +import React from "react"; +import $box from "@/styles/Box.module.css"; +import $typo from "@/styles/Typography.module.css"; + +const SorryPage: React.FC = () => { + return ( +
+

Access Denied

+

Your account is not authorized to use this app.

+
+ ); +}; + +export default SorryPage; From 11dc2fb0e5c3b25c51b22c15925e3441ac6da473 Mon Sep 17 00:00:00 2001 From: thom Date: Sat, 12 Oct 2024 08:02:13 -0700 Subject: [PATCH 11/25] feat: auth guards + type fix in trpc.ts --- src/backend/routers/case_manager.ts | 22 +++++++++++----------- src/backend/routers/para.ts | 10 +++++----- src/backend/routers/student.ts | 10 +++++----- src/backend/trpc.ts | 9 ++++++--- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/backend/routers/case_manager.ts b/src/backend/routers/case_manager.ts index 7d7091c4..1ebb1b36 100644 --- a/src/backend/routers/case_manager.ts +++ b/src/backend/routers/case_manager.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasAuthenticated, router } from "../trpc"; +import { hasCaseManager, router } from "../trpc"; import { createPara, assignParaToCaseManager, @@ -10,7 +10,7 @@ export const case_manager = router({ /** * Get all students assigned to the current user */ - getMyStudents: hasAuthenticated.query(async (req) => { + getMyStudents: hasCaseManager.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db @@ -22,7 +22,7 @@ export const case_manager = router({ return result; }), - getMyStudentsAndIepInfo: hasAuthenticated.query(async (req) => { + getMyStudentsAndIepInfo: hasCaseManager.query(async (req) => { const { userId } = req.ctx.auth; const studentData = await req.ctx.db @@ -50,7 +50,7 @@ export const case_manager = router({ * it doesn't already exist. Throws an error if the student is already * assigned to another CM. */ - addStudent: hasAuthenticated + addStudent: hasCaseManager .input( z.object({ first_name: z.string(), @@ -72,7 +72,7 @@ export const case_manager = router({ /** * Edits the given student in the CM's roster. Throws an error if the student was not found in the db. */ - editStudent: hasAuthenticated + editStudent: hasCaseManager .input( z.object({ student_id: z.string(), @@ -115,7 +115,7 @@ export const case_manager = router({ /** * Removes the case manager associated with this student. */ - removeStudent: hasAuthenticated + removeStudent: hasCaseManager .input( z.object({ student_id: z.string(), @@ -131,7 +131,7 @@ export const case_manager = router({ .execute(); }), - getMyParas: hasAuthenticated.query(async (req) => { + getMyParas: hasCaseManager.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db @@ -152,7 +152,7 @@ export const case_manager = router({ * Handles creation of para and assignment to user, attempts to send * email but does not await email success */ - addStaff: hasAuthenticated + addStaff: hasCaseManager .input( z.object({ first_name: z.string(), @@ -180,7 +180,7 @@ export const case_manager = router({ /** * Deprecated: use addStaff instead */ - addPara: hasAuthenticated + addPara: hasCaseManager .input( z.object({ para_id: z.string(), @@ -195,7 +195,7 @@ export const case_manager = router({ return; }), - editPara: hasAuthenticated + editPara: hasCaseManager .input( z.object({ para_id: z.string(), @@ -236,7 +236,7 @@ export const case_manager = router({ .executeTakeFirstOrThrow(); }), - removePara: hasAuthenticated + removePara: hasCaseManager .input( z.object({ para_id: z.string(), diff --git a/src/backend/routers/para.ts b/src/backend/routers/para.ts index 44acccee..7e34217f 100644 --- a/src/backend/routers/para.ts +++ b/src/backend/routers/para.ts @@ -1,9 +1,9 @@ import { z } from "zod"; -import { hasAuthenticated, router } from "../trpc"; +import { hasCaseManager, router } from "../trpc"; import { createPara } from "../lib/db_helpers/case_manager"; export const para = router({ - getParaById: hasAuthenticated + getParaById: hasCaseManager .input(z.object({ user_id: z.string().uuid() })) .query(async (req) => { const { user_id } = req.input; @@ -17,7 +17,7 @@ export const para = router({ return result; }), - getParaByEmail: hasAuthenticated + getParaByEmail: hasCaseManager .input(z.object({ email: z.string() })) .query(async (req) => { const { email } = req.input; @@ -34,7 +34,7 @@ export const para = router({ /** * Deprecated: use case_manager.addStaff instead */ - createPara: hasAuthenticated + createPara: hasCaseManager .input( z.object({ first_name: z.string(), @@ -61,7 +61,7 @@ export const para = router({ // TODO elsewhere: add "email_verified_at" timestamp when para first signs in with their email address (entered into db by cm) }), - getMyTasks: hasAuthenticated.query(async (req) => { + getMyTasks: hasCaseManager.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db diff --git a/src/backend/routers/student.ts b/src/backend/routers/student.ts index dc9e6bd4..701b42c7 100644 --- a/src/backend/routers/student.ts +++ b/src/backend/routers/student.ts @@ -3,7 +3,7 @@ import { hasAuthenticated, hasCaseManager, router } from "../trpc"; // TODO: define .output() schemas for all procedures export const student = router({ - getStudentById: hasAuthenticated + getStudentById: hasCaseManager .input(z.object({ student_id: z.string().uuid() })) .query(async (req) => { const { student_id } = req.input; @@ -17,7 +17,7 @@ export const student = router({ return result; }), - getStudentByTaskId: hasAuthenticated + getStudentByTaskId: hasCaseManager .input(z.object({ task_id: z.string().uuid() })) .query(async (req) => { const { task_id } = req.input; @@ -38,7 +38,7 @@ export const student = router({ /** * Adds a new IEP for the given student. */ - addIep: hasAuthenticated + addIep: hasCaseManager .input( z.object({ student_id: z.string(), @@ -67,7 +67,7 @@ export const student = router({ /** * Adds a new IEP for the given student. */ - editIep: hasAuthenticated + editIep: hasCaseManager .input( z.object({ student_id: z.string(), @@ -104,7 +104,7 @@ export const student = router({ /** * Returns all the IEPs associated with the given student. */ - getIeps: hasAuthenticated + getIeps: hasCaseManager .input( z.object({ student_id: z.string(), diff --git a/src/backend/trpc.ts b/src/backend/trpc.ts index 6eabeed8..443cfcf2 100644 --- a/src/backend/trpc.ts +++ b/src/backend/trpc.ts @@ -27,6 +27,9 @@ function hasMinimumRole(auth: Auth, requiredRole: UserType): boolean { ); } +// Add this type at the top of the file +type AuthenticatedAuth = Extract; + // initialize tRPC exactly once per application: export const t = initTRPC.context().create({ // SuperJSON allows us to transparently use, e.g., standard Date/Map/Sets @@ -55,7 +58,7 @@ const atLeastPara = t.middleware(({ next, ctx }) => { return next({ ctx: { ...ctx, - auth: ctx.auth, + auth: ctx.auth as AuthenticatedAuth, }, }); }); @@ -68,7 +71,7 @@ const atLeastCaseManager = t.middleware(({ next, ctx }) => { return next({ ctx: { ...ctx, - auth: ctx.auth, + auth: ctx.auth as AuthenticatedAuth, }, }); }); @@ -81,7 +84,7 @@ const isAdmin = t.middleware(({ next, ctx }) => { return next({ ctx: { ...ctx, - auth: ctx.auth, + auth: ctx.auth as AuthenticatedAuth, }, }); }); From 5258bc4268514224661d96600fffc1da2bc06870 Mon Sep 17 00:00:00 2001 From: thom Date: Wed, 16 Oct 2024 16:42:06 -0700 Subject: [PATCH 12/25] feat: all roles land on correct page + paras can see their tasks --- src/backend/routers/para.ts | 4 ++-- src/pages/signInPage.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/routers/para.ts b/src/backend/routers/para.ts index 7e34217f..7c070eb0 100644 --- a/src/backend/routers/para.ts +++ b/src/backend/routers/para.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasCaseManager, router } from "../trpc"; +import { hasCaseManager, hasPara, router } from "../trpc"; import { createPara } from "../lib/db_helpers/case_manager"; export const para = router({ @@ -61,7 +61,7 @@ export const para = router({ // TODO elsewhere: add "email_verified_at" timestamp when para first signs in with their email address (entered into db by cm) }), - getMyTasks: hasCaseManager.query(async (req) => { + getMyTasks: hasPara.query(async (req) => { const { userId } = req.ctx.auth; const result = await req.ctx.db diff --git a/src/pages/signInPage.tsx b/src/pages/signInPage.tsx index aca09591..d7b68d3b 100644 --- a/src/pages/signInPage.tsx +++ b/src/pages/signInPage.tsx @@ -32,7 +32,7 @@ const SignInPage = () => { className={$button.default} onClick={() => signIn("google", { - callbackUrl: "/students", + callbackUrl: "/", }) } > From 013b29e7f2f857016c02d50fa8ff3412a208b993 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 17:59:21 -0700 Subject: [PATCH 13/25] Specify UNAUTHORIZED as error --- src/backend/routers/admin.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/routers/admin.test.ts b/src/backend/routers/admin.test.ts index 05a94a55..b9475061 100644 --- a/src/backend/routers/admin.test.ts +++ b/src/backend/routers/admin.test.ts @@ -12,7 +12,13 @@ test("getPostgresInfo", async (t) => { test("getPostgresInfo (throws if not admin)", async (t) => { const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); - await t.throwsAsync(async () => { + const error = await t.throwsAsync(async () => { await trpc.admin.getPostgresInfo.query(); }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); }); From 2c020f8db1e6f14c00ea6c7fea8e10e29dfac3d2 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 21:45:59 -0700 Subject: [PATCH 14/25] Add specs to test authentication of each of case_maanger router endpoints --- src/backend/routers/case_manager.test.ts | 129 ++++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-) diff --git a/src/backend/routers/case_manager.test.ts b/src/backend/routers/case_manager.test.ts index 438782b3..cf0b04e1 100644 --- a/src/backend/routers/case_manager.test.ts +++ b/src/backend/routers/case_manager.test.ts @@ -7,7 +7,7 @@ import { } from "@/backend/lib/db_helpers/case_manager"; import { UserType } from "@/types/auth"; -test("getMyStudents", async (t) => { +test("getMyStudents - can fetch students", async (t) => { const { trpc, db, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, }); @@ -29,6 +29,20 @@ test("getMyStudents", async (t) => { t.is(myStudents[0].student_id, student_id); }); +test("getMyStudents - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.getMyStudents.query(); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("getMyStudentsAndIepInfo - student does not have IEP", async (t) => { const { trpc, db, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -82,6 +96,20 @@ test("getMyStudentsAndIepInfo - student has IEP", async (t) => { t.deepEqual(myStudentsAfter[0].end_date, iep.end_date); }); +test("getMyStudentsAndIepInfo - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.getMyStudentsAndIepInfo.query(); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("addStudent - student doesn't exist in db", async (t) => { const { trpc, db } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -278,6 +306,25 @@ test("addStudent - invalid email", async (t) => { } }); +test("addStudent - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.addStudent.mutate({ + first_name: "Foo", + last_name: "Bar", + email: "invalid-email", + grade: 6, + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("removeStudent", async (t) => { const { trpc, db, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -306,6 +353,22 @@ test("removeStudent", async (t) => { t.is(after.length, 0); }); +test("removeStudent - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.removeStudent.mutate({ + student_id: "student_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("getMyParas", async (t) => { const { trpc, db, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -326,6 +389,20 @@ test("getMyParas", async (t) => { t.is(myParas.length, 1); }); +test("getMyParas - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.getMyParas.query(); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("addStaff", async (t) => { const { trpc } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -351,6 +428,24 @@ test("addStaff", async (t) => { t.is(createdPara.email, newParaData.email); }); +test("addStaff - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.addStaff.mutate({ + first_name: "Foo", + last_name: "Bar", + email: "foo@bar.com", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("addPara", async (t) => { const { trpc, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -367,6 +462,22 @@ test("addPara", async (t) => { t.is(myParas.length, 1); }); +test("addPara - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.addPara.mutate({ + para_id: "para_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("removePara", async (t) => { const { trpc, db, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -390,3 +501,19 @@ test("removePara", async (t) => { myParas = await trpc.case_manager.getMyParas.query(); t.is(myParas.length, 0); }); + +test("removePara - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.case_manager.removePara.mutate({ + para_id: "para_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); From b1e5c2f8fdfa83114e9ea50bdbb017a7fb559ad9 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 21:52:04 -0700 Subject: [PATCH 15/25] Only paras and up can upload files --- src/backend/routers/file.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/backend/routers/file.ts b/src/backend/routers/file.ts index 1206a89e..04d78b73 100644 --- a/src/backend/routers/file.ts +++ b/src/backend/routers/file.ts @@ -5,12 +5,12 @@ import { GetObjectCommand, } from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; -import { hasAuthenticated, router } from "../trpc"; +import { hasPara, router } from "../trpc"; import { randomUUID } from "crypto"; import { deleteFile } from "../lib/files"; export const file = router({ - getMyFiles: hasAuthenticated.query(async (req) => { + getMyFiles: hasPara.query(async (req) => { return req.ctx.db .selectFrom("file") .selectAll() @@ -18,7 +18,7 @@ export const file = router({ .execute(); }), - getPresignedUrlForFileDownload: hasAuthenticated + getPresignedUrlForFileDownload: hasPara .input( z.object({ file_id: z.string().uuid(), @@ -50,7 +50,7 @@ export const file = router({ }; }), - getPresignedUrlForFileUpload: hasAuthenticated + getPresignedUrlForFileUpload: hasPara .input( z.object({ type: z.string(), @@ -71,7 +71,7 @@ export const file = router({ return { url, key }; }), - finishFileUpload: hasAuthenticated + finishFileUpload: hasPara .input( z.object({ filename: z.string(), @@ -99,7 +99,7 @@ export const file = router({ return file; }), - deleteFile: hasAuthenticated + deleteFile: hasPara .input( z.object({ file_id: z.string().uuid(), From 46ce087820e67c961833da4f49f33d8a52ca57af Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 23:31:57 -0700 Subject: [PATCH 16/25] Only allow case managers to access iep router routes, and add two api endpoint tests --- src/backend/routers/iep.test.ts | 32 +++++++++++++++++++++++++++++ src/backend/routers/iep.ts | 36 ++++++++++++++++----------------- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/backend/routers/iep.test.ts b/src/backend/routers/iep.test.ts index f8df0797..424edf16 100644 --- a/src/backend/routers/iep.test.ts +++ b/src/backend/routers/iep.test.ts @@ -169,3 +169,35 @@ test("edit goal", async (t) => { t.is(modifiedGoal!.goal_id, goal!.goal_id); t.is(modifiedGoal?.description, "modified goal 1"); }); +test("editGoal - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.iep.editGoal.mutate({ + goal_id: "goal_id", + description: "description", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + +test("getGoal - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.iep.getGoal.query({ + goal_id: "goal_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); diff --git a/src/backend/routers/iep.ts b/src/backend/routers/iep.ts index 2f5a716a..fecb6237 100644 --- a/src/backend/routers/iep.ts +++ b/src/backend/routers/iep.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasAuthenticated, router } from "../trpc"; +import { hasCaseManager, router } from "../trpc"; import { jsonArrayFrom } from "kysely/helpers/postgres"; import { deleteFile } from "../lib/files"; import { substituteTransactionOnContext } from "../lib/utils/context"; @@ -7,7 +7,7 @@ import { TRPCError } from "@trpc/server"; // TODO: define .output() schemas for all procedures export const iep = router({ - addGoal: hasAuthenticated + addGoal: hasCaseManager .input( z.object({ iep_id: z.string(), @@ -31,7 +31,7 @@ export const iep = router({ return result; }), - editGoal: hasAuthenticated + editGoal: hasCaseManager .input( z.object({ goal_id: z.string(), @@ -70,7 +70,7 @@ export const iep = router({ return result; }), - addSubgoal: hasAuthenticated + addSubgoal: hasCaseManager .input( z.object({ // current_level not included, should be calculated as trial data is collected @@ -123,7 +123,7 @@ export const iep = router({ return result; }), - addTask: hasAuthenticated + addTask: hasCaseManager .input( z.object({ subgoal_id: z.string(), @@ -148,7 +148,7 @@ export const iep = router({ return result; }), - assignTaskToParas: hasAuthenticated + assignTaskToParas: hasCaseManager .input( z.object({ subgoal_id: z.string().uuid(), @@ -175,7 +175,7 @@ export const iep = router({ return result; }), //Temporary function to easily assign tasks to self for testing - tempAddTaskToSelf: hasAuthenticated + tempAddTaskToSelf: hasCaseManager .input( z.object({ subgoal_id: z.string(), @@ -217,7 +217,7 @@ export const iep = router({ return result; }), - addTrialData: hasAuthenticated + addTrialData: hasCaseManager .input( z.object({ task_id: z.string(), @@ -246,7 +246,7 @@ export const iep = router({ return result; }), - updateTrialData: hasAuthenticated + updateTrialData: hasCaseManager .input( z.object({ trial_data_id: z.string(), @@ -271,7 +271,7 @@ export const iep = router({ .execute(); }), - getGoals: hasAuthenticated + getGoals: hasCaseManager .input( z.object({ iep_id: z.string(), @@ -289,7 +289,7 @@ export const iep = router({ return result; }), - getGoal: hasAuthenticated + getGoal: hasCaseManager .input( z.object({ goal_id: z.string(), @@ -307,7 +307,7 @@ export const iep = router({ return result; }), - getSubgoals: hasAuthenticated + getSubgoals: hasCaseManager .input( z.object({ goal_id: z.string(), @@ -325,7 +325,7 @@ export const iep = router({ return result; }), - getSubgoal: hasAuthenticated + getSubgoal: hasCaseManager .input( z.object({ subgoal_id: z.string(), @@ -342,7 +342,7 @@ export const iep = router({ return result; }), - getSubgoalsByAssignee: hasAuthenticated + getSubgoalsByAssignee: hasCaseManager .input( z.object({ assignee_id: z.string(), @@ -361,7 +361,7 @@ export const iep = router({ return result; }), - getSubgoalAndTrialData: hasAuthenticated + getSubgoalAndTrialData: hasCaseManager .input( z.object({ task_id: z.string(), @@ -424,7 +424,7 @@ export const iep = router({ return result; }), - markAsSeen: hasAuthenticated + markAsSeen: hasCaseManager .input( z.object({ task_id: z.string(), @@ -442,7 +442,7 @@ export const iep = router({ .execute(); }), - attachFileToTrialData: hasAuthenticated + attachFileToTrialData: hasCaseManager .input( z.object({ trial_data_id: z.string(), @@ -461,7 +461,7 @@ export const iep = router({ .execute(); }), - removeFileFromTrialDataAndDelete: hasAuthenticated + removeFileFromTrialDataAndDelete: hasCaseManager .input( z.object({ trial_data_id: z.string(), From 5b382d7030ba924fa6f7211042a8e9db5584ffc7 Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 23:38:59 -0700 Subject: [PATCH 17/25] Add tests for authenticated access to para controller routes --- src/backend/routers/para.test.ts | 72 ++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/backend/routers/para.test.ts b/src/backend/routers/para.test.ts index af7db797..ac7101b7 100644 --- a/src/backend/routers/para.test.ts +++ b/src/backend/routers/para.test.ts @@ -22,6 +22,22 @@ test("getParaById", async (t) => { t.is(para?.user_id, user_id); }); +test("getParaById - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.para.getParaById.query({ + user_id: "user_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("getParaByEmail", async (t) => { const { trpc, db } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -42,6 +58,22 @@ test("getParaByEmail", async (t) => { t.is(para?.email, email); }); +test("getParaByEmail - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.para.getParaByEmail.query({ + email: "foo@bar.com", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("createPara", async (t) => { const { trpc, db, nodemailerMock } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -68,6 +100,24 @@ test("createPara", async (t) => { ); }); +test("createPara - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.para.createPara.mutate({ + first_name: "Foo", + last_name: "Bar", + email: "foo.bar@email.com", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("paras are deduped by email", async (t) => { const { trpc, db } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -200,3 +250,25 @@ test("getMyTasks", async (t) => { t.is(task[0].instructions, INSTRUCTIONS); t.is(task[0].trial_count, TRIAL_COUNT); }); + +test("getMyTasks - paras do have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + await t.notThrowsAsync(async () => { + await trpc.para.getMyTasks.query(); + }); +}); + +test("getMyTasks - regular users don't have access", async (t) => { + const { trpc } = await getTestServer(t, {}); + + const error = await t.throwsAsync(async () => { + await trpc.para.getMyTasks.query(); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); From d4f862207cbea72a9c8cfd3b00c6759003a6a3bb Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 23:47:18 -0700 Subject: [PATCH 18/25] Add some specs to student router endpoints for controlled access --- src/backend/routers/student.test.ts | 48 ++++++++++++++++++++++++++++- src/backend/routers/student.ts | 2 +- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/backend/routers/student.test.ts b/src/backend/routers/student.test.ts index 3f876619..b3e57641 100644 --- a/src/backend/routers/student.test.ts +++ b/src/backend/routers/student.test.ts @@ -3,7 +3,7 @@ import { getTestServer } from "@/backend/tests"; import { parseISO } from "date-fns"; import { UserType } from "@/types/auth"; -test("getStudentById", async (t) => { +test("getStudentById - can query by student id", async (t) => { const { trpc, db, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, }); @@ -24,6 +24,34 @@ test("getStudentById", async (t) => { t.is(student?.student_id, student_id); }); +test("getStudentById - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.student.getStudentById.query({ student_id: "student_id" }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + +test("getStudentByTaskId - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.student.getStudentByTaskId.query({ task_id: "task_id" }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + // TODO: This test looks to be testing the `UNIQUE` constraing on the schema. // Improve this test test("doNotAddDuplicateEmails", async (t) => { @@ -135,6 +163,24 @@ test("editIep", async (t) => { t.deepEqual(got[0].end_date, updated_end_date); }); +test("editIep - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.student.editIep.mutate({ + student_id: "student_id", + start_date: new Date(parseISO("2023-03-02")), + end_date: new Date(parseISO("2023-03-02")), + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("getActiveStudentIep - return only one iep object", async (t) => { const { trpc, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, diff --git a/src/backend/routers/student.ts b/src/backend/routers/student.ts index 701b42c7..066bfd16 100644 --- a/src/backend/routers/student.ts +++ b/src/backend/routers/student.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasAuthenticated, hasCaseManager, router } from "../trpc"; +import { hasCaseManager, router } from "../trpc"; // TODO: define .output() schemas for all procedures export const student = router({ From cbe4152794f33953f77afb23d16bfe9ecf7ab24a Mon Sep 17 00:00:00 2001 From: Vincent Shuali Date: Wed, 16 Oct 2024 23:51:42 -0700 Subject: [PATCH 19/25] Finish adding specs to student router --- src/backend/routers/student.test.ts | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/backend/routers/student.test.ts b/src/backend/routers/student.test.ts index b3e57641..caa13d0f 100644 --- a/src/backend/routers/student.test.ts +++ b/src/backend/routers/student.test.ts @@ -111,6 +111,40 @@ test("addIep and getIep", async (t) => { t.deepEqual(got[0].end_date, added.end_date); }); +test("addIep - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.student.addIep.mutate({ + student_id: "student_id", + start_date: new Date("2023-01-01"), + end_date: new Date("2023-01-01"), + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + +test("getIeps - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.student.getIeps.query({ + student_id: "student_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("editIep", async (t) => { const { trpc, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, @@ -205,6 +239,22 @@ test("getActiveStudentIep - return only one iep object", async (t) => { t.deepEqual(studentWithIep?.end_date, addedIep.end_date); }); +test("getActiveStudentIep - paras do not have access", async (t) => { + const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); + + const error = await t.throwsAsync(async () => { + await trpc.student.getActiveStudentIep.query({ + student_id: "student_id", + }); + }); + + t.is( + error?.message, + "UNAUTHORIZED", + "Expected an 'unauthorized' error message" + ); +}); + test("checkAddedIEPEndDates", async (t) => { const { trpc, seed } = await getTestServer(t, { authenticateAs: UserType.CaseManager, From 3e1f540bc99a55985b3b733885bf5f2fb967a1da Mon Sep 17 00:00:00 2001 From: thom Date: Thu, 17 Oct 2024 10:08:53 -0700 Subject: [PATCH 20/25] fix: remove 401 hook as it doesn't work with all routes --- src/pages/_app.tsx | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/pages/_app.tsx b/src/pages/_app.tsx index c5dadc3a..57a411e9 100644 --- a/src/pages/_app.tsx +++ b/src/pages/_app.tsx @@ -17,7 +17,6 @@ import { AdapterDateFns } from "@mui/x-date-pickers/AdapterDateFns"; import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles"; import { compassTheme as theme } from "@/theme"; import { FontProvider } from "@/components/font-provider"; -import { useRouter } from "next/router"; interface CustomPageProps { session: Session; @@ -37,7 +36,6 @@ export default function App({ Component, pageProps, }: AppProps) { - const router = useRouter(); const [errorMessage, setErrorMessage] = useState(""); const [queryClient] = useState( @@ -46,18 +44,15 @@ export default function App({ queryCache: new QueryCache({ onError: (error) => { if (error instanceof Error) { - if (error.message === "UNAUTHORIZED") { - void router.push("/sorry"); - } else { - const errorMessages: { [key: string]: string } = { - BAD_REQUEST: "400: Bad request, please try again", - NOT_FOUND: "404: Page not found", - }; + const errorMessages: { [key: string]: string } = { + BAD_REQUEST: "400: Bad request, please try again", + UNAUTHORIZED: "401: Unauthorized Error", + NOT_FOUND: "404: Page not found", + }; - const defaultMessage = "An error occurred. Please try again"; - const msg = errorMessages[error.message] || defaultMessage; - setErrorMessage(msg); - } + const defaultMessage = "An error occured. Please try again"; + const msg = errorMessages[error.message] || defaultMessage; + setErrorMessage(msg); } }, }), From 642dad718fb876f6477668a15f84d6fd139d39b0 Mon Sep 17 00:00:00 2001 From: thom Date: Fri, 18 Oct 2024 13:12:36 -0700 Subject: [PATCH 21/25] feat: link accounts to compass app when they first log in if they were pre-added by case manager or admin --- src/backend/auth/adapter.ts | 15 +++++++--- src/backend/auth/options.ts | 60 +++++++++++++++++++++++++++++-------- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/backend/auth/adapter.ts b/src/backend/auth/adapter.ts index d6b15ad9..7d659103 100644 --- a/src/backend/auth/adapter.ts +++ b/src/backend/auth/adapter.ts @@ -1,4 +1,4 @@ -import { Adapter, AdapterSession } from "next-auth/adapters"; +import { Adapter, AdapterSession, AdapterAccount } from "next-auth/adapters"; import { KyselyDatabaseInstance, KyselySchema, @@ -7,6 +7,12 @@ import { import { InsertObject, Selectable } from "kysely"; import { CustomAdapterUser, UserType } from "@/types/auth"; +// Extend the Adapter interface to include the implemented methods +export interface ExtendedAdapter extends Adapter { + getUserByEmail: (email: string) => Promise; + linkAccount: (account: AdapterAccount) => Promise; +} + const mapStoredUserToAdapterUser = ( user: Selectable> ): CustomAdapterUser => ({ @@ -35,17 +41,18 @@ const mapStoredSessionToAdapterSession = ( */ export const createPersistedAuthAdapter = ( db: KyselyDatabaseInstance -): Adapter => ({ +): ExtendedAdapter => ({ async createUser(user) { const numOfUsers = await db .selectFrom("user") .select((qb) => qb.fn.count("user_id").as("count")) .executeTakeFirstOrThrow(); - // First created user is an admin + // First created user is an admin, else make them a user. This is to ensure there is always an admin user, but also to ensure we don't grant + // para or case_manager to folks not pre-added to the system. // todo: this should be pulled from an invite or something else instead of defaulting to a para - currently devs signing in are being assigned as paras const role = - Number(numOfUsers.count) === 0 ? UserType.Admin : UserType.Para; + Number(numOfUsers.count) === 0 ? UserType.Admin : UserType.User; const [first_name, last_name] = user.name?.split(" ") ?? [ user.email?.split("@")[0], diff --git a/src/backend/auth/options.ts b/src/backend/auth/options.ts index a08fa6b5..08f57ca5 100644 --- a/src/backend/auth/options.ts +++ b/src/backend/auth/options.ts @@ -2,18 +2,54 @@ import GoogleProvider from "next-auth/providers/google"; import { createPersistedAuthAdapter } from "@/backend/auth/adapter"; import { KyselyDatabaseInstance } from "../lib"; import type { NextAuthOptions } from "next-auth"; +import type { ExtendedAdapter } from "@/backend/auth/adapter"; export const getNextAuthOptions = ( db: KyselyDatabaseInstance -): NextAuthOptions => ({ - providers: [ - GoogleProvider({ - clientId: process.env.GOOGLE_CLIENT_ID as string, - clientSecret: process.env.GOOGLE_CLIENT_SECRET as string, - }), - ], - adapter: createPersistedAuthAdapter(db), - pages: { - signIn: "/signInPage", - }, -}); +): NextAuthOptions => { + const adapter: ExtendedAdapter = createPersistedAuthAdapter(db); + + return { + providers: [ + GoogleProvider({ + clientId: process.env.GOOGLE_CLIENT_ID as string, + clientSecret: process.env.GOOGLE_CLIENT_SECRET as string, + }), + ], + adapter, + pages: { + signIn: "/signInPage", + }, + callbacks: { + // hook into the sign in process so we can link accounts if needed + async signIn({ user, account }) { + if (account?.provider === "google") { + const existingUser = await adapter.getUserByEmail(user.email!); + + if (existingUser) { + // user exists, check if account is linked + const linkedAccount = await db + .selectFrom("account") + .where("user_id", "=", existingUser.id) + .where("provider_name", "=", account.provider) + .where("provider_account_id", "=", account.providerAccountId) + .selectAll() + .executeTakeFirst(); + + if (!linkedAccount) { + // user was added by case manager or admin but hasn't logged in before so + // we need to link the user's google account + if (adapter.linkAccount) { + await adapter.linkAccount({ + ...account, + userId: existingUser.id, + }); + } + } + } + } + return true; + }, + }, + }; +}; From 6e1731eec9c5feecb2b3035dc16486cd5a063bdb Mon Sep 17 00:00:00 2001 From: thom Date: Fri, 18 Oct 2024 14:14:36 -0700 Subject: [PATCH 22/25] fix: some auth guards were wrong, removed a test. --- src/backend/routers/iep.ts | 10 +++++----- src/backend/routers/student.test.ts | 14 -------------- src/backend/routers/student.ts | 4 ++-- 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/backend/routers/iep.ts b/src/backend/routers/iep.ts index fecb6237..62e620ea 100644 --- a/src/backend/routers/iep.ts +++ b/src/backend/routers/iep.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasCaseManager, router } from "../trpc"; +import { hasCaseManager, hasPara, router } from "../trpc"; import { jsonArrayFrom } from "kysely/helpers/postgres"; import { deleteFile } from "../lib/files"; import { substituteTransactionOnContext } from "../lib/utils/context"; @@ -217,7 +217,7 @@ export const iep = router({ return result; }), - addTrialData: hasCaseManager + addTrialData: hasPara .input( z.object({ task_id: z.string(), @@ -246,7 +246,7 @@ export const iep = router({ return result; }), - updateTrialData: hasCaseManager + updateTrialData: hasPara .input( z.object({ trial_data_id: z.string(), @@ -361,7 +361,7 @@ export const iep = router({ return result; }), - getSubgoalAndTrialData: hasCaseManager + getSubgoalAndTrialData: hasPara .input( z.object({ task_id: z.string(), @@ -424,7 +424,7 @@ export const iep = router({ return result; }), - markAsSeen: hasCaseManager + markAsSeen: hasPara .input( z.object({ task_id: z.string(), diff --git a/src/backend/routers/student.test.ts b/src/backend/routers/student.test.ts index caa13d0f..2c70127a 100644 --- a/src/backend/routers/student.test.ts +++ b/src/backend/routers/student.test.ts @@ -38,20 +38,6 @@ test("getStudentById - paras do not have access", async (t) => { ); }); -test("getStudentByTaskId - paras do not have access", async (t) => { - const { trpc } = await getTestServer(t, { authenticateAs: UserType.Para }); - - const error = await t.throwsAsync(async () => { - await trpc.student.getStudentByTaskId.query({ task_id: "task_id" }); - }); - - t.is( - error?.message, - "UNAUTHORIZED", - "Expected an 'unauthorized' error message" - ); -}); - // TODO: This test looks to be testing the `UNIQUE` constraing on the schema. // Improve this test test("doNotAddDuplicateEmails", async (t) => { diff --git a/src/backend/routers/student.ts b/src/backend/routers/student.ts index 066bfd16..e21dfa09 100644 --- a/src/backend/routers/student.ts +++ b/src/backend/routers/student.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { hasCaseManager, router } from "../trpc"; +import { hasCaseManager, hasPara, router } from "../trpc"; // TODO: define .output() schemas for all procedures export const student = router({ @@ -17,7 +17,7 @@ export const student = router({ return result; }), - getStudentByTaskId: hasCaseManager + getStudentByTaskId: hasPara .input(z.object({ task_id: z.string().uuid() })) .query(async (req) => { const { task_id } = req.input; From 8ec54a075ad5f4ee94f464c3d6ffd900ae6411b3 Mon Sep 17 00:00:00 2001 From: canjalal Date: Sun, 27 Oct 2024 17:03:59 -0700 Subject: [PATCH 23/25] Tweak misleading test --- src/backend/routers/case_manager.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/routers/case_manager.test.ts b/src/backend/routers/case_manager.test.ts index cf0b04e1..38efbad4 100644 --- a/src/backend/routers/case_manager.test.ts +++ b/src/backend/routers/case_manager.test.ts @@ -313,7 +313,7 @@ test("addStudent - paras do not have access", async (t) => { await trpc.case_manager.addStudent.mutate({ first_name: "Foo", last_name: "Bar", - email: "invalid-email", + email: "foo@bar.com", grade: 6, }); }); From df71b5bef4ddb60d1a40265682a70322010e7ff3 Mon Sep 17 00:00:00 2001 From: thom Date: Thu, 31 Oct 2024 21:58:25 -0700 Subject: [PATCH 24/25] fix: migration collision --- src/backend/db/migrations/{2_rbac_roles.sql => 3_rbac_roles.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/backend/db/migrations/{2_rbac_roles.sql => 3_rbac_roles.sql} (100%) diff --git a/src/backend/db/migrations/2_rbac_roles.sql b/src/backend/db/migrations/3_rbac_roles.sql similarity index 100% rename from src/backend/db/migrations/2_rbac_roles.sql rename to src/backend/db/migrations/3_rbac_roles.sql From 99fa3089db94cca98b146331b3ebd842ea5d765c Mon Sep 17 00:00:00 2001 From: thom Date: Thu, 31 Oct 2024 22:02:48 -0700 Subject: [PATCH 25/25] fix: type check --- src/backend/routers/iep.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/routers/iep.test.ts b/src/backend/routers/iep.test.ts index ca306a3c..48589dcd 100644 --- a/src/backend/routers/iep.test.ts +++ b/src/backend/routers/iep.test.ts @@ -106,7 +106,7 @@ test("basic flow - add/get goals, subgoals, tasks", async (t) => { test("addTask - no duplicate subgoal_id + assigned_id combo", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const para_id = seed.para.user_id; @@ -162,7 +162,7 @@ test("addTask - no duplicate subgoal_id + assigned_id combo", async (t) => { test("assignTaskToParas - no duplicate subgoal_id + para_id combo", async (t) => { const { trpc, seed } = await getTestServer(t, { - authenticateAs: "case_manager", + authenticateAs: UserType.CaseManager, }); const para_1 = seed.para;