Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
df1e196
feat: add initial extended model for consistency and additional storage
mariajgrimaldi Oct 14, 2025
9d3b395
feat: add model to be used as backreference to maintain rules up to date
mariajgrimaldi Oct 16, 2025
833cc2c
refactor!: use registry pattern to extend base models
mariajgrimaldi Oct 20, 2025
b4f7555
refactor: consider when deleting extended model so the casbin rule is…
mariajgrimaldi Oct 21, 2025
338ab81
refactor: run make format
mariajgrimaldi Oct 23, 2025
1ec58fe
refactor: regenerate migrations
mariajgrimaldi Oct 24, 2025
d3bf9c1
refactor: fix issues when rebasing
mariajgrimaldi Oct 24, 2025
3ea8a84
refactor: drop unused variables from data classes
mariajgrimaldi Oct 24, 2025
b2b873b
refactor: address integration test failure
mariajgrimaldi Nov 4, 2025
27b6e5c
refactor: consider double namespace instead
mariajgrimaldi Nov 4, 2025
d3f1632
test: add minimal unittest suite for extended casbin model
mariajgrimaldi Nov 4, 2025
5996a49
refactor: address quality issues
mariajgrimaldi Nov 5, 2025
59053d7
feat: add handler to remove roles when user is retired
mariajgrimaldi Oct 22, 2025
2925a21
refactor: address PR review
mariajgrimaldi Oct 23, 2025
e6096e2
refactor: address issues after rebase
mariajgrimaldi Oct 24, 2025
59ab46b
refactor: address quality issues after rebase
mariajgrimaldi Nov 6, 2025
282c6b6
refactor: drop load policy in favor of cache invalidation
mariajgrimaldi Nov 13, 2025
cbec3e6
refactor: (temp) use no-index by default to avoid duplicate warnings
mariajgrimaldi Nov 14, 2025
41da28b
docs: update changelog
mariajgrimaldi Nov 14, 2025
737c07a
refactor: consider later changes in namespaces
mariajgrimaldi Nov 14, 2025
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
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ Unreleased

*

0.17.0 - 2025-11-14
********************

Added
=====

* Signal to clear policies associated to a user when they are retired.

0.16.0 - 2025-11-13
********************

Expand Down
9 changes: 8 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,12 @@ def on_init(app): # pylint: disable=unused-argument
# If we are, assemble the path manually
bin_path = os.path.abspath(os.path.join(sys.prefix, "bin"))
apidoc_path = os.path.join(bin_path, apidoc_path)

# Set SPHINX_APIDOC_OPTIONS to add :no-index: to generated automodule directives
# This prevents duplicate object warnings for re-exported API members
env = os.environ.copy()
env['SPHINX_APIDOC_OPTIONS'] = 'members,show-inheritance,undoc-members,no-index'
Comment on lines +561 to +564
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a temporary solution so we can finally merge this. We can address a more long-term solution (like improving all docs structure) as part of this issue #145


check_call(
[
apidoc_path,
Expand All @@ -565,7 +571,8 @@ def on_init(app): # pylint: disable=unused-argument
os.path.join(root_path, "openedx_authz"),
os.path.join(root_path, "openedx_authz/migrations"),
os.path.join(root_path, "openedx_authz/tests"),
]
],
env=env
)


Expand Down
2 changes: 1 addition & 1 deletion openedx_authz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

import os

__version__ = "0.16.0"
__version__ = "0.17.0"

ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))
14 changes: 14 additions & 0 deletions openedx_authz/api/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"get_all_subject_role_assignments_in_scope",
"get_subject_role_assignments",
"get_scopes_for_subject_and_permission",
"unassign_subject_from_all_roles",
]

# TODO: these are the concerns we still have to address:
Expand Down Expand Up @@ -418,3 +419,16 @@ def get_scopes_for_subject_and_permission(
if permission in role.permissions and role_assignment.scope not in scopes:
scopes.append(role_assignment.scope)
return scopes


def unassign_subject_from_all_roles(subject: SubjectData) -> bool:
"""Unassign a subject from all roles across all scopes.

Args:
subject: The SubjectData object representing the subject to unassign.

Returns:
bool: True if any roles were removed, False otherwise.
"""
enforcer = AuthzEnforcer.get_enforcer()
return enforcer.remove_filtered_grouping_policy(GroupingPolicyIndex.SUBJECT.value, subject.namespaced_key)
14 changes: 14 additions & 0 deletions openedx_authz/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
get_subject_role_assignments_in_scope,
get_subjects_for_role_in_scope,
unassign_role_from_subject_in_scope,
unassign_subject_from_all_roles,
)

__all__ = [
Expand All @@ -36,6 +37,7 @@
"is_user_allowed",
"get_scopes_for_user_and_permission",
"get_users_for_role_in_scope",
"unassign_all_roles_from_user",
]


Expand Down Expand Up @@ -223,3 +225,15 @@ def get_scopes_for_user_and_permission(
UserData(external_key=user_external_key),
PermissionData(action=ActionData(external_key=action_external_key)),
)


def unassign_all_roles_from_user(user_external_key: str) -> bool:
"""Unassign all roles from a user across all scopes.

Args:
user_external_key (str): ID of the user (e.g., 'john_doe').

Returns:
bool: True if any roles were removed, False otherwise.
"""
return unassign_subject_from_all_roles(UserData(external_key=user_external_key))
34 changes: 34 additions & 0 deletions openedx_authz/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,14 @@
from django.db.models.signals import post_delete
from django.dispatch import receiver

from openedx_authz.api.users import unassign_all_roles_from_user
from openedx_authz.models.core import ExtendedCasbinRule

try:
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL
except ImportError:
USER_RETIRE_LMS_CRITICAL = None

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -48,3 +54,31 @@ def delete_casbin_rule_on_extended_rule_deletion(sender, instance, **kwargs): #
instance.casbin_rule_id,
exc_info=exc,
)


def unassign_roles_on_user_retirement(sender, user, **kwargs): # pylint: disable=unused-argument
"""
Unassign roles from a user when they are retired.

This handler is triggered when a user is retired in the LMS. It ensures that
any roles assigned to the user are removed, maintaining the integrity of the
authorization system.

Args:
sender: The model class (User).
user: The user instance being retired.
**kwargs: Additional keyword arguments from the signal.
"""
try:
unassign_all_roles_from_user(user.username)
except Exception as exc: # pylint: disable=broad-exception-caught
logger.exception(
"Error unassigning roles from user %s during retirement",
user.id,
exc_info=exc,
)


# Only register the handler if the signal is available (i.e., running in Open edX)
if USER_RETIRE_LMS_CRITICAL is not None:
USER_RETIRE_LMS_CRITICAL.connect(unassign_roles_on_user_retirement)
154 changes: 154 additions & 0 deletions openedx_authz/tests/api/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
get_subject_role_assignments_in_scope,
get_subjects_for_role_in_scope,
unassign_role_from_subject_in_scope,
unassign_subject_from_all_roles,
)
from openedx_authz.constants import permissions, roles
from openedx_authz.constants.roles import (
Expand Down Expand Up @@ -976,3 +977,156 @@ def test_assign_role_creates_extended_casbin_rule(self):
self.assertIn(role_data.namespaced_key, extended_rule.casbin_rule_key)
self.assertIn(subject_data.namespaced_key, extended_rule.casbin_rule_key)
self.assertIn(scope_data.namespaced_key, extended_rule.casbin_rule_key)


@ddt_data(
# Test user with single role in single scope
("alice", ["lib:Org1:math_101"], {"library_admin"}),
# Test user with multiple roles in different scopes
(
"eve",
["lib:Org2:physics_401", "lib:Org2:chemistry_501", "lib:Org2:biology_601"],
{"library_admin", "library_author", "library_user"},
),
# Test user with same role in multiple scopes
("liam", ["lib:Org4:art_101", "lib:Org4:art_201", "lib:Org4:art_301"], {"library_author"}),
# Test user with multiple different roles in multiple scopes
(
"peter",
["lib:Org6:project_alpha", "lib:Org6:project_beta", "lib:Org6:project_gamma", "lib:Org6:project_delta"],
{"library_admin", "library_author", "library_contributor", "library_user"},
),
)
@unpack
def test_unassign_subject_from_all_roles_removes_all_assignments(self, subject_name, scopes, expected_roles_before):
"""Test that unassign_subject_from_all_roles removes all role assignments.

Expected result:
- Before unassignment: Subject has roles in specified scopes
- Function returns True indicating roles were removed
- After unassignment: Subject has no role assignments in any scope
- Querying role assignments returns empty list
"""
subject = SubjectData(external_key=subject_name)

# Verify the subject has roles before unassignment
assignments_before = get_subject_role_assignments(subject)
self.assertGreater(len(assignments_before), 0)

# Verify roles are what we expect before removal
roles_before = {r.external_key for assignment in assignments_before for r in assignment.roles}
self.assertEqual(roles_before, expected_roles_before)

# Verify assignments exist in each expected scope
for scope_name in scopes:
scope_assignments = get_subject_role_assignments_in_scope(subject, ScopeData(external_key=scope_name))
self.assertGreater(len(scope_assignments), 0)

# Unassign all roles from the subject
result = unassign_subject_from_all_roles(subject)

# Verify the function returns True (indicating roles were removed)
self.assertTrue(result)

# Verify the subject has no role assignments after unassignment
assignments_after = get_subject_role_assignments(subject)
self.assertEqual(len(assignments_after), 0)

# Verify no assignments in any of the previous scopes
for scope_name in scopes:
scope_assignments = get_subject_role_assignments_in_scope(subject, ScopeData(external_key=scope_name))
self.assertEqual(len(scope_assignments), 0)

def test_unassign_subject_with_no_roles_returns_false(self):
"""Test that unassigning a subject with no roles returns False.

Expected result:
- Function returns False when subject has no role assignments
- No errors occur when trying to unassign from non-existent subject
"""
non_existent_subject = SubjectData(external_key="user_with_no_roles")

# Verify the subject has no roles
assignments_before = get_subject_role_assignments(non_existent_subject)
self.assertEqual(len(assignments_before), 0)

# Unassign all roles (should return False since there are none)
result = unassign_subject_from_all_roles(non_existent_subject)

# Verify the function returns False (no roles to remove)
self.assertFalse(result)

# Verify still no assignments after the operation
assignments_after = get_subject_role_assignments(non_existent_subject)
self.assertEqual(len(assignments_after), 0)

def test_unassign_subject_does_not_affect_other_subjects(self):
"""Test that unassigning one subject does not affect other subjects.

Expected result:
- When unassigning roles from one subject, other subjects retain their roles
- Other subjects with the same roles in the same scopes are unaffected
"""
# Use subjects that share the same scope
subject_to_unassign = SubjectData(external_key="grace")
other_subject = SubjectData(external_key="heidi")
shared_scope = ScopeData(external_key="lib:Org1:math_advanced")

# Verify both subjects have roles in the shared scope before
grace_assignments_before = get_subject_role_assignments_in_scope(subject_to_unassign, shared_scope)
heidi_assignments_before = get_subject_role_assignments_in_scope(other_subject, shared_scope)

self.assertGreater(len(grace_assignments_before), 0)
self.assertGreater(len(heidi_assignments_before), 0)

# Unassign all roles from grace
result = unassign_subject_from_all_roles(subject_to_unassign)
self.assertTrue(result)

# Verify grace has no assignments after unassignment
grace_assignments_after = get_subject_role_assignments(subject_to_unassign)
self.assertEqual(len(grace_assignments_after), 0)

# Verify heidi still has her assignments
heidi_assignments_after = get_subject_role_assignments_in_scope(other_subject, shared_scope)
self.assertEqual(len(heidi_assignments_after), len(heidi_assignments_before))

# Verify heidi still has the library_contributor role
heidi_roles = {r.external_key for assignment in heidi_assignments_after for r in assignment.roles}
self.assertIn("library_contributor", heidi_roles)

def test_unassign_and_reassign_subject(self):
"""Test that a subject can be reassigned roles after being unassigned.

Expected result:
- Subject has roles initially
- After unassignment, subject has no roles
- Subject can be assigned new roles
- Newly assigned roles work correctly
"""
subject = SubjectData(external_key="bob")
new_scope = ScopeData(external_key="lib:Org1:new_library")
new_role = RoleData(external_key="library_admin")

# Verify bob has roles initially
assignments_before = get_subject_role_assignments(subject)
self.assertGreater(len(assignments_before), 0)

# Unassign all roles
result = unassign_subject_from_all_roles(subject)
self.assertTrue(result)

# Verify no roles after unassignment
assignments_after_unassign = get_subject_role_assignments(subject)
self.assertEqual(len(assignments_after_unassign), 0)

# Assign a new role in a new scope
assign_result = assign_role_to_subject_in_scope(subject, new_role, new_scope)
self.assertTrue(assign_result)

# Verify the new assignment works
new_assignments = get_subject_role_assignments_in_scope(subject, new_scope)
self.assertEqual(len(new_assignments), 1)

new_roles = {r.external_key for assignment in new_assignments for r in assignment.roles}
self.assertIn("library_admin", new_roles)
Loading