4268 ip groups broken #4367
4268 ip groups broken #4367
Conversation
pdurbin
commented
Dec 7, 2017
•
pdurbin
commented
- connects to IP Groups: Permissions assigned to group are no longer applied except by indexing. #4268 IP Groups: Permissions assigned to group are no longer applied except by indexing
- connects to Some Question about IP Group #4386 Some Question about IP Group
Small changes to IP Group guide
eea4655 commented out the "Add permissions gained from groups" code so we are re-enabling it. FileDownloadHelper was making an assumption that a Guest user can never download a file. If you are using IP Groups, it is a desired behavior for a Guest user to be able to download a file if the user is part of an IP Group.
| // we required isSessionUserAuthenticated to return true. Now we require | ||
| // that the User is not an instance of GuestUser, which is similar in | ||
| // spirit to the previous check. | ||
| // (2) A Guest user can only download a restricted file through membership of an IP Group. | ||
| // -------------------------------------------------------------------- | ||
|
|
||
| if (session.getUser() instanceof GuestUser){ |
There was a problem hiding this comment.
Is there a need to treat guest user any differently? Just below, there is a call to check permissions (a call to a method, doesSessionUserHavePermission. I believe we should be able to change that method to use:
this.permissionService.On(objectToCheck).has(permissionToCheck);
instead of:
this.permissionService.userOn(this.session.getUser(), objectToCheck).has(permissionToCheck);
This would be a nice code cleanup.
There was a problem hiding this comment.
The discussion continues about this permission check at at #4268 (comment)
There was a problem hiding this comment.
It looks good. I appreciate the reorganized documentation and extra tests. But I have the same request/question that @scolapasta asked (in the PR, in FileDownloadHelper) - it does look like it would be cleaner, not to treat the user case in any special case at all; just comment out the "if (session.getUser() instanceof GuestUser)" part copmpletely, let it fall through - and then call permissionService.on(dvobject).has(permission) - which should cover everything, including group membership.
This fixes a bug where IP Group membership wasn't allowing authenticated users to download a file. Now both the Guest user and authenticated users can download a restricted file if they are members of an IP Group that has been granted access.
Expanded the Roles/Permissions sections of the user guide to explain the feature more clearly, particularly how groups work.
pdurbin
modified the milestones:
4.9 - Persistent Identifiers for Files,
4.8.5 - Guestbook and IP Groups fixes
