UFAL/Shibboleth - netid-header should use getArrayProperty everywhere

dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java

@@ -238,17 +238,25 @@ 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.
         initialize(context);
 
         // 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 {

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 {

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

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());
+    }
 }

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);
+    }
 }

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;
+    }
 }
 

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.