fix: limited staff cohorts and gradebook access [BB-7962]

[0x29a]

Description

http://<LMS>/courses/<COURSE_KEY>/cohorts/ responds with 404 status code for users with Limited Staff role. This breaks "Cohorts" instructor dashboard tab, it's just empty.

It happens due to this check:
https://github.com/openedx/edx-platform/blob/bef05ab6649fe9dd7e04f05c445b5cc006c4cd97/openedx/core/djangoapps/course_groups/views.py#L175-L176
https://github.com/openedx/edx-platform/blob/bef05ab6649fe9dd7e04f05c445b5cc006c4cd97/common/djangoapps/student/auth.py#L125-L129
https://github.com/openedx/edx-platform/blob/bef05ab6649fe9dd7e04f05c445b5cc006c4cd97/common/djangoapps/student/auth.py#L109-L122

So currently this endpoint requires Studio write permissions for both GET and POST requests. Unfortunately, the story is the same for many other endpoints that are necessary for LMS, at least: SubsectionGradeView, CourseGradingView, GradebookView, GradebookBulkUpdateView.

A proper solution would be to leave Limited Staff without any Studio access and expand this list with new flags:
https://github.com/openedx/edx-platform/blob/bef05ab6649fe9dd7e04f05c445b5cc006c4cd97/common/djangoapps/student/auth.py#L29-L35

However, in this case we'd have to add more decorators like course_author_access_required and / or do some refactoring. I don't know whether it's feasible giving the upcoming RBAC.

Testing instructions

1. Create a user with "Limited Staff" course team access role.
2. Open the "Cohorts" instructor dashboard tab. You should see a checkbox there.

[openedx-webhooks]

Thanks for the pull request, @0x29a! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[Agrendalath]

@0x29a, given the upcoming RBAC changes, I believe it's a reasonable tradeoff to make this work. Just please annotate this as a # HACK and move the detailed explanation to the actual code instead of keeping it in the test - it's more discoverable there.

[0x29a]

Thanks for the fast review, @Agrendalath! Updated.

[Agrendalath]

👍

• I tested this: checked that the cohorts tab and gradebook work correctly
• I read through the code
• I checked for accessibility issues: n/a
• Includes documentation: n/a
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

[rgraber]

Can we add something in the comment linking to an epic or ticket or something that will enable the hack to be removed? And ideally a target removal date?
It seems scary to me to grant these permissions but I'll defer to those who know the context better

[Agrendalath]

@rgraber, we added this role (along with a simple implementation of the role inheritance) in #32570, as there was no way of restricting staff member access to Studio. From this document it looks like that the LMS and Studio permissions will be separated in openedx/platform-roadmap#246, so we should be able to remove this then (or it will be reworked in the meantime, depending on the exact scope of the project).

[rgraber]

@rgraber, we added this role (along with a simple implementation of the role inheritance) in #32570, as there was no way of restricting staff member access to Studio. From this document it looks like that the LMS and Studio permissions will be separated in openedx/platform-roadmap#246, so we should be able to remove this then (or it will be reworked in the meantime, depending on the exact scope of the project).

Can we put that in the code comment? Not a blocker, just want to reduce the risk of this malingering

[Agrendalath]

@rgraber, sure thing. I added this context to the code comment.

[openedx-webhooks]

@0x29a 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.