From ee73b8dd62ea993676bd228fff7b39ed9f4be9a6 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Wed, 13 Nov 2024 18:10:04 +0100 Subject: [PATCH 1/2] The user is not signed in without using link with the verification token from the email/ --- .../clarin/ClarinShibAuthentication.java | 12 +++++- .../ClarinShibbolethLoginFilterIT.java | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) 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 3aa767ee58b..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 @@ -239,7 +239,17 @@ 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[] netidHeaders = configurationService.getArrayProperty("authentication-shibboleth.netid-header"); - clarinVerificationToken = clarinVerificationTokenService.findByNetID(context, netidHeaders, shibheaders); + + // 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. 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 11e4304f2b5..3f0827ca41a 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 @@ -621,6 +621,43 @@ public void shouldAskForEmailWhenHasPersistentId() throws Exception { 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))); + } + private EPerson checkUserWasCreated(String netIdValue, String idpValue, String email, String name) throws SQLException { // Check if was created a user with such email and netid. From 8c648c17069637511a2e724195e4334679d5a02f Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Thu, 14 Nov 2024 13:20:26 +0100 Subject: [PATCH 2/2] Send a redirect to UI with specific parameter that the Shibboleth authorization wasn't successful --- .../src/main/java/org/dspace/core/Utils.java | 17 ++++++++++++++++ .../test/java/org/dspace/core/UtilsTest.java | 20 +++++++++++++++++++ .../clarin/ClarinShibbolethLoginFilter.java | 6 ++++++ .../ClarinShibbolethLoginFilterIT.java | 12 +++++++++++ 4 files changed, 55 insertions(+) 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 93d64e0fb3b..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); 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 3f0827ca41a..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 @@ -658,6 +658,18 @@ public void shouldNotAuthenticateOnSecondAttemptWithoutVerificationTokenInReques 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.