-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat: libraries v2 support for studio perms XBlock service #39074
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: master
Are you sure you want to change the base?
Changes from all commits
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,26 @@ | ||
| """ Fixtures for AuthZ-aware tests """ | ||
| import casbin | ||
| import pkg_resources | ||
| from openedx_authz.engine.enforcer import AuthzEnforcer | ||
| from openedx_authz.engine.utils import migrate_policy_between_enforcers | ||
|
|
||
|
|
||
| def seed_policies(): | ||
| """Seed the database with AuthZ policies.""" | ||
| global_enforcer = AuthzEnforcer.get_enforcer() | ||
| global_enforcer.load_policy() | ||
|
|
||
| model_path = pkg_resources.resource_filename( | ||
| "openedx_authz.engine", | ||
| "config/model.conf", | ||
| ) | ||
|
|
||
| policy_path = pkg_resources.resource_filename( | ||
| "openedx_authz.engine", | ||
| "config/authz.policy", | ||
| ) | ||
|
|
||
| migrate_policy_between_enforcers( | ||
| source_enforcer=casbin.Enforcer(model_path, policy_path), | ||
| target_enforcer=global_enforcer, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| """ | ||
| Services for learning content | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| from opaque_keys.edx.locator import LibraryLocatorV2 | ||
| from openedx_authz import api as authz_api | ||
| from openedx_authz.constants.permissions import EDIT_LIBRARY_CONTENT, VIEW_LIBRARY | ||
|
|
||
| from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access | ||
|
|
||
|
|
||
| class StudioPermissionsService: | ||
| """ | ||
| Service that can provide information about a user's permissions. | ||
| """ | ||
|
|
||
| def __init__(self, user): | ||
| self._user = user | ||
|
|
||
| def can_read(self, context_key): | ||
| """ Does the user have read access to the given course/library? """ | ||
| if isinstance(context_key, LibraryLocatorV2): | ||
| return self._user.is_active and authz_api.is_user_allowed( | ||
| self._user, | ||
| VIEW_LIBRARY.identifier, | ||
| str(context_key), | ||
| ) | ||
| return has_studio_read_access(self._user, context_key) | ||
|
|
||
| def can_write(self, context_key): | ||
| """ Does the user have write access to the given course/library? """ | ||
| if isinstance(context_key, LibraryLocatorV2): | ||
| return self._user.is_active and authz_api.is_user_allowed( | ||
| self._user, | ||
| EDIT_LIBRARY_CONTENT.identifier, | ||
| str(context_key), | ||
| ) | ||
| return has_studio_write_access(self._user, context_key) | ||
|
Comment on lines
+21
to
+39
Contributor
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. I guess we need |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| """ | ||
| Tests for content XBlock Services | ||
| """ | ||
| from django.contrib.auth import get_user_model | ||
| from django.test import TransactionTestCase | ||
| from opaque_keys.edx.locator import LibraryLocatorV2 | ||
| from organizations.models import Organization | ||
|
|
||
| from common.djangoapps.student.auth import update_org_role | ||
| from common.djangoapps.student.roles import OrgStaffRole | ||
| from common.djangoapps.student.tests.factories import UserFactory | ||
| from openedx.core.djangoapps.authz.tests.fixtures import seed_policies | ||
| from openedx.core.djangoapps.content.services import StudioPermissionsService | ||
| from openedx.core.djangoapps.content_libraries.api import ( | ||
| AccessLevel, | ||
| ContentLibraryMetadata, | ||
| assign_library_role_to_user, | ||
| create_library, | ||
| ) | ||
| from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase | ||
| from xmodule.modulestore.tests.factories import CourseFactory | ||
|
|
||
| User = get_user_model() | ||
|
|
||
|
|
||
| class StudioPermissionsServiceTestCase(ModuleStoreTestCase, TransactionTestCase): | ||
| """ | ||
| Test the studio permissions service. | ||
| """ | ||
|
|
||
| def setUp(self) -> None: | ||
| super().setUp() | ||
| seed_policies() | ||
| self.org = Organization.objects.create(name="Organization A", short_name="orgA") | ||
| self.staff = UserFactory.create( | ||
| is_staff=True, | ||
| ) | ||
|
|
||
| def _create_privileged_org_user(self) -> User: | ||
| user = UserFactory.create() | ||
| update_org_role(self.staff, OrgStaffRole, user, [self.org.short_name]) | ||
| return user | ||
|
|
||
| def test_user_can_read_course(self) -> None: | ||
| course = CourseFactory.create(org=self.org.short_name) | ||
| user = self._create_privileged_org_user() | ||
| service = StudioPermissionsService(user=user) | ||
| assert service.can_read(course.location) | ||
|
|
||
| def test_user_can_write_course(self) -> None: | ||
| course = CourseFactory.create(org=self.org.short_name) | ||
| user = self._create_privileged_org_user() | ||
| service = StudioPermissionsService(user=user) | ||
| assert service.can_write(course.location) | ||
|
|
||
| def test_user_cannot_read_course(self) -> None: | ||
| course = CourseFactory.create(org=self.org.short_name) | ||
| user = UserFactory.create() | ||
| service = StudioPermissionsService(user=user) | ||
| assert not service.can_read(course.location) | ||
|
|
||
| def test_user_cannot_write_course(self) -> None: | ||
| course = CourseFactory.create(org=self.org.short_name) | ||
| user = UserFactory.create() | ||
| service = StudioPermissionsService(user=user) | ||
| assert not service.can_write(course.location) | ||
|
|
||
| def _create_library(self) -> ContentLibraryMetadata: | ||
| return create_library( | ||
| org=self.org, | ||
| slug="lib", | ||
| title="Library Org", | ||
| description="This is a library from Org", | ||
| ) | ||
|
|
||
| def _create_privileged_library_user(self, library_key: LibraryLocatorV2) -> User: | ||
| user = UserFactory.create() | ||
| assign_library_role_to_user(library_key, user, AccessLevel.ADMIN_LEVEL) | ||
| return user | ||
|
|
||
| def test_user_can_read_library(self) -> None: | ||
| library = self._create_library() | ||
| user = self._create_privileged_library_user(library.key) | ||
| service = StudioPermissionsService(user=user) | ||
| assert service.can_read(library.key) | ||
|
|
||
| def test_user_can_write_library(self) -> None: | ||
| library = self._create_library() | ||
| user = self._create_privileged_library_user(library.key) | ||
| service = StudioPermissionsService(user=user) | ||
| assert service.can_write(library.key) | ||
|
|
||
| def test_user_cannot_read_library(self) -> None: | ||
| library = self._create_library() | ||
| user = UserFactory.create() | ||
| service = StudioPermissionsService(user=user) | ||
| assert not service.can_read(library.key) | ||
|
|
||
| def test_user_cannot_write_library(self) -> None: | ||
| library = self._create_library() | ||
| user = UserFactory.create() | ||
| service = StudioPermissionsService(user=user) | ||
| assert not service.can_write(library.key) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -415,7 +415,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata: | |
|
|
||
|
|
||
| def create_library( | ||
| org: str, | ||
| org: Organization, | ||
|
Contributor
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. It's a little concerning that mypy hadn't flagged any error here ?
Contributor
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. @bradenmacdonald Yes, it is. And after playing with it for a bit, I can't seem to make it fail. I don't want to detour too far here, but something's up. I tried changing mypy's file includes to globs just to make sure the directory paths weren't insufficient, and I tried switching the types back and forth, but despite the fact I know some files that call this function are in the includes, they aren't considered wrong whether I set it to Organization or string, and whether I comment out the assert below or not. |
||
| slug: str, | ||
| title: str, | ||
| description: str = "", | ||
|
|
||
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.
"Hmm, this StudioPermissionsService is marked as Deprecated, and now this PR is un-deprecating it. I wonder who marked it as deprecated and what the replacement is."
(looks at git blame)
Oh.