Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/pylint-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
- module-name: openedx-1
path: "openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/djangoapps/course_live/"
- module-name: openedx-2
path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/envs/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/"
path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/envs/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/ openedx/core/djangoapps/authz/"
- module-name: common
path: "common"
- module-name: cms
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/unit-test-shards.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
"openedx/core/djangoapps/xblock/",
"openedx/core/djangoapps/xmodule_django/",
"openedx/core/djangoapps/zendesk_proxy/",
"openedx/core/djangoapps/authz/",
"openedx/core/djangolib/",
"openedx/core/lib/",
"openedx/core/tests/",
Expand Down Expand Up @@ -227,6 +228,7 @@
"openedx/core/djangoapps/xblock/",
"openedx/core/djangoapps/xmodule_django/",
"openedx/core/djangoapps/zendesk_proxy/",
"openedx/core/djangoapps/authz/",
"openedx/core/lib/",
"openedx/tests/"
]
Expand Down
50 changes: 49 additions & 1 deletion cms/djangoapps/contentstore/api/tests/test_quality.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
Tests for the course import API views
"""


from rest_framework.test import APIClient
from rest_framework import status
from openedx_authz.constants.roles import COURSE_STAFF

from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin
from .base import BaseCourseViewTest


Expand Down Expand Up @@ -67,3 +70,48 @@ def test_student_fails(self):
self.client.login(username=self.student.username, password=self.password)
resp = self.client.get(self.get_url(self.course_key))
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)


class CourseQualityAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest):
"""
Tests Course Quality API authorization using openedx-authz.
"""

view_name = "courses_api:course_quality"
authz_roles_to_assign = [COURSE_STAFF.external_key]

def test_authorized_user_can_access(self):
"""User with COURSE_STAFF role can access."""
resp = self.authorized_client.get(self.get_url(self.course_key))
self.assertEqual(resp.status_code, status.HTTP_200_OK)

def test_unauthorized_user_cannot_access(self):
"""User without role cannot access."""
resp = self.unauthorized_client.get(self.get_url(self.course_key))
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

def test_role_scoped_to_course(self):
"""Authorization should only apply to the assigned course."""
other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id)

resp = self.authorized_client.get(self.get_url(other_course.id))
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

def test_staff_user_allowed_via_legacy(self):
"""
Staff users should still pass through legacy fallback.
"""
self.client.login(username=self.staff.username, password=self.password)

resp = self.client.get(self.get_url(self.course_key))
self.assertEqual(resp.status_code, status.HTTP_200_OK)

def test_superuser_allowed(self):
"""Superusers should always be allowed."""
superuser = UserFactory(is_superuser=True)

client = APIClient()
client.force_authenticate(user=superuser)

resp = client.get(self.get_url(self.course_key))
self.assertEqual(resp.status_code, status.HTTP_200_OK)
68 changes: 68 additions & 0 deletions cms/djangoapps/contentstore/api/tests/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,23 @@

import ddt
import factory

from django.conf import settings
from django.contrib.auth import get_user_model
from django.test.utils import override_settings
from django.urls import reverse
from rest_framework import status
from rest_framework.test import APITestCase
from rest_framework.test import APIClient
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin
from openedx_authz.constants.roles import COURSE_STAFF

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
from common.djangoapps.student.tests.factories import StaffFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory
from cms.djangoapps.contentstore.api.tests.base import BaseCourseViewTest

User = get_user_model()

Expand Down Expand Up @@ -272,3 +277,66 @@ def test_list_ready_to_update_reference_success(self, mock_block, mock_auth):
{'usage_key': str(self.block2.location)},
])
mock_auth.assert_called_once()


class CourseValidationAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest):
"""
Tests Course Validation API authorization using openedx-authz.
"""

view_name = "courses_api:course_validation"
authz_roles_to_assign = [COURSE_STAFF.external_key]

def test_authorized_user_can_access(self):
"""
User with COURSE_STAFF role should be allowed via AuthZ.
"""
resp = self.authorized_client.get(self.get_url(self.course_key))

self.assertEqual(resp.status_code, status.HTTP_200_OK)

def test_unauthorized_user_cannot_access(self):
"""
User without permissions should be denied.
"""
resp = self.unauthorized_client.get(self.get_url(self.course_key))

self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

def test_role_scoped_to_course(self):
"""
Authorization should only apply to the assigned course scope.
"""
other_course = self.store.create_course(
"OtherOrg",
"OtherCourse",
"Run",
self.staff.id,
)

resp = self.authorized_client.get(self.get_url(other_course.id))

self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

def test_staff_user_allowed_via_legacy(self):
Copy link
Member

Choose a reason for hiding this comment

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

What about a non-staff user? Is this tested anywhere else?

"""
Course staff should pass through legacy fallback when AuthZ denies.
"""
self.client.login(username=self.staff.username, password=self.password)

resp = self.client.get(self.get_url(self.course_key))

self.assertEqual(resp.status_code, status.HTTP_200_OK)

def test_superuser_allowed(self):
"""
Superusers should always be allowed through legacy fallback.
"""
superuser = UserFactory(is_superuser=True)

client = APIClient()
client.force_authenticate(user=superuser)

resp = client.get(self.get_url(self.course_key))

self.assertEqual(resp.status_code, status.HTTP_200_OK)
7 changes: 5 additions & 2 deletions cms/djangoapps/contentstore/api/views/course_quality.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@
from edxval.api import get_course_videos_qset
from rest_framework.generics import GenericAPIView
from rest_framework.response import Response
from openedx.core.djangoapps.authz.constants import LegacyPermission
from scipy import stats
from openedx_authz.constants.permissions import COURSES_VIEW_COURSE

from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
from openedx.core.lib.cache_utils import request_cached
from openedx.core.lib.graph_traversals import traverse_pre_order
from openedx.core.djangoapps.authz.decorators import authz_permission_required
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order

from .utils import course_author_access_required, get_bool_param
from .utils import get_bool_param

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -82,7 +85,7 @@ class CourseQualityView(DeveloperErrorViewMixin, GenericAPIView):
# does not specify a serializer class.
swagger_schema = None

@course_author_access_required
@authz_permission_required(COURSES_VIEW_COURSE.identifier, LegacyPermission.READ)
def get(self, request, course_key):
"""
Returns validation information for the given course.
Expand Down
5 changes: 4 additions & 1 deletion cms/djangoapps/contentstore/api/views/course_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
from rest_framework import serializers, status
from rest_framework.generics import GenericAPIView
from rest_framework.response import Response
from openedx.core.djangoapps.authz.constants import LegacyPermission
from user_tasks.models import UserTaskStatus
from user_tasks.views import StatusViewSet
from openedx_authz.constants.permissions import COURSES_VIEW_COURSE

from cms.djangoapps.contentstore.course_info_model import get_course_updates
from cms.djangoapps.contentstore.tasks import migrate_course_legacy_library_blocks_to_item_bank
Expand All @@ -19,6 +21,7 @@
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.core.lib.api.serializers import StatusSerializerWithUuid
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
from openedx.core.djangoapps.authz.decorators import authz_permission_required
from xmodule.course_metadata_utils import DEFAULT_GRADING_POLICY # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order

Expand Down Expand Up @@ -80,7 +83,7 @@ class CourseValidationView(DeveloperErrorViewMixin, GenericAPIView):
# does not specify a serializer class.
swagger_schema = None

@course_author_access_required
@authz_permission_required(COURSES_VIEW_COURSE.identifier, LegacyPermission.READ)
def get(self, request, course_key):
"""
Returns validation information for the given course.
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/api/views/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def course_author_access_required(view):
Usage::
@course_author_access_required
def my_view(request, course_key):
# Some functionality ...
# Some functionality...
"""
def _wrapper_view(self, request, course_id, *args, **kwargs):
"""
Expand Down
3 changes: 3 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,9 @@ def make_lms_template_path(settings):
# alternative swagger generator for CMS API
'drf_spectacular',

# Authz
'openedx.core.djangoapps.authz',

'openedx_events',

# Core models to represent courses
Expand Down
3 changes: 3 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2018,6 +2018,9 @@
# Notifications
'openedx.core.djangoapps.notifications',

# Authz
'openedx.core.djangoapps.authz',

'openedx_events',

# Core models to represent courses
Expand Down
Empty file.
16 changes: 16 additions & 0 deletions openedx/core/djangoapps/authz/apps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
"""Django app configuration for authz app."""

from django.apps import AppConfig


class AuthzConfig(AppConfig):
"""Django application configuration for the Open edX Authorization (AuthZ) app.

This app provides a centralized location for integrations with the
openedx-authz library, including permission helpers, decorators,
and other utilities used to enforce RBAC-based authorization across
the platform."""

default_auto_field = 'django.db.models.BigAutoField'
name = 'openedx.core.djangoapps.authz'
verbose_name = "Open edX Authorization Framework"
15 changes: 15 additions & 0 deletions openedx/core/djangoapps/authz/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
"""Constants used by the Open edX Authorization (AuthZ) framework."""

from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access
from openedx.core.lib.teams_config import Enum
Copy link
Member

Choose a reason for hiding this comment

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

I'd avoid importing from other apps unless necessary like in L3:

Suggested change
from openedx.core.lib.teams_config import Enum
from enum import Enum



class LegacyPermission(Enum):
Copy link
Member

Choose a reason for hiding this comment

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

Maybe more specific would be better, also adding a docstring would be useful as well.

Suggested change
class LegacyPermission(Enum):
class LegacyAuthoringPermission(Enum):

READ = "read"
WRITE = "write"


LEGACY_PERMISSION_HANDLER_MAP = {
LegacyPermission.READ: has_studio_read_access,
LegacyPermission.WRITE: has_studio_write_access,
}
Loading
Loading