KEYCLOAK-17612- Invalid SAML Response : Invalid Destination
This commit is contained in:
parent
455e93856c
commit
8255cba930
4 changed files with 238 additions and 5 deletions
|
@ -67,8 +67,8 @@ import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
|
|||
import org.keycloak.saml.processing.core.util.XMLSignatureUtil;
|
||||
import org.keycloak.saml.processing.web.util.PostBindingUtil;
|
||||
import org.keycloak.services.ErrorPage;
|
||||
import org.keycloak.services.Urls;
|
||||
import org.keycloak.services.managers.AuthenticationManager;
|
||||
import org.keycloak.services.managers.ClientSessionCode;
|
||||
import org.keycloak.services.messages.Messages;
|
||||
|
||||
import javax.ws.rs.Consumes;
|
||||
|
@ -282,7 +282,7 @@ public class SAMLEndpoint {
|
|||
event.error(Errors.INVALID_REQUEST);
|
||||
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST);
|
||||
}
|
||||
if (! destinationValidator.validate(session.getContext().getUri().getAbsolutePath(), requestAbstractType.getDestination())) {
|
||||
if (! destinationValidator.validate(getExpectedDestination(config.getAlias(), null), requestAbstractType.getDestination())) {
|
||||
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
|
||||
event.detail(Details.REASON, Errors.INVALID_DESTINATION);
|
||||
event.error(Errors.INVALID_SAML_RESPONSE);
|
||||
|
@ -594,7 +594,7 @@ public class SAMLEndpoint {
|
|||
event.error(Errors.INVALID_SAML_LOGOUT_RESPONSE);
|
||||
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST);
|
||||
}
|
||||
if (! destinationValidator.validate(session.getContext().getUri().getAbsolutePath(), statusResponse.getDestination())) {
|
||||
if (! destinationValidator.validate(getExpectedDestination(config.getAlias(), clientId), statusResponse.getDestination())) {
|
||||
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
|
||||
event.detail(Details.REASON, Errors.INVALID_DESTINATION);
|
||||
event.error(Errors.INVALID_SAML_RESPONSE);
|
||||
|
@ -643,6 +643,13 @@ public class SAMLEndpoint {
|
|||
}
|
||||
return AuthenticationManager.finishBrowserLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers);
|
||||
}
|
||||
|
||||
private String getExpectedDestination(String providerAlias, String clientId) {
|
||||
if(clientId != null) {
|
||||
return session.getContext().getUri().getAbsolutePath().toString();
|
||||
}
|
||||
return Urls.identityProviderAuthnResponse(session.getContext().getUri().getBaseUri(), providerAlias, realm.getName()).toString();
|
||||
}
|
||||
}
|
||||
|
||||
protected class PostBinding extends Binding {
|
||||
|
|
|
@ -70,13 +70,17 @@ public class SamlClientBuilder {
|
|||
* @return Client that executed the steps
|
||||
*/
|
||||
public SamlClient execute(Consumer<CloseableHttpResponse> resultConsumer) {
|
||||
final SamlClient samlClient = new SamlClient();
|
||||
final SamlClient samlClient = createSamlClient();
|
||||
samlClient.executeAndTransform(r -> {
|
||||
resultConsumer.accept(r);
|
||||
return null;
|
||||
}, steps);
|
||||
return samlClient;
|
||||
}
|
||||
|
||||
protected SamlClient createSamlClient() {
|
||||
return new SamlClient();
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute the current steps and pass the final response to the {@code resultTransformer} for processing.
|
||||
|
@ -84,7 +88,7 @@ public class SamlClientBuilder {
|
|||
* @return Value returned by {@code resultTransformer}
|
||||
*/
|
||||
public <T> T executeAndTransform(ResultExtractor<T> resultTransformer) {
|
||||
return new SamlClient().executeAndTransform(resultTransformer, steps);
|
||||
return createSamlClient().executeAndTransform(resultTransformer, steps);
|
||||
}
|
||||
|
||||
public List<Step> getSteps() {
|
||||
|
|
|
@ -64,6 +64,7 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
|
|||
realm.setEnabled(true);
|
||||
realm.setRealm(REALM_CONS_NAME);
|
||||
realm.setResetPasswordAllowed(true);
|
||||
realm.setEventsListeners(Arrays.asList("jboss-logging", "event-queue"));
|
||||
|
||||
return realm;
|
||||
}
|
||||
|
|
|
@ -0,0 +1,221 @@
|
|||
package org.keycloak.testsuite.broker;
|
||||
|
||||
import org.apache.http.conn.ssl.NoopHostnameVerifier;
|
||||
import org.apache.http.conn.ssl.TrustAllStrategy;
|
||||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.apache.http.ssl.SSLContextBuilder;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.keycloak.dom.saml.v2.protocol.ResponseType;
|
||||
import org.keycloak.events.Errors;
|
||||
import org.keycloak.events.EventType;
|
||||
import org.keycloak.representations.idm.ClientRepresentation;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
|
||||
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
|
||||
import org.keycloak.testsuite.AssertEvents;
|
||||
import org.keycloak.testsuite.util.Matchers;
|
||||
import org.keycloak.testsuite.util.ReverseProxy;
|
||||
import org.keycloak.testsuite.util.SamlClient;
|
||||
import org.keycloak.testsuite.util.SamlClientBuilder;
|
||||
|
||||
import javax.ws.rs.core.Response;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.net.URI;
|
||||
import java.net.URISyntaxException;
|
||||
import java.net.URLEncoder;
|
||||
import java.security.KeyManagementException;
|
||||
import java.security.KeyStoreException;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.containsString;
|
||||
import static org.hamcrest.CoreMatchers.startsWith;
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestConstants.IDP_SAML_ALIAS;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestConstants.REALM_CONS_NAME;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_EMAIL;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_LOGIN;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_PASSWORD;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
|
||||
|
||||
public final class KcSamlBrokerFrontendUrlTest extends AbstractBrokerTest {
|
||||
|
||||
@Rule
|
||||
public ReverseProxy proxy = new ReverseProxy();
|
||||
|
||||
@Rule
|
||||
public AssertEvents events = new AssertEvents(this);
|
||||
|
||||
@Override
|
||||
protected BrokerConfiguration getBrokerConfiguration() {
|
||||
return new KcSamlBrokerConfiguration() {
|
||||
@Override
|
||||
public RealmRepresentation createConsumerRealm() {
|
||||
RealmRepresentation realm = super.createConsumerRealm();
|
||||
|
||||
Map<String, String> attributes = new HashMap<>();
|
||||
|
||||
attributes.put("frontendUrl", proxy.getUrl());
|
||||
|
||||
realm.setAttributes(attributes);
|
||||
|
||||
return realm;
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<ClientRepresentation> createProviderClients() {
|
||||
List<ClientRepresentation> clients = super.createProviderClients();
|
||||
|
||||
List<String> redirectUris = new ArrayList<>();
|
||||
|
||||
redirectUris.add(proxy.getUrl() + "/realms/" + REALM_CONS_NAME + "/broker/" + IDP_SAML_ALIAS + "/endpoint/*");
|
||||
|
||||
clients.get(0).setRedirectUris(redirectUris);
|
||||
|
||||
return clients;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getIDPClientIdInProviderRealm() {
|
||||
return proxy.getUrl() + "/realms/" + consumerRealmName();
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
private SamlClientBuilder clientBuilderTrustingAllCertificates() {
|
||||
return new SamlClientBuilder() {
|
||||
@Override
|
||||
protected SamlClient createSamlClient() {
|
||||
return new SamlClient() {
|
||||
@Override
|
||||
protected HttpClientBuilder createHttpClientBuilderInstance() {
|
||||
try {
|
||||
return super.createHttpClientBuilderInstance()
|
||||
.setSSLContext(new SSLContextBuilder().loadTrustMaterial(null, TrustAllStrategy.INSTANCE).build())
|
||||
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE);
|
||||
} catch (NoSuchAlgorithmException | KeyManagementException | KeyStoreException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@Test
|
||||
@Override
|
||||
public void testLogInAsUserInIDP() {
|
||||
updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin);
|
||||
createUser(bc.consumerRealmName(), "consumer", "password", "FirstName", "LastName", "consumer@localhost.com");
|
||||
|
||||
driver.navigate().to(proxy.getUrl() + "/realms/consumer/account");
|
||||
log.debug("Clicking social " + bc.getIDPAlias());
|
||||
loginPage.clickSocial(bc.getIDPAlias());
|
||||
waitForPage(driver, "sign in to", true);
|
||||
log.debug("Logging in");
|
||||
|
||||
// make sure the frontend url is used to build the redirect uri when redirecting to the broker
|
||||
try {
|
||||
assertThat(driver.getCurrentUrl(), containsString("client_id=" + URLEncoder.encode(proxy.getUrl(), "UTF-8")));
|
||||
} catch (UnsupportedEncodingException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
|
||||
loginPage.login(bc.getUserLogin(), bc.getUserPassword());
|
||||
waitForPage(driver, "account management", true);
|
||||
accountUpdateProfilePage.assertCurrent();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFrontendUrlInDestinationExpected() throws URISyntaxException {
|
||||
SAMLDocumentHolder samlResponse = clientBuilderTrustingAllCertificates()
|
||||
.idpInitiatedLogin(new URI(proxy.getUrl() + "/realms/" + bc.consumerRealmName() + "/protocol/saml"), "sales-post").build()
|
||||
// Request login via kc-saml-idp
|
||||
.login().idp(IDP_SAML_ALIAS).build()
|
||||
|
||||
.processSamlResponse(SamlClient.Binding.POST) // AuthnRequest to producer IdP
|
||||
.targetAttributeSamlRequest()
|
||||
.build()
|
||||
|
||||
// Login in provider realm
|
||||
.login().user(USER_LOGIN, USER_PASSWORD).build()
|
||||
|
||||
// Send the response to the consumer realm
|
||||
.processSamlResponse(SamlClient.Binding.POST)
|
||||
.transformObject(saml2Object -> {
|
||||
assertThat(saml2Object, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
|
||||
ResponseType response = (ResponseType) saml2Object;
|
||||
|
||||
assertThat(response.getDestination(), startsWith(proxy.getUrl()));
|
||||
|
||||
return saml2Object;
|
||||
})
|
||||
.build()
|
||||
|
||||
// Create account in comsumer realm
|
||||
.updateProfile().username(USER_LOGIN).email(USER_EMAIL).firstName("Firstname").lastName("Lastname").build()
|
||||
.followOneRedirect()
|
||||
|
||||
// Obtain the response sent to the app
|
||||
.getSamlResponse(SamlClient.Binding.POST);
|
||||
|
||||
assertThat(samlResponse.getSamlObject(), Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeycloakRejectsRealUrlWhenFrontendUrlConfigured() throws URISyntaxException {
|
||||
clientBuilderTrustingAllCertificates()
|
||||
.idpInitiatedLogin(new URI(proxy.getUrl() + "/realms/" + bc.consumerRealmName() + "/protocol/saml"), "sales-post").build()
|
||||
// Request login via kc-saml-idp
|
||||
.login().idp(IDP_SAML_ALIAS).build()
|
||||
|
||||
.processSamlResponse(SamlClient.Binding.POST) // AuthnRequest to producer IdP
|
||||
.targetAttributeSamlRequest()
|
||||
.build()
|
||||
|
||||
// Login in provider realm
|
||||
.login().user(USER_LOGIN, USER_PASSWORD).build()
|
||||
|
||||
// Send the response to the consumer realm
|
||||
.processSamlResponse(SamlClient.Binding.POST)
|
||||
.transformObject(saml2Object -> {
|
||||
assertThat(saml2Object, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
|
||||
ResponseType response = (ResponseType) saml2Object;
|
||||
|
||||
assertThat(response.getDestination(), startsWith(proxy.getUrl()));
|
||||
|
||||
response.setDestination(getConsumerRoot() + "/auth/realms/" + REALM_CONS_NAME + "/broker/" + IDP_SAML_ALIAS + "/endpoint");
|
||||
return saml2Object;
|
||||
})
|
||||
.build()
|
||||
|
||||
// Obtain the response sent to the app
|
||||
.execute(response -> {
|
||||
assertThat(response, Matchers.statusCodeIsHC(Response.Status.BAD_REQUEST));
|
||||
String consumerRealmId = realmsResouce().realm(bc.consumerRealmName()).toRepresentation().getId();
|
||||
events.expect(EventType.IDENTITY_PROVIDER_RESPONSE_ERROR)
|
||||
.clearDetails()
|
||||
.session((String) null)
|
||||
.realm(consumerRealmId)
|
||||
.user((String) null)
|
||||
.client((String) null)
|
||||
.error(Errors.INVALID_SAML_RESPONSE)
|
||||
.detail("reason", Errors.INVALID_DESTINATION)
|
||||
.assertEvent();
|
||||
events.assertEmpty();
|
||||
});
|
||||
}
|
||||
|
||||
@Ignore
|
||||
@Test
|
||||
@Override
|
||||
public void loginWithExistingUser() {
|
||||
// no-op
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue