freeCodeCamp / classroom

BSD 3-Clause "New" or "Revised" License
144 stars 120 forks source link

Adding support for multiple teachers in a classroom & setting up RBAC for individual classrooms #310

Open GuillermoFloresV opened 1 year ago

GuillermoFloresV commented 1 year ago

While reviewing this PR (https://github.com/freeCodeCamp/classroom/pull/252), we noticed some significant areas for improvement. However, the PR is so old, that we figured it makes more sense to log the changes as a new issue.

Work on this was started here: https://github.com/freeCodeCamp/classroom/pull/252

The contributors ended up doing some migrations that looked awkward/hacky. After changing the teacherId to an array, the UserID foreign key constraint broke. They tried to fix the issue by making the foreign key constraint optional. A better solution is to create a new role (classroomOwner). The previous UserID foreign key should refer to the classroomOwner's Id.

Eventually, we also want to be able to use this foundation to set up RBAC inside a classroom. (Hence the need for a classroomOwner role)

theGaryLarson commented 1 year ago

Hi @GuillermoFloresV I was assigned issue 166. But noticed this other thread where it had been closed and incorporated here. I'm glad I found this because it addresses the questions raised regarding permissions given to a teacher and the research I've been doing.

I have a few questions I need answered for clarification:

  1. First off the issue I was assigned is related to this issue, correct? Before I started making changes I want to ensure I understood the scope of the problem and what kind of problems changing the model would create. I understand I will have to change and test the other areas of code and will use the references link in my originally assigned issue.

Regarding Permissions

  1. When creating a class this is when the teacher would be assigned ownership of the class, correct? If so, I would assign the classroom owner role through a newly created api route that updates the field role with classroomOwner using Prisma. Am I correct in assuming that with the current RBAC structure a user is only allowed one role as well? Would adjusting so an user can have more than one role be within scope of this issue or should it be broken up iteratively? I foresee problems where a refactor would need to be done to ensure teachers that are owners can see the classrooms page and join other classes as well. image

  2. Should only the owning teacher be allowed to EDIT and DELETE the class? If so I'd like to hide these options from non-owning teachers. image

  3. Finally, is it ok if I handle the role checks within the component or will I need to modify the API routes as well?

I am looking into each of these more on my own as well but some feedback and guidance on what has been established would be great.

Outline of My Steps

Steps: Data Model Changes: Modify the Classroom model in the Prisma schema to replace the classroomTeacherId field with a classroomTeacherIds field that can hold multiple user IDs. This allows us to associate multiple users (teachers) with each classroom.

Define Roles and Permissions: Define the various roles (like "Admin", "Teacher", "Student") and the permissions associated with each role. This could be done in a separate configuration file or directly in the code.

Update User Model: Expand the role field in the User model to support more roles, or introduce a new Role model that can hold more detailed role information and permissions.

Implement Role Checking: Update the application logic to check the role of the user and enforce the appropriate permissions. This could involve adding checks in various parts of your code to see if the current user has the necessary role to perform a certain action.

Update Server-Side Logic: In addition to checking the user's role on the client side, also check it on the server side when handling the API request to delete a classroom. This provides an extra layer of security.

Testing: Create tests for various scenarios and edge cases to ensure that permissions are enforced correctly and that classrooms can only be deleted by users with the appropriate role.

Iterate and Refine: As you test your RBAC system, you may find areas that need refinement or additional features that could be added. Plan for multiple iterations of development and testing.

Associated Files: Prisma Schema (prisma/schema.prisma): Update the Classroom model to replace the classroomTeacherId field with a classroomTeacherIds field that can hold multiple user IDs. You might also need to add a Role model to store role information and permissions.

API Routes: Update the server-side logic in your API routes to check the role of the user and enforce the appropriate permissions. This would likely involve changes to several files in the pages/api directory, such as create_class_teacher.js, delete_class.js, and others.

Page Components: Update the client-side logic in your page components to check the role of the user before performing certain actions. This could involve changes to several files in the components directory, such as adminTable.js, dashtable.js, and others.

Authentication Logic (pages/api/auth/[...nextauth].js): Update the authentication logic to include the user's role in the session information. This would allow you to access the user's role in your page components and API routes.

Tests: Need to update them to account for the new RBAC logic. This could involve changes to several files in a tests directory.

lloydchang commented 1 year ago

@theGaryLarson @GuillermoFloresV

FWIW, in case you want something else to compare and contrast with, this problem space sounds similar to https://docs.github.com/en/education/manage-coursework-with-github-classroom/teach-with-github-classroom/manage-classrooms#about-management-of-classrooms

About management of classrooms

GitHub Classroom uses organization accounts on GitHub to manage permissions, administration, and security for each classroom that you create. Each organization can have multiple classrooms.

After you create a classroom, GitHub Classroom will prompt you to invite teaching assistants (TAs) and admins to the classroom. Each classroom can have one or more admins. Admins can be teachers, TAs, or any other course administrator who you'd like to have control over your classrooms on GitHub Classroom.

Invite TAs and admins to your classroom by inviting the personal accounts on GitHub to your organization as organization owners and sharing the URL for your classroom. Organization owners can administer any classroom for the organization. For more information, see "Roles in an organization" and "Inviting users to join your organization."

When you're done using a classroom, you can archive the classroom and refer to the classroom, roster, and assignments later, or you can delete the classroom if you no longer need the classroom.

You can reuse existing assignments in any other classroom you have admin access to, including classrooms in a different organization. For more information, see "Reuse an assignment."

You can also view your classrooms and assignments directly from the GitHub command line interface with the GitHub Classroom extension. For more information, see "Using GitHub Classroom with GitHub CLI."

lloydchang commented 1 year ago

@theGaryLarson @GuillermoFloresV

FWIW, as an additional reference guide:

A 2020 archive of https://classroom.github.com/ source code is at https://github.com/education/classroom

While https://github.com/education/classroom is written in Ruby on Rails, I believe Ruby computer programming language and Rails web framework are acceptable as reading material — if you want to dig into their source code to understand better how GitHub implemented classroom back in 2020.