You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
That is the entire model. It is stored per course/TA, managed at PUT /:courseId/ta-permissions/:taId, read at GET /:courseId/ta-permissions/:taId and GET /:courseId/ta-permissions, and consumed by five frontend scripts:
canAccessCourses is doing far too much work — it currently gates everything course-related regardless of how sensitive that thing is.
⚠️ Behavior change already noted
GET /publish-status and GET /pass-threshold are now staff-only, and TAs need the courses permission. Both are only called from instructor pages today, so the blast radius is small — but this is exactly the coupling this issue should fix: an endpoint's access should not be an accident of which broad boolean happens to cover it.
Scope
Define the permission set. Candidates, based on what TAs actually touch: view/edit course materials, manage units, view flags, respond to flags, view student list, view student transcripts, run AI generation, manage questions, edit settings. Keep it small — a permission nobody sets is worse than none.
Decide roles vs. flags. The comment asks for "specific roles". Named role presets (e.g. Grader, Content TA, Full TA) that expand to a permission set are friendlier than a wall of checkboxes, as long as the underlying storage stays flag-based so a role can be customized.
Migrate existing data. Every current TA has the two booleans; they need a defined mapping into the new set so nobody silently gains or loses access on deploy.
Enforce server-side, once. Today each route hand-rolls its check. A single middleware taking a required permission is the point of this work — otherwise the new granularity just multiplies the places to get it wrong.
Update the TA hub UI (public/instructor/scripts/ta-hub.js) to manage the new model, and the TA-side pages to hide what the TA cannot do.
Audit every existing TA-reachable route and assign it a permission explicitly, including the two named above.
Acceptance criteria
A TA with no permissions can reach no course data.
Each permission gates exactly the routes documented for it, verified by tests hitting the API directly rather than through the UI.
Existing TAs' effective access after migration matches their access before it.
The TA-side UI does not offer actions the server would reject.
Permission changes take effect without the TA re-logging-in, or the requirement to re-login is documented.
Open questions
Roles, raw flags, or roles-with-overrides?
Should a TA ever see student transcripts, or is that instructor-only regardless of permission? Worth deciding deliberately given Deletion of Student data after __ years #500 and the mental-health flag data.
What exists today
TA permissions are two booleans, nothing more. From
src/routes/courses.js:3357:That is the entire model. It is stored per course/TA, managed at
PUT /:courseId/ta-permissions/:taId, read atGET /:courseId/ta-permissions/:taIdandGET /:courseId/ta-permissions, and consumed by five frontend scripts:public/instructor/scripts/ta-hub.js:246,:440public/instructor/scripts/instructor-ta.js:163public/instructor/scripts/flagged.js:446,:1591public/ta/scripts/ta-settings.js:138public/ta/scripts/ta-home.js:143canAccessCoursesis doing far too much work — it currently gates everything course-related regardless of how sensitive that thing is.GET /publish-statusandGET /pass-thresholdare now staff-only, and TAs need thecoursespermission. Both are only called from instructor pages today, so the blast radius is small — but this is exactly the coupling this issue should fix: an endpoint's access should not be an accident of which broad boolean happens to cover it.Scope
public/instructor/scripts/ta-hub.js) to manage the new model, and the TA-side pages to hide what the TA cannot do.Acceptance criteria
Open questions