Merge pull request #4350 from hmlnarik/KEYCLOAK-4446-Failed-to-process-response-when-reject-consent-with-turned-on-encryption

KEYCLOAK-4446 Do not encrypt SAML status messages
This commit is contained in:
Marek Posolda 2017-07-26 15:31:54 +02:00 committed by GitHub
commit dd6a7b23c3
5 changed files with 69 additions and 11 deletions

View file

@ -18,7 +18,10 @@
package org.keycloak.adapters.saml;
import org.keycloak.adapters.spi.AuthenticationError;
import org.keycloak.dom.saml.v2.protocol.StatusCodeType;
import org.keycloak.dom.saml.v2.protocol.StatusResponseType;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import java.util.Objects;
/**
* Object that describes the SAML error that happened.
@ -27,6 +30,7 @@ import org.keycloak.dom.saml.v2.protocol.StatusResponseType;
* @version $Revision: 1 $
*/
public class SamlAuthenticationError implements AuthenticationError {
public static enum Reason {
EXTRACTION_FAILURE,
INVALID_SIGNATURE,
@ -59,7 +63,18 @@ public class SamlAuthenticationError implements AuthenticationError {
@Override
public String toString() {
return "SamlAuthenticationError [reason=" + reason + ", status=" + status + "]";
return "SamlAuthenticationError [reason=" + reason + ", status="
+ ((status == null || status.getStatus() == null) ? "UNKNOWN" : extractStatusCode(status.getStatus().getStatusCode()))
+ "]";
}
private String extractStatusCode(StatusCodeType statusCode) {
if (statusCode == null || statusCode.getValue() == null) {
return "UNKNOWN";
}
if (Objects.equals(JBossSAMLURIConstants.STATUS_RESPONDER.get(), statusCode.getValue().toString())) {
return extractStatusCode(statusCode.getStatusCode());
}
return statusCode.getValue().toString();
}
}

View file

@ -190,16 +190,9 @@ public class SamlProtocol implements LoginProtocol {
}
binding.signatureAlgorithm(samlClient.getSignatureAlgorithm()).signWith(keyName, keys.getPrivateKey(), keys.getPublicKey(), keys.getCertificate()).signDocument();
}
if (samlClient.requiresEncryption()) {
PublicKey publicKey;
try {
publicKey = SamlProtocolUtils.getEncryptionKey(client);
} catch (Exception e) {
logger.error("failed", e);
return ErrorPage.error(session, Messages.FAILED_TO_PROCESS_RESPONSE);
}
binding.encrypt(publicKey);
}
// There is no support for encrypting status messages in SAML.
// Only assertions, attributes, base ID and name ID can be encrypted
// See Chapter 6 of saml-core-2.0-os.pdf
Document document = builder.buildDocument();
return buildErrorResponse(authSession, binding, document);
} catch (Exception e) {

View file

@ -27,6 +27,7 @@ import java.net.URL;
*/
public class SalesPostServlet extends SAMLServlet {
public static final String DEPLOYMENT_NAME = "sales-post";
public static final String CLIENT_NAME = "http://localhost:8081/sales-post/";
@ArquillianResource
@OperateOnDeployment(DEPLOYMENT_NAME)

View file

@ -35,6 +35,11 @@ public class ClientAttributeUpdater {
return this;
}
public ClientAttributeUpdater setConsentRequired(Boolean consentRequired) {
rep.setConsentRequired(consentRequired);
return this;
}
public ClientAttributeUpdater setFrontchannelLogout(Boolean frontchannelLogout) {
rep.setFrontchannelLogout(frontchannelLogout);
return this;

View file

@ -661,6 +661,50 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd
}
}
@Test
public void salesPostEncRejectConsent() throws Exception {
ClientRepresentation salesPostEncClient = testRealmResource().clients().findByClientId(SalesPostEncServlet.CLIENT_NAME).get(0);
try (Closeable client = new ClientAttributeUpdater(testRealmResource().clients().get(salesPostEncClient.getId()))
.setConsentRequired(true)
.update()) {
new SamlClientBuilder()
.navigateTo(salesPostEncServletPage.toString())
.processSamlResponse(Binding.POST).build()
.login().user(bburkeUser).build()
.consentRequired().approveConsent(false).build()
.processSamlResponse(Binding.POST).build()
.execute(r -> {
assertThat(r, statusCodeIsHC(Response.Status.OK));
assertThat(r, bodyHC(containsString("urn:oasis:names:tc:SAML:2.0:status:RequestDenied"))); // TODO: revisit - should the HTTP status be 403 too?
});
} finally {
salesPostEncServletPage.logout();
}
}
@Test
public void salesPostRejectConsent() throws Exception {
ClientRepresentation salesPostClient = testRealmResource().clients().findByClientId(SalesPostServlet.CLIENT_NAME).get(0);
try (Closeable client = new ClientAttributeUpdater(testRealmResource().clients().get(salesPostClient.getId()))
.setConsentRequired(true)
.update()) {
new SamlClientBuilder()
.navigateTo(salesPostServletPage.toString())
.processSamlResponse(Binding.POST).build()
.login().user(bburkeUser).build()
.consentRequired().approveConsent(false).build()
.processSamlResponse(Binding.POST).build()
.execute(r -> {
assertThat(r, statusCodeIsHC(Response.Status.OK));
assertThat(r, bodyHC(containsString("urn:oasis:names:tc:SAML:2.0:status:RequestDenied"))); // TODO: revisit - should the HTTP status be 403 too?
});
} finally {
salesPostServletPage.logout();
}
}
@Test
public void salesPostPassiveTest() {
salesPostPassiveServletPage.navigateTo();