You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Manage attribute checks in addition to RBAC checks #87
In the current library's permission system, we use Bridgekeeper to implement rules that are checked at enforcement points to make decisions. Those rules are a combination of role checks with attribute/context checks.
Our current matcher only supports explicit role assignment (with the permissions of the role and with inheritance of roles).
We need a way to perform attribute/context checks at our enforcement points, ideally using Casbin.
changed the title [-]Manage custom attributes checks apart from the RBAC checks[/-][+]Manage attribute checks in addition to RBAC checks[/+]on Oct 8, 2025
Proposal:
Having those checks in a custom function for the MVP, something like:
frombridgekeeperimportrulesdefcheck_custom_conditions(**kwargs):
""" Evaluates custom conditions based on the action and contextual keyword arguments. """ifaction=="act^view_library":
# Condition 1: Global Staff (a common requirement)ifrules.is_stafforrules.is_superuser:
returnTrue# Condition 2: Custom Resource Attribute# Here we use the resource_object fetched from the scope IDif<another_condition>:
returnTrueelifaction=="act^publish_library":
...
Then register it in the enforcement and use it in our matcher.
This is with the idea of maintaining the enforcement decision in Casbin.
The problem with this in the long term is that we will have to split the information to make a decision between the policy and the specific checks in the function. A long-term suggestion would be to be able to add those conditions to the policy and have a function that only checks the conditions within the policy. Example: p, role^library_admin, act^view_library, lib^*, allow, condition_1, condition_2
Or having another group like: g4, act^view_library, condition_1, condition_2
Discarded:
Use the current bridgekeeper permissions and use inside them the Casbin enforcement: the benefits are that we are using something we already have, but the decision will remain in edx-platform and not in openedx-authz, and the Casbin enforcement.
Use the Casbin enforcement + my custom attributes checks in the enforcement points: it is easy, but we will delegate the responsibility to code, and it doesn't align with quality and clean code standards.
Hi @MaferMazu, I think it'll be easier to discuss the approach in a PR, but in the meantime, here’s my first impression. What concerns me about the matcher, and really about any matcher, is how to make it general enough so we don’t need to change the model often and risk breaking backward compatibility. In this case, I’m mainly worried about hardcoding the actions in the matcher. How could we handle that? Let me know once you have a PR ready for review!
The PR is not ready for review, but it has the info needed for testing and seeing how I picture it.
PR: #101
Commit with the changes: 905e51c
About your concerns, @mariajgrimaldi, for the MVP, we only have 3 cases:
global_staff (access to everything without explicit permission)
the creta_library permission (that is not in the policy because it is not part of a library role, and we check course_creator)
the view_library (that checks if the library has a bool attribute)
My concern with my approach is that we will have the policy split between the policy file and the custom_function, which is not ideal. However, it is the easiest approach I found to fulfill our needs for the MVP without incurring a lot of technical debt.
To improve this in the future, the idea is to include those "special conditions" in the policy. For example (g_n, act^new_act, condition1, condition2, continion_n), and I don't think we are going to have more than a certain number of conditions. But in this ideal situation, as you mentioned, it will be difficult to think of a function able to manage those special conditions. The approach here will be to identify the special conditions we will need to cover for the current roles and permissions and try to group them (for example, the global staff can do anything, which is something we probably want to map for every permission), and definitely, we will have conditions regarding the resource/scope attribute. <- and we can try to solve this with a specific format and naming for conditions, but I think it's a conversation for the future.
For future improvements, I suggest we try to centralize the permission checking in the policy (find a way to evaluate attributes with the matcher), to avoid the permission logic spreading (it should live in the policy in some way).
Activity
[-]Manage custom attributes checks apart from the RBAC checks[/-][+]Manage attribute checks in addition to RBAC checks[/+]MaferMazu commentedon Oct 8, 2025
Proposal:
Having those checks in a custom function for the MVP, something like:
Then register it in the enforcement and use it in our matcher.
This is with the idea of maintaining the enforcement decision in Casbin.
The problem with this in the long term is that we will have to split the information to make a decision between the policy and the specific checks in the function. A long-term suggestion would be to be able to add those conditions to the policy and have a function that only checks the conditions within the policy. Example:
p, role^library_admin, act^view_library, lib^*, allow, condition_1, condition_2Or having another group like:
g4, act^view_library, condition_1, condition_2Discarded:
MaferMazu commentedon Oct 8, 2025
I'll try that, but @mariajgrimaldi @BryanttV, if you have any early feedback regarding that idea, I would appreciate it.
mariajgrimaldi commentedon Oct 9, 2025
Hi @MaferMazu, I think it'll be easier to discuss the approach in a PR, but in the meantime, here’s my first impression. What concerns me about the matcher, and really about any matcher, is how to make it general enough so we don’t need to change the model often and risk breaking backward compatibility. In this case, I’m mainly worried about hardcoding the actions in the matcher. How could we handle that? Let me know once you have a PR ready for review!
MaferMazu commentedon Oct 17, 2025
The PR is not ready for review, but it has the info needed for testing and seeing how I picture it.
PR: #101
Commit with the changes: 905e51c
About your concerns, @mariajgrimaldi, for the MVP, we only have 3 cases:
My concern with my approach is that we will have the policy split between the policy file and the custom_function, which is not ideal. However, it is the easiest approach I found to fulfill our needs for the MVP without incurring a lot of technical debt.
To improve this in the future, the idea is to include those "special conditions" in the policy. For example (g_n, act^new_act, condition1, condition2, continion_n), and I don't think we are going to have more than a certain number of conditions. But in this ideal situation, as you mentioned, it will be difficult to think of a function able to manage those special conditions. The approach here will be to identify the special conditions we will need to cover for the current roles and permissions and try to group them (for example, the global staff can do anything, which is something we probably want to map for every permission), and definitely, we will have conditions regarding the resource/scope attribute. <- and we can try to solve this with a specific format and naming for conditions, but I think it's a conversation for the future.
cc @mariajgrimaldi
MaferMazu commentedon Nov 6, 2025
The #114 help with those cases, but, we'll still need to manage:
CAN_CREATE_CONTENT_LIBRARY⚠️
Conditions: global_staff ✅ or course_creator.
CAN_VIEW_THIS_CONTENT_LIBRARY⚠️ , explicit read permissions (admin, author, read). ✅
Conditions: global staff ✅, attribute ‘allow_public_read’
MaferMazu commentedon Dec 4, 2025
For the MVP we are handling them using Bridgekeeper. Reference: https://github.com/eduNEXT/edx-platform/blob/release/ulmo/openedx/core/djangoapps/content_libraries/api/libraries.py#L797
For future improvements, I suggest we try to centralize the permission checking in the policy (find a way to evaluate attributes with the matcher), to avoid the permission logic spreading (it should live in the policy in some way).