diff --git a/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java index 98c030be80c..53adf9d79b8 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java @@ -238,9 +238,18 @@ public int authenticate(Context context, String username, String password, // The user e-mail is not stored in the `shibheaders` but in the `clarinVerificationToken`. // The email was added to the `clarinVerificationToken` in the ClarinShibbolethFilter. - String netidHeader = configurationService.getProperty("authentication-shibboleth.netid-header"); - clarinVerificationToken = clarinVerificationTokenService.findByNetID(context, - shibheaders.get_single(netidHeader)); + String[] netidHeaders = configurationService.getArrayProperty("authentication-shibboleth.netid-header"); + + // Load the verification token from the request header or from the request parameter. + // This is only set if the user is trying to authenticate with the `verification-token`. + String VERIFICATION_TOKEN = "verification-token"; + String verificationTokenFromRequest = StringUtils.defaultIfBlank(request.getHeader(VERIFICATION_TOKEN), + request.getParameter(VERIFICATION_TOKEN)); + if (StringUtils.isNotEmpty(verificationTokenFromRequest)) { + log.info("Verification token from request header `{}`: {}", VERIFICATION_TOKEN, + verificationTokenFromRequest); + clarinVerificationToken = clarinVerificationTokenService.findByToken(context, verificationTokenFromRequest); + } // CLARIN // Initialize the additional EPerson metadata. @@ -248,7 +257,6 @@ public int authenticate(Context context, String username, String password, // Should we auto register new users. boolean autoRegister = configurationService.getBooleanProperty("authentication-shibboleth.autoregister", true); - String[] netidHeaders = configurationService.getArrayProperty("authentication-shibboleth.netid-header"); // Four steps to authenticate a user try { diff --git a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinVerificationTokenServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinVerificationTokenServiceImpl.java index 27b4ca7cad8..33eff897579 100644 --- a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinVerificationTokenServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinVerificationTokenServiceImpl.java @@ -12,6 +12,7 @@ import java.util.Objects; import org.apache.commons.lang.NullArgumentException; +import org.dspace.authenticate.clarin.ShibHeaders; import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.service.AuthorizeService; import org.dspace.content.dao.clarin.ClarinVerificationTokenDAO; @@ -75,6 +76,19 @@ public ClarinVerificationToken findByNetID(Context context, String netID) throws return clarinVerificationTokenDAO.findByNetID(context, netID); } + @Override + public ClarinVerificationToken findByNetID(Context context, String[] netIdHeaders, ShibHeaders shibHeaders) + throws SQLException { + for (String netidHeader : netIdHeaders) { + String netID = shibHeaders.get_single(netidHeader); + ClarinVerificationToken clarinVerificationToken = clarinVerificationTokenDAO.findByNetID(context, netID); + if (Objects.nonNull(clarinVerificationToken)) { + return clarinVerificationToken; + } + } + return null; + } + @Override public void delete(Context context, ClarinVerificationToken clarinVerificationToken) throws SQLException { diff --git a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinVerificationTokenService.java b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinVerificationTokenService.java index ec9d23cde5a..653a765929f 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinVerificationTokenService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinVerificationTokenService.java @@ -10,6 +10,7 @@ import java.sql.SQLException; import java.util.List; +import org.dspace.authenticate.clarin.ShibHeaders; import org.dspace.authorize.AuthorizeException; import org.dspace.content.clarin.ClarinVerificationToken; import org.dspace.core.Context; @@ -69,6 +70,18 @@ public interface ClarinVerificationTokenService { */ ClarinVerificationToken findByNetID(Context context, String netID) throws SQLException; + /** + * Find the clarin verification token object from the shibboleth headers trying every netId header + * until the object is found + * @param context DSpace context object + * @param netIdHeaders array of the netId headers - values from the configuration + * @param shibHeaders object with the shibboleth headers + * @return found clarin verification token object or null + * @throws SQLException if database error + */ + ClarinVerificationToken findByNetID(Context context, String[] netIdHeaders, ShibHeaders shibHeaders) + throws SQLException; + /** * Remove the clarin verification token from DB * @param context DSpace context object diff --git a/dspace-api/src/main/java/org/dspace/core/Utils.java b/dspace-api/src/main/java/org/dspace/core/Utils.java index ea9ed57eca0..6831f45b5c5 100644 --- a/dspace-api/src/main/java/org/dspace/core/Utils.java +++ b/dspace-api/src/main/java/org/dspace/core/Utils.java @@ -506,4 +506,21 @@ public static String interpolateConfigsInString(String string) { ConfigurationService config = DSpaceServicesFactory.getInstance().getConfigurationService(); return StringSubstitutor.replace(string, config.getProperties()); } + + /** + * Replace the last occurrence of a substring within a string. + * + * @param input The input string + * @param toReplace The substring to replace + * @param replacement The replacement substring + * @return Replaced input string or the original input string if the substring to replace is not found + */ + public static String replaceLast(String input, String toReplace, String replacement) { + int lastIndex = input.lastIndexOf(toReplace); + if (lastIndex == -1) { + return input; // No replacement if not found + } + + return input.substring(0, lastIndex) + replacement + input.substring(lastIndex + toReplace.length()); + } } diff --git a/dspace-api/src/test/java/org/dspace/core/UtilsTest.java b/dspace-api/src/test/java/org/dspace/core/UtilsTest.java index 291561ac253..eec5b014595 100644 --- a/dspace-api/src/test/java/org/dspace/core/UtilsTest.java +++ b/dspace-api/src/test/java/org/dspace/core/UtilsTest.java @@ -132,4 +132,24 @@ public void testInterpolateConfigsInString() { // remove the config we added configurationService.setProperty(configName, null); } + + // Replace the last occurrence of a substring + @Test + public void testReplaceLast_SingleOccurrence() { + String input = "/login/"; + String result = Utils.replaceLast(input, "/", "replacement"); + + // Expected output: "/loginreplacement" + assertEquals("/loginreplacement", result); + } + + // No replacement when the substring is not found + @Test + public void testReplaceLast_NoMatch() { + String input = "/login"; + String result = Utils.replaceLast(input, "/", "replacement"); + + // Expected output: "/login" + assertEquals("replacementlogin", result); + } } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/clarin/ClarinShibbolethLoginFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/clarin/ClarinShibbolethLoginFilter.java index 821be38ed2a..774cdcd8085 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/clarin/ClarinShibbolethLoginFilter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/clarin/ClarinShibbolethLoginFilter.java @@ -261,6 +261,7 @@ protected void unsuccessfulAuthentication(HttpServletRequest request, String missingHeadersUrl = "missing-headers"; String userWithoutEmailUrl = "auth-failed"; String duplicateUser = "duplicate-user"; + String cannotAuthenticate = "shibboleth-authentication-failed"; // Compose the redirect URL if (this.isMissingHeadersFromIdp) { @@ -270,6 +271,11 @@ protected void unsuccessfulAuthentication(HttpServletRequest request, } else if (StringUtils.isNotEmpty(this.netId)) { // netId is set if the user doesn't have the email redirectUrl += userWithoutEmailUrl + "?netid=" + this.netId; + } else { + // Remove the last slash from the URL `login/` + String redirectUrlWithoutSlash = redirectUrl.endsWith("/") ? + Utils.replaceLast(redirectUrl, "/", "") : redirectUrl; + redirectUrl = redirectUrlWithoutSlash + "?error=" + cannotAuthenticate; } response.sendRedirect(redirectUrl); @@ -344,23 +350,21 @@ protected void setMissingUserEmail(HttpServletRequest req, Context context = ContextUtil.obtainContext(req); String authenticateHeaderValue = restAuthenticationService.getWwwAuthenticateHeaderValue(req, res); - // Load header keys from cfg - String netidHeader = configurationService.getProperty("authentication-shibboleth.netid-header"); - // Store the header which the Idp has sent to the ShibHeaders object and save that header into the table // `verification_token` because after successful authentication the Idp headers will be showed for the user in // the another page. // Store header values in the ShibHeaders because of String issues. ShibHeaders shib_headers = new ShibHeaders(req); - String netid = shib_headers.get_single(netidHeader); + String[] netIdHeaders = shib_headers.getNetIdHeaders(); + String netId = getNetIdFromShibHeaders(netIdHeaders, shib_headers); // Store the Idp headers associated with the current netid. + ClarinVerificationToken clarinVerificationToken; try { - ClarinVerificationToken clarinVerificationToken = - clarinVerificationTokenService.findByNetID(context, netid); + clarinVerificationToken = clarinVerificationTokenService.findByNetID(context, netId); if (Objects.isNull(clarinVerificationToken)) { clarinVerificationToken = clarinVerificationTokenService.create(context); - clarinVerificationToken.setePersonNetID(netid); + clarinVerificationToken.setePersonNetID(netId); } clarinVerificationToken.setShibHeaders(shib_headers.toString()); clarinVerificationTokenService.update(context, clarinVerificationToken); @@ -370,7 +374,7 @@ protected void setMissingUserEmail(HttpServletRequest req, + e.getMessage()); } - this.netId = netid; + this.netId = netId; } private String getEpersonEmail(EPerson ePerson) { @@ -379,5 +383,20 @@ private String getEpersonEmail(EPerson ePerson) { } return ePerson.getEmail(); } + + /** + * Get the netId from the ShibHeaders object. The netId is stored in the headers which are defined in the + * `authentication-shibboleth.netid-headers` property. + * @return netId or null + */ + private String getNetIdFromShibHeaders(String[] netIdHeaders, ShibHeaders shibHeaders) { + for (String netidHeader : netIdHeaders) { + String netID = shibHeaders.get_single(netidHeader); + if (StringUtils.isNotEmpty(netID)) { + return netID; + } + } + return null; + } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/security/ClarinShibbolethLoginFilterIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/security/ClarinShibbolethLoginFilterIT.java index 6cf8c6058d7..e828dd9dc2d 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/security/ClarinShibbolethLoginFilterIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/security/ClarinShibbolethLoginFilterIT.java @@ -606,6 +606,70 @@ public void testDuplicateEmailError() throws Exception { deleteShibbolethUser(ePerson); } + // mail=null, eppn=null, persistent-id=somestring + @Test + public void shouldAskForEmailWhenHasPersistentId() throws Exception { + String persistentId = "some pid"; + + // Try to log in a user without email, but with persistent id. The user should be redirected to the page where + // he will fill in the user email. + getClient().perform(get("/api/authn/shibboleth") + .header("Shib-Identity-Provider", IDP_TEST_EPERSON) + .header(NET_ID_PERSISTENT_ID, persistentId)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("http://localhost:4000/login/auth-failed?netid=" + + Util.formatNetId(persistentId, IDP_TEST_EPERSON))); + } + + // The user was registered and signed in with the verification token on the second attempt, after the email + // containing the verification token was sent. + @Test + public void shouldNotAuthenticateOnSecondAttemptWithoutVerificationTokenInRequest() throws Exception { + String email = "test@mail.epic"; + String netId = email; + String idp = "Test Idp"; + + // Try to authenticate but the Shibboleth doesn't send the email in the header, so the user won't be registered + // but the user will be redirected to the page where he will fill in the user email. + getClient().perform(get("/api/authn/shibboleth") + .header("Shib-Identity-Provider", idp) + .header("SHIB-NETID", netId)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("http://localhost:4000/login/auth-failed?netid=" + + Util.formatNetId(netId, idp))); + + // Send the email with the verification token. + String tokenAdmin = getAuthToken(admin.getEmail(), password); + getClient(tokenAdmin).perform(post("/api/autoregistration?netid=" + netId + "&email=" + email) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isOk()); + + // Load the created verification token. + ClarinVerificationToken clarinVerificationToken = clarinVerificationTokenService.findByNetID(context, netId); + assertTrue(Objects.nonNull(clarinVerificationToken)); + + // Try to authenticate the user again, and it should NOT to be automatically registered and signed in, + // because the verification token is not passed in the request header. + getClient().perform(get("/api/authn/shibboleth") + .header("Shib-Identity-Provider", idp) + .header("SHIB-NETID", netId)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("http://localhost:4000/login/auth-failed?netid=" + + Util.formatNetId(netId, idp))); + } + + @Test + public void shouldSendShibbolethAuthError() throws Exception { + String idp = "Test Idp"; + + // Try to authenticate but the Shibboleth doesn't send the email or netid in the header, + // so the user won't be registered but the user will be redirected to the login page with the error message. + getClient().perform(get("/api/authn/shibboleth") + .header("Shib-Identity-Provider", idp)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("http://localhost:4000/login?error=shibboleth-authentication-failed")); + } + private EPerson checkUserWasCreated(String netIdValue, String idpValue, String email, String name) throws SQLException { // Check if was created a user with such email and netid.