-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: implement org retrieval for user roles in authz compatibility layer #38216
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
base: master
Are you sure you want to change the base?
Changes from all commits
644258a
daf6196
30ffbe1
6cfd1b5
88619e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -532,9 +532,13 @@ def _authz_get_orgs_for_user(self, user) -> list[str]: | |
| Returns a list of org short names for the user with given role. | ||
| AuthZ compatibility layer | ||
| """ | ||
| # TODO: This will be implemented on Milestone 1 | ||
| # of the Authz for Course Authoring project | ||
| return [] | ||
| role = get_authz_role_from_legacy_role(self._role_name) | ||
| assignments = authz_api.get_user_role_assignments_filtered( | ||
| user_external_key=user.username, | ||
| role_external_key=role, | ||
| ) | ||
| orgs = {assignment.scope.org for assignment in assignments if assignment.scope.org is not None} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have one doubt about the desired functionality for this function. My first impression was that this function should return the list of org-wide assignations. However on further inspection, I see that the old code doesn't make a distinction between CourseAccessRoles with or without course_id, and according to the docstring at CourseAccessRole definiton, org-wide assignations are the ones that have course_id as None. So just to confirm that we are all on the same understanding, this function (legacy version or new one), returns all Orgs to which the user has at least access to one course, right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, both |
||
| return list(orgs) | ||
|
|
||
| def _legacy_get_orgs_for_user(self, user) -> list[str]: | ||
| """ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
out of scope question: Is this filtering only for courses, or could it be used for libraries as well? I understand that roles are not shared between namespaces, but it worries me that we're relying on this implicit condition to not mix namespace scopes.
EDIT: although roles are specified per namespace, so it should be enough using a course role here to only get course scopes.
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.
I'll resolve my comment.
Uh oh!
There was an error while loading. Please reload this page.
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.
(non-blocking) I'm going to resurface my question because of this discussion we're having here: https://github.com/openedx/openedx-platform/pull/38199/changes#r3015158428. Is this approach also applicable here?
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, here since we are passing a role (for example,
course_auditor), it will only return course assignments (both fromCourseOverviewDataandOrgCourseOverviewGlobData). It works as you mentioned. There cannot be acourse_auditorwith assignments in libraries, only course assignments can exist. If the role is for libraries, then it will only return library assignments.