DSpace icon indicating copy to clipboard operation
DSpace copied to clipboard

Different implementations of "special group" mapping should be factored (and redesigned?)

Open mwoodiupui opened this issue 3 years ago • 5 comments

Describe the bug Most of DSpace's authentication providers implement a mapping from some property of a session to DSpace groups. Each is configured differently, and each configuration is cumbersome. We may be able to consolidate the code for handling these mappings. We should be able to make the mappings easier to understand and configure.

Expected behavior

  • If possible, session-specific permissions should be configured in the same way regardless of the provider.
  • Role mapping should be easier and less error-prone.

Related work Link to any related tickets or PRs here.

mwoodiupui avatar Jul 14 '22 18:07 mwoodiupui

The following conversation is copied from Slack.

Wayne Kool Jul 12th at 16:47 I’m hoping there is someone out there that has some insight into the following question. We are running DSpace 7.3 with Open ID Connect (OIDC) Authentication setup. This is working great for authentication. The problem that we are trying to solve is being able to pull the user’s groups from oidc scopes. The property authentication-oidc.scopes = openid, email, profile, does not seem to support groups. Is groups something that can be mapped into DSpace in 7.x? If one wanted to be able to have these groups mapped to internal DSpace groups, is there any suggestions that one can offer?

A bit of why this is necessary for us… We need the ability to implement a “Need To Know” feature that determines if a logged-in user is a member of certain AD Groups. If the user is in these groups, they should be granted access. If not, then no access is granted. These groups are managed outside of DSpace by a completely different organization. The Admins of DSpace will not be putting people in/out of these groups, but simply passing along the groups to DSpace to see if the user is in the group or not. Any assistance on figuring out how we can accomplish this would be welcome. (edited)

Tim Donohue 1 day ago @Wayne Kool: I think the quick answer is that the OIDC plugin doesn't yet support group mapping (at least I don't see it mentioned in the docs): https://wiki.lyrasis.org/display/DSDOC7x/Authentication+Plugins#AuthenticationPlugins-OpenIDConnect(OIDC)Authentication

There are other auth plugins that do support group mapping (like LDAP and Shibboleth -- both of those have configs which can map their groups/roles into DSpace groups)

I think it's completely logical that OIDC should support this mapping...it just doesn't appear to be built yet. It seems like it might be possible to look at how it is implemented for LDAP or Shibboleth and see if a similar concept could be achieved in OIDC. (I don't personally know enough about OIDC to provide great advice here, but maybe others do)

Wayne Kool 1 day ago @tdonohue, that’s what I suspected, as well. I did see LDAP and Shibboleth, but we really need OIDC to have this. I am in the same boat about not knowing enough about the implementation of OIDC, but willing to learn . If anyone else has some pointers on the best place to start on such an effort — or if anyone has actually written something… that would be fabulous. Even a bit of direction on what classes to look at to get a closer landing zone on where to begin would be great.

Tim Donohue 1 day ago @Wayne Kool: At a basic level, all the auth plugin classes are available here: https://github.com/DSpace/DSpace/tree/main/dspace-api/src/main/java/org/dspace/authenticate

I'm not sure if the ShibAuthentication or the LDAPAuthentication class would be more "similar" to OIDC, but those might be worth looking at.

Essentially, there's no real way to "inherit" this group mapping across all authentication plugins at this time... each plugin must implement their own group mapping concept, and it appears that OIDC just doesn't have one yet.

Tim Donohue 1 day ago (It is also possible someone else has done this or is looking into this same thing... I don't know the answer to that, but hopefully someone else will respond here if so)

Wayne Kool 1 day ago Another opportunity to excel, as they say. If someone has been looking at this, I’d love to exchange notes. Who knows, maybe we can contribute back and make it available to everyone. Thanks for the quick reply, Tim. (edited)

Tim Donohue 1 day ago If you do build it, please do contribute! I think others will want this.

IIRC, this OIDC auth feature was initially built by 4Science, so they might be a resource here (or you might be able to hire them to add the feature & give it back) (edited)

Wayne Kool 1 day ago I’m definitely interested in this. I’ll check out 4Science.

Kim Shepherd 1 day ago on this note, if we wanted something more generic that other auth plugins could take advantage of as well, perhaps we should look at a more generic "eperson metadata --> group" mapping? haven't fully thought through all the implications there, it just occurred to me..

Wayne Kool 1 day ago This is certainly a topic that warrants further investigation. In my world, we have disparate applications where Active Directory groups are used to manage roles across the organization. In other words, central control of groups in AD just makes a lot of sense for being able to determine who can do what in real time. If someone leaves a certain role, the application that DSpace serves needs to know about that change right away… so as to not allow that user access to files they should no longer see. For us, DSpace is serving as a repository for items where “Need to Know” access is paramount.

Kim Shepherd 22 hours ago yes, for sure, and mapping user to special groups at time of login is a use case that's been supported for a long time. Extending the OIDC plugin shouldn't be too much work, i was just further wondering if the special group handling itself could be made more generic in a way that simplifies how the auth plugins work... quickest solution is definitely looking to change OIDC plugin to do similar special group mapping that Shib and LDAP do

Mirko Grothe 21 hours ago Maybe we should look into supporting synchronization of LDAP groups as well, that would be much more scalable and useful in big organizations than special group mapping via config files.

Kim Shepherd 21 hours ago that's true, too, as long as you trusted the data, there could be some dynamic group mapping (create if doesn't already exist) with a special prefix or something

Wayne Kool 21 hours ago Something that manages the automatic inclusion / eviction of these groups for the E-Person upon login. Upon expiration of user session, a refresh of these groups that are associated to the User within DSpace would be ideal.

Mirko Grothe 20 hours ago Other software, I'm thinking of Confluence, periodically synchronize all LDAP users and groups within the configured scope. That would be my wish for DSpace 

Wayne Kool 20 hours ago Lots of interest around this topic.

Tim Donohue 20 hours ago I think the core question here is whether DSpace should replicate all this data. It could. Currently, the approach has been more "lightweight". DSpace literally just "look ups" your current LDAP (or auth system) groups at the point when you login to DSpace. Then it uses those to decide which permissions you get in DSpace (via that mapping to DSpace groups).

This lightweight approach means that LDAP (or auth system) remains the "point of record". DSpace doesn't ever have outdated info "cached" in it, as it just looks up your permissions when it needs them.

In any case, not against it...just pointing out why things are designed how they are designed.

Tim Donohue 20 hours ago (This approach also means that if you have permissions removed from LDAP, they are immediately revoked in DSpace as well. There's no cache that needs re-synced or updated.) (edited)

Tim Donohue 20 hours ago That said, I completely agree the current approach of managing all this in a config file is a pain. It should somehow be manageable from the Admin UI someday

Mark Wood 1 hour ago Looks like we still don't do a good enough job of explaining that special-group membership is a property of the session, not of the user.

Mark Wood 1 hour ago Here I would prefer that DSpace not cache "foreign" authorization data. I would expect that most ADS- or LDAP-based applications work this way.

Mark Wood 1 hour ago I suspect that a security audit would frown on caching group memberships.

Tim Donohue 36 minutes ago I admit, I also prefer the current approach (of using session-based "special groups") instead of copying all the data into DSpace. However, I fully agree that it could be easier to manage/maintain (managing role to DSpace group mappings in a config file is a pain). I don't have a better solution in mind myself, but I'd welcome folks brainstorming on it and bringing back ideas

Wayne Kool 25 minutes ago I couldn’t agree more. Caching of outside groups is not a good thing from a security perspective. Being able to easily add groups to the session with OIDC and/or any other method is key. I’m just in a “newbie” place with some of this, as I do not have a deep understanding of how OIDC does this. So, looking at the existing way this is done with ShibAuthentication or LDAPAuthentication and applying it to to the OIDCAuthentication plugin is the challenge at the moment. It’s not exactly turn key

Mark Wood 22 minutes ago Sharing a dab of research, because I was curious: the Shibboleth authentication provider is probably the one to study. LDAP and Password implement a single "special" group each, to mark sessions that were authenticated by those providers. LDAP wants custom code to do more than that.

One reason that the current mapping is awful is that it is such a mismatch to the Properties structure, which can't represent maps or arrays and causes much typing and parsing to simulate them. Configuring these using Spring would be more natural and cheaply done, but some folks are allergic to XML. These data would be a good fit to a relation, but that will require a user interface.

Wayne Kool 16 minutes ago Down the road it would be a “nice to have” for an interface. In our use case, this mapping is critical for our needs. We need to present ALL results to users of what items are available in the system — not just the items where they might have permission. We call this feature “NTK or Need to Know”. Of course, we would not want an abstract to contain anything that is not consumable to anyone that does not have permission, but the metadata, title and existence that there is an item in the system is required. A user can then request access to that item by going through a process to be added to the correct groups. This is the problem that we are trying to solve. Those groups are managed in Active Directory and are relevant to other systems, as well. So, being able to read-in what groups someone belongs to and map those to DSpace groups for permissions would fix this for us.

Mark Wood 15 minutes ago Hm, maybe the comments in authentication-ldap.cfg are outdated: further down there is a lot of material about authentication-ldap.login.groupmap.N. I didn't see it because I grepped for "special" rather than "group". authentication-ip.cfg also does mapping from specific IP ranges into DSpace groups.

Mark Wood 13 minutes ago @kshepherd’s point is well taken: there is probably enough regularity across authentication providers' use of role mapping that we can factor it out to a common format.

mwoodiupui avatar Jul 14 '22 18:07 mwoodiupui

I support this idea of refactoring "special group" logic in authentication plugins to be more "shared code". But, I'm not sure how to make this actionable yet... so, I've flagged as "needs discussion" for now.

tdonohue avatar Jul 20 '22 15:07 tdonohue

[tangent] If the eventual changes in configuration are significant enough, it would be an opportunity to replace the IMHO misleading term "special groups." There's nothing special about these groups; what's unusual is how membership is reckoned. I confess that I haven't thought of a compelling replacement. But people do get confused about the ephemeral nature of "special group" membership.

I'm happy to discuss this tangent if anyone else cares, but I won't continue arguing here if no one does.

mwoodiupui avatar Jul 21 '22 11:07 mwoodiupui

@mwoodiupui : Per your tangent, I've often wondered myself if "special groups" should be renamed something like "session-based groups" or something similar. That'd at least be more accurate, as your membership to that group is only for your current "session". (I do agree that the term "special groups" has no purposeful meaning)

tdonohue avatar Jul 21 '22 13:07 tdonohue

Thoughts on names: The important aspects are that these Group memberships are assigned based on data inspected at login, and are ephemeral / session-based - I've seen people get confused in the past about why they can't see those memberships for a user that is logged out. "Session-based" could help with that latter aspect, and maybe the first? "Dynamic" was my thought for the first but doesn't make it clear that the membership only lasts for the duration of a session.

I'll think more about shared code.. maybe it turns out that there's not much code to share, if 90% of the work of each plugin is specific handling / parsing of auth provider responses that doesn't apply elsewhere

kshepherd avatar Jul 21 '22 21:07 kshepherd