Skip to content

Commit

Permalink
fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtVa…
Browse files Browse the repository at this point in the history
…lidationRule
  • Loading branch information
richardtreier committed Dec 21, 2023
1 parent a10ed75 commit e2b19c9
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class Oauth2ServiceConfiguration {
private String publicCertificateAlias;
private String providerAudience;
private int notBeforeValidationLeeway;
private int issuedAtValidationLeeway;
private String endpointAudience;

private Long tokenExpiration;
Expand Down Expand Up @@ -77,6 +78,10 @@ public int getNotBeforeValidationLeeway() {
return notBeforeValidationLeeway;
}

public int getIssuedAtValidationLeeway() {
return issuedAtValidationLeeway;
}

public String getEndpointAudience() {
return endpointAudience;
}
Expand Down Expand Up @@ -146,6 +151,11 @@ public Builder notBeforeValidationLeeway(int notBeforeValidationLeeway) {
return this;
}

public Builder issuedAtValidationLeeway(int issuedAtValidationLeeway) {
configuration.issuedAtValidationLeeway = issuedAtValidationLeeway;
return this;
}

public Builder endpointAudience(String endpointAudience) {
configuration.endpointAudience = endpointAudience;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,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 token")
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")
private static final String ISSUED_AT_LEEWAY = "edc.oauth.validation.issued.at.leeway";
private IdentityProviderKeyResolver providerKeyResolver;

@Inject
Expand Down Expand Up @@ -167,6 +169,7 @@ private Oauth2ServiceConfiguration createConfig(ServiceExtensionContext context)
.privateKeyResolver(privateKeyResolver)
.certificateResolver(certificateResolver)
.notBeforeValidationLeeway(context.getSetting(NOT_BEFORE_LEEWAY, 10))
.issuedAtValidationLeeway(context.getSetting(ISSUED_AT_LEEWAY, 10))
.tokenExpiration(TimeUnit.MINUTES.toSeconds(tokenExpiration))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@
public class Oauth2ExpirationIssuedAtValidationRule implements TokenValidationRule {

private final Clock clock;
private final int issuedAtValidationLeeway;

public Oauth2ExpirationIssuedAtValidationRule(Clock clock) {
public Oauth2ExpirationIssuedAtValidationRule(Clock clock, int issuedAtValidationLeeway) {
this.clock = clock;
this.issuedAtValidationLeeway = issuedAtValidationLeeway;
}

@Override
Expand All @@ -51,7 +53,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(issuedAtValidationLeeway).isBefore(issuedAt)) {
return Result.failure("Current date/time before issued at (iat) claim in token");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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.getIssuedAtValidationLeeway()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ 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 int issuedAtValidationLeeway = 5;
private final TokenValidationRule rule = new Oauth2ExpirationIssuedAtValidationRule(clock, issuedAtValidationLeeway);

@Test
void validationOk() {
Expand All @@ -46,6 +47,45 @@ void validationOk() {
assertThat(result.succeeded()).isTrue();
}

@Test
void validationOkBecauseIssuedAtInFutureButWithinLeeway() {
var token = ClaimToken.Builder.newInstance()
.claim(EXPIRATION_TIME, Date.from(now.plusSeconds(60)))
.claim(ISSUED_AT, Date.from(now.plusSeconds(2)))
.build();

var result = rule.checkRule(token, emptyMap());

assertThat(result.succeeded()).isTrue();
}

/**
* 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();
}

@Test
void validationKoBecauseExpirationTimeNotRespected() {
var token = ClaimToken.Builder.newInstance()
Expand Down Expand Up @@ -95,6 +135,4 @@ void validationKoBecauseIssuedAtInFuture() {
assertThat(result.succeeded()).isFalse();
assertThat(result.getFailureMessages()).hasSize(1).contains("Current date/time before issued at (iat) claim in token");
}


}

0 comments on commit e2b19c9

Please sign in to comment.