Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtValidationRule #3728

Merged
merged 9 commits into from
Dec 22, 2023
Merged

fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtValidationRule #3728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8f910ab
fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtVa…
richardtreier Dec 21, 2023
3aab663
feat: issuedAt validation leeway is now opt-in
richardtreier Dec 21, 2023
396269f
feat: warning with INFO when issuedAt leeway unconfigured
richardtreier Dec 21, 2023
8175b71
chore: code style, reworded INFO message
richardtreier Dec 21, 2023
5f6c547
chore: fix codestyle, no concatenated strings
richardtreier Dec 21, 2023
2e7ad58
chore: shorten variable name
richardtreier Dec 21, 2023
6a6d8ea
chore: contribution documentation for changed files
richardtreier Dec 22, 2023
e39f31d
chore: add default and type to changed settings
richardtreier Dec 22, 2023
9dbadb6
chore: update DEPENDENCIES
richardtreier Dec 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions DEPENDENCIES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ maven/mavencentral/io.rest-assured/rest-assured/5.4.0, Apache-2.0, approved, #12
maven/mavencentral/io.rest-assured/xml-path/5.4.0, Apache-2.0, approved, #12038
maven/mavencentral/io.setl/rdf-urdna/1.1, Apache-2.0, approved, clearlydefined
maven/mavencentral/io.swagger.core.v3/swagger-annotations-jakarta/2.2.15, Apache-2.0, approved, #5947
maven/mavencentral/io.swagger.core.v3/swagger-annotations-jakarta/2.2.19, Apache-2.0, approved, #5947
maven/mavencentral/io.swagger.core.v3/swagger-annotations-jakarta/2.2.20, Apache-2.0, approved, #5947
maven/mavencentral/io.swagger.core.v3/swagger-annotations/2.2.15, Apache-2.0, approved, #11362
maven/mavencentral/io.swagger.core.v3/swagger-annotations/2.2.8, Apache-2.0, approved, #11362
maven/mavencentral/io.swagger.core.v3/swagger-core-jakarta/2.2.15, Apache-2.0, approved, #5929
Expand Down Expand Up @@ -226,26 +226,26 @@ maven/mavencentral/org.eclipse.edc/autodoc-processor/0.4.2-SNAPSHOT, Apache-2.0,
maven/mavencentral/org.eclipse.edc/runtime-metamodel/0.4.2-SNAPSHOT, Apache-2.0, approved, technology.edc
maven/mavencentral/org.eclipse.jetty.toolchain/jetty-jakarta-servlet-api/5.0.2, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.toolchain/jetty-jakarta-websocket-api/2.0.0, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-core-client/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-core-common/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-core-server/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-jakarta-client/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-jakarta-common/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-jakarta-server/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-servlet/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-alpn-client/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-annotations/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-client/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-http/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-io/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-jndi/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-plus/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-security/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-server/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-servlet/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-util/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-webapp/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-xml/11.0.18, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-core-client/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-core-common/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-core-server/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-jakarta-client/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-jakarta-common/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-jakarta-server/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty.websocket/websocket-servlet/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-alpn-client/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-annotations/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-client/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-http/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-io/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-jndi/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-plus/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-security/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-server/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-servlet/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-util/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-webapp/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.eclipse.jetty/jetty-xml/11.0.19, EPL-2.0 OR Apache-2.0, approved, rt.jetty
maven/mavencentral/org.glassfish.hk2.external/aopalliance-repackaged/3.0.5, EPL-2.0 OR GPL-2.0-only with Classpath-exception-2.0, approved, ee4j.glassfish
maven/mavencentral/org.glassfish.hk2/hk2-api/3.0.5, EPL-2.0 OR GPL-2.0-only with Classpath-exception-2.0, approved, ee4j.glassfish
maven/mavencentral/org.glassfish.hk2/hk2-locator/3.0.5, EPL-2.0 OR GPL-2.0-only with Classpath-exception-2.0, approved, ee4j.glassfish
Expand Down
11 changes: 11 additions & 0 deletions ...uth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*
* Contributors:
* Microsoft Corporation - initial API and implementation
* sovity GmbH - added issuedAt leeway
*
*/

Expand All @@ -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;
Expand Down Expand Up @@ -77,6 +79,10 @@ public int getNotBeforeValidationLeeway() {
return notBeforeValidationLeeway;
}

public int getIssuedAtLeeway() {
return issuedAtLeeway;
}

public String getEndpointAudience() {
return endpointAudience;
}
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 19 additions & 1 deletion ...m/oauth2/oauth2-core/src/main/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Contributors:
* Microsoft Corporation - initial API and implementation
* Fraunhofer Institute for Software and Systems Engineering - Improvements
* sovity GmbH - added issuedAt leeway
*
*/

Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

}
7 changes: 5 additions & 2 deletions ...src/main/java/org/eclipse/edc/iam/oauth2/rule/Oauth2ExpirationIssuedAtValidationRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*
* Contributors:
* Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
* sovity GmbH - added issuedAt leeway
*
*/

Expand All @@ -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
Expand All @@ -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");
}
}
Expand Down
3 changes: 2 additions & 1 deletion ...core/src/main/java/org/eclipse/edc/iam/oauth2/rule/Oauth2ValidationRulesRegistryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*
* Contributors:
* Amadeus - Initial implementation
* sovity GmbH - added issuedAt leeway
*
*/

Expand All @@ -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()));
}
}
70 changes: 62 additions & 8 deletions ...uth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/Oauth2ServiceExtensionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}
}
}
Loading
Loading