Skip to content

fix: course editor gets 403 copying a unit to the clipboard #39075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mariajgrimaldi merged 2 commits into from
Sep 10, 2026
+50 −2
Merged

fix: course editor gets 403 copying a unit to the clipboard #39075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
daee143
fix: Course Editor gets 403 copying a unit to the clipboard
efortish Sep 3, 2026
319f4b8
test: wrap the 403 assertion in allow_transaction_exception
efortish Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions openedx/core/djangoapps/content_staging/tests/test_clipboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
from typing import cast
from xml.etree import ElementTree

import ddt
from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF
from rest_framework.test import APIClient

from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
from openedx.core.djangoapps.content_staging import api as python_api
from xmodule.contentstore.django import contentstore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course
Expand Down Expand Up @@ -378,3 +382,40 @@ def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> None:
a = ElementTree.canonicalize(xml_str_a, strip_text=True)
b = ElementTree.canonicalize(xml_str_b, strip_text=True)
assert a == b


@ddt.ddt
class ClipboardAuthzTest(CourseAuthoringAuthzTestMixin, ModuleStoreTestCase):
"""
Regression test for openedx-authz#403: ClipboardEndpoint.post() required legacy read
access via has_studio_read_access(), so AuthZ-native roles with no legacy equivalent
(course_auditor, course_editor) got a 403 when copying a unit to the clipboard despite
holding COURSES_VIEW_COURSE.
"""

@ddt.data(
COURSE_STAFF.external_key,
COURSE_ADMIN.external_key,
COURSE_AUDITOR.external_key,
COURSE_EDITOR.external_key,
)
def test_course_roles_can_copy_unit_to_clipboard(self, role_key):
course_key = ToyCourseFactory.create().id
html_key = course_key.make_usage_key("html", "toyhtml")

role_user = UserFactory(password=self.password)
self.add_user_to_role_in_course(role_user, role_key, course_key)

client = APIClient()
client.force_authenticate(user=role_user)
response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(html_key)}, format="json")

assert response.status_code == 200

def test_unauthorized_user_gets_permission_denied(self):
course_key = ToyCourseFactory.create().id
html_key = course_key.make_usage_key("html", "toyhtml")

with self.allow_transaction_exception():
response = self.unauthorized_client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(html_key)}, format="json")
assert response.status_code == 403
11 changes: 9 additions & 2 deletions openedx/core/djangoapps/content_staging/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2
from openedx_authz.constants import permissions as authz_permissions
from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError
from rest_framework.response import Response
from rest_framework.views import APIView

from common.djangoapps.student.auth import has_studio_read_access
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
from openedx.core.djangoapps.xblock import api as xblock_api
from openedx.core.lib.api.view_utils import view_auth_classes
from xmodule.modulestore.django import modulestore
Expand Down Expand Up @@ -102,7 +104,12 @@ def post(self, request):
try:
if isinstance(course_key, CourseLocator):
# Make sure the user has permission on that course
if not has_studio_read_access(request.user, course_key):
if not user_has_course_permission(
request.user,
COURSES_VIEW_COURSE.identifier,

@BryanttV BryanttV Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure if COURSES_VIEW_COURSE is the right permission to validate here, since the Course Auditor could also use the endpoint, and in theory, that role should only have view permissions. Maybe we should check for COURSES_EDIT_COURSE instead?

I'd like to get Product's perspective on this @gviedma-aulasneo

@gviedma-aulasneo gviedma-aulasneo Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BryanttV @rodmgwgu
Once a user is able to view the course content, the “copy” action is really just a faster way to replicate that content. I don’t see much harm in allowing users with view access to copy it as well.

That said, it is a bit “dirty” to tie the action to a permission that technically represents something else. It sounds like a fairly niche permission to split out and validate separately, though.

So let’s proceed with the view permission for now, since it is the most basic permission across the authoring roles.

@BryanttV BryanttV Sep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gviedma-aulasneo.

To clarify, this is only the copy endpoint. There's also an endpoint to paste the content into the course, which is currently restricted for the Course Auditor because they only have view permissions.

Similarly, on the frontend, all buttons and menus are currently being hidden so that the Course Auditor cannot perform any paste actions.

Considering this, I believe the correct permission to validate should be COURSES_EDIT_COURSE, rather than the view permission. Even though the pasted content is based on existing content, it still involves adding another unit/component, etc., to the course. The way I see it, the Course Auditor shouldn't be able to do that.

What do you think?

cc @rodmgwgu

@BryanttV BryanttV Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a conversation with @gviedma-aulasneo, we decided that the permission to validate will be courses.view_course (as it is now).

A proposed use case is when a user is a Course Auditor for Course A and a Course Editor for Course B. The system should allow copying the content from Course A and then pasting it into Course B. Therefore, the view permission validation will be maintained. Nothing will change on the Frontend (the option will not be enabled), but it will be available in case it needs to be implemented in the future.

course_key,
LegacyAuthoringPermission.READ,
):
raise PermissionDenied(
"You must be a member of the course team in Studio to export OLX using this API."
)
Expand Down
Loading