KEYCLOAK-19177 Disable ECP flow by default for all Saml clients; ecp flow creates only transient users sessions

This commit is contained in:
Michal Hajas 2021-10-04 15:26:29 +02:00 committed by Hynek Mlnařík
parent b5f70d8a32
commit 44000caaf5
12 changed files with 135 additions and 4 deletions

View file

@ -120,6 +120,14 @@ public class SamlClient extends ClientConfigResolver {
client.setAttribute(SamlConfigAttributes.SAML_FORCE_NAME_ID_FORMAT_ATTRIBUTE, Boolean.toString(val));
}
public boolean allowECPFlow() {
return "true".equals(resolveAttribute(SamlConfigAttributes.SAML_ALLOW_ECP_FLOW));
}
public void setAllowECPFlow(boolean val) {
client.setAttribute(SamlConfigAttributes.SAML_ALLOW_ECP_FLOW, Boolean.toString(val));
}
public boolean forceArtifactBinding(){
return "true".equals(resolveAttribute(SamlConfigAttributes.SAML_ARTIFACT_BINDING));
}

View file

@ -44,4 +44,5 @@ public interface SamlConfigAttributes {
String SAML_ENCRYPTION_PRIVATE_KEY_ATTRIBUTE = "saml.encryption." + CertificateInfoHelper.PRIVATE_KEY;
String SAML_ASSERTION_LIFESPAN = "saml.assertion.lifespan";
String SAML_ARTIFACT_BINDING_IDENTIFIER = "saml.artifact.binding.identifier";
String SAML_ALLOW_ECP_FLOW = "saml.allow.ecp.flow";
}

View file

@ -154,6 +154,10 @@ public class SamlProtocolFactory extends AbstractLoginProtocolFactory {
client.setForceNameIDFormat(false);
}
if (rep.getAllowEcpFlow() == null) {
client.setAllowECPFlow(false);
}
if (rep.getSamlServerSignature() == null) {
client.setRequiresRealmSignature(true);
}

View file

@ -61,6 +61,11 @@ public class SamlRepresentationAttributes {
return getAttributes().get(SamlConfigAttributes.SAML_FORCE_NAME_ID_FORMAT_ATTRIBUTE);
}
public String getAllowEcpFlow() {
if (getAttributes() == null) return null;
return getAttributes().get(SamlConfigAttributes.SAML_ALLOW_ECP_FLOW);
}
public String getSamlArtifactBinding() {
if (getAttributes() == null) return null;
return getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING);

View file

@ -26,6 +26,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.models.utils.DefaultAuthenticationFlows;
import org.keycloak.protocol.saml.JaxrsSAML2BindingBuilder;
import org.keycloak.protocol.saml.SamlClient;
import org.keycloak.protocol.saml.SamlConfigAttributes;
import org.keycloak.protocol.saml.SamlProtocol;
import org.keycloak.protocol.saml.SamlService;
@ -36,6 +37,7 @@ import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import org.keycloak.saml.common.exceptions.ConfigurationException;
import org.keycloak.saml.common.exceptions.ProcessingException;
import org.keycloak.saml.validators.DestinationValidator;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.w3c.dom.Document;
@ -44,7 +46,6 @@ import javax.xml.soap.SOAPException;
import javax.xml.soap.SOAPHeaderElement;
import java.io.IOException;
import java.io.InputStream;
import java.util.Map;
import java.util.Objects;
/**
@ -79,6 +80,12 @@ public class SamlEcpProfileService extends SamlService {
@Override
protected Response loginRequest(String relayState, AuthnRequestType requestAbstractType, ClientModel client) {
// Do not allow ECP login when client does not support it
if (!new SamlClient(client).allowECPFlow()) {
logger.errorf("Client %s is not allowed to execute ECP flow", client.getClientId());
throw new RuntimeException("Client is not allowed to use ECP profile.");
}
// force passive authentication when executing this profile
requestAbstractType.setIsPassive(true);
requestAbstractType.setDestination(session.getContext().getUri().getAbsolutePath());
@ -99,6 +106,8 @@ public class SamlEcpProfileService extends SamlService {
@Override
protected Response newBrowserAuthentication(AuthenticationSessionModel authSession, boolean isPassive, boolean redirectToAuthentication, SamlProtocol samlProtocol) {
// Saml ECP flow creates only TRANSIENT user sessions
authSession.setClientNote(AuthenticationManager.USER_SESSION_PERSISTENT_STATE, UserSessionModel.SessionPersistenceState.TRANSIENT.toString());
return super.newBrowserAuthentication(authSession, isPassive, redirectToAuthentication, createEcpSamlProtocol());
}

View file

@ -17,17 +17,34 @@
package org.keycloak.testsuite.saml;
import org.junit.Test;
import org.keycloak.dom.saml.v2.SAML2Object;
import org.keycloak.dom.saml.v2.assertion.AuthnStatementType;
import org.keycloak.dom.saml.v2.protocol.ResponseType;
import org.keycloak.dom.saml.v2.protocol.StatusResponseType;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.protocol.saml.SamlConfigAttributes;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
import org.keycloak.testsuite.util.SamlClientBuilder;
import javax.ws.rs.core.Response;
import javax.xml.soap.MessageFactory;
import javax.xml.soap.SOAPException;
import javax.xml.soap.SOAPMessage;
import java.io.IOException;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertThat;
import static org.hamcrest.Matchers.nullValue;
import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
import static org.keycloak.testsuite.util.Matchers.statusCodeIsHC;
import static org.keycloak.testsuite.util.SamlClient.Binding.POST;
import static org.keycloak.testsuite.util.SamlClient.Binding.SOAP;
@ -214,4 +231,64 @@ public class SOAPBindingTest extends AbstractSamlTest {
assertThat(response.getSamlObject(), instanceOf(StatusResponseType.class));
}
@Test
public void soapBindingIsNotPossibleForClientsWithSamlEcpFlowAttributeFalse() {
// Disable ECP_FLOW_ENABLED switch
getCleanup().addCleanup(ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_ECP_SP)
.setAttribute(SamlConfigAttributes.SAML_ALLOW_ECP_FLOW, "false")
.setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "false")
.setAttribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, "false")
.update());
new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_ECP_SP, SAML_ASSERTION_CONSUMER_URL_ECP_SP, SOAP)
.basicAuthentication(bburkeUser)
.build()
.execute(response -> {
assertThat(response, statusCodeIsHC(Response.Status.INTERNAL_SERVER_ERROR));
try {
MessageFactory messageFactory = MessageFactory.newInstance();
SOAPMessage soapMessage = messageFactory.createMessage(null, response.getEntity().getContent());
String faultDetail = soapMessage.getSOAPBody().getFault().getDetail().getValue();
assertThat(faultDetail, is(equalTo("Client is not allowed to use ECP profile.")));
} catch (SOAPException | IOException e) {
throw new RuntimeException(e);
}
});
}
@Test
public void ecpFlowCreatesTransientSessions() {
// Disable ECP_FLOW_ENABLED switch
getCleanup().addCleanup(ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_ECP_SP)
.setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "false")
.setAttribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, "false")
.update());
// Successfully login using ECP flow
SAML2Object samlObject = new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_ECP_SP, SAML_ASSERTION_CONSUMER_URL_ECP_SP, SOAP)
.basicAuthentication(bburkeUser)
.build()
.executeAndTransform(SOAP::extractResponse).getSamlObject();
assertThat(samlObject, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
ResponseType loginResp1 = (ResponseType) samlObject;
AuthnStatementType sessionId = (AuthnStatementType) loginResp1.getAssertions().get(0).getAssertion().getStatements().iterator().next();
String userSessionId = sessionId.getSessionIndex().split("::")[0];
// Test that the user session with the given ID does not exist
testingClient.server().run(session -> {
RealmModel realmByName = session.realms().getRealmByName(REALM_NAME);
UserSessionModel userSession = session.sessions().getUserSession(realmByName, userSessionId);
assertThat(userSession, nullValue());
});
}
}

View file

@ -716,7 +716,8 @@
"saml.signature.algorithm": "RSA_SHA256",
"saml.client.signature": "true",
"saml.authnstatement": "true",
"saml.signing.certificate": "MIIB1DCCAT0CBgFJGP5dZDANBgkqhkiG9w0BAQsFADAwMS4wLAYDVQQDEyVodHRwOi8vbG9jYWxob3N0OjgwODAvc2FsZXMtcG9zdC1zaWcvMB4XDTE0MTAxNjEyNDQyM1oXDTI0MTAxNjEyNDYwM1owMDEuMCwGA1UEAxMlaHR0cDovL2xvY2FsaG9zdDo4MDgwL3NhbGVzLXBvc3Qtc2lnLzCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA1RvGu8RjemSJA23nnMksoHA37MqY1DDTxOECY4rPAd9egr7GUNIXE0y1MokaR5R2crNpN8RIRwR8phQtQDjXL82c6W+NLQISxztarQJ7rdNJIYwHY0d5ri1XRpDP8zAuxubPYiMAVYcDkIcvlbBpwh/dRM5I2eElRK+eSiaMkCUCAwEAATANBgkqhkiG9w0BAQsFAAOBgQCLms6htnPaY69k1ntm9a5jgwSn/K61cdai8R8B0ccY7zvinn9AfRD7fiROQpFyY29wKn8WCLrJ86NBXfgFUGyR5nLNHVy3FghE36N2oHy53uichieMxffE6vhkKJ4P8ChfJMMOZlmCPsQPDvjoAghHt4mriFiQgRdPgIy/zDjSNw=="
"saml.signing.certificate": "MIIB1DCCAT0CBgFJGP5dZDANBgkqhkiG9w0BAQsFADAwMS4wLAYDVQQDEyVodHRwOi8vbG9jYWxob3N0OjgwODAvc2FsZXMtcG9zdC1zaWcvMB4XDTE0MTAxNjEyNDQyM1oXDTI0MTAxNjEyNDYwM1owMDEuMCwGA1UEAxMlaHR0cDovL2xvY2FsaG9zdDo4MDgwL3NhbGVzLXBvc3Qtc2lnLzCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA1RvGu8RjemSJA23nnMksoHA37MqY1DDTxOECY4rPAd9egr7GUNIXE0y1MokaR5R2crNpN8RIRwR8phQtQDjXL82c6W+NLQISxztarQJ7rdNJIYwHY0d5ri1XRpDP8zAuxubPYiMAVYcDkIcvlbBpwh/dRM5I2eElRK+eSiaMkCUCAwEAATANBgkqhkiG9w0BAQsFAAOBgQCLms6htnPaY69k1ntm9a5jgwSn/K61cdai8R8B0ccY7zvinn9AfRD7fiROQpFyY29wKn8WCLrJ86NBXfgFUGyR5nLNHVy3FghE36N2oHy53uichieMxffE6vhkKJ4P8ChfJMMOZlmCPsQPDvjoAghHt4mriFiQgRdPgIy/zDjSNw==",
"saml.allow.ecp.flow": "true"
}
},
{

View file

@ -4,6 +4,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import org.keycloak.protocol.saml.SamlConfigAttributes;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.testsuite.console.page.clients.CreateClientForm;
import org.keycloak.testsuite.console.page.fragment.OnOffSwitch;
@ -262,6 +263,7 @@ public class ClientSettingsForm extends CreateClientForm {
public static final String SAML_ASSERTION_CONSUMER_URL_REDIRECT = "saml_assertion_consumer_url_redirect";
public static final String SAML_FORCE_NAME_ID_FORMAT = "saml_force_name_id_format";
public static final String SAML_NAME_ID_FORMAT = "saml_name_id_format";
public static final String SAML_ALLOW_ECP_FLOW = SamlConfigAttributes.SAML_ALLOW_ECP_FLOW;
public static final String SAML_SIGNATURE_CANONICALIZATION_METHOD = "saml_signature_canonicalization_method";
public static final String SAML_SINGLE_LOGOUT_SERVICE_URL_POST = "saml_single_logout_service_url_post";
public static final String SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT = "saml_single_logout_service_url_redirect";

View file

@ -23,10 +23,10 @@ import static org.keycloak.testsuite.auth.page.login.OIDCLogin.OIDC;
import static org.keycloak.testsuite.auth.page.login.OIDCLogin.SAML;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_AUTHNSTATEMENT;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_CLIENT_SIGNATURE;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_ALLOW_ECP_FLOW;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_FORCE_NAME_ID_FORMAT;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_FORCE_POST_BINDING;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_NAME_ID_FORMAT;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_ONETIMEUSE_CONDITION;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_SERVER_SIGNATURE;
import static org.keycloak.testsuite.console.page.clients.settings.ClientSettingsForm.SAMLClientSettingsForm.SAML_SIGNATURE_ALGORITHM;
import static org.keycloak.testsuite.util.AttributesAssert.assertEqualsBooleanAttributes;
@ -89,6 +89,7 @@ public abstract class AbstractClientTest extends AbstractConsoleTest {
attributes.put(SAML_SIGNATURE_ALGORITHM, "RSA_SHA256");
attributes.put(SAML_FORCE_NAME_ID_FORMAT, "false");
attributes.put(SAML_NAME_ID_FORMAT, "username");
attributes.put(SAML_ALLOW_ECP_FLOW, "false");
attributes.put(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER, ArtifactBindingUtils.computeArtifactBindingIdentifierString("saml"));
return attributes;
}

View file

@ -371,6 +371,8 @@ front-channel-logout-session-required.tooltip=Specifying whether a sid (session
force-name-id-format=Force Name ID Format
force-name-id-format.tooltip=Ignore requested NameID subject format and use admin console configured one.
allow-ecp-flow=Allow ECP Flow
allow-ecp-flow.tooltip=This client is allowed to use ECP flow for authenticating users.
name-id-format=Name ID Format
name-id-format.tooltip=The name ID format to use for the subject.
mapper.nameid.format.tooltip=Name ID Format using Mapper

View file

@ -1197,6 +1197,7 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, flows, $ro
$scope.samlEncrypt = false;
$scope.samlForcePostBinding = false;
$scope.samlForceNameIdFormat = false;
$scope.samlAllowECPFlow = false;
$scope.samlXmlKeyNameTranformer = $scope.xmlKeyNameTranformers[1];
$scope.disableAuthorizationTab = !client.authorizationServicesEnabled;
$scope.disableServiceAccountRolesTab = !client.serviceAccountsEnabled;
@ -1351,6 +1352,13 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, flows, $ro
$scope.samlForceNameIdFormat = false;
}
}
if ($scope.client.attributes["saml.allow.ecp.flow"]) {
if ($scope.client.attributes["saml.allow.ecp.flow"] == "true") {
$scope.samlAllowECPFlow = true;
} else {
$scope.samlAllowECPFlow = false;
}
}
if ($scope.client.attributes["saml.multivalued.roles"]) {
if ($scope.client.attributes["saml.multivalued.roles"] == "true") {
$scope.samlMultiValuedRoles = true;
@ -1961,6 +1969,12 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, flows, $ro
} else {
$scope.clientEdit.attributes["saml_force_name_id_format"] = "false";
}
if ($scope.samlAllowECPFlow == true) {
$scope.clientEdit.attributes["saml.allow.ecp.flow"] = "true";
} else {
$scope.clientEdit.attributes["saml.allow.ecp.flow"] = "false";
}
if ($scope.samlMultiValuedRoles == true) {
$scope.clientEdit.attributes["saml.multivalued.roles"] = "true";

View file

@ -299,6 +299,13 @@
</div>
<kc-tooltip>{{:: 'force-name-id-format.tooltip' | translate}}</kc-tooltip>
</div>
<div class="form-group clearfix block" data-ng-show="protocol == 'saml'">
<label class="col-md-2 control-label" for="samlAllowECPFlow">{{:: 'allow-ecp-flow' | translate}}</label>
<div class="col-sm-6">
<input ng-model="samlAllowECPFlow" ng-click="switchChange()" name="samlAllowECPFlow" id="samlAllowECPFlow" onoffswitch on-text="{{:: 'onText' | translate}}" off-text="{{:: 'offText' | translate}}"/>
</div>
<kc-tooltip>{{:: 'allow-ecp-flow.tooltip' | translate}}</kc-tooltip>
</div>
<div class="form-group" data-ng-show="protocol == 'saml'">
<label class="col-md-2 control-label" for="samlNameIdFormat">{{:: 'name-id-format' | translate}}</label>
<div class="col-sm-6">