fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtVa…

…lidationRule (#3728)

* fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtValidationRule

* feat: issuedAt validation leeway is now opt-in

* feat: warning with INFO when issuedAt leeway unconfigured

* chore: code style, reworded INFO message

* chore: fix codestyle, no concatenated strings

* chore: shorten variable name

* chore: contribution documentation for changed files

* chore: add default and type to changed settings

* chore: update DEPENDENCIES

extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceConfiguration.java

@@ -9,6 +9,7 @@
  *
  *  Contributors:
  *       Microsoft Corporation - initial API and implementation
+ *       sovity GmbH - added issuedAt leeway
  *
  */
 
@@ -33,6 +34,7 @@ public class Oauth2ServiceConfiguration {
     private String publicCertificateAlias;
     private String providerAudience;
     private int notBeforeValidationLeeway;
+    private int issuedAtLeeway;
     private String endpointAudience;
 
     private Long tokenExpiration;
@@ -77,6 +79,10 @@ public int getNotBeforeValidationLeeway() {
         return notBeforeValidationLeeway;
     }
 
+    public int getIssuedAtLeeway() {
+        return issuedAtLeeway;
+    }
+
     public String getEndpointAudience() {
         return endpointAudience;
     }
@@ -146,6 +152,11 @@ public Builder notBeforeValidationLeeway(int notBeforeValidationLeeway) {
             return this;
         }
 
+        public Builder issuedAtLeeway(int issuedAtLeeway) {
+            configuration.issuedAtLeeway = issuedAtLeeway;
+            return this;
+        }
+
         public Builder endpointAudience(String endpointAudience) {
             configuration.endpointAudience = endpointAudience;
             return this;

extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtension.java

@@ -10,6 +10,7 @@
  *  Contributors:
  *       Microsoft Corporation - initial API and implementation
  *       Fraunhofer Institute for Software and Systems Engineering - Improvements
+ *       sovity GmbH - added issuedAt leeway
  *
  */
 
@@ -46,6 +47,8 @@
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
+import static java.lang.String.format;
+
 /**
  * Provides OAuth2 client credentials flow support.
  */
@@ -74,8 +77,10 @@ public class Oauth2ServiceExtension implements ServiceExtension {
     private static final String TOKEN_EXPIRATION = "edc.oauth.token.expiration"; // in minutes
     @Setting
     private static final String CLIENT_ID = "edc.oauth.client.id";
-    @Setting
+    @Setting(value = "Leeway in seconds for validating the not before (nbf) claim in the token.", defaultValue = "10", type = "int")
     private static final String NOT_BEFORE_LEEWAY = "edc.oauth.validation.nbf.leeway";
+    @Setting(value = "Leeway in seconds for validating the issuedAt claim in the token. By default it is 0 seconds.", defaultValue = "0", type = "int")
+    private static final String ISSUED_AT_LEEWAY = "edc.oauth.validation.issued.at.leeway";
     private IdentityProviderKeyResolver providerKeyResolver;
 
     @Inject
@@ -167,8 +172,21 @@ private Oauth2ServiceConfiguration createConfig(ServiceExtensionContext context)
                 .privateKeyResolver(privateKeyResolver)
                 .certificateResolver(certificateResolver)
                 .notBeforeValidationLeeway(context.getSetting(NOT_BEFORE_LEEWAY, 10))
+                .issuedAtLeeway(getIssuedAtLeeway(context))
                 .tokenExpiration(TimeUnit.MINUTES.toSeconds(tokenExpiration))
                 .build();
     }
 
+    private int getIssuedAtLeeway(ServiceExtensionContext context) {
+        if (!context.getConfig().hasKey(ISSUED_AT_LEEWAY)) {
+            var message = format(
+                    "No value was configured for '%s'. Consider setting a leeway of 2-5s in production to avoid problems with clock skew.",
+                    ISSUED_AT_LEEWAY
+            );
+            context.getMonitor().info(message);
+        }
+
+        return context.getSetting(ISSUED_AT_LEEWAY, 0);
+    }
+
 }

extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/rule/Oauth2ExpirationIssuedAtValidationRule.java

@@ -9,6 +9,7 @@
  *
  *  Contributors:
  *       Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
+ *       sovity GmbH - added issuedAt leeway
  *
  */
 
@@ -32,9 +33,11 @@
 public class Oauth2ExpirationIssuedAtValidationRule implements TokenValidationRule {
 
     private final Clock clock;
+    private final int issuedAtLeeway;
 
-    public Oauth2ExpirationIssuedAtValidationRule(Clock clock) {
+    public Oauth2ExpirationIssuedAtValidationRule(Clock clock, int issuedAtLeeway) {
         this.clock = clock;
+        this.issuedAtLeeway = issuedAtLeeway;
     }
 
     @Override
@@ -51,7 +54,7 @@ public Result<Void> checkRule(@NotNull ClaimToken toVerify, @Nullable Map<String
         if (issuedAt != null) {
             if (issuedAt.isAfter(expires)) {
                 return Result.failure("Issued at (iat) claim is after expiration time (exp) claim in token");
-            } else if (now.isBefore(issuedAt)) {
+            } else if (now.plusSeconds(issuedAtLeeway).isBefore(issuedAt)) {
                 return Result.failure("Current date/time before issued at (iat) claim in token");
             }
         }

extensions/common/iam/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/rule/Oauth2ValidationRulesRegistryImpl.java

@@ -9,6 +9,7 @@
  *
  *  Contributors:
  *       Amadeus - Initial implementation
+ *       sovity GmbH - added issuedAt leeway
  *
  */
 
@@ -28,6 +29,6 @@ public class Oauth2ValidationRulesRegistryImpl extends TokenValidationRulesRegis
     public Oauth2ValidationRulesRegistryImpl(Oauth2ServiceConfiguration configuration, Clock clock) {
         this.addRule(new Oauth2AudienceValidationRule(configuration.getEndpointAudience()));
         this.addRule(new Oauth2NotBeforeValidationRule(clock, configuration.getNotBeforeValidationLeeway()));
-        this.addRule(new Oauth2ExpirationIssuedAtValidationRule(clock));
+        this.addRule(new Oauth2ExpirationIssuedAtValidationRule(clock, configuration.getIssuedAtLeeway()));
     }
 }

extensions/common/iam/oauth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtensionTest.java

@@ -9,12 +9,14 @@
  *
  *  Contributors:
  *       Microsoft Corporation - initial API and implementation
+ *       sovity GmbH - added issuedAt leeway
  *
  */
 
 package org.eclipse.edc.iam.oauth2;
 
 import org.eclipse.edc.junit.extensions.DependencyInjectionExtension;
+import org.eclipse.edc.spi.monitor.Monitor;
 import org.eclipse.edc.spi.security.CertificateResolver;
 import org.eclipse.edc.spi.security.PrivateKeyResolver;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
@@ -29,6 +31,7 @@
 import java.util.Map;
 
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.contains;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -49,24 +52,75 @@ void setup(ServiceExtensionContext context) {
     }
 
     @Test
-    void verifyExtensionWithCertificateAlias(Oauth2ServiceExtension extension, ServiceExtensionContext context) throws CertificateEncodingException {
+    void verifyExtensionWithCertificateAlias(Oauth2ServiceExtension extension, ServiceExtensionContext context) {
         var config = spy(ConfigFactory.fromMap(Map.of(
                 "edc.oauth.client.id", "id",
                 "edc.oauth.token.url", "url",
                 "edc.oauth.certificate.alias", "alias",
                 "edc.oauth.private.key.alias", "p_alias")));
-        when(context.getConfig(any())).thenReturn(config);
-        var certificate = mock(X509Certificate.class);
-        var privateKey = mock(PrivateKey.class);
-        when(privateKey.getAlgorithm()).thenReturn("RSA");
-        when(certificate.getEncoded()).thenReturn(new byte[] {});
-        when(certificateResolver.resolveCertificate("alias")).thenReturn(certificate);
-        when(privateKeyResolver.resolvePrivateKey("p_alias", PrivateKey.class)).thenReturn(privateKey);
+        mockCertificate("alias");
+        mockRsaPrivateKey("p_alias");
 
+        when(context.getConfig(any())).thenReturn(config);
         extension.initialize(context);
 
         verify(config, times(1)).getString("edc.oauth.certificate.alias");
         verify(config, never()).getString("edc.oauth.public.key.alias");
     }
 
+    @Test
+    void leewayWarningLoggedWhenLeewayUnconfigured(Oauth2ServiceExtension extension, ServiceExtensionContext context) {
+        var config = spy(ConfigFactory.fromMap(Map.of(
+                "edc.oauth.client.id", "id",
+                "edc.oauth.token.url", "url",
+                "edc.oauth.certificate.alias", "alias",
+                "edc.oauth.private.key.alias", "p_alias")));
+        mockCertificate("alias");
+        mockRsaPrivateKey("p_alias");
+
+        var monitor = mock(Monitor.class);
+        when(context.getMonitor()).thenReturn(monitor);
+        when(context.getConfig(any())).thenReturn(config);
+        extension.initialize(context);
+
+        var message = "No value was configured for 'edc.oauth.validation.issued.at.leeway'.";
+        verify(monitor, times(1)).info(contains(message));
+    }
+
+    @Test
+    void leewayNoWarningWhenLeewayConfigured(Oauth2ServiceExtension extension, ServiceExtensionContext context) {
+        var config = spy(ConfigFactory.fromMap(Map.of(
+                "edc.oauth.client.id", "id",
+                "edc.oauth.token.url", "url",
+                "edc.oauth.certificate.alias", "alias",
+                "edc.oauth.private.key.alias", "p_alias",
+                "edc.oauth.validation.issued.at.leeway", "5")));
+        mockCertificate("alias");
+        mockRsaPrivateKey("p_alias");
+
+        var monitor = mock(Monitor.class);
+        when(context.getMonitor()).thenReturn(monitor);
+        when(context.getConfig(any())).thenReturn(config);
+        extension.initialize(context);
+
+        var message = "No value was configured for 'edc.oauth.validation.issued.at.leeway'.";
+        verify(monitor, never()).info(contains(message));
+    }
+
+    private void mockRsaPrivateKey(String alias) {
+        var privateKey = mock(PrivateKey.class);
+        when(privateKey.getAlgorithm()).thenReturn("RSA");
+        when(privateKeyResolver.resolvePrivateKey(alias, PrivateKey.class)).thenReturn(privateKey);
+    }
+
+    private void mockCertificate(String alias) {
+        try {
+            var certificate = mock(X509Certificate.class);
+            when(certificate.getEncoded()).thenReturn(new byte[] {});
+            when(certificateResolver.resolveCertificate(alias)).thenReturn(certificate);
+        } catch (CertificateEncodingException e) {
+            // Should never happen, it's a checked exception in the way of mocking
+            throw new RuntimeException(e);
+        }
+    }
 }

extensions/common/iam/oauth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/rule/Oauth2ExpirationIssuedAtValidationRuleTest.java

@@ -9,6 +9,7 @@
  *
  *  Contributors:
  *       Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
+ *       sovity GmbH - added issuedAt leeway
  *
  */
 
@@ -33,7 +34,7 @@ class Oauth2ExpirationIssuedAtValidationRuleTest {
 
     private final Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
     private final Clock clock = Clock.fixed(now, UTC);
-    private final TokenValidationRule rule = new Oauth2ExpirationIssuedAtValidationRule(clock);
+    private final TokenValidationRule rule = new Oauth2ExpirationIssuedAtValidationRule(clock,  0);
 
     @Test
     void validationOk() {
@@ -96,5 +97,87 @@ void validationKoBecauseIssuedAtInFuture() {
         assertThat(result.getFailureMessages()).hasSize(1).contains("Current date/time before issued at (iat) claim in token");
     }
 
+    @Test
+    void validationKoBecauseIssuedAtInFutureOutsideLeeway() {
+        var rule = new Oauth2ExpirationIssuedAtValidationRule(clock, 5);
+
+        var token = ClaimToken.Builder.newInstance()
+                .claim(EXPIRATION_TIME, Date.from(now.plusSeconds(60)))
+                .claim(ISSUED_AT, Date.from(now.plusSeconds(10)))
+                .build();
+
+        var result = rule.checkRule(token, emptyMap());
+
+        assertThat(result.succeeded()).isFalse();
+        assertThat(result.getFailureMessages()).hasSize(1).contains("Current date/time before issued at (iat) claim in token");
+    }
+
+    @Test
+    void validationOkBecauseIssuedAtInFutureButWithinLeeway() {
+        var rule = new Oauth2ExpirationIssuedAtValidationRule(clock, 20);
+
+        var token = ClaimToken.Builder.newInstance()
+                .claim(EXPIRATION_TIME, Date.from(now.plusSeconds(60)))
+                .claim(ISSUED_AT, Date.from(now.plusSeconds(10)))
+                .build();
+
+        var result = rule.checkRule(token, emptyMap());
+
+        assertThat(result.succeeded()).isTrue();
+    }
 
+    /**
+     * Demonstrates situation where rounded dates in the JWT token cause validation failures
+     * <br>
+     * Rounding of dates in JWT is within spec and the direction of rounding is platform-dependant.
+     */
+    @Test
+    void validationKoWithRoundedIssuedAtAndNoLeeway() {
+        // time skew: tokens have dates rounded up to the second
+        var issuedAt = Instant.now().truncatedTo(ChronoUnit.SECONDS);
+        var expiresAt = issuedAt.plusSeconds(60);
+
+        // time skew: the connector is still in the previous second, with unrounded dates
+        var now = issuedAt.minus(250, ChronoUnit.MILLIS);
+
+        var clock = Clock.fixed(now, UTC);
+        var rule = new Oauth2ExpirationIssuedAtValidationRule(clock, 0);
+
+        var token = ClaimToken.Builder.newInstance()
+                .claim(EXPIRATION_TIME, Date.from(expiresAt))
+                .claim(ISSUED_AT, Date.from(issuedAt))
+                .build();
+
+        var result = rule.checkRule(token, emptyMap());
+
+        assertThat(result.succeeded()).isFalse();
+        assertThat(result.getFailureMessages()).hasSize(1).contains("Current date/time before issued at (iat) claim in token");
+    }
+
+    /**
+     * Regression test against clock skew and platform-dependant rounding of dates, solved with a 2s leeway.
+     * <br>
+     * Rounding of dates in JWT is within spec and the direction of rounding is platform-dependant.
+     */
+    @Test
+    void validationOkWithRoundedIssuedAtAndMinimalLeeway() {
+        // time skew: tokens have dates rounded up to the second
+        var issuedAt = Instant.now().truncatedTo(ChronoUnit.SECONDS);
+        var expiresAt = issuedAt.plusSeconds(60);
+
+        // time skew: the connector is still in the previous second, with unrounded dates
+        var now = issuedAt.minus(250, ChronoUnit.MILLIS);
+
+        var clock = Clock.fixed(now, UTC);
+        var rule = new Oauth2ExpirationIssuedAtValidationRule(clock, 2);
+
+        var token = ClaimToken.Builder.newInstance()
+                .claim(EXPIRATION_TIME, Date.from(expiresAt))
+                .claim(ISSUED_AT, Date.from(issuedAt))
+                .build();
+
+        var result = rule.checkRule(token, emptyMap());
+
+        assertThat(result.succeeded()).isTrue();
+    }
 }