Builds out roles + role-specific landing UX + tighten up auth guards #455

thomhickey · 2024-10-18T02:21:37Z

migration changes users.role to four roles: admin, case_manager, para, user
user.role added to session context (this is done via db lookup on each request - can be optimized to instead store user.role in session at login that's a @todo)
new UserType enum for role string constants, and applied everywhere instead of using string literals
trpc.ts exports four procedures for trpc use and implements cascading access control user -> para -> case_manager -> admin. so para can do everything user can do, case_manager can do everything para and user can do, etc.
all endpoints tightened up to use the appropriate trpc procedure according to role base access control
oauth redirect route is now / instead of /students, with each role being forwarded to appropriate landing page including a new 'sorry' page that says you cannot access compass
tests updated

to test:

change your role in db
log out/log in
--> user goes to /sorry
--> para goes to /benchmarks and has limited nav menus
--> case_manager goes to /students and has appropriate nav menus
--> admin goes to /students but has all nav

…r or admin

…ling of 401

…ints

… endpoint tests

…e pre-added by case manager or admin

nickvisut · 2024-10-22T23:46:06Z

src/backend/trpc.ts

+};
+
+// Function to compare roles
+function hasMinimumRole(auth: Auth, requiredRole: UserType): boolean {


this function assumes that each role is a superset of the prior one; does this mean a case manager should be able to do what a para does?

Yes. Uncommon work flows could be removed from the UI, but I'm thinking in the future there could be an administrator panel that could change database records through trpc calls

Yes, @nickvisut for now cascading roles on the backend seems like a good fit for POC/MVP. And as mentioned by Vince, the front-end doesn't do much in the way of auth guards, it just hides ui, but entering a route by hand will render the page, even if the api won't fill the page with data. We can add stories for those use cases as well.

nickvisut · 2024-10-22T23:48:59Z

src/types/auth.ts

+  };
+}
+
+export enum UserType {


Honestly I'm wondering if it would be possible to just use the default TypeScript enum where UserType.User = 0, UserType.Para = 1, and then use it as our ROLE_LEVEL, and instead have some fixed function called getUserTypeString(userType: UserType) that returns "user", "para", "case_manager", "admin", and is used only for writing to the database, which stores these strings rather than the enum.

nickvisut · 2024-10-23T00:25:05Z

added Compass Engineering Reviewers as test of the new team

canjalal · 2024-10-27T23:32:57Z

src/backend/routers/case_manager.test.ts

+    await trpc.case_manager.addStudent.mutate({
+      first_name: "Foo",
+      last_name: "Bar",
+      email: "invalid-email",


Nitpick: this shouldn't be an invalid email because that's not what we're testing. I'm going to fix this

canjalal · 2024-10-27T23:37:27Z

src/backend/tests/fixtures/get-test-server.ts


 export interface GetTestServerOptions {
-  authenticateAs?: "case_manager" | "para" | "admin";
+  authenticateAs?: UserType.CaseManager | UserType.Para | UserType.Admin;


Do we want to just set the type to UserType, and make it default to UserType.User if it's not specified?

^^ Just tried this, there's not much benefit because the seed file doesn't have a "UserType.User" key so it becomes more trouble than it's worth

canjalal · 2024-10-27T23:47:11Z

src/pages/sorry.tsx

+import $box from "@/styles/Box.module.css";
+import $typo from "@/styles/Typography.module.css";
+
+const SorryPage: React.FC = () => {


If we aren't going to use this element in this PR, let's remove it and add it to a new branch split from this one.

canjalal · 2024-10-27T23:51:03Z

src/backend/routers/para.test.ts

+});
+
+test("getMyTasks - regular users don't have access", async (t) => {
+  const { trpc } = await getTestServer(t, {});


~~See my comment on making authenticateAs default to UserType.User if not specified, and have authenticateAs just have type UserType~~

thomhickey and others added 20 commits September 30, 2024 11:41

feat: adding role to session, will be needed for lots of features

08eae0c

fix: refactor types into src/types/auth.ts

69acc50

fix: move type to auth.ts types

c4607be

fix: refactor trpc auth guard name + add two more

be24b11

feat: migration to build out roles in db + one rename of auth guard

473ba16

Add UserType enum and refactor hasMinimumRole

46e7330

Do not display staff link on frontend unless user role is case manage…

9c4040b

…r or admin

Fix typescript types

c76a507

Move UserType enum to auth types, only let Paras see Assigned link

75ee0f4

feat: navitems are role-based, sorry page added, front-end error hand…

774ab08

…ling of 401

feat: auth guards + type fix in trpc.ts

11dc2fb

feat: all roles land on correct page + paras can see their tasks

5258bc4

Specify UNAUTHORIZED as error

013b29e

Add specs to test authentication of each of case_maanger router endpo…

2c020f8

…ints

Only paras and up can upload files

b1e5c2f

Only allow case managers to access iep router routes, and add two api…

46ce087

… endpoint tests

Add tests for authenticated access to para controller routes

5b382d7

Add some specs to student router endpoints for controlled access

d4f8622

Finish adding specs to student router

cbe4152

fix: remove 401 hook as it doesn't work with all routes

3e1f540

thomhickey linked an issue Oct 18, 2024 that may be closed by this pull request

Working roles/UI #452

Closed

thomhickey requested review from mrabbitt, nickvisut, KCCPMG and canjalal October 18, 2024 02:22

thomhickey added 3 commits October 18, 2024 08:12

taking main

38eb042

feat: link accounts to compass app when they first log in if they wer…

642dad7

…e pre-added by case manager or admin

fix: some auth guards were wrong, removed a test.

6e1731e

nickvisut reviewed Oct 22, 2024

View reviewed changes

nickvisut requested a review from a team October 23, 2024 00:24

canjalal reviewed Oct 27, 2024

View reviewed changes

Tweak misleading test

8ec54a0

francisli approved these changes Oct 31, 2024

View reviewed changes

thomhickey added 3 commits October 31, 2024 21:37

taking main

3b95394

fix: migration collision

df71b5b

fix: type check

99fa308

thomhickey merged commit 3c95751 into main Nov 1, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Builds out roles + role-specific landing UX + tighten up auth guards #455

Builds out roles + role-specific landing UX + tighten up auth guards #455

thomhickey commented Oct 18, 2024

nickvisut Oct 22, 2024

canjalal Oct 24, 2024

thomhickey Oct 25, 2024

nickvisut Oct 22, 2024

canjalal Oct 27, 2024

nickvisut commented Oct 23, 2024

canjalal Oct 27, 2024

canjalal Oct 27, 2024

canjalal Oct 28, 2024

canjalal Oct 27, 2024

canjalal Oct 27, 2024 •

edited

Loading

Builds out roles + role-specific landing UX + tighten up auth guards #455

Builds out roles + role-specific landing UX + tighten up auth guards #455

Conversation

thomhickey commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickvisut commented Oct 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

canjalal Oct 27, 2024 • edited Loading

Choose a reason for hiding this comment

canjalal Oct 27, 2024 •

edited

Loading