-
Notifications
You must be signed in to change notification settings - Fork 301
Bugfix on ACLs #1242
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
Bugfix on ACLs #1242
Conversation
tomchop
commented
Mar 27, 2025
- Return all ACLs by default (useful when checking)
- Have an option to only return default ACLs (useful when editing)
- Add a test
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.
Pull Request Overview
This PR fixes ACL handling by returning all ACLs by default while adding an option to retrieve only default ACLs, and includes new tests to verify the behavior.
- Updated test data and added a new test (test_get_acls) in tests/schemas/rbac.py
- Modified ACL retrieval in core/web/apiv2/rbac.py and core/schemas/model.py to support a "direct" option
- Updated dfiq logic in core/schemas/dfiq.py to initialize ACLs via rbac.set_acls
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/schemas/rbac.py | Updated user, group, and entity names; added a new test to verify ACL retrieval |
core/web/apiv2/rbac.py | Modified get_acl_details to retrieve ACLs using the new direct parameter |
core/schemas/model.py | Updated get_acls to support a direct parameter, affecting the traversal logic |
core/schemas/dfiq.py | Added call to rbac.set_acls during dfiq object processing to ensure ACLs are set |
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.
one small nit other than that LGTM.
@@ -96,18 +96,19 @@ def __init__(self, **data): | |||
def acls(self): | |||
return self._acls | |||
|
|||
def get_acls(self) -> None: | |||
def get_acls(self, direct=False) -> None: | |||
"""Returns the permissions assigned to a user. | |||
Args: | |||
user: The user to check permissions for. |
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.
Looks like Args documentation are no longer relevant