Unpublished DvObjects' cards don't show for IpGroups contained within a group that has permission to view them.

[michbarsinai]

Given a dataverse that:

1. Contains unpublished Datasets
2. Has an explicit group with Admin role (call that eg)
3. eg has an IpGroup as a member (we'll call that ipg)

When a user logs in from an IP address that's a member of ipg, she does not see the card of the unpublished dataset. Note that when the role is assigned to ipg directly, these cards are shown.

[michbarsinai]

@kcondon - is that right?

[kcondon]

Yes, that's right, thanks

[pdurbin]

Yesterday I replicated the problem (IP groups within explicit groups are not honored) but I'm blocked on having a new method to call to collects even more groups. From chatting with @michbarsinai in Slack it sounds like one should supply a RoleAssignee rather than a User to the method. My preference would be to have a working method first and then refactor all the search code as necessary. @michbarsinai can you supply the method? Unfortunately, I'm out until the first of September after today.

[pdurbin]

@michbarsinai as of 18e1037 in the 1380-honor-ip-groups this is how the method looks in GroupServiceBean that would need to be enhanced:

    public Set<Group> groupsFor( DataverseRequest dr ) {
        Set<Group> groups = new HashSet<>();

        // get the global groups
        groups.addAll( groupsFor(dr,null) );

        // add the explicit groups
        groups.addAll( explicitGroupService.findGroups(dr.getUser()) );

        return groups;
    }

That is to say, this is the method that SearchServiceBean relies on to gather as many groups as possible for the DataverseRequest in question.

Here's a handy link to the method above:

dataverse/src/main/java/edu/harvard/iq/dataverse/authorization/groups/GroupServiceBean.java

Line 138 in 18e1037

public Set<Group> groupsFor( DataverseRequest dr ) {

Also related to "groups within groups" is this issue about MyData: #3056

[pdurbin]

Oh, my other thought on this issue to simply stop supporting "groups within groups". They're a little buggy as currently implemented and I wonder if they're more trouble than they're worth.

[djbrooke]

@michbarsinai - we are trying to get the IP Group fixes in 4.5.1 but this method update is blocking this issue. What are your thoughts? Do you have time to work on it or should we take a stab at it on this side? Thanks!

[michbarsinai]

@djbrooke I was just talking (slacking?) with @pdurbin about this. I need to implement a method, will get it done by early next week.

[djbrooke]

@michbarsinai awesome! Thanks much.

[michbarsinai]

Method implemented. As the groupsFor method is used twice, and I didn't want to break existing usages, I've added another method to the GroupServiceBean, called flattenGroupsCollection. Given a collection of groups, this method returns a stream of the groups in the collection and all their group descendants.

See unit test for sample usage. In the context of this issue, I think it would be enough to replace groupsFor(X) with flattenGroupsCollection(groupsFor(X)).

Unit test: https://github.com/IQSS/dataverse/blob/1380-honor-ip-groups/src/test/java/edu/harvard/iq/dataverse/authorization/groups/GroupServiceBeanTest.java#L65

[pdurbin]

@michbarsinai I'm having good luck with the new collectAncestors method you added in 1c1c60b. Thanks! Unlike before, the following scenario now works:

• scripts/api/data/ipGroup-all.json is used to create an IP group that matches all IP address (in the GUI you can find this group as &ip/ipGroup3 - "IP group to match all IPv4 and IPv6 addresses")
• in an unpublished subdataverse, create an explicit group and add the IP group above as a member. This is a "group within a group"
• assign the expicit group to the subdataverse with the admin role
• confirm that cards the unpublished subdataverse and unpublished direct child datasets and their files are viewable anonymously
• revoke that role and assign it at an unpublished dataset instead to confirm that the unpublished dataset card and its files are viewable anonymously

@michbarsinai there's still an issue the branch we're working on is behind "develop". Unfortunately, there are merge conflicts so pull request #3103 can't be merged. I'll take a swing at getting the branch up to date with "develop" and I'll ping you if I'm unclear about how to handle any of the merge conflicts.

[pdurbin]

Ok, resolving the merge conflict was pretty thorny and resulting in me opening #3340 and leaving a long comment in 80268c3 to explain what I did but both and before and after the merge this issue seems to have been fixed. Please see my last comment about "groups within groups" for how I've been testing. Passing to QA.

[kcondon]

OK, tested and it works now.