Skip to content

Commit

Permalink
UFAL/Shibboleth - netid-header should use getArrayProperty everywhere (
Browse files Browse the repository at this point in the history
…#807)

* Fetch netid as array from the cfg. Now netid as array is used everywhere. Added integration test to ask for an email when the user send only persistent-id in the shib header.

* Fixed checkstyle issue

* The user is not signed in without using link with the verification token from the email/ (#809)

* UFAL/Shibboleth - show error in the UI when shibboleth authentication is failed (#810)

* The user is not signed in without using link with the verification token from the email/

* Send a redirect to UI with specific parameter that the Shibboleth authorization wasn't successful
  • Loading branch information
milanmajchrak authored Nov 19, 2024
1 parent dad2ea2 commit 32f61b3
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions dspace-api/src/main/java/org/dspace/core/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
20 changes: 20 additions & 0 deletions dspace-api/src/test/java/org/dspace/core/UtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -370,7 +374,7 @@ protected void setMissingUserEmail(HttpServletRequest req,
+ e.getMessage());
}

this.netId = netid;
this.netId = netId;
}

private String getEpersonEmail(EPerson ePerson) {
Expand All @@ -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;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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 = "[email protected]";
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.
Expand Down

0 comments on commit 32f61b3

Please sign in to comment.