libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Fix error type in SAML response on missing destination

Browse source 
We now use INVALID_SAML_RESPONSE insteadof INVALID_LOGOUT_RESPONSE.
Added proposed test case.

Closes #11178

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
Co-authored-by: Michal Hajas <mhajas@redhat.com>
Co-authored-by: Chris Dolphy <cdolphy@redhat.com>
This commit is contained in:
Thomas Darimont 2024-01-31 09:32:14 +01:00 committed by GitHub
parent 2480fab6f9
commit 346c2926f6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 82 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
View file

@ -658,7 +658,7 @@ public class SAMLEndpoint {
&& statusResponse.getDestination() == null && containsUnencryptedSignature(holder)) { && statusResponse.getDestination() == null && containsUnencryptedSignature(holder)) {
event.event(EventType.IDENTITY_PROVIDER_RESPONSE); event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.detail(Details.REASON, Errors.MISSING_REQUIRED_DESTINATION); event.detail(Details.REASON, Errors.MISSING_REQUIRED_DESTINATION);
event.error(Errors.INVALID_SAML_LOGOUT_RESPONSE); event.error(Errors.INVALID_SAML_RESPONSE);
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST); return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST);
} }
if (! destinationValidator.validate(getExpectedDestination(config.getAlias(), clientId), statusResponse.getDestination())) { if (! destinationValidator.validate(getExpectedDestination(config.getAlias(), clientId), statusResponse.getDestination())) {

81
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerDestinationTest.java Normal file
View file

@ -0,0 +1,81 @@
package org.keycloak.testsuite.broker;
import org.junit.Rule;
import org.junit.Test;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.protocol.saml.SamlConfigAttributes;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
import org.keycloak.testsuite.util.SamlClient;
import org.keycloak.testsuite.util.SamlClientBuilder;
import org.w3c.dom.Element;
import jakarta.ws.rs.core.Response;
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.REALM_PROV_NAME;
import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_LOGIN;
import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_PASSWORD;
import static org.keycloak.testsuite.util.Matchers.statusCodeIsHC;
public class KcSamlBrokerDestinationTest extends AbstractBrokerTest {
@Rule
public AssertEvents events = new AssertEvents(this);
@Override
protected BrokerConfiguration getBrokerConfiguration() {
return KcSamlBrokerConfiguration.INSTANCE;
}
@Test
public void testNullDestinationInResponseShouldReturnInvalidSamlResponse() {
getCleanup(REALM_PROV_NAME)
.addCleanup(ClientAttributeUpdater.forClient(adminClient, bc.providerRealmName(), bc.getIDPClientIdInProviderRealm())
.setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, Boolean.toString(true))
.setAttribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, "false") // Do not require client signature
.update()
);
new SamlClientBuilder()
.idpInitiatedLogin(getConsumerSamlEndpoint(REALM_CONS_NAME), "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)
.transformDocument(doc -> {
Element documentElement = doc.getDocumentElement();
documentElement.removeAttribute("Destination");
})
.build()
.execute(response -> {
assertThat(response, statusCodeIsHC(Response.Status.BAD_REQUEST));
String consumerRealmId = realmsResouce().realm(bc.consumerRealmName()).toRepresentation().getId();
String expectedError = Errors.INVALID_SAML_RESPONSE;
events.expect(EventType.IDENTITY_PROVIDER_RESPONSE)
.clearDetails()
.session((String) null)
.realm(consumerRealmId)
.user((String) null)
.client((String) null)
.error(expectedError)
.detail("reason", Errors.MISSING_REQUIRED_DESTINATION)
.assertEvent();
events.assertEmpty();
});
}
}