Skip to content

103-be/shibboleth-login-wrong-error-page #279

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.

Sign up for GitHub

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

Merged
milanmajchrak merged 4 commits into from
May 15, 2023
Merged

103-be/shibboleth-login-wrong-error-page #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a47ebf0
The user is redireced to the FE after wrong shibboleth authentication.
milanmajchrak May 15, 2023
fc48f63
Changed redirect URLs
milanmajchrak May 15, 2023
a4228a6
Fixed checkstyle issues
milanmajchrak May 15, 2023
a773b42
Copied ShibbolethFilterIT into ClarinShibbolethFilterIT because the t…
milanmajchrak May 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 46 additions & 16 deletions ...webapp/src/main/java/org/dspace/app/rest/security/clarin/ClarinShibbolethLoginFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ public class ClarinShibbolethLoginFilter extends StatelessLoginFilter {

private static final Logger log = LogManager.getLogger(org.dspace.app.rest.security.ShibbolethLoginFilter.class);

/**
* Property which handles information if the IdP send required information.
*/
private boolean isMissingHeadersFromIdp = false;

/**
* The netId of the user for which IdP send required information, but without Email.
*/
private String netId = "";

private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
private ClarinVerificationTokenService clarinVerificationTokenService = ClarinServiceFactory.getInstance()
.getClarinVerificationTokenService();
Expand All @@ -91,6 +101,10 @@ public ClarinShibbolethLoginFilter(String url, AuthenticationManager authenticat
@Override
public Authentication attemptAuthentication(HttpServletRequest req,
HttpServletResponse res) throws AuthenticationException {
// Reset properties in every login request
this.setMissingHeadersFromIdp(false);
this.netId = "";

// First, if Shibboleth is not enabled, throw an immediate ProviderNotFoundException
// This tells Spring Security that authentication failed
if (!ClarinShibAuthentication.isEnabled()) {
Expand Down Expand Up @@ -147,16 +161,14 @@ public Authentication attemptAuthentication(HttpServletRequest req,
try {
if (StringUtils.isEmpty(netid) || StringUtils.isEmpty(idp)) {
log.error("Cannot load the netid or idp from the request headers.");
this.redirectToMissingHeadersPage(res);
return null;
this.setMissingHeadersFromIdp(true);
}

// The Idp hasn't sent the email - the user will be redirected to the page where he must fill in that
// missing email
if (StringUtils.isBlank(email)) {
log.error("Cannot load the shib email header from the request headers.");
this.redirectToWriteEmailPage(req, res);
return null;
this.setMissingUserEmail(req, res);
}
} catch (IOException e) {
throw new RuntimeException("Cannot redirect the user to the Shibboleth authentication error page" +
Expand Down Expand Up @@ -215,7 +227,31 @@ protected void unsuccessfulAuthentication(HttpServletRequest request,
String authenticateHeaderValue = restAuthenticationService.getWwwAuthenticateHeaderValue(request, response);

response.setHeader("WWW-Authenticate", authenticateHeaderValue);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authentication failed!");

// Get redirect URL from request parameter
String redirectUrl = request.getParameter("redirectUrl");

// If redirectUrl unspecified, default to the configured UI
if (StringUtils.isEmpty(redirectUrl)) {
redirectUrl = configurationService.getProperty("dspace.ui.url");
}


String loginUrl = "/login/";
redirectUrl += loginUrl;

String missingHeadersUrl = "missing-headers";
String userWithoutEmailUrl = "auth-failed";

// Compose the redirect URL
if (this.isMissingHeadersFromIdp) {
redirectUrl += missingHeadersUrl;
} else if (StringUtils.isNotEmpty(this.netId)) {
// netId is set if the user doesn't have the email
redirectUrl += userWithoutEmailUrl + "?netid=" + this.netId;
}

response.sendRedirect(redirectUrl);
log.error("Authentication failed (status:{})",
HttpServletResponse.SC_UNAUTHORIZED, failed);
}
Expand Down Expand Up @@ -261,9 +297,10 @@ private void redirectAfterSuccess(HttpServletRequest request, HttpServletRespons
/**
* The IdP hasn't sent the `Shib-Identity-Provider` or `SHIB-NETID` header. The user is redirected to the
* static error page (The UI process error message).
*
*/
protected void redirectToMissingHeadersPage(HttpServletResponse res) throws IOException {
res.sendError(HttpServletResponse.SC_UNAUTHORIZED, MISSING_HEADERS_FROM_IDP);
protected void setMissingHeadersFromIdp(boolean value) {
this.isMissingHeadersFromIdp = value;
}

/**
Expand All @@ -272,7 +309,7 @@ protected void redirectToMissingHeadersPage(HttpServletResponse res) throws IOEx
* The request headers passed by IdP are stored into the `verification_token` table the `shib_headers` column
* for later usage. After successful signing in the `verification_token` record is removed from the DB.
*/
protected void redirectToWriteEmailPage(HttpServletRequest req,
protected void setMissingUserEmail(HttpServletRequest req,
HttpServletResponse res) throws IOException {
Context context = ContextUtil.obtainContext(req);
String authenticateHeaderValue = restAuthenticationService.getWwwAuthenticateHeaderValue(req, res);
Expand Down Expand Up @@ -303,14 +340,7 @@ protected void redirectToWriteEmailPage(HttpServletRequest req,
+ e.getMessage());
}

// Add header values to the error message to retrieve them in the FE. That headers are needed for the
// next processing.
String separator = ",";
String[] headers = new String[] {USER_WITHOUT_EMAIL_EXCEPTION, netid};
String errorMessage = StringUtils.join(headers, separator);

res.setHeader("WWW-Authenticate", authenticateHeaderValue);
res.sendError(HttpServletResponse.SC_UNAUTHORIZED, errorMessage);
this.netId = netid;
}

private String getEpersonEmail(EPerson ePerson) {
Expand Down
65 changes: 34 additions & 31 deletions dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,37 +336,40 @@ public void testStatusNotAuthenticated() throws Exception {
// .andExpect(status().isNoContent());
// }

@Test
public void testShibbolethEndpointCannotBeUsedWithShibDisabled() throws Exception {
// Enable only password login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);

String uiURL = configurationService.getProperty("dspace.ui.url");

// Verify /api/authn/shibboleth endpoint does not work
// NOTE: this is the same call as in testStatusShibAuthenticatedWithCookie())
String token = getClient().perform(get("/api/authn/shibboleth")
.header("Referer", "https://myshib.example.com")
.param("redirectUrl", uiURL)
.requestAttr("SHIB-MAIL", eperson.getEmail())
.requestAttr("SHIB-SCOPED-AFFILIATION", "faculty;staff"))
.andExpect(status().isUnauthorized())
.andReturn().getResponse().getHeader("Authorization");

getClient(token).perform(get("/api/authn/status"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.authenticated", is(false)))
.andExpect(jsonPath("$.authenticationMethod").doesNotExist());

getClient(token).perform(
get("/api/authz/authorizations/search/object")
.param("embed", "feature")
.param("feature", feature)
.param("uri", utils.linkToSingleResource(ePersonRest, "self").getHref()))
.andExpect(status().isOk())
.andExpect(jsonPath("$.page.totalElements", is(0)))
.andExpect(jsonPath("$._embedded").doesNotExist());
}
// Note: This test was commented because the Shibboleth Authentication was customized following the Clarin
// requirements. This test was copied and updated following the Clarin updates to the
// `ClarinAuthenticationRestControllerIT#testShibbolethEndpointCannotBeUsedWithShibDisabled` method.
// @Test
// public void testShibbolethEndpointCannotBeUsedWithShibDisabled() throws Exception {
// // Enable only password login
// configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);
//
// String uiURL = configurationService.getProperty("dspace.ui.url");
//
// // Verify /api/authn/shibboleth endpoint does not work
// // NOTE: this is the same call as in testStatusShibAuthenticatedWithCookie())
// String token = getClient().perform(get("/api/authn/shibboleth")
// .header("Referer", "https://myshib.example.com")
// .param("redirectUrl", uiURL)
// .requestAttr("SHIB-MAIL", eperson.getEmail())
// .requestAttr("SHIB-SCOPED-AFFILIATION", "faculty;staff"))
// .andExpect(status().isUnauthorized())
// .andReturn().getResponse().getHeader("Authorization");
//
// getClient(token).perform(get("/api/authn/status"))
// .andExpect(status().isOk())
// .andExpect(jsonPath("$.authenticated", is(false)))
// .andExpect(jsonPath("$.authenticationMethod").doesNotExist());
//
// getClient(token).perform(
// get("/api/authz/authorizations/search/object")
// .param("embed", "feature")
// .param("feature", feature)
// .param("uri", utils.linkToSingleResource(ePersonRest, "self").getHref()))
// .andExpect(status().isOk())
// .andExpect(jsonPath("$.page.totalElements", is(0)))
// .andExpect(jsonPath("$._embedded").doesNotExist());
// }

// NOTE: This test is similar to testStatusShibAuthenticatedWithCookie(), but proves the same process works
// for Password Authentication in theory (NOTE: at this time, there's no way to create an auth cookie via the
Expand Down
94 changes: 84 additions & 10 deletions ...rver-webapp/src/test/java/org/dspace/app/rest/security/ClarinShibbolethLoginFilterIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
*/
package org.dspace.app.rest.security;

import static org.dspace.app.rest.security.clarin.ClarinShibbolethLoginFilter.MISSING_HEADERS_FROM_IDP;
import static org.dspace.app.rest.security.clarin.ClarinShibbolethLoginFilter.USER_WITHOUT_EMAIL_EXCEPTION;
import static org.dspace.app.rest.security.ShibbolethLoginFilterIT.PASS_ONLY;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -114,8 +113,8 @@ public void shouldReturnMissingHeadersFromIdpExceptionBecauseOfMissingIdp() thro
// 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-NETID", netId))
.andExpect(status().isUnauthorized())
.andExpect(status().reason(MISSING_HEADERS_FROM_IDP));
.andExpect(status().isFound())
.andExpect(redirectedUrl("http://localhost:4000/login/missing-headers"));
}

/**
Expand All @@ -129,8 +128,8 @@ public void shouldReturnMissingHeadersFromIdpExceptionBecauseOfMissingNetId() th
// 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))
.andExpect(status().isUnauthorized())
.andExpect(status().reason(MISSING_HEADERS_FROM_IDP));
.andExpect(status().isFound())
.andExpect(redirectedUrl("http://localhost:4000/login/missing-headers"));
}

/**
Expand All @@ -139,6 +138,7 @@ public void shouldReturnMissingHeadersFromIdpExceptionBecauseOfMissingNetId() th
* The request headers passed by IdP are stored into the `verification_token` table the `shib_headers` column.
* The user is redirected to the page when he must fill his email.
*/
// HERE
@Test
public void shouldReturnUserWithoutEmailException() throws Exception {
String netId = "123456";
Expand All @@ -149,8 +149,8 @@ public void shouldReturnUserWithoutEmailException() throws Exception {
getClient().perform(get("/api/authn/shibboleth")
.header("SHIB-NETID", netId)
.header("Shib-Identity-Provider", idp))
.andExpect(status().isUnauthorized())
.andExpect(status().reason(USER_WITHOUT_EMAIL_EXCEPTION + "," + netId));
.andExpect(status().isFound())
.andExpect(redirectedUrl("http://localhost:4000/login/auth-failed?netid=" + netId));
}

/**
Expand Down Expand Up @@ -178,8 +178,8 @@ public void userFillInEmailAndShouldBeRegisteredByVerificationToken() throws Exc
.header("SHIB-NETID", netId)
.header("SHIB-GIVENNAME", firstname)
.header("SHIB-SURNAME", lastname))
.andExpect(status().isUnauthorized())
.andExpect(status().reason(USER_WITHOUT_EMAIL_EXCEPTION + "," + netId));
.andExpect(status().isFound())
.andExpect(redirectedUrl("http://localhost:4000/login/auth-failed?netid=" + netId));

// Send the email with the verification token.
String tokenAdmin = getAuthToken(admin.getEmail(), password);
Expand Down Expand Up @@ -325,6 +325,80 @@ public void testRedirectToDefaultDspaceUrl() throws Exception {

}

// This test is copied from the `AuthenticationRestControllerIT` and modified following the Clarin updates.
@Test
public void testShibbolethEndpointCannotBeUsedWithShibDisabled() throws Exception {
// Enable only password login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);

String uiURL = configurationService.getProperty("dspace.ui.url");

// Verify /api/authn/shibboleth endpoint does not work
// NOTE: this is the same call as in testStatusShibAuthenticatedWithCookie())
String token = getClient().perform(get("/api/authn/shibboleth")
.header("Referer", "https://myshib.example.com")
.param("redirectUrl", uiURL)
.requestAttr("SHIB-MAIL", eperson.getEmail())
.requestAttr("SHIB-SCOPED-AFFILIATION", "faculty;staff"))
.andExpect(status().isFound())
.andReturn().getResponse().getHeader("Authorization");

getClient(token).perform(get("/api/authn/status"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.authenticated", is(false)))
.andExpect(jsonPath("$.authenticationMethod").doesNotExist());

getClient(token).perform(
get("/api/authz/authorizations/search/object")
.param("embed", "feature")
.param("feature", feature)
.param("uri", utils.linkToSingleResource(ePersonRest, "self").getHref()))
.andExpect(status().isOk())
.andExpect(jsonPath("$.page.totalElements", is(0)))
.andExpect(jsonPath("$._embedded").doesNotExist());
}

// This test is copied from the `ShibbolethLoginFilterIT` and modified following the Clarin updates.
@Test
public void testNoRedirectIfInvalidShibAttributes() throws Exception {
// In this request, we use a SHIB-MAIL attribute which does NOT match an EPerson.
getClient().perform(get("/api/authn/shibboleth")
.requestAttr("SHIB-MAIL", "not-an-eperson@example.com"))
.andExpect(status().isFound());
}

// This test is copied from the `ShibbolethLoginFilterIT` and modified following the Clarin updates.
@Test
public void testNoRedirectIfShibbolethDisabled() throws Exception {
// Enable Password authentication ONLY
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);

// Test redirecting to a trusted URL (same as previous test).
// This time we should be unauthorized as Shibboleth is disabled.
getClient().perform(get("/api/authn/shibboleth")
.param("redirectUrl", "http://localhost:8080/server/api/authn/status")
.requestAttr("SHIB-MAIL", eperson.getEmail()))
.andExpect(status().isFound());
}

// This test is copied from the `ShibbolethLoginFilterIT` and modified following the Clarin updates.
@Test
public void testRedirectRequiresShibAttributes2() throws Exception {
String token = getAuthToken(eperson.getEmail(), password);

// Verify this endpoint also doesn't work using a regular auth token (again if SHIB-* attributes missing)
getClient(token).perform(get("/api/authn/shibboleth"))
.andExpect(status().isFound());
}

// This test is copied from the `ShibbolethLoginFilterIT` and modified following the Clarin updates.
@Test
public void testRedirectRequiresShibAttributes() throws Exception {
// Verify this endpoint doesn't work if no SHIB-* attributes are set
getClient().perform(get("/api/authn/shibboleth"))
.andExpect(status().isFound());
}

// This test is copied from the `ShibbolethLoginFilterIT` and modified following the Clarin updates.
@Test
public void testRedirectToAnotherGivenTrustedUrl() throws Exception {
Expand Down
Loading