From 82ef5b7927648a51fe3c5a340a5b8113e92ba5b8 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Fri, 15 Nov 2019 10:29:26 +0100 Subject: [PATCH] KEYCLOAK-12000: Allow overriding time lifespans on a SAML client --- .../keycloak/protocol/saml/SamlClient.java | 18 ++++ .../protocol/saml/SamlConfigAttributes.java | 1 + .../keycloak/protocol/saml/SamlProtocol.java | 5 +- .../updaters/ClientAttributeUpdater.java | 3 + .../saml/SessionNotOnOrAfterTest.java | 97 +++++++++++++++++-- .../messages/admin-messages_en.properties | 2 + .../admin/resources/js/controllers/clients.js | 9 ++ .../resources/partials/client-detail.html | 23 ++++- 8 files changed, 145 insertions(+), 13 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java b/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java index 38d253d9fe..32c9838a9a 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java @@ -17,6 +17,7 @@ package org.keycloak.protocol.saml; +import org.jboss.logging.Logger; import org.keycloak.models.ClientConfigResolver; import org.keycloak.models.ClientModel; import org.keycloak.saml.SignatureAlgorithm; @@ -31,6 +32,8 @@ import org.keycloak.saml.common.util.XmlKeyInfoKeyNameTransformer; */ public class SamlClient extends ClientConfigResolver { + protected static final Logger logger = Logger.getLogger(SamlClient.class); + public static final XmlKeyInfoKeyNameTransformer DEFAULT_XML_KEY_INFO_KEY_NAME_TRANSFORMER = XmlKeyInfoKeyNameTransformer.KEY_ID; public SamlClient(ClientModel client) { @@ -231,5 +234,20 @@ public class SamlClient extends ClientConfigResolver { client.setAttribute(SamlConfigAttributes.SAML_ONETIMEUSE_CONDITION, Boolean.toString(val)); } + public void setAssertionLifespan(int assertionLifespan) { + client.setAttribute(SamlConfigAttributes.SAML_ASSERTION_LIFESPAN, Integer.toString(assertionLifespan)); + } + public int getAssertionLifespan() { + String value = client.getAttribute(SamlConfigAttributes.SAML_ASSERTION_LIFESPAN); + if (value == null || value.isEmpty()) { + return -1; + } + try { + return Integer.parseInt(value); + } catch (NumberFormatException e) { + logger.warnf("Invalid numeric value for saml attribute \"%s\": %s", SamlConfigAttributes.SAML_ASSERTION_LIFESPAN, value); + return -1; + } + } } diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java b/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java index ef700268d0..7453e27192 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java @@ -41,4 +41,5 @@ public interface SamlConfigAttributes { String SAML_SIGNING_CERTIFICATE_ATTRIBUTE = "saml.signing." + CertificateInfoHelper.X509CERTIFICATE; String SAML_ENCRYPTION_CERTIFICATE_ATTRIBUTE = "saml.encryption." + CertificateInfoHelper.X509CERTIFICATE; String SAML_ENCRYPTION_PRIVATE_KEY_ATTRIBUTE = "saml.encryption." + CertificateInfoHelper.PRIVATE_KEY; + String SAML_ASSERTION_LIFESPAN = "saml.assertion.lifespan"; } diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index 697ac2fa3e..bbc3425476 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -397,12 +397,13 @@ public class SamlProtocol implements LoginProtocol { clientSession.setNote(SAML_NAME_ID, nameId); clientSession.setNote(SAML_NAME_ID_FORMAT, nameIdFormat); + int assertionLifespan = samlClient.getAssertionLifespan(); SAML2LoginResponseBuilder builder = new SAML2LoginResponseBuilder(); builder.requestID(requestID) .destination(redirectUri) .issuer(responseIssuer) - .assertionExpiration(realm.getAccessCodeLifespan()) - .subjectExpiration(realm.getAccessTokenLifespan()) + .assertionExpiration(assertionLifespan <= 0? realm.getAccessCodeLifespan() : assertionLifespan) + .subjectExpiration(assertionLifespan <= 0? realm.getAccessTokenLifespan() : assertionLifespan) .sessionExpiration(realm.getSsoSessionMaxLifespan()) .requestIssuer(clientSession.getClient().getClientId()) .nameIdentifier(nameIdFormat, nameId) diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java index 9f276b0801..5e345d50ae 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java @@ -68,6 +68,9 @@ public class ClientAttributeUpdater extends ServerResourceUpdater statements = resp.getAssertions().get(0).getAssertion().getStatements(); + Assert.assertNotNull(resp); + Assert.assertNotNull(resp.getAssertions()); + Assert.assertThat(resp.getAssertions().size(), greaterThan(0)); + Assert.assertNotNull(resp.getAssertions().get(0)); + Assert.assertNotNull(resp.getAssertions().get(0).getAssertion()); + + // session lifespan + Assert.assertNotNull(resp.getAssertions().get(0).getAssertion().getStatements()); + Set statements = resp.getAssertions().get(0).getAssertion().getStatements(); AuthnStatementType authType = statements.stream() .filter(statement -> statement instanceof AuthnStatementType) .map(s -> (AuthnStatementType) s) @@ -37,7 +56,28 @@ public class SessionNotOnOrAfterTest extends AbstractSamlTest { assertThat(authType, notNullValue()); assertThat(authType.getSessionNotOnOrAfter(), notNullValue()); - assertThat(authType.getSessionNotOnOrAfter(), is(XMLTimeUtil.add(authType.getAuthnInstant(), SSO_MAX_LIFESPAN * 1000))); + assertThat(authType.getSessionNotOnOrAfter(), is(XMLTimeUtil.add(authType.getAuthnInstant(), ssoMaxLifespan * 1000))); + + // Conditions + Assert.assertNotNull(resp.getAssertions().get(0).getAssertion().getConditions()); + Assert.assertNotNull(resp.getAssertions().get(0).getAssertion().getConditions()); + ConditionsType condition = resp.getAssertions().get(0).getAssertion().getConditions(); + + Assert.assertEquals(XMLTimeUtil.add(condition.getNotBefore(), accessCodeLifespan * 1000), condition.getNotOnOrAfter()); + + // SubjectConfirmation (confirmationData has no NotBefore, using the previous one because it's the same) + Assert.assertNotNull(resp.getAssertions().get(0).getAssertion().getSubject()); + Assert.assertNotNull(resp.getAssertions().get(0).getAssertion().getSubject().getConfirmation()); + List confirmations = resp.getAssertions().get(0).getAssertion().getSubject().getConfirmation(); + + SubjectConfirmationDataType confirmationData = confirmations.stream() + .map(c -> c.getSubjectConfirmationData()) + .filter(c -> c != null) + .findFirst() + .orElse(null); + + Assert.assertNotNull(confirmationData); + Assert.assertEquals(XMLTimeUtil.add(condition.getNotBefore(), accessTokenLifespan * 1000), confirmationData.getNotOnOrAfter()); return null; } @@ -45,13 +85,17 @@ public class SessionNotOnOrAfterTest extends AbstractSamlTest { @Test public void testSamlResponseContainsSessionNotOnOrAfterIdpInitiatedLogin() throws Exception { try(AutoCloseable c = new RealmAttributeUpdater(adminClient.realm(REALM_NAME)) - .updateWith(r -> r.setSsoSessionMaxLifespan(SSO_MAX_LIFESPAN)) + .updateWith(r -> { + r.setSsoSessionMaxLifespan(SSO_MAX_LIFESPAN); + r.setAccessCodeLifespan(ACCESS_CODE_LIFESPAN); + r.setAccessTokenLifespan(ACCESS_TOKEN_LIFESPAN); + }) .update()) { new SamlClientBuilder() .idpInitiatedLogin(getAuthServerSamlEndpoint(REALM_NAME), "sales-post").build() .login().user(bburkeUser).build() .processSamlResponse(SamlClient.Binding.POST) - .transformObject(this::checkSessionNotOnOrAfter) + .transformObject(r -> checkSessionNotOnOrAfter(r, SSO_MAX_LIFESPAN, ACCESS_CODE_LIFESPAN, ACCESS_TOKEN_LIFESPAN)) .build() .execute(); } @@ -60,14 +104,51 @@ public class SessionNotOnOrAfterTest extends AbstractSamlTest { @Test public void testSamlResponseContainsSessionNotOnOrAfterAuthnLogin() throws Exception { try(AutoCloseable c = new RealmAttributeUpdater(adminClient.realm(REALM_NAME)) - .updateWith(r -> r.setSsoSessionMaxLifespan(SSO_MAX_LIFESPAN)) + .updateWith(r -> { + r.setSsoSessionMaxLifespan(SSO_MAX_LIFESPAN); + r.setAccessCodeLifespan(ACCESS_CODE_LIFESPAN); + r.setAccessTokenLifespan(ACCESS_TOKEN_LIFESPAN); + }) .update()) { new SamlClientBuilder() .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, SamlClient.Binding.POST) .build() .login().user(bburkeUser).build() .processSamlResponse(SamlClient.Binding.POST) - .transformObject(this::checkSessionNotOnOrAfter) + .transformObject(r -> checkSessionNotOnOrAfter(r, SSO_MAX_LIFESPAN, ACCESS_CODE_LIFESPAN, ACCESS_TOKEN_LIFESPAN)) + .build() + .execute(); + } + } + + @Test + public void testSamlResponseClientConfigurationIdpInitiatedLogin() throws Exception { + int ssoMaxLifespan = adminClient.realm(REALM_NAME).toRepresentation().getSsoSessionMaxLifespan(); + try(AutoCloseable c = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST) + .setAttribute(SamlConfigAttributes.SAML_ASSERTION_LIFESPAN, "2000") + .update()) { + new SamlClientBuilder() + .idpInitiatedLogin(getAuthServerSamlEndpoint(REALM_NAME), "sales-post").build() + .login().user(bburkeUser).build() + .processSamlResponse(SamlClient.Binding.POST) + .transformObject(r -> checkSessionNotOnOrAfter(r, ssoMaxLifespan, 2000, 2000)) + .build() + .execute(); + } + } + + @Test + public void testSamlResponseClientConfigurationAfterAuthnLogin() throws Exception { + int ssoMaxLifespan = adminClient.realm(REALM_NAME).toRepresentation().getSsoSessionMaxLifespan(); + try(AutoCloseable c = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST) + .setAttribute(SamlConfigAttributes.SAML_ASSERTION_LIFESPAN, "1800") + .update()) { + new SamlClientBuilder() + .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, SamlClient.Binding.POST) + .build() + .login().user(bburkeUser).build() + .processSamlResponse(SamlClient.Binding.POST) + .transformObject(r -> checkSessionNotOnOrAfter(r, ssoMaxLifespan, 1800, 1800)) .build() .execute(); } diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties index 5d6caaaf26..abeca2f35b 100644 --- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties +++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties @@ -137,6 +137,8 @@ action-token-generated-by-admin-lifespan=Default Admin-Initiated Action Lifespan action-token-generated-by-admin-lifespan.tooltip=Maximum time before an action permit sent to a user by administrator is expired. This value is recommended to be long to allow administrators send e-mails for users that are currently offline. The default timeout can be overridden immediately before issuing the token. action-token-generated-by-user-lifespan=User-Initiated Action Lifespan action-token-generated-by-user-lifespan.tooltip=Maximum time before an action permit sent by a user (such as a forgot password e-mail) is expired. This value is recommended to be short because it is expected that the user would react to self-created action quickly. +saml-assertion-lifespan=Assertion Lifespan +saml-assertion-lifespan.tooltip=Lifespan set in the SAML assertion conditions. After that time the assertion will be invalid. The "SessionNotOnOrAfter" attribute is not modified and continue using the "SSO Session Max" time defined at realm level. action-token-generated-by-user.execute-actions=Execute Actions action-token-generated-by-user.idp-verify-account-via-email=IdP Account E-mail Verification diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js index f3b6877dd7..0e8cc3a777 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js @@ -1037,6 +1037,7 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, flows, $ro $scope.tlsClientCertificateBoundAccessTokens = false; $scope.accessTokenLifespan = TimeUnit2.asUnit(client.attributes['access.token.lifespan']); + $scope.samlAssertionLifespan = TimeUnit2.asUnit(client.attributes['saml.assertion.lifespan']); if(client.origin) { if ($scope.access.viewRealm) { @@ -1362,6 +1363,14 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, flows, $ro } } + $scope.updateAssertionLifespan = function() { + if ($scope.samlAssertionLifespan.time) { + $scope.clientEdit.attributes['saml.assertion.lifespan'] = $scope.samlAssertionLifespan.toSeconds(); + } else { + $scope.clientEdit.attributes['saml.assertion.lifespan'] = null; + } + } + function configureAuthorizationServices() { if ($scope.clientEdit.authorizationServicesEnabled) { if ($scope.accessType == 'public') { diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html b/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html index d24f87e342..7bb63af0a1 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/client-detail.html @@ -503,10 +503,10 @@ -
+
{{:: 'advanced-client-settings' | translate}} {{:: 'advanced-client-settings.tooltip' | translate}} -
+
@@ -523,6 +523,23 @@ {{:: 'access-token-lifespan.tooltip' | translate}}
+
+ + +
+ + +
+ {{:: 'saml-assertion-lifespan.tooltip' | translate}} +
+
@@ -581,4 +598,4 @@
- \ No newline at end of file +