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:

edx-platform/openedx/core/djangoapps/course_groups/views.py

Lines 175 to 176 in bef05ab

	if not has_course_author_access(request.user, course_key):
	raise Http404('The requesting user does not have course author permissions.')

edx-platform/common/djangoapps/student/auth.py

Lines 125 to 129 in bef05ab

	def has_course_author_access(user, course_key):
	"""
	Old name for has_studio_write_access
	"""
	return has_studio_write_access(user, course_key)

edx-platform/common/djangoapps/student/auth.py

Lines 109 to 122 in bef05ab

	def has_studio_write_access(user, course_key):
	"""
	Return True if user has studio write access to the given course.
	Note that the CMS permissions model is with respect to courses.
	There is a super-admin permissions if user.is_staff is set.
	Also, since we're unifying the user database between LMS and CAS,
	I'm presuming that the course instructor (formally known as admin)
	will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our
	queries here as INSTRUCTOR has all the rights that STAFF do.
	:param user:
	:param course_key: a CourseKey
	"""
	return bool(STUDIO_EDIT_CONTENT & get_user_permissions(user, course_key))

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:

edx-platform/common/djangoapps/student/auth.py

Lines 29 to 35 in bef05ab

	# Studio permissions:
	STUDIO_EDIT_ROLES = 8
	STUDIO_VIEW_USERS = 4
	STUDIO_EDIT_CONTENT = 2
	STUDIO_VIEW_CONTENT = 1
	STUDIO_NO_PERMISSIONS = 0
	# In addition to the above, one is always allowed to "demote" oneself to a lower role within a course, or remove oneself

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.