Skip to content

Commit 5192606

Browse files
[FC-0099] feat: add handler to remove roles when user is retired (#110)
1 parent 5564afd commit 5192606

9 files changed

Lines changed: 678 additions & 2 deletions

File tree

CHANGELOG.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ Unreleased
1616

1717
*
1818

19+
0.17.0 - 2025-11-14
20+
********************
21+
22+
Added
23+
=====
24+
25+
* Signal to clear policies associated to a user when they are retired.
26+
1927
0.16.0 - 2025-11-13
2028
********************
2129

docs/conf.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,12 @@ def on_init(app): # pylint: disable=unused-argument
557557
# If we are, assemble the path manually
558558
bin_path = os.path.abspath(os.path.join(sys.prefix, "bin"))
559559
apidoc_path = os.path.join(bin_path, apidoc_path)
560+
561+
# Set SPHINX_APIDOC_OPTIONS to add :no-index: to generated automodule directives
562+
# This prevents duplicate object warnings for re-exported API members
563+
env = os.environ.copy()
564+
env['SPHINX_APIDOC_OPTIONS'] = 'members,show-inheritance,undoc-members,no-index'
565+
560566
check_call(
561567
[
562568
apidoc_path,
@@ -565,7 +571,8 @@ def on_init(app): # pylint: disable=unused-argument
565571
os.path.join(root_path, "openedx_authz"),
566572
os.path.join(root_path, "openedx_authz/migrations"),
567573
os.path.join(root_path, "openedx_authz/tests"),
568-
]
574+
],
575+
env=env
569576
)
570577

571578

openedx_authz/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44

55
import os
66

7-
__version__ = "0.16.0"
7+
__version__ = "0.17.0"
88

99
ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))

openedx_authz/api/roles.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"get_all_subject_role_assignments_in_scope",
4242
"get_subject_role_assignments",
4343
"get_scopes_for_subject_and_permission",
44+
"unassign_subject_from_all_roles",
4445
]
4546

4647
# TODO: these are the concerns we still have to address:
@@ -418,3 +419,16 @@ def get_scopes_for_subject_and_permission(
418419
if permission in role.permissions and role_assignment.scope not in scopes:
419420
scopes.append(role_assignment.scope)
420421
return scopes
422+
423+
424+
def unassign_subject_from_all_roles(subject: SubjectData) -> bool:
425+
"""Unassign a subject from all roles across all scopes.
426+
427+
Args:
428+
subject: The SubjectData object representing the subject to unassign.
429+
430+
Returns:
431+
bool: True if any roles were removed, False otherwise.
432+
"""
433+
enforcer = AuthzEnforcer.get_enforcer()
434+
return enforcer.remove_filtered_grouping_policy(GroupingPolicyIndex.SUBJECT.value, subject.namespaced_key)

openedx_authz/api/users.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
get_subject_role_assignments_in_scope,
2323
get_subjects_for_role_in_scope,
2424
unassign_role_from_subject_in_scope,
25+
unassign_subject_from_all_roles,
2526
)
2627

2728
__all__ = [
@@ -36,6 +37,7 @@
3637
"is_user_allowed",
3738
"get_scopes_for_user_and_permission",
3839
"get_users_for_role_in_scope",
40+
"unassign_all_roles_from_user",
3941
]
4042

4143

@@ -223,3 +225,15 @@ def get_scopes_for_user_and_permission(
223225
UserData(external_key=user_external_key),
224226
PermissionData(action=ActionData(external_key=action_external_key)),
225227
)
228+
229+
230+
def unassign_all_roles_from_user(user_external_key: str) -> bool:
231+
"""Unassign all roles from a user across all scopes.
232+
233+
Args:
234+
user_external_key (str): ID of the user (e.g., 'john_doe').
235+
236+
Returns:
237+
bool: True if any roles were removed, False otherwise.
238+
"""
239+
return unassign_subject_from_all_roles(UserData(external_key=user_external_key))

openedx_authz/handlers.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,14 @@
1010
from django.db.models.signals import post_delete
1111
from django.dispatch import receiver
1212

13+
from openedx_authz.api.users import unassign_all_roles_from_user
1314
from openedx_authz.models.core import ExtendedCasbinRule
1415

16+
try:
17+
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL
18+
except ImportError:
19+
USER_RETIRE_LMS_CRITICAL = None
20+
1521
logger = logging.getLogger(__name__)
1622

1723

@@ -48,3 +54,31 @@ def delete_casbin_rule_on_extended_rule_deletion(sender, instance, **kwargs): #
4854
instance.casbin_rule_id,
4955
exc_info=exc,
5056
)
57+
58+
59+
def unassign_roles_on_user_retirement(sender, user, **kwargs): # pylint: disable=unused-argument
60+
"""
61+
Unassign roles from a user when they are retired.
62+
63+
This handler is triggered when a user is retired in the LMS. It ensures that
64+
any roles assigned to the user are removed, maintaining the integrity of the
65+
authorization system.
66+
67+
Args:
68+
sender: The model class (User).
69+
user: The user instance being retired.
70+
**kwargs: Additional keyword arguments from the signal.
71+
"""
72+
try:
73+
unassign_all_roles_from_user(user.username)
74+
except Exception as exc: # pylint: disable=broad-exception-caught
75+
logger.exception(
76+
"Error unassigning roles from user %s during retirement",
77+
user.id,
78+
exc_info=exc,
79+
)
80+
81+
82+
# Only register the handler if the signal is available (i.e., running in Open edX)
83+
if USER_RETIRE_LMS_CRITICAL is not None:
84+
USER_RETIRE_LMS_CRITICAL.connect(unassign_roles_on_user_retirement)

openedx_authz/tests/api/test_roles.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
get_subject_role_assignments_in_scope,
3636
get_subjects_for_role_in_scope,
3737
unassign_role_from_subject_in_scope,
38+
unassign_subject_from_all_roles,
3839
)
3940
from openedx_authz.constants import permissions, roles
4041
from openedx_authz.constants.roles import (
@@ -976,3 +977,156 @@ def test_assign_role_creates_extended_casbin_rule(self):
976977
self.assertIn(role_data.namespaced_key, extended_rule.casbin_rule_key)
977978
self.assertIn(subject_data.namespaced_key, extended_rule.casbin_rule_key)
978979
self.assertIn(scope_data.namespaced_key, extended_rule.casbin_rule_key)
980+
981+
982+
@ddt_data(
983+
# Test user with single role in single scope
984+
("alice", ["lib:Org1:math_101"], {"library_admin"}),
985+
# Test user with multiple roles in different scopes
986+
(
987+
"eve",
988+
["lib:Org2:physics_401", "lib:Org2:chemistry_501", "lib:Org2:biology_601"],
989+
{"library_admin", "library_author", "library_user"},
990+
),
991+
# Test user with same role in multiple scopes
992+
("liam", ["lib:Org4:art_101", "lib:Org4:art_201", "lib:Org4:art_301"], {"library_author"}),
993+
# Test user with multiple different roles in multiple scopes
994+
(
995+
"peter",
996+
["lib:Org6:project_alpha", "lib:Org6:project_beta", "lib:Org6:project_gamma", "lib:Org6:project_delta"],
997+
{"library_admin", "library_author", "library_contributor", "library_user"},
998+
),
999+
)
1000+
@unpack
1001+
def test_unassign_subject_from_all_roles_removes_all_assignments(self, subject_name, scopes, expected_roles_before):
1002+
"""Test that unassign_subject_from_all_roles removes all role assignments.
1003+
1004+
Expected result:
1005+
- Before unassignment: Subject has roles in specified scopes
1006+
- Function returns True indicating roles were removed
1007+
- After unassignment: Subject has no role assignments in any scope
1008+
- Querying role assignments returns empty list
1009+
"""
1010+
subject = SubjectData(external_key=subject_name)
1011+
1012+
# Verify the subject has roles before unassignment
1013+
assignments_before = get_subject_role_assignments(subject)
1014+
self.assertGreater(len(assignments_before), 0)
1015+
1016+
# Verify roles are what we expect before removal
1017+
roles_before = {r.external_key for assignment in assignments_before for r in assignment.roles}
1018+
self.assertEqual(roles_before, expected_roles_before)
1019+
1020+
# Verify assignments exist in each expected scope
1021+
for scope_name in scopes:
1022+
scope_assignments = get_subject_role_assignments_in_scope(subject, ScopeData(external_key=scope_name))
1023+
self.assertGreater(len(scope_assignments), 0)
1024+
1025+
# Unassign all roles from the subject
1026+
result = unassign_subject_from_all_roles(subject)
1027+
1028+
# Verify the function returns True (indicating roles were removed)
1029+
self.assertTrue(result)
1030+
1031+
# Verify the subject has no role assignments after unassignment
1032+
assignments_after = get_subject_role_assignments(subject)
1033+
self.assertEqual(len(assignments_after), 0)
1034+
1035+
# Verify no assignments in any of the previous scopes
1036+
for scope_name in scopes:
1037+
scope_assignments = get_subject_role_assignments_in_scope(subject, ScopeData(external_key=scope_name))
1038+
self.assertEqual(len(scope_assignments), 0)
1039+
1040+
def test_unassign_subject_with_no_roles_returns_false(self):
1041+
"""Test that unassigning a subject with no roles returns False.
1042+
1043+
Expected result:
1044+
- Function returns False when subject has no role assignments
1045+
- No errors occur when trying to unassign from non-existent subject
1046+
"""
1047+
non_existent_subject = SubjectData(external_key="user_with_no_roles")
1048+
1049+
# Verify the subject has no roles
1050+
assignments_before = get_subject_role_assignments(non_existent_subject)
1051+
self.assertEqual(len(assignments_before), 0)
1052+
1053+
# Unassign all roles (should return False since there are none)
1054+
result = unassign_subject_from_all_roles(non_existent_subject)
1055+
1056+
# Verify the function returns False (no roles to remove)
1057+
self.assertFalse(result)
1058+
1059+
# Verify still no assignments after the operation
1060+
assignments_after = get_subject_role_assignments(non_existent_subject)
1061+
self.assertEqual(len(assignments_after), 0)
1062+
1063+
def test_unassign_subject_does_not_affect_other_subjects(self):
1064+
"""Test that unassigning one subject does not affect other subjects.
1065+
1066+
Expected result:
1067+
- When unassigning roles from one subject, other subjects retain their roles
1068+
- Other subjects with the same roles in the same scopes are unaffected
1069+
"""
1070+
# Use subjects that share the same scope
1071+
subject_to_unassign = SubjectData(external_key="grace")
1072+
other_subject = SubjectData(external_key="heidi")
1073+
shared_scope = ScopeData(external_key="lib:Org1:math_advanced")
1074+
1075+
# Verify both subjects have roles in the shared scope before
1076+
grace_assignments_before = get_subject_role_assignments_in_scope(subject_to_unassign, shared_scope)
1077+
heidi_assignments_before = get_subject_role_assignments_in_scope(other_subject, shared_scope)
1078+
1079+
self.assertGreater(len(grace_assignments_before), 0)
1080+
self.assertGreater(len(heidi_assignments_before), 0)
1081+
1082+
# Unassign all roles from grace
1083+
result = unassign_subject_from_all_roles(subject_to_unassign)
1084+
self.assertTrue(result)
1085+
1086+
# Verify grace has no assignments after unassignment
1087+
grace_assignments_after = get_subject_role_assignments(subject_to_unassign)
1088+
self.assertEqual(len(grace_assignments_after), 0)
1089+
1090+
# Verify heidi still has her assignments
1091+
heidi_assignments_after = get_subject_role_assignments_in_scope(other_subject, shared_scope)
1092+
self.assertEqual(len(heidi_assignments_after), len(heidi_assignments_before))
1093+
1094+
# Verify heidi still has the library_contributor role
1095+
heidi_roles = {r.external_key for assignment in heidi_assignments_after for r in assignment.roles}
1096+
self.assertIn("library_contributor", heidi_roles)
1097+
1098+
def test_unassign_and_reassign_subject(self):
1099+
"""Test that a subject can be reassigned roles after being unassigned.
1100+
1101+
Expected result:
1102+
- Subject has roles initially
1103+
- After unassignment, subject has no roles
1104+
- Subject can be assigned new roles
1105+
- Newly assigned roles work correctly
1106+
"""
1107+
subject = SubjectData(external_key="bob")
1108+
new_scope = ScopeData(external_key="lib:Org1:new_library")
1109+
new_role = RoleData(external_key="library_admin")
1110+
1111+
# Verify bob has roles initially
1112+
assignments_before = get_subject_role_assignments(subject)
1113+
self.assertGreater(len(assignments_before), 0)
1114+
1115+
# Unassign all roles
1116+
result = unassign_subject_from_all_roles(subject)
1117+
self.assertTrue(result)
1118+
1119+
# Verify no roles after unassignment
1120+
assignments_after_unassign = get_subject_role_assignments(subject)
1121+
self.assertEqual(len(assignments_after_unassign), 0)
1122+
1123+
# Assign a new role in a new scope
1124+
assign_result = assign_role_to_subject_in_scope(subject, new_role, new_scope)
1125+
self.assertTrue(assign_result)
1126+
1127+
# Verify the new assignment works
1128+
new_assignments = get_subject_role_assignments_in_scope(subject, new_scope)
1129+
self.assertEqual(len(new_assignments), 1)
1130+
1131+
new_roles = {r.external_key for assignment in new_assignments for r in assignment.roles}
1132+
self.assertIn("library_admin", new_roles)

0 commit comments

Comments
 (0)