[DS-652] Wrong behaviour of special groups at login. Use only special groups of the authetication that DID authenticate the user.

[dspace-bot]

Imported from JIRA [DS-652] created by nuncanada

We have internal users autheticating thru LDAP. And external users are still able to create new users.

Unfortunally the authentication is putting all external users created thru the Login Authentication also in the ldap.login.specialgroup, of course that is not expected.

Looking at code at AuthenticationManager it becomes clear that it is adding ALL the special groups of ALL the possible authentication mechanism, which doesnt make any sense whatsoever...

It should only add special groups of the authentication mechanism that DID authenticate the user.

[dspace-bot]

sandsfish said:

This mechanism allows for implicit authentication methods (e.g. IPAuthentication) to add certain groups based on the characteristics of the authentication (i.e. which IP range the request came from).

So there are use-cases where it makes sense, though I don't know that there isn't an argument for enabling/disabling this.

[dspace-bot]

nuncanada said:

I do not follow why the wrong behaviour is necessary in your case. I am not proposing for the feature to disappear...
Unless you are using this as a hack to get multiple groups inserted for a user? The right solution would be to be able to insert multiple groups for each authentication method.

[dspace-bot]

nuncanada said:

Patch to fix the behaviour of the special groups during authentication.
Now only the special group related to that specific authentication will be added to the session. (It's trivial to change it to accept multiple special groups for each authentication method)

The only part left untouched is how to deal with the loginAs part... It should need UI changes so the admin is able to choose also specialGroups related to specific authentication mechanisms so he/she can have the same exact view as the user.

[dspace-bot]

tdonohue said:

Flavio,

I think this change will need more discussion amongst the developers/committers, as it seems to be changing how the 'special groups' feature was actually meant to work. The 'special groups' feature was originally designed so that special groups could be added even without authentication.

As a basic example, this is a feature specifically useful to Libraries who want to provide special access rights to their walkup patrons (i.e. users who are accessing the site directly from a Library computer). In that scenario, I know of several institutions who want patrons at a Library computer to have special access rights to materials in DSpace.

So, here's a more specific example:

• Suppose your DSpace installation contains several works (perhaps Theses or Dissertations) that are only accessible to people at your University.
• In DSpace, you'd create a special group called something like "University Users". People should be added to this group in one of several specific scenarios:
  (1) If the user is able to login via your Campus authentication system (LDAP or similar) they should be auto-added to this special group
  (2) If someone is accessing DSpace from a specific range of IP addresses (e.g. a computer in your Library) they should be auto-added to this group even if they are not authenticated. This would allow your local users to get automatic access to research from a Library computer.

As you can see, #2 above isn't possible unless you allow people to be added to Special Groups even if the authentication plugin didn't truly authenticate the user.

I wonder if there may be another way around the problem you are seeing? It looks like the LDAPAuthentication.getSpecialGroups() method is specifically trying to make sure it doesn't add anonymous or non-LDAP users (users not having a 'netID' stored in DSpace) to the 'ldap.login.specialgroup'. I'm trying to determine why this doesn't seem to work in your scenario (and if it doesn't, maybe there's a bug in the LDAPAuthentication.getSpecialGroups() method that can fix your problem in an easier way). Perhaps we can help you figure out a better workaround if you can describe which Authentication plugin(s) you are using and what your configurations are in dspace.cfg for those plugins.

[dspace-bot]

nuncanada said:

Right now we use PasswordAuthentication for external users, but we do
not want them to have access to some special collections only internal
users should.
They are being added to the special collection group by the current
code where we configured only ldap.login.specialgroup.

The IPAuthentication is a separate authentication mechanism so with my
patch in place your problem will also be easily solved.
Just specify the same special group with read access to the special
collection to both the ldap.specialgroup and to the
authentication.ip.SPECIAL_GROUP = iprange[, iprange ...].

The only use-case where a problem could appear was if you you were
using the current wrong behaviour to assign multiple groups to the
authenticated users, something that can be trivially adapted (10 min
work) in the patch (and current) code for DSpace to be able to handle
multiple groups for each authentication mechanism.

[dspace-bot]

tdonohue said:

Flavio,

I still feel your patch seems to be changing more than it really should be.

Although IPAuthentication is separate, your patch now requires that an Authentication Plugin return 'AuthenticationMethod.SUCCESS' in order for Special Groups to be added. The IPAuthentication never returns SUCCESS as it doesn't actually do authentication, it only serves to add you to special groups if you are in an IP range. So, based on your code changes, the IPAuthentication plugin will no longer work (as far as I can tell). Also, if we changed IPAuthentication to return SUCCESS, we'd need to make sure that didn't cause other issues in the AuthenticationManager (as SUCCESS is supposed to only be returned when you truly are logged into the system).

So, what you may be misunderstanding is that anonymous users can also sometimes be added to Special Groups. The primary example of that is IPAuthentication which will specifically add any anonymous users to special group(s) if they are in a given IP range. So, these users are never truly "logged into" the system (i.e. they cannot see a user 'profile' page or submit a new Item or anything else that requires an actual login), but they may be given special viewing/access rights based on their IP range.

So, that's why I was wondering if there's actually a bug in LDAPAuthentication that is the cause of your problems. Obviously, logging in via PasswordAuthentication should not add you to the ldap.login.specialgroup. I agree with that, and if that's what is happening, we need to figure out why the LDAPAuthentication.getSpecialGroups() method isn't working properly for you. That method should not be adding anyone who wasn't authenticated via LDAP, but it obviously must be for you.

[dspace-bot]

stuartlewis said:

LDAPAuthentication getSpecialGroups looks like it should work ok, as it contains the following check before setting the special groups:

if (!context.getCurrentUser().getNetid().equals(""))

The only problem might be if a user has authenticated with a method other than LDAP where the netid is used and set.

[dspace-bot]

nuncanada said:

if (!context.getCurrentUser().getNetid().equals(""))

I am not at work right now to check our database, but this code might not be working because getNetid may return a null instead of an empty string?

[dspace-bot]

nuncanada said:

Nevermind. If it were null a NullPointerException would be thrown.

[dspace-bot]

stuartlewis said:

Hi Flávio,

That's right - this is caught, and is taken to show that they do not have a netid set. It isn't the prettiest of code, but should work.

catch (Exception npe) {
// The user is not an LDAP user, so we don't need to worry about them
}
return new int[0];

[dspace-bot]

nuncanada said:

The problem is we have a pretty old version of the code it seems (or i mangled something during merging).

[dspace-bot]

tdonohue said:

Hi Flavio,

If the problem seems to be in your older version of the code, are there any other questions you have about this issue?

Does the explanation around how special groups work make sense? Is there anything else that needs to be answered/discussed, or should we close out this issue?

[dspace-bot]

nuncanada said:

This issue should be closed.

[dspace-bot]

tdonohue said:

Closing this issue and marking as "Answered". No fixes were necessary, but several questions on how special groups work were answered (see above comments).