Shibboleth special groups don't populate when IP authentication also used in 7.6.1

[pybrarian]

Describe the bug

When using IP-based groups assignment along with Shibboleth groups assignment, the Shibboleth groups do not get applied. This seems to be related to the if condition introduced in PR #9130 for dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java.

Logs show the getSpecialGroups method of dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java only returning the cached special groups, and when investigated the cached special groups are only the ones from IP authentication. It never reaches the "Starting to determine special groups" log line to apply the Shibboleth-configured groups.

I don't know the code or authentication flows super well, but my best guess is that when dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java does the check to make sure the login type aligns with the context, it causes the Shibboleth groups to be applied later in the order, after the IP authentication groups have already been added to the context. Then in dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java the check on line 296 can only check if there's anything in the specialGroups in the context object, but doesn't know what authentication plugin put the special groups there to know if the Shibboleth plugin has already applied groups. It sees the groups from IP authentication and just returns the cached groups.

If I turn off the IPAuthentication plugin, the Shibboleth groups get applied as expected. I am unsure if the same applies to password or other forms of Authentication in relation to IP Authentication.

We have not experienced this issue with 7.6 or earlier 7.x versions of DSpace.

To Reproduce

Steps to reproduce the behavior:

1. In DSpace 7.6.1, configure groups to be added by the IP Authentication Plugin and the Shibboleth Authentication plugin
2. Log in with Shibboleth from an IP address configured to apply a special group
3. Check groups in user's profile

Expected behavior

Groups from both IP Authentication and Shibboleth authentication would be added to the user.

Related work

Seems to be related to:
Issue #9127
PR #9130

[tdonohue]

Thanks @pybrarian . I haven't had a chance to verify this, but it does sound like either a possible bug, or some sort of misconfiguration. @LucaGiamminonni pinging you as you were involved in the but fix in #9130 in case you have any ideas of what the problem may be.

[pybrarian]

I've been investigating this more and think I'm close to a fix, but I don't understand everything that is happening with authentication so would appreciate input from folks more familiar with the whole picture. It looks to me like Shib is the only authentication method affected by this.

In this commit replacing session based auth, an object that explicitly tracked shib special groups was replaced with querying the Context object, which stores just the special groups but not the method used to set them. So I think my hunch above was correct as far as it goes.

I added some code to the Context object to track whether an authentication method has been used to set special groups and augmented the check on line line 296 of ShibAuthentication.java to check whether the Shib authentication method had been used yet to set special groups (and set those values later in the body of the method before returning the groups). This solved my problem, I start seeing special groups from both Shibboleth and IP, but at the trade-off that it never seems to return them from the cached groups on the Context object.

This is (one of) the areas where not understanding the whole authentication picture lets me down. I assume the early return is there to avoid extra unnecessary processing, but it seems possible that it's less effective with the way DSpace handles auth in 7.x+ and could be removed.

If somebody who has a better understanding has 15 minutes to pair with me, that would be really helpful. Or if it would be better just to submit a couple of PRs (one with the additions to the Context method, one with the context check just taken out of ShibAuthentication) and let folks look at it, I can do that too.