Skip to content
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

Refactoring methods in LdapClient and Authorizer #118

Closed
Tracked by #116
longshuicy opened this issue May 5, 2023 · 0 comments · Fixed by #133
Closed
Tracked by #116

Refactoring methods in LdapClient and Authorizer #118

longshuicy opened this issue May 5, 2023 · 0 comments · Fixed by #133
Assignees
Labels
5storypoints Between 16-24hrs, may involve chainings, new pipelines, very complex models
Milestone

Comments

@longshuicy
Copy link
Member

longshuicy commented May 5, 2023

Backwards tracing:

1. No need for this LdapClient to exist anymore

In this file:

2. add additional userGroups parameter to private List<Space> getSpacesUserCanRead(List<Space> spaces, String username) {

add additional parameter List<String> userGroups

3.add additional userGroups parameter to private boolean canUserAccessSpace(Space space, String username, PrivilegeLevel privilegeLevel) {

add additional parameter List<String> userGroups

Further pass in that userGroups to getGroupSpecificPrivileges(username, space.getPrivileges())
e.g.
https://github.com/IN-CORE/incore-services/blob/04c33a1ce6ca6e99b2cb88de61e5b8883a3b6c12/server/incore-common/src/main/java/edu/illinois/ncsa/incore/common/auth/Authorizer.java#LL303C19-L303C19

4. modify getGroupSpecificPrivileges(username, space.getPrivileges())

private Set<PrivilegeLevel> getGroupSpecificPrivileges(String user, Privileges spec, List<String> userGroups) {
...
//            Set<String> userGroups = ldapClient.getUserGroups(user);

Commented out the part that fetch userGroups from ldap

5. modify getGroupSpecificPrivileges(username, space.getPrivileges()) continue

if (userGroups.contains(System.getenv("AUTH_LDAP_VIEW_ALL"))) {

This should change to check if incore_admin or incore_ncsa exist in the group or not; if exist, consider to grant them the same view all privileges

6. modify public boolean isUserAdmin(String username) {

pass List<String> userGroups to that function
No need to check LDAP but instead check for the same incore_admin and/or incore_ncsa

return userGroups.contains(System.getenv("AUTH_LDAP_ADMINS"));

7. pass List<String> userGroups to public Set<PrivilegeLevel> getPrivilegesFor(String user, Privileges spec) {

8. pass List<String> userGroups to canRead, canWrite, canDelete etc

9. wire in the groups info from every single controller. Documented in another task

@longshuicy longshuicy added the 2storypoint Between 2-6 hours of work, requiring email and/or a brief meeting label May 5, 2023
@longshuicy longshuicy added this to the 1.14.0 milestone May 5, 2023
@longshuicy longshuicy added 5storypoints Between 16-24hrs, may involve chainings, new pipelines, very complex models 3storypoints Between 7-15 hours of work, requiring back and forth communications to clarify a complex problem and removed 2storypoint Between 2-6 hours of work, requiring email and/or a brief meeting 5storypoints Between 16-24hrs, may involve chainings, new pipelines, very complex models 3storypoints Between 7-15 hours of work, requiring back and forth communications to clarify a complex problem labels May 5, 2023
@Rashmil-1999 Rashmil-1999 linked a pull request Jun 5, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5storypoints Between 16-24hrs, may involve chainings, new pipelines, very complex models
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants