Skip to content

Commit

Permalink
Add config option to turn audience claim check off.
Browse files Browse the repository at this point in the history
Signed-off-by: Tomáš Kraus <tomas.kraus@oracle.com>
  • Loading branch information
Tomas-Kraus committed Aug 16, 2023
1 parent e6ca9c0 commit 72a3f6f
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 14 deletions.
48 changes: 42 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,22 @@ 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 check is turned on. Validation will fail when {@code true}
* and audience claim is null
* @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 +982,43 @@ 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 check is configured as mandatory. Validation will fail when {@code true}
* and audience claim is null
* @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,8 +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
// 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 Down Expand Up @@ -127,6 +129,7 @@ public B config(Config config) {

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 @@ -424,7 +427,7 @@ public B scopeAudience(String audience) {
/**
* Allow audience claim to be optional.
*
* @param optional whether the audience claim is optional (true) or not (false)
* @param optional whether the audience claim is optional ({@code true}) or not ({@code false})
* @return updated builder instance
*/
@ConfiguredOption("false")
Expand All @@ -433,6 +436,18 @@ public B optionalAudience(boolean 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 @@ -473,6 +488,10 @@ String audience() {
return audience;
}

boolean checkAudience() {
return checkAudience;
}

String serverType() {
return serverType;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,11 @@
* <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 @@ -16,6 +16,11 @@

package io.helidon.security.providers.oidc.common;

import java.net.URI;
import java.time.Duration;
import java.util.Arrays;
import java.util.Map;

import io.helidon.common.http.Http;
import io.helidon.config.Config;
import io.helidon.config.ConfigSources;
Expand Down Expand Up @@ -44,11 +49,6 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;

import java.net.URI;
import java.time.Duration;
import java.util.Arrays;
import java.util.Map;

/**
* Unit test for {@link OidcConfig}.
*/
Expand Down Expand Up @@ -205,6 +205,17 @@ void testOptionalAudience() {
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
Expand Up @@ -20,6 +20,7 @@

import org.junit.jupiter.api.Test;

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

Expand Down Expand Up @@ -51,4 +52,10 @@ void testOptionalAudience() {
assertThat(audience, nullValue());
}

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

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,9 @@ security:
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 @@ -440,7 +440,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

0 comments on commit 72a3f6f

Please sign in to comment.