KEYCLOAK-10884: Need clock skew for SAML identity provider

server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java

@@ -28,6 +28,8 @@ import java.util.Map;
  */
 public class IdentityProviderModel implements Serializable {
 
+    public static final String ALLOWED_CLOCK_SKEW = "allowedClockSkew";
+
     private String internalId;
 
     /**

services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderConfig.java

@@ -111,12 +111,12 @@ public class OIDCIdentityProviderConfig extends OAuth2IdentityProviderConfig {
     }
 
     public int getAllowedClockSkew() {
-        String allowedClockSkew = getConfig().get("allowedClockSkew");
+        String allowedClockSkew = getConfig().get(ALLOWED_CLOCK_SKEW);
         if (allowedClockSkew == null || allowedClockSkew.isEmpty()) {
             return 0;
         }
         try {
-            return Integer.parseInt(getConfig().get("allowedClockSkew"));
+            return Integer.parseInt(getConfig().get(ALLOWED_CLOCK_SKEW));
         } catch (NumberFormatException e) {
             // ignore it and use default
             return 0;

services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java

@@ -433,7 +433,8 @@ public class SAMLEndpoint {
                     identity.setToken(samlResponse);
                 }
 
-                ConditionsValidator.Builder cvb = new ConditionsValidator.Builder(assertion.getID(), assertion.getConditions(), destinationValidator);
+                ConditionsValidator.Builder cvb = new ConditionsValidator.Builder(assertion.getID(), assertion.getConditions(), destinationValidator)
+                        .clockSkewInMillis(1000 * config.getAllowedClockSkew());
                 try {
                     String issuerURL = getEntityId(session.getContext().getUri(), realm);
                     cvb.addAllowedAudience(URI.create(issuerURL));

services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java

@@ -230,4 +230,27 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
             : xmlSigKeyInfoKeyNameTransformer.name());
     }
 
+    public int getAllowedClockSkew() {
+        int result = 0;
+        String allowedClockSkew = getConfig().get(ALLOWED_CLOCK_SKEW);
+        if (allowedClockSkew != null && !allowedClockSkew.isEmpty()) {
+            try {
+                result = Integer.parseInt(allowedClockSkew);
+                if (result < 0) {
+                    result = 0;
+                }
+            } catch (NumberFormatException e) {
+                // ignore it and use 0
+            }
+        }
+        return result;
+    }
+
+    public void setAllowedClockSkew(int allowedClockSkew) {
+        if (allowedClockSkew < 0) {
+            getConfig().remove(ALLOWED_CLOCK_SKEW);
+        } else {
+            getConfig().put(ALLOWED_CLOCK_SKEW, String.valueOf(allowedClockSkew));
+        }
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerAllowedClockSkewTest.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.keycloak.testsuite.broker;
+
+import java.io.Closeable;
+import javax.ws.rs.core.Response;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
+import static org.junit.Assert.assertThat;
+import org.junit.Test;
+import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
+import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
+import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
+import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
+import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
+import org.keycloak.testsuite.saml.AbstractSamlTest;
+import org.keycloak.testsuite.updaters.IdentityProviderAttributeUpdater;
+import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
+import static org.keycloak.testsuite.util.Matchers.statusCodeIsHC;
+import org.keycloak.testsuite.util.SamlClient;
+import org.keycloak.testsuite.util.SamlClientBuilder;
+import org.w3c.dom.Document;
+
+/**
+ *
+ * @author rmartinc
+ */
+public class KcSamlBrokerAllowedClockSkewTest extends AbstractInitializedBaseBrokerTest {
+
+    @Override
+    protected BrokerConfiguration getBrokerConfiguration() {
+        return KcSamlBrokerConfiguration.INSTANCE;
+    }
+
+    @Test
+    public void loginClientExpiredResponseFromIdP() throws Exception {
+        AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
+
+        Document doc = SAML2Request.convert(loginRep);
+
+        new SamlClientBuilder()
+          .authnRequest(getAuthServerSamlEndpoint(bc.consumerRealmName()), doc, SamlClient.Binding.POST).build()   // Request to consumer IdP
+          .login().idp(bc.getIDPAlias()).build()
+
+          .processSamlResponse(SamlClient.Binding.POST)    // AuthnRequest to producer IdP
+            .targetAttributeSamlRequest()
+            .build()
+
+          .login().user(bc.getUserLogin(), bc.getUserPassword()).build()
+
+          .addStep(() -> KcSamlBrokerAllowedClockSkewTest.this.setTimeOffset(-30)) // offset to the past to invalidate the request
+          .processSamlResponse(SamlClient.Binding.POST)    // Response from producer IdP should fail
+            .build()
+            .execute(hr -> assertThat(hr, statusCodeIsHC(Response.Status.BAD_REQUEST)));
+    }
+
+    @Test
+    public void loginClientExpiredResponseFromIdPWithClockSkew() throws Exception {
+        try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
+                .setAttribute(SAMLIdentityProviderConfig.ALLOWED_CLOCK_SKEW, "60")
+                .update()) {
+
+            AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
+
+            Document doc = SAML2Request.convert(loginRep);
+
+            SAMLDocumentHolder samlResponse = new SamlClientBuilder()
+              .authnRequest(getAuthServerSamlEndpoint(bc.consumerRealmName()), doc, SamlClient.Binding.POST).build()   // Request to consumer IdP
+              .login().idp(bc.getIDPAlias()).build()
+
+              .processSamlResponse(SamlClient.Binding.POST)    // AuthnRequest to producer IdP
+                .targetAttributeSamlRequest()
+                .build()
+
+              .login().user(bc.getUserLogin(), bc.getUserPassword()).build()
+
+              .addStep(() -> KcSamlBrokerAllowedClockSkewTest.this.setTimeOffset(-30)) // offset to the past but inside the clock skew
+              .processSamlResponse(SamlClient.Binding.POST)    // Response from producer IdP expired but valid with the clock skew
+                .build()
+
+              // first-broker flow
+              .updateProfile().firstName("a").lastName("b").email(bc.getUserEmail()).username(bc.getUserLogin()).build()
+              .followOneRedirect()
+
+              .getSamlResponse(SamlClient.Binding.POST);       // Response from consumer IdP
+
+            Assert.assertThat(samlResponse, Matchers.notNullValue());
+            Assert.assertThat(samlResponse.getSamlObject(), isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
+        }
+    }
+}

themes/src/main/resources/theme/base/admin/resources/js/app.js

@@ -3282,6 +3282,21 @@ module.directive('kcValidPage', function() {
    }
 });
 
+// Directive to parse/format strings into numbers
+module.directive('stringToNumber', function() {
+    return {
+        require: 'ngModel',
+        link: function(scope, element, attrs, ngModel) {
+            ngModel.$parsers.push(function(value) {
+                return (typeof value === 'undefined' || value === null)? '' : '' + value;
+            });
+            ngModel.$formatters.push(function(value) {
+                return parseFloat(value);
+            });
+        }
+    };
+});
+
 // filter used for paged tables
 module.filter('startFrom', function () {
     return function (input, start) {

themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html

@@ -229,6 +229,13 @@
                 </div>
                 <kc-tooltip>{{:: 'validating-x509-certificate.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group">
+                <label class="col-md-2 control-label" for="allowedClockSkew">{{:: 'allowed-clock-skew' | translate}}</label>
+                <div class="col-md-6 time-selector">
+                    <input class="form-control" string-to-number type="number" min="0" max="2147483" step="1" ng-model="identityProvider.config.allowedClockSkew" id="allowedClockSkew"/>
+                </div>
+                <kc-tooltip>{{:: 'identity-provider.allowed-clock-skew.tooltip' | translate}}</kc-tooltip>
+            </div>
          </fieldset>
         <fieldset data-ng-show="newIdentityProvider">
             <legend uncollapsed><span class="text">{{:: 'import-external-idp-config' | translate}}</span> <kc-tooltip>{{:: 'import-external-idp-config.tooltip' | translate}}</kc-tooltip></legend>
@@ -274,4 +281,4 @@
     </form>
 </div>
 
-<kc-menu></kc-menu>
+<kc-menu></kc-menu>