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.

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

4.x: Make check for audience claim in access token optional in OIDC provider #6959

Merged
merged 5 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 40 additions & 6 deletions security/jwt/src/main/java/io/helidon/security/jwt/Jwt.java
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,21 @@ public Errors validate(String issuer, String audience) {
return validate(issuer, audience == null ? Set.of() : Set.of(audience));
}

/**
* Validates all default values.
* Values validated: {@link #validate(String, Set, boolean)}
*
* @param issuer validates that this JWT was issued by this issuer. Setting this to non-null value will make
* issuer claim mandatory
* @param audience validates that this JWT was issued for this audience. Setting this to non-null value will make
* audience claim mandatory
* @param checkAudience whether audience claim validation should be executed
* @return errors instance to check for validation result
*/
public Errors validate(String issuer, String audience, boolean checkAudience) {
return validate(issuer, audience == null ? Set.of() : Set.of(audience), checkAudience);
}

/**
* Validates all default values.
* Values validated:
Expand All @@ -966,23 +981,42 @@ public Errors validate(String issuer, String audience) {
* issuer claim mandatory
* @param audience validates that this JWT was issued for this audience. Setting this to non-null value and with
* any non-null value in the Set will make audience claim mandatory
* @param checkAudience whether audience claim validation should be executed
* @return errors instance to check for validation result
*/
public Errors validate(String issuer, Set<String> audience) {
public Errors validate(String issuer, Set<String> audience, boolean checkAudience) {
List<Validator<Jwt>> validators = defaultTimeValidators();
if (null != issuer) {
addIssuerValidator(validators, issuer, true);
}
if (null != audience) {
audience.stream()
.filter(Objects::nonNull)
.findAny()
.ifPresent(it -> addAudienceValidator(validators, audience, true));
// Audience check is turned on
if (checkAudience) {
if (null != audience) {
audience.stream()
.filter(Objects::nonNull)
.findAny()
.ifPresent(it -> addAudienceValidator(validators, audience, true));
}
}
addUserPrincipalValidator(validators);
return validate(validators);
}

/**
* Validates all default values.
* Audience claim check is not mandatory.
* Values validated: {@link #validate(String, Set, boolean)}
*
* @param issuer validates that this JWT was issued by this issuer. Setting this to non-null value will make
* issuer claim mandatory
* @param audience validates that this JWT was issued for this audience. Setting this to non-null value and with
* any non-null value in the Set will make audience claim mandatory
* @return errors instance to check for validation result
*/
public Errors validate(String issuer, Set<String> audience) {
return validate(issuer, audience, true);
}

/**
* Adds a validator that makes sure the {@link Jwt#userPrincipal()} is present.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Builder<B,
private URI introspectUri;
private String scopeAudience;
private boolean useWellKnown = true;
// Whether audience claim is optional (turned off by default)
private boolean optionalAudience = false;
// Whether to check audience claim (turned on by default)
private boolean checkAudience = true;

BaseBuilder() {
}
Expand All @@ -78,7 +82,7 @@ void buildConfiguration() {
OidcUtil.validateExists(collector, clientSecret, "Client Secret", "client-secret");
OidcUtil.validateExists(collector, identityUri, "Identity URI", "identity-uri");

if ((audience == null) && (identityUri != null)) {
if (audience == null && !optionalAudience && identityUri != null) {
this.audience = identityUri.toString();
}
// first set of validations
Expand Down Expand Up @@ -124,6 +128,8 @@ public B config(Config config) {
config.get("server-type").asString().ifPresent(this::serverType);

config.get("client-timeout-millis").asLong().ifPresent(this::clientTimeoutMillis);
config.get("optional-audience").asBoolean().ifPresent(this::optionalAudience);
config.get("check-audience").asBoolean().ifPresent(this::checkAudience);
return identity();
}

Expand Down Expand Up @@ -418,6 +424,30 @@ public B scopeAudience(String audience) {
return identity();
}

/**
* Allow audience claim to be optional.
*
* @param optional whether the audience claim is optional ({@code true}) or not ({@code false})
* @return updated builder instance
*/
@ConfiguredOption("false")
public B optionalAudience(boolean optional) {
this.optionalAudience = optional;
return identity();
}

/**
* Configure audience claim check.
*
* @param checkAudience whether the audience claim will be checked ({@code true}) or not ({@code false})
* @return updated builder instance
*/
@ConfiguredOption("false")
public B checkAudience(boolean checkAudience) {
this.checkAudience = checkAudience;
return identity();
}

private void clientTimeoutMillis(long millis) {
this.clientTimeout(Duration.ofMillis(millis));
}
Expand Down Expand Up @@ -458,6 +488,10 @@ String audience() {
return audience;
}

boolean checkAudience() {
return checkAudience;
}

String serverType() {
return serverType;
}
Expand Down Expand Up @@ -501,4 +535,5 @@ String scopeAudience() {
String name() {
return TenantConfigFinder.DEFAULT_TENANT_ID;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@
* <td>Force https for redirects to identity provider.
* This is helpful if you have a frontend SSL or cloud load balancer in front and Helidon is serving plain http.</td>
* </tr>
* <tr>
* <td>{@code optional-audience}</td>
* <td>{@code false}</td>
* <td>Allow audience claim to be optional.</td>
* </tr>
* <tr>
* <td>{@code check-audience}</td>
* <td>{@code true}</td>
* <td>Turn audience claim check on when {@code true} or off when {@code false}.</td>
* </tr>
* </table>
*/
public final class OidcConfig extends TenantConfigImpl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ static Builder tenantBuilder() {
*/
String audience();

/**
* Whether to validate audience token.
*
* @return audience
*/
boolean checkAudience();

/**
* Audience URI of custom scopes.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class TenantConfigImpl implements TenantConfig {
private final boolean validateJwtWithJwk;
private final String issuer;
private final String audience;
private final boolean checkAudience;
private final String realm;
private final OidcConfig.ClientAuthentication tokenEndpointAuthentication;
private final Duration clientTimeout;
Expand All @@ -60,6 +61,7 @@ class TenantConfigImpl implements TenantConfig {
this.validateJwtWithJwk = builder.validateJwtWithJwk();
this.issuer = builder.issuer();
this.audience = builder.audience();
this.checkAudience = builder.checkAudience();
this.identityUri = builder.identityUri();
this.realm = builder.realm();
this.tokenEndpointUri = builder.tokenEndpointUri();
Expand Down Expand Up @@ -144,6 +146,11 @@ public String audience() {
return audience;
}

@Override
public boolean checkAudience() {
return checkAudience;
}

@Override
public String scopeAudience() {
return scopeAudience;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,29 @@ void testCookieEncryptionPasswordFromBuilderConfig() {
}
}

@Test
void testOptionalAudience() {
OidcConfig config = OidcConfig.builder()
.identityUri(URI.create("http://localhost/identity"))
.clientSecret("top-secret")
.clientId("client-id")
.optionalAudience(true)
.build();
String audience = config.audience();
assertThat(audience, nullValue());
}

@Test
void testCheckAudience() {
OidcConfig config = OidcConfig.builder()
.identityUri(URI.create("http://localhost/identity"))
.clientSecret("top-secret")
.clientId("client-id")
.checkAudience(false)
.build();
assertThat(config.checkAudience(), is(false));
}

// Stub the Builder class to be able to retrieve the cookie-encryption-password value
private static class TestOidcConfigBuilder extends OidcConfig.Builder {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021 Oracle and/or its affiliates.
* Copyright (c) 2018, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,14 +18,21 @@

import io.helidon.config.Config;

import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;

/**
* Unit test for {@link OidcConfig}.
*/
class OidcConfigFromConfigTest extends OidcConfigAbstractTest {
private OidcConfig oidcConfig;
private Config config;

OidcConfigFromConfigTest() {
Config config = Config.builder()
config = Config.builder()
.disableSystemPropertiesSource()
.disableEnvironmentVariablesSource()
.build();
Expand All @@ -37,4 +44,18 @@ class OidcConfigFromConfigTest extends OidcConfigAbstractTest {
OidcConfig getConfig() {
return oidcConfig;
}

@Test
void testOptionalAudience() {
OidcConfig oidcConfig = OidcConfig.create(config.get("security.oidc-optional-aud"));
String audience = oidcConfig.audience();
assertThat(audience, nullValue());
}

@Test
void testDisabledAudience() {
OidcConfig oidcConfig = OidcConfig.create(config.get("security.oidc-disabled-aud"));
assertThat(oidcConfig.checkAudience(), is(false));
}

}
13 changes: 13 additions & 0 deletions security/providers/oidc-common/src/test/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

security:
config.require-encryption: false

oidc-test:
identity-uri: "https://identity.oracle.com"
scope-audience: "https://something:7987/test-application"
Expand All @@ -29,3 +30,15 @@ security:
authorization-endpoint-uri: "https://identity.oracle.com/authorization"
introspect-endpoint-uri: "https://identity.oracle.com/introspect"
relative-uris: true

oidc-optional-aud:
identity-uri: "https://my.identity"
client-id: "my-id"
client-secret: "my-well-known-secret"
optional-audience: true

oidc-disabled-aud:
identity-uri: "https://my.identity"
client-id: "my-id"
client-secret: "my-well-known-secret"
check-audience: false
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,9 @@ private AuthenticationResponse processValidationResult(ProviderRequest providerR
Errors.Collector collector) {
Jwt jwt = signedJwt.getJwt();
Errors errors = collector.collect();
Errors validationErrors = jwt.validate(tenant.issuer(), tenantConfig.audience());
Errors validationErrors = jwt.validate(tenant.issuer(),
tenantConfig.audience(),
tenantConfig.checkAudience());

if (errors.isValid() && validationErrors.isValid()) {

Expand Down