-
Notifications
You must be signed in to change notification settings - Fork 0
Crls/deny publish action for staff #69
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.
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
base: open-release/teak.nelp
Are you sure you want to change the base?
Changes from all commits
1c114c2
b219dd1
379238b
9a5352e
ef8b9cf
5281d21
e479076
a43e9a3
96a324e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| """ | ||
| API Serializer for the current user's course role. | ||
| """ | ||
|
|
||
| from rest_framework import serializers | ||
|
|
||
|
|
||
| class CourseUserRoleSerializer(serializers.Serializer): | ||
| """ | ||
| Serializer for the current user's role in a given course. | ||
| """ | ||
| role = serializers.CharField(allow_null=True) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| from .course_rerun import CourseRerunView | ||
| from .course_waffle_flags import CourseWaffleFlagsView | ||
| from .course_team import CourseTeamView | ||
| from .course_user_role import CourseUserRoleView | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you use it?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's used. Registered in urls.py at the route course_user_role/{COURSE_ID_PATTERN}, making it an active API endpoint accessible at /api/contentstore/v1/course_user_role/{course_id} |
||
| from .grading import CourseGradingView | ||
| from .group_configurations import CourseGroupConfigurationsView | ||
| from .help_urls import HelpUrlsView | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| """ API View for current user's course role """ | ||
|
|
||
| import edx_api_doc_tools as apidocs | ||
| from opaque_keys.edx.keys import CourseKey | ||
| from rest_framework.request import Request | ||
| from rest_framework.response import Response | ||
| from rest_framework.views import APIView | ||
|
|
||
| from common.djangoapps.student.auth import has_studio_read_access | ||
| from common.djangoapps.student.roles import get_user_course_role | ||
| from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes | ||
|
|
||
| from ..serializers import CourseUserRoleSerializer | ||
|
|
||
|
|
||
| @view_auth_classes(is_authenticated=True) | ||
| class CourseUserRoleView(DeveloperErrorViewMixin, APIView): | ||
| """ | ||
| View for getting the authenticated user's role for a course. | ||
| """ | ||
|
|
||
| @apidocs.schema( | ||
| parameters=[ | ||
| apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), | ||
| ], | ||
| responses={ | ||
| 200: CourseUserRoleSerializer, | ||
| 401: "The requester is not authenticated.", | ||
| 403: "The requester cannot access the specified course.", | ||
| 404: "The requested course does not exist.", | ||
| }, | ||
| ) | ||
| @verify_course_exists() | ||
| def get(self, request: Request, course_id: str): | ||
| """ | ||
| Get the authenticated user's role for the specified course. | ||
|
|
||
| **Example Request** | ||
|
|
||
| GET /api/contentstore/v1/course_user_role/{course_id} | ||
|
|
||
| **Example Response** | ||
|
|
||
| ```json | ||
| { "role": "instructor" } | ||
| ``` | ||
| """ | ||
| user = request.user | ||
| course_key = CourseKey.from_string(course_id) | ||
|
|
||
| if not has_studio_read_access(user, course_key): | ||
| self.permission_denied(request) | ||
|
|
||
| role = get_user_course_role(user, course_key) | ||
| print(f"role: {role}") | ||
|
|
||
| serializer = CourseUserRoleSerializer({"role": role}) | ||
| return Response(serializer.data) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -46,6 +46,8 @@ | |||||
| has_studio_read_access, | ||||||
| has_studio_write_access, | ||||||
| ) | ||||||
| from common.djangoapps.student.models import CourseAccessRole | ||||||
| from common.djangoapps.student.roles import GlobalStaff | ||||||
| from common.djangoapps.util.date_utils import get_default_time_display | ||||||
| from common.djangoapps.util.json_request import JsonResponse, expect_json | ||||||
| from openedx.core.djangoapps.bookmarks import api as bookmarks_api | ||||||
|
|
@@ -155,6 +157,77 @@ def _get_block_parent_children(xblock): | |||||
| return response | ||||||
|
|
||||||
|
|
||||||
| def _check_publish_permissions(request, usage_key): | ||||||
| """ | ||||||
| Check if the user has permission to publish content. | ||||||
|
|
||||||
| This function validates that only instructors (not staff-only users) can publish | ||||||
| content. Staff members are denied publish permissions even if they are GlobalStaff. | ||||||
|
|
||||||
| Args: | ||||||
| request: The HTTP request object containing the publish action and user information. | ||||||
| usage_key: The usage key identifying the xblock/content being published. | ||||||
|
|
||||||
| Returns: | ||||||
| None if permission check passes, or JsonResponse with error message and 403 status | ||||||
| if the user lacks publish permissions. | ||||||
| """ | ||||||
| try: | ||||||
| publish_action = ( | ||||||
| request.json.get("publish") | ||||||
| if hasattr(request, "json") and request.json | ||||||
| else None | ||||||
| ) | ||||||
| log.info( | ||||||
| f"Publish action check: method={request.method}, " | ||||||
| f"publish={publish_action}, user={request.user.username}, " | ||||||
| f"is_superuser={request.user.is_superuser}" | ||||||
| ) | ||||||
|
|
||||||
| if publish_action == "make_public": | ||||||
| # Check the user's course access role from database first | ||||||
| # This check applies to all users, including GlobalStaff | ||||||
| user_course_roles = list(CourseAccessRole.objects.filter( | ||||||
| user=request.user, | ||||||
| course_id=usage_key.course_key, | ||||||
| role__in=["instructor", "staff"] | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| ).values_list("role", flat=True)) | ||||||
|
|
||||||
| is_global_staff = GlobalStaff().has_user(request.user) | ||||||
| log.info( | ||||||
| f"Publish permission check: user={request.user.username}, " | ||||||
| f"roles={user_course_roles}, " | ||||||
| f"is_global_staff={is_global_staff}, " | ||||||
| f"course={usage_key.course_key}" | ||||||
| ) | ||||||
|
|
||||||
| # If user is only staff (not instructor), deny publish permission | ||||||
| # This applies even to GlobalStaff users | ||||||
| if "staff" in user_course_roles and "instructor" not in user_course_roles: | ||||||
| log.warning( | ||||||
| f"Publish DENIED for staff-only user: " | ||||||
| f"{request.user.username} (global_staff={is_global_staff})" | ||||||
| ) | ||||||
| return JsonResponse( | ||||||
| { | ||||||
| "error": _( | ||||||
| "Only instructors can publish content. " | ||||||
| "Staff members do not have publish permissions." | ||||||
| ) | ||||||
| }, | ||||||
| status=403, | ||||||
| ) | ||||||
| else: | ||||||
| log.info( | ||||||
| f"Publish ALLOWED for user: {request.user.username}, " | ||||||
| f"roles={user_course_roles}, global_staff={is_global_staff}" | ||||||
| ) | ||||||
| except Exception as e: # lint-amnesty, pylint: disable=broad-exception-caught | ||||||
| log.error(f"Error checking publish permissions: {e}", exc_info=True) | ||||||
|
|
||||||
| return None | ||||||
|
|
||||||
|
|
||||||
| def handle_xblock(request, usage_key_string=None): | ||||||
| """ | ||||||
| Service method with all business logic for handling xblock requests. | ||||||
|
|
@@ -173,6 +246,22 @@ def handle_xblock(request, usage_key_string=None): | |||||
| if not access_check(request.user, usage_key.course_key): | ||||||
| raise PermissionDenied() | ||||||
|
|
||||||
| # Debug logging to see what's in the request | ||||||
| log.info( | ||||||
| f"=== XBLOCK REQUEST DEBUG === method={request.method}, " | ||||||
| f"user={request.user.username}, usage_key={usage_key}" | ||||||
| ) | ||||||
| log.info( | ||||||
| f"request.json exists: {hasattr(request, 'json')}, " | ||||||
| f"request.json value: {getattr(request, 'json', None)}" | ||||||
| ) | ||||||
|
|
||||||
| # Check if user is trying to publish and if they have permission | ||||||
| if request.method in ("POST", "PUT", "PATCH"): | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why |
||||||
| permission_response = _check_publish_permissions(request, usage_key) | ||||||
| if permission_response: | ||||||
| return permission_response | ||||||
|
|
||||||
| if request.method == "GET": | ||||||
| accept_header = request.META.get("HTTP_ACCEPT", "application/json") | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CourseUserRoleSerializerDo you need it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, needed. It's used by CourseUserRoleView to serialize the response data.