The IdP returns the assertion as a cross-site POST to the assertion consumer service. + * A {@code SameSite=Lax} cookie is not sent on such a request, so the shipped default in + * {@code tomcat_config.properties} has to be changed for SAML:
+ *+ * tomcat.sameSiteCookies = none + *+ *
{@code none} is only accepted by browsers on a {@code Secure} cookie, so Fess must be + * served over HTTPS.
+ * * @see Fess Documentation */ public class SamlAuthenticator implements SsoAuthenticator { @@ -311,9 +321,9 @@ public LoginCredential getLoginCredential() { final HttpServletResponse response = LaResponseUtil.getResponse(); - final HttpSession session = request.getSession(false); - if (session != null) { - final String requestId = (String) session.getAttribute(SAML_STATE); + if (containsSamlResponse(request)) { + final HttpSession session = request.getSession(false); + final String requestId = session == null ? null : (String) session.getAttribute(SAML_STATE); if (StringUtil.isNotBlank(requestId)) { session.removeAttribute(SAML_STATE); try { @@ -336,6 +346,14 @@ public LoginCredential getLoginCredential() { return null; } } + // The assertion arrived but the matching AuthnRequest ID is unreachable. Sending + // another AuthnRequest would come straight back here in the same state, looping + // forever, so fail once instead. + logger.warn("Received a SAML response with no matching AuthnRequest ID in the session." + + " The assertion consumer service is a cross-site POST, which does not carry a SameSite=Lax cookie;" + + " see tomcat.sameSiteCookies in tomcat_config.properties." + + " An IdP-initiated (unsolicited) response is rejected for the same reason."); + return null; } try { @@ -351,6 +369,25 @@ public LoginCredential getLoginCredential() { }).orElse(null); } + /** + * Returns whether the request carries a SAML response, which is what the IdP posts to the + * assertion consumer service. + * + *The session is deliberately not consulted here: it is the session cookie that goes + * missing when the browser refuses to send it on the cross-site POST, and a callback that is + * mistaken for a fresh visit is redirected back to the IdP forever.
+ * + *Only solicited responses are accepted. Fess binds every response to the ID of the + * AuthnRequest it sent, so an unsolicited (IdP-initiated) response has nothing to match + * against and is rejected rather than answered with a fresh AuthnRequest.
+ * + * @param request The HTTP request. + * @return true if the request carries a SAML response. + */ + protected boolean containsSamlResponse(final HttpServletRequest request) { + return StringUtil.isNotBlank(request.getParameter("SAMLResponse")); + } + /** * Creates a login credential. * @param request The HTTP request. diff --git a/src/test/java/org/codelibs/fess/sso/saml/SamlAuthenticatorTest.java b/src/test/java/org/codelibs/fess/sso/saml/SamlAuthenticatorTest.java index c02fce2bd..3613313bf 100644 --- a/src/test/java/org/codelibs/fess/sso/saml/SamlAuthenticatorTest.java +++ b/src/test/java/org/codelibs/fess/sso/saml/SamlAuthenticatorTest.java @@ -30,12 +30,15 @@ import org.codelibs.core.lang.StringUtil; import org.codelibs.core.misc.DynamicProperties; +import org.codelibs.fess.app.web.base.login.ActionResponseCredential; import org.codelibs.fess.exception.SsoMessageException; import org.codelibs.fess.sso.SsoResponseType; import org.codelibs.fess.unit.UnitFessTestCase; import org.codelibs.fess.util.ComponentUtil; import org.codelibs.saml2.core.settings.Saml2Settings; +import org.dbflute.utflute.mocklet.MockletHttpServletRequest; import org.junit.jupiter.api.Test; +import org.lastaflute.web.login.credential.LoginCredential; public class SamlAuthenticatorTest extends UnitFessTestCase { @@ -195,6 +198,96 @@ public void test_getLogoutResponse_withoutIdpSingleLogoutServiceUrl() throws Exc } } + // =================================================================================== + // Assertion Consumer Service + // ========================== + + /** Minimal IdP settings, so that an AuthnRequest can actually be built. */ + private void setUpIdp(final DynamicProperties systemProperties) { + systemProperties.setProperty("saml.idp.entityid", "https://idp.example.com/metadata"); + systemProperties.setProperty("saml.idp.single_sign_on_service.url", "https://idp.example.com/sso"); + systemProperties.setProperty("saml.idp.certfingerprint", "afe71c28ef740bc87425be13a2263d37971da1f9"); + } + + private void tearDownIdp(final DynamicProperties systemProperties) { + systemProperties.remove("saml.idp.entityid"); + systemProperties.remove("saml.idp.single_sign_on_service.url"); + systemProperties.remove("saml.idp.certfingerprint"); + } + + @Test + public void test_containsSamlResponse() throws Exception { + final SamlAuthenticator authenticator = new SamlAuthenticator(); + + assertFalse(authenticator.containsSamlResponse(getMockRequest())); + + final MockletHttpServletRequest blank = getMockRequest(); + blank.setParameter("SAMLResponse", " "); + assertFalse(authenticator.containsSamlResponse(blank)); + + final MockletHttpServletRequest posted = getMockRequest(); + posted.setParameter("SAMLResponse", "PHNhbWxwOlJlc3BvbnNlIC8+"); + assertTrue(authenticator.containsSamlResponse(posted)); + } + + @Test + public void test_getLoginCredential_unmatchedResponseFailsInsteadOfRedirecting() throws Exception { + final SamlAuthenticator authenticator = createAuthenticator(); + final DynamicProperties systemProperties = ComponentUtil.getSystemProperties(); + final LogCapturingAppender appender = LogCapturingAppender.attach(SamlAuthenticator.class); + try { + setUpIdp(systemProperties); + // the IdP posts the assertion cross-site, so a SameSite=Lax cookie is not sent back + // and the session holding the AuthnRequest ID is unreachable + final MockletHttpServletRequest request = getMockRequest(); + request.setMethod("POST"); + request.setParameter("SAMLResponse", "PHNhbWxwOlJlc3BvbnNlIC8+"); + + // redirecting to the IdP again would come straight back in the same state + assertNull(authenticator.getLoginCredential()); + assertEquals(1, appender.warnings().size()); + assertTrue(appender.warnings().get(0), appender.warnings().get(0).contains("no matching AuthnRequest ID")); + } finally { + tearDownIdp(systemProperties); + appender.detach(); + } + } + + @Test + public void test_getLoginCredential_requestWithoutResponseStartsLogin() throws Exception { + final SamlAuthenticator authenticator = createAuthenticator(); + final DynamicProperties systemProperties = ComponentUtil.getSystemProperties(); + try { + setUpIdp(systemProperties); + final MockletHttpServletRequest request = getMockRequest(); + + final LoginCredential credential = authenticator.getLoginCredential(); + + assertTrue(String.valueOf(credential), credential instanceof ActionResponseCredential); + assertNotNull(request.getSession(false).getAttribute("SAML_STATE")); + } finally { + tearDownIdp(systemProperties); + } + } + + @Test + public void test_getLoginCredential_requestWithoutResponseKeepsPendingRequestId() throws Exception { + final SamlAuthenticator authenticator = createAuthenticator(); + final DynamicProperties systemProperties = ComponentUtil.getSystemProperties(); + try { + setUpIdp(systemProperties); + // a plain visit to /sso/ while a login is in flight must not consume the pending ID + final MockletHttpServletRequest request = getMockRequest(); + request.getSession().setAttribute("SAML_STATE", "ONELOGIN_pending"); + + authenticator.getLoginCredential(); + + assertNotNull(request.getSession(false).getAttribute("SAML_STATE")); + } finally { + tearDownIdp(systemProperties); + } + } + @Test public void test_buildDefaultUrl_withDefaultBaseUrl() throws Exception { assertEquals("http://localhost:8080/sso/metadata", new SamlAuthenticator().buildDefaultUrl("/sso/metadata"));