-
Notifications
You must be signed in to change notification settings - Fork 22
[wip] identity linking #1382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: clarin-v7
Are you sure you want to change the base?
[wip] identity linking #1382
Changes from 1 commit
a1b38d0
6bd3abf
64b523b
76f7b89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,7 @@ | |
| import org.dspace.eperson.factory.EPersonServiceFactory; | ||
| import org.dspace.eperson.service.EPersonService; | ||
| import org.dspace.eperson.service.GroupService; | ||
| import org.dspace.eperson.service.clarin.ClarinIdentityService; | ||
| import org.dspace.services.ConfigurationService; | ||
| import org.dspace.services.factory.DSpaceServicesFactory; | ||
|
|
||
|
|
@@ -124,6 +125,7 @@ public class ClarinShibAuthentication implements AuthenticationMethod { | |
| ClarinServiceFactory.getInstance().getClarinUserRegistration(); | ||
| protected ClarinVerificationTokenService clarinVerificationTokenService = ClarinServiceFactory.getInstance() | ||
| .getClarinVerificationTokenService(); | ||
| protected ClarinIdentityService identityService = EPersonServiceFactory.getInstance().getClarinIdentityService(); | ||
|
|
||
| /** | ||
| * Authenticate the given or implicit credentials. This is the heart of the | ||
|
|
@@ -558,7 +560,18 @@ protected EPerson findEPerson(Context context, HttpServletRequest request, Strin | |
|
|
||
| // 1) First, look for a netid header. | ||
| if (netidHeaders != null) { | ||
| eperson = findEpersonByNetId(netidHeaders, shibheaders, ePersonService, context, true); | ||
| eperson = findEpersonByNetId(netidHeaders, shibheaders, identityService, context, true); | ||
| if (eperson != null) { | ||
| foundNetID = true; | ||
| } | ||
| } | ||
|
|
||
| // 1b) CLARIN: an allowlisted identity proxy (e.g. e-INFRA CZ/Perun) may release | ||
| // voperson_external_id, listing this user's other registered external identities. | ||
| // If exactly one existing EPerson already holds one of them, auto-link this login | ||
| // to it instead of falling through to email matching / auto-registration. | ||
| if (eperson == null && netidHeaders != null) { | ||
| eperson = tryAutoLink(context, netidHeaders); | ||
| if (eperson != null) { | ||
| foundNetID = true; | ||
| } | ||
|
|
@@ -643,6 +656,37 @@ protected EPerson findEPerson(Context context, HttpServletRequest request, Strin | |
| return eperson; | ||
| } | ||
|
|
||
| /** | ||
| * Try to auto-link this login to an existing EPerson via the allowlisted proxy's | ||
| * {@code voperson_external_id} attribute (Perun's "merging by all registered external | ||
| * identities"). See {@link ClarinIdentityService#autoLink}. | ||
| * | ||
| * @return the EPerson to log into, or null if auto-linking does not apply/match. | ||
| */ | ||
| protected EPerson tryAutoLink(Context context, String[] netidHeaders) throws SQLException { | ||
| String idp = shibheaders.get_idp(); | ||
| if (!identityService.isAllowlistedProxy(idp)) { | ||
| return null; | ||
| } | ||
|
|
||
| String vopersonExternalIdHeader = configurationService | ||
| .getProperty("authentication-shibboleth.voperson-external-id-header"); | ||
| if (vopersonExternalIdHeader == null) { | ||
| return null; | ||
| } | ||
| List<String> releasedEppns = shibheaders.get(vopersonExternalIdHeader); | ||
| if (releasedEppns == null || releasedEppns.isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| String proxyNetid = getFirstNetId(netidHeaders); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why just the first one? Don't we need the full fallback dance? Maybe not, we should be pretty sure of what the proxy releases
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deliberate: the alias must record the same netid |
||
| if (proxyNetid == null) { | ||
| return null; | ||
| } | ||
|
|
||
| return identityService.autoLink(context, releasedEppns, idp, proxyNetid); | ||
| } | ||
|
|
||
| /** | ||
| * Register a new eperson object. This method is called when no existing user was | ||
| * found for the NetID or Email and autoregister is enabled. When these conditions | ||
|
|
@@ -1310,9 +1354,12 @@ public String getEmailAcceptedOrNull(String email) { | |
|
|
||
| /** | ||
| * Find an EPerson by a NetID header. The method will go through all the netid headers and try to find a user. | ||
| * Resolution goes through {@link ClarinIdentityService}'s alias table only - {@code EPerson.netid} is a | ||
| * denormalized display field that nothing at login reads. | ||
| */ | ||
|
Comment on lines
1370
to
1375
|
||
| public static EPerson findEpersonByNetId(String[] netidHeaders, ShibHeaders shibheaders, | ||
| EPersonService ePersonService, Context context, boolean logAllowed) | ||
| ClarinIdentityService identityService, Context context, | ||
| boolean logAllowed) | ||
| throws SQLException { | ||
| // Go through all the netid headers and try to find a user. It could be e.g., `eppn`, `persistent-id`,.. | ||
| for (String netidHeader : netidHeaders) { | ||
|
|
@@ -1322,7 +1369,7 @@ public static EPerson findEpersonByNetId(String[] netidHeaders, ShibHeaders shib | |
| continue; | ||
| } | ||
|
|
||
| EPerson eperson = ePersonService.findByNetid(context, netid); | ||
| EPerson eperson = identityService.resolve(context, netid); | ||
|
|
||
| if (eperson == null && logAllowed) { | ||
| log.info( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| /** | ||
| * The contents of this file are subject to the license and copyright | ||
| * detailed in the LICENSE and NOTICE files at the root of the source | ||
| * tree and available online at | ||
| * | ||
| * http://www.dspace.org/license/ | ||
| */ | ||
| package org.dspace.eperson.clarin; | ||
|
|
||
| import java.sql.SQLException; | ||
| import java.util.ArrayList; | ||
| import java.util.Date; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| import org.apache.commons.lang3.ArrayUtils; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.dspace.core.Context; | ||
| import org.dspace.eperson.EPerson; | ||
| import org.dspace.eperson.dao.clarin.EPersonNetidAliasDAO; | ||
| import org.dspace.eperson.service.EPersonService; | ||
| import org.dspace.eperson.service.clarin.ClarinIdentityService; | ||
| import org.dspace.services.ConfigurationService; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
|
|
||
| /** | ||
| * @see ClarinIdentityService | ||
| * | ||
| * @author Ondrej Kosarko | ||
| */ | ||
| public class ClarinIdentityServiceImpl implements ClarinIdentityService { | ||
|
|
||
| private static final Logger log = LogManager.getLogger(ClarinIdentityServiceImpl.class); | ||
|
|
||
| public static final String SOURCE_MIGRATION = "migration"; | ||
| public static final String SOURCE_AUTO_VOPERSON = "auto-voperson"; | ||
| public static final String SOURCE_ADMIN = "admin"; | ||
|
|
||
| @Autowired | ||
| private EPersonNetidAliasDAO ePersonNetidAliasDAO; | ||
|
|
||
| @Autowired | ||
| private ConfigurationService configurationService; | ||
|
|
||
| @Autowired | ||
| private EPersonService ePersonService; | ||
|
|
||
| @Override | ||
| public EPerson resolve(Context context, String netid) throws SQLException { | ||
| if (StringUtils.isBlank(netid)) { | ||
| return null; | ||
| } | ||
| EPersonNetidAlias alias = ePersonNetidAliasDAO.findByNetid(context, netid); | ||
| if (alias != null) { | ||
| return alias.getEPerson(); | ||
| } | ||
| // Fall back to the legacy denormalized eperson.netid column: EPersons whose netid was | ||
| // set directly (e.g. ClarinShibAuthentication#updateEPerson locking a legacy account to | ||
| // its first-seen netid) never get an alias row of their own, only pre-existing netids | ||
| // captured by the migration backfill do. Without this fallback such accounts would | ||
| // silently stop resolving by netid once login moved to alias-only lookup. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair — rewritten in plain language in 76f7b89: alias rows exist only for netids that went through |
||
| return ePersonService.findByNetid(context, netid); | ||
| } | ||
|
|
||
| @Override | ||
| public EPersonNetidAlias attach(Context context, EPerson ePerson, String netid, String source, EPerson createdBy) | ||
| throws SQLException { | ||
| EPersonNetidAlias existing = ePersonNetidAliasDAO.findByNetid(context, netid); | ||
| if (existing != null) { | ||
| if (!existing.getEPerson().getID().equals(ePerson.getID())) { | ||
| throw new IllegalStateException( | ||
| "netid '" + netid + "' is already attached to a different EPerson (" + | ||
|
Comment on lines
+69
to
+73
|
||
| existing.getEPerson().getID() + "), refusing to reattach to " + ePerson.getID()); | ||
| } | ||
| return existing; | ||
| } | ||
|
|
||
| EPersonNetidAlias alias = new EPersonNetidAlias(); | ||
| alias.setEPerson(ePerson); | ||
| alias.setNetid(netid); | ||
| alias.setSource(source); | ||
| alias.setCreatedBy(createdBy); | ||
| alias.setCreatedDate(new Date()); | ||
| return ePersonNetidAliasDAO.create(context, alias); | ||
|
Comment on lines
+69
to
+85
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged, and deliberately left as check-then-insert. The unique constraint on |
||
| } | ||
|
|
||
| @Override | ||
| public List<EPersonNetidAlias> findAliases(Context context, EPerson ePerson) throws SQLException { | ||
| return ePersonNetidAliasDAO.findByEPerson(context, ePerson); | ||
| } | ||
|
|
||
| @Override | ||
| public void detach(Context context, EPersonNetidAlias alias) throws SQLException { | ||
| ePersonNetidAliasDAO.delete(context, alias); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isAllowlistedProxy(String authority) { | ||
| if (StringUtils.isBlank(authority)) { | ||
| return false; | ||
| } | ||
| String[] allowlist = configurationService.getArrayProperty("identity.auto-link.proxy-allowlist"); | ||
| return ArrayUtils.contains(allowlist, authority); | ||
| } | ||
|
|
||
| @Override | ||
| public EPerson autoLink(Context context, List<String> releasedEppns, String proxyAuthority, String proxyNetid) | ||
| throws SQLException { | ||
| if (releasedEppns == null || releasedEppns.isEmpty() || StringUtils.isBlank(proxyNetid)) { | ||
| return null; | ||
| } | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 190d5bc: |
||
| if (!isAllowlistedProxy(proxyAuthority)) { | ||
| log.warn("Auto-link via proxy '{}' refused: proxy is not on the identity.auto-link.proxy-allowlist.", | ||
| proxyAuthority); | ||
| return null; | ||
| } | ||
|
|
||
| // Already linked (e.g. concurrent/repeat login racing this method) - nothing to do. | ||
| EPerson alreadyLinked = resolve(context, proxyNetid); | ||
| if (alreadyLinked != null) { | ||
| return alreadyLinked; | ||
| } | ||
|
|
||
| Set<EPerson> matches = new LinkedHashSet<>(); | ||
| List<String> matchedOn = new ArrayList<>(); | ||
| for (String eppn : releasedEppns) { | ||
| if (StringUtils.isBlank(eppn)) { | ||
| continue; | ||
| } | ||
| for (EPersonNetidAlias alias : ePersonNetidAliasDAO.findByValuePrefix(context, eppn)) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why by value prefix and not the whole value?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the released
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any benefit in keeping the authority value, should the migration drop it? |
||
| if (matches.add(alias.getEPerson())) { | ||
| matchedOn.add(eppn); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (matches.isEmpty()) { | ||
| return null; | ||
| } | ||
| if (matches.size() > 1) { | ||
| log.warn("Auto-link via proxy '{}' matched {} different EPersons for released identities {} - " + | ||
| "refusing to guess, flag for admin merge (proxy netid '{}').", | ||
| proxyAuthority, matches.size(), matchedOn, proxyNetid); | ||
| return null; | ||
|
kosarko marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| EPerson matched = matches.iterator().next(); | ||
|
kosarko marked this conversation as resolved.
|
||
| attach(context, matched, proxyNetid, SOURCE_AUTO_VOPERSON, null); | ||
| log.info("Auto-linked netid '{}' to EPerson {} via proxy '{}' voperson_external_id match.", | ||
| proxyNetid, matched.getID(), proxyAuthority); | ||
| return matched; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| /** | ||
| * The contents of this file are subject to the license and copyright | ||
| * detailed in the LICENSE and NOTICE files at the root of the source | ||
| * tree and available online at | ||
| * | ||
| * http://www.dspace.org/license/ | ||
| */ | ||
| package org.dspace.eperson.clarin; | ||
|
|
||
| import java.util.Date; | ||
| import javax.persistence.Column; | ||
| import javax.persistence.Entity; | ||
| import javax.persistence.FetchType; | ||
| import javax.persistence.GeneratedValue; | ||
| import javax.persistence.GenerationType; | ||
| import javax.persistence.Id; | ||
| import javax.persistence.JoinColumn; | ||
| import javax.persistence.ManyToOne; | ||
| import javax.persistence.SequenceGenerator; | ||
| import javax.persistence.Table; | ||
| import javax.persistence.Temporal; | ||
| import javax.persistence.TemporalType; | ||
|
|
||
| import org.dspace.core.ReloadableEntity; | ||
| import org.dspace.eperson.EPerson; | ||
|
|
||
| /** | ||
| * An alias binding one identity value (a formatted "value[authority]" netid, | ||
| * where authority is a SAML IdP entityID or OIDC issuer URL) to an EPerson. | ||
| * | ||
| * This table is the primary source netid-based login resolution consults; | ||
| * {@link EPerson#getNetid()} is otherwise a denormalized display field, but | ||
| * is still consulted as a fallback by | ||
| * {@link org.dspace.eperson.service.clarin.ClarinIdentityService#resolve} | ||
| * for EPersons that do not (yet) have an alias row of their own. | ||
| * | ||
| * @author Ondrej Kosarko | ||
| */ | ||
| @Entity | ||
| @Table(name = "eperson_netid_alias") | ||
| public class EPersonNetidAlias implements ReloadableEntity<Integer> { | ||
|
|
||
| @Id | ||
| @Column(name = "eperson_netid_alias_id") | ||
| @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "eperson_netid_alias_id_seq") | ||
| @SequenceGenerator(name = "eperson_netid_alias_id_seq", sequenceName = "eperson_netid_alias_id_seq", | ||
| allocationSize = 1) | ||
| private Integer id; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "eperson_id", nullable = false) | ||
| private EPerson ePerson; | ||
|
|
||
| /** | ||
| * Formatted "value[authority]" identity, e.g. "novak@cuni.cz[https://cas.cuni.cz/idp/shibboleth]". | ||
| */ | ||
| @Column(name = "netid", nullable = false, unique = true) | ||
| private String netid; | ||
|
Copilot marked this conversation as resolved.
Outdated
|
||
|
|
||
| /** | ||
| * How this alias was created: 'migration' | 'auto-voperson' | 'admin' | 'merge'. | ||
| */ | ||
| @Column(name = "source", nullable = false) | ||
| private String source; | ||
|
Copilot marked this conversation as resolved.
Outdated
|
||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "created_by") | ||
| private EPerson createdBy; | ||
|
|
||
| @Column(name = "created_date", nullable = false) | ||
| @Temporal(TemporalType.TIMESTAMP) | ||
| private Date createdDate; | ||
|
|
||
| protected EPersonNetidAlias() { | ||
| } | ||
|
|
||
| @Override | ||
| public Integer getID() { | ||
| return id; | ||
| } | ||
|
|
||
| public EPerson getEPerson() { | ||
| return ePerson; | ||
| } | ||
|
|
||
| public void setEPerson(EPerson ePerson) { | ||
| this.ePerson = ePerson; | ||
| } | ||
|
|
||
| public String getNetid() { | ||
| return netid; | ||
| } | ||
|
|
||
| public void setNetid(String netid) { | ||
| this.netid = netid; | ||
| } | ||
|
|
||
| public String getSource() { | ||
| return source; | ||
| } | ||
|
|
||
| public void setSource(String source) { | ||
| this.source = source; | ||
| } | ||
|
|
||
| public EPerson getCreatedBy() { | ||
| return createdBy; | ||
| } | ||
|
|
||
| public void setCreatedBy(EPerson createdBy) { | ||
| this.createdBy = createdBy; | ||
| } | ||
|
|
||
| public Date getCreatedDate() { | ||
| return createdDate; | ||
| } | ||
|
|
||
| public void setCreatedDate(Date createdDate) { | ||
| this.createdDate = createdDate; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.