-
Notifications
You must be signed in to change notification settings - Fork 6
[FC-0099] feat: implement custom matcher in casbin model #114
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
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
c5262fb
feat: add custom matcher in casbin model
BryanttV 8d401e7
refactor: enhance custom condition checks for authorization in matche…
BryanttV 6458b59
feat: add tests for staff and superuser automatic permission grants
BryanttV 1d7087a
test: validate permissions for not library scopes
BryanttV 4bd8f2d
refactor: drop /tests package in favor of openedx_authz/tests
mariajgrimaldi c3f0d69
refactor: address PR reviews
mariajgrimaldi 4b15aeb
refactor: drop sc: references
mariajgrimaldi 6d039c9
refactor: add test case for non-existent lib scope
mariajgrimaldi fd8dca3
refactor: go back to previous configuration for tox.ini
mariajgrimaldi f423be1
refactor: address quality issues
mariajgrimaldi b5de0c8
chore: update docs for release
mariajgrimaldi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| """Custom condition checker. Note only used for data_library scope""" | ||
|
|
||
| from django.contrib.auth import get_user_model | ||
|
|
||
| from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData | ||
| from openedx_authz.rest_api.utils import get_user_by_username_or_email | ||
|
|
||
| User = get_user_model() | ||
|
|
||
|
|
||
| def is_admin_or_superuser_check(request_user: str, request_action: str, request_scope: str) -> bool: # pylint: disable=unused-argument | ||
| """ | ||
| Evaluates custom, non-role-based conditions for authorization checks. | ||
|
|
||
| Checks attribute-based conditions that don't rely on role assignments. | ||
| Currently handles ContentLibraryData scopes by granting access to staff | ||
| and superusers. | ||
|
|
||
| Args: | ||
| request_user (str): Namespaced user key (format: "user::<username>") | ||
| request_action (str): Namespaced action key (format: "action::<action_name>") | ||
| request_scope (str): Namespaced scope key (format: "scope_type::<scope_id>") | ||
|
|
||
| Returns: | ||
| bool: True if the condition is satisfied (user is staff/superuser for | ||
| ContentLibraryData scopes), False otherwise (including when user | ||
| doesn't exist or scope type is not supported) | ||
| """ | ||
| try: | ||
| username = UserData(namespaced_key=request_user).external_key | ||
| user = get_user_by_username_or_email(username) | ||
| except User.DoesNotExist: | ||
| return False | ||
|
|
||
| scope = ScopeData(namespaced_key=request_scope) | ||
|
|
||
| if isinstance(scope, ContentLibraryData): | ||
| return user.is_staff or user.is_superuser | ||
|
|
||
| return False |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,10 +10,13 @@ | |
| from unittest import TestCase | ||
|
|
||
| import casbin | ||
| import pytest | ||
| from ddt import data, ddt, unpack | ||
| from django.contrib.auth import get_user_model | ||
|
|
||
| from openedx_authz import ROOT_DIRECTORY | ||
| from openedx_authz.constants import roles | ||
| from openedx_authz.engine.matcher import is_admin_or_superuser_check | ||
| from openedx_authz.tests.test_utils import ( | ||
| make_action_key, | ||
| make_library_key, | ||
|
|
@@ -22,6 +25,8 @@ | |
| make_user_key, | ||
| ) | ||
|
|
||
| User = get_user_model() | ||
|
|
||
|
|
||
| class AuthRequest(TypedDict): | ||
| """ | ||
|
|
@@ -44,6 +49,7 @@ class AuthRequest(TypedDict): | |
| ] | ||
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| @ddt | ||
| class CasbinEnforcementTestCase(TestCase): | ||
| """ | ||
|
|
@@ -65,6 +71,7 @@ def setUpClass(cls) -> None: | |
| raise FileNotFoundError(f"Model file not found: {model_file}") | ||
|
|
||
| cls.enforcer = casbin.Enforcer(model_file) | ||
| cls.enforcer.add_function("is_staff_or_superuser", is_admin_or_superuser_check) | ||
|
|
||
| def _load_policy(self, policy: list[str]) -> None: | ||
| """ | ||
|
|
@@ -573,3 +580,84 @@ def test_wildcard_library_access(self, scope: str, expected_result: bool): | |
| "expected_result": expected_result, | ||
| } | ||
| self._test_enforcement(self.POLICY, request) | ||
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| @ddt | ||
| class StaffSuperuserAccessTests(CasbinEnforcementTestCase): | ||
| """ | ||
| Tests for staff and superuser automatic permission grants via is_staff_or_superuser. | ||
|
|
||
| This test class verifies that staff members and superusers are automatically | ||
| granted access to ContentLibrary scopes through the is_admin_or_superuser_check function, | ||
| without requiring explicit role assignments. | ||
| """ | ||
|
|
||
| # Empty policy - no role assignments for staff/superuser users | ||
| POLICY = [] | ||
|
|
||
| def setUp(self) -> None: | ||
| """Set up the test environment.""" | ||
| super().setUp() | ||
| User.objects.create_user(username="staff_user", email="[email protected]", password="test", is_staff=True) | ||
| User.objects.create_superuser(username="superuser", email="[email protected]", password="test") | ||
| User.objects.create_user(username="regular_user", email="[email protected]", password="test") | ||
|
|
||
| @data( | ||
| # Staff user has automatic access to any library scope | ||
| ( | ||
| make_user_key("staff_user"), | ||
| make_action_key("view_library"), | ||
| make_library_key("lib:TestOrg:TestLib"), | ||
| True, | ||
| ), | ||
| ( | ||
| make_user_key("staff_user"), | ||
| make_action_key("edit_library"), | ||
| make_library_key("lib:AnyOrg:AnyLib"), | ||
| True, | ||
| ), | ||
| # Superuser has automatic access to any library scope | ||
| ( | ||
| make_user_key("superuser"), | ||
| make_action_key("view_library"), | ||
| make_library_key("lib:TestOrg:TestLib"), | ||
| True, | ||
| ), | ||
| ( | ||
| make_user_key("superuser"), | ||
| make_action_key("delete_library"), | ||
| make_library_key("lib:AnyOrg:AnyLib"), | ||
| True, | ||
| ), | ||
| # Regular user without role assignment has no access | ||
| ( | ||
| make_user_key("regular_user"), | ||
| make_action_key("view_library"), | ||
| make_library_key("lib:TestOrg:TestLib"), | ||
| False, | ||
| ), | ||
| # Non existent library scope access denied | ||
| ( | ||
| make_user_key("regular_user"), | ||
| make_action_key("view_library"), | ||
| make_library_key("lib:NonExistent:NoLib"), | ||
| False, | ||
| ), | ||
| ) | ||
| @unpack | ||
| def test_staff_superuser_guaranteed_permissions(self, subject: str, action: str, scope: str, expected_result: bool): | ||
| """Test that staff and superusers have guaranteed permissions for ContentLibrary scopes. | ||
|
|
||
| This test validates that: | ||
| - Staff users automatically have access to all library scopes without role assignments | ||
| - Superusers automatically have access to all library scopes without role assignments | ||
| - Regular users require explicit role assignments to access libraries | ||
| - Access is granted through the is_staff_or_superuser matcher function | ||
|
|
||
| Expected result: | ||
| - Staff and superusers can perform any action on any ContentLibrary scope | ||
| - Regular users are denied access without role assignments | ||
| """ | ||
| request = {"subject": subject, "action": action, "scope": scope, "expected_result": expected_result} | ||
| self._test_enforcement(self.POLICY, request) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we need a test that handles non-existent library keys? I assume that would return False, right?
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! I added a test case here: 7805698