Skip to content

Set correct default values in constructors of OIDC config builders #47451

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

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
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
package io.quarkus.oidc.client.registration;

import static io.quarkus.oidc.client.registration.runtime.OidcClientRegistrationsConfig.getDefaultClientRegistration;

import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import io.quarkus.oidc.client.registration.runtime.OidcClientRegistrationsConfig;
import io.quarkus.oidc.common.runtime.config.OidcCommonConfig;
import io.smallrye.config.SmallRyeConfigBuilder;
import io.smallrye.config.WithDefault;

//https://datatracker.ietf.org/doc/html/rfc7592
Expand Down Expand Up @@ -74,13 +69,7 @@ interface Metadata {
* @return OidcClientRegistrationConfigBuilder builder
*/
static OidcClientRegistrationConfigBuilder builder() {
var clientRegistrationsConfig = new SmallRyeConfigBuilder()
.addDiscoveredConverters()
.withMapping(OidcClientRegistrationsConfig.class)
.build()
.getConfigMapping(OidcClientRegistrationsConfig.class);
var clientRegistrationWithDefaultValues = getDefaultClientRegistration(clientRegistrationsConfig);
return new OidcClientRegistrationConfigBuilder(clientRegistrationWithDefaultValues);
return new OidcClientRegistrationConfigBuilder();
}

/**
Expand All @@ -90,7 +79,6 @@ static OidcClientRegistrationConfigBuilder builder() {
* @return OidcClientRegistrationConfigBuilder
*/
static OidcClientRegistrationConfigBuilder builder(OidcClientRegistrationConfig config) {
Objects.requireNonNull(config);
return new OidcClientRegistrationConfigBuilder(config);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package io.quarkus.oidc.client.registration;

import static io.quarkus.oidc.client.registration.runtime.OidcClientRegistrationsConfig.getDefaultClientRegistration;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import io.quarkus.oidc.client.registration.runtime.OidcClientRegistrationsConfig;
import io.quarkus.oidc.common.runtime.config.OidcCommonConfigBuilder;
import io.smallrye.config.SmallRyeConfigBuilder;

/**
* The {@link OidcClientRegistrationConfig} builder. This builder is not thread safe.
Expand Down Expand Up @@ -60,14 +64,31 @@ public Metadata metadata() {
}
}

/**
* {@link OidcClientRegistrationConfig} with documented defaults.
* Cached here so that we avoid building the SmallRye Config again and again when no-args builder constructors
* are used.
*/
private static volatile OidcClientRegistrationConfig configWithDefaults = null;

private Optional<String> id;
private boolean registrationEnabled;
private boolean registerEarly;
private Optional<String> initialToken;
private OidcClientRegistrationConfig.Metadata metadata;

OidcClientRegistrationConfigBuilder(OidcClientRegistrationConfig config) {
super(config);
/**
* Creates {@link OidcClientRegistrationConfig} builder populated with documented default values.
*/
public OidcClientRegistrationConfigBuilder() {
this(getConfigWithDefaults());
}

/**
* Creates {@link OidcClientRegistrationConfig} builder populated with {@code config} values.
*/
public OidcClientRegistrationConfigBuilder(OidcClientRegistrationConfig config) {
super(Objects.requireNonNull(config));
this.id = config.id();
this.registrationEnabled = config.registrationEnabled();
this.registerEarly = config.registerEarly();
Expand Down Expand Up @@ -171,7 +192,7 @@ public MetadataBuilder(OidcClientRegistrationConfigBuilder configBuilder) {
}

public MetadataBuilder() {
this.configBuilder = null;
this(new OidcClientRegistrationConfigBuilder());
}

public OidcClientRegistrationConfig.Metadata build() {
Expand Down Expand Up @@ -235,4 +256,16 @@ public MetadataBuilder extraProps(Map<String, String> extraProps) {
return this;
}
}

private static OidcClientRegistrationConfig getConfigWithDefaults() {
if (configWithDefaults == null) {
final OidcClientRegistrationsConfig clientRegistrationsConfig = new SmallRyeConfigBuilder()
.addDiscoveredConverters()
.withMapping(OidcClientRegistrationsConfig.class)
.build()
.getConfigMapping(OidcClientRegistrationsConfig.class);
configWithDefaults = getDefaultClientRegistration(clientRegistrationsConfig);
}
return configWithDefaults;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ public class OidcClientRegistrationConfigBuilderTest {
@Test
public void testDefaultValues() {
var config = OidcClientRegistrationConfig.builder().build();
testDefaultValues(config);
config = new OidcClientRegistrationConfigBuilder().build();
testDefaultValues(config);
}

private static void testDefaultValues(OidcClientRegistrationConfig config) {
// OidcClientRegistrationConfig methods
assertTrue(config.id().isEmpty());
assertTrue(config.registrationEnabled());
Expand Down Expand Up @@ -265,4 +270,13 @@ public void testCreateBuilderShortcuts() {
assertEquals("registration-path", config.registrationPath().orElse(null));
assertEquals("redirect-uri", config.metadata().redirectUri().orElse(null));
}

@Test
public void testMetadataBuilderDefaults() {
var metadata = new MetadataBuilder().build();
assertTrue(metadata.clientName().isEmpty());
assertTrue(metadata.postLogoutUri().isEmpty());
assertTrue(metadata.redirectUri().isEmpty());
assertTrue(metadata.extraProps().isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@

import io.quarkus.oidc.client.runtime.OidcClientConfig;
import io.quarkus.oidc.client.runtime.OidcClientConfig.Grant;
import io.quarkus.oidc.client.runtime.OidcClientsConfig;
import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfigBuilder;
import io.smallrye.config.SmallRyeConfigBuilder;

/**
* Builder for the {@link io.quarkus.oidc.client.runtime.OidcClientConfig}. This builder is not thread-safe.
Expand Down Expand Up @@ -103,6 +105,13 @@ public Map<String, String> headers() {
}
}

/**
* {@link OidcClientConfig} with documented defaults.
* Cached here so that we avoid building the SmallRye Config again and again when no-args builder constructors
* are used.
*/
private static volatile OidcClientConfig configWithDefaults = null;

private final Map<String, String> headers = new HashMap<>();
private boolean earlyTokensAcquisition;
private final Map<String, Map<String, String>> grantOptions = new HashMap<>();
Expand All @@ -115,6 +124,13 @@ public Map<String, String> headers() {
private boolean clientEnabled;
private Optional<String> id;

/**
* Creates {@link OidcClientConfigBuilder} builder populated with documented default values.
*/
public OidcClientConfigBuilder() {
this(getConfigWithDefaults());
}

/**
* @param config created either by this builder or SmallRye Config; config methods must never return null
*/
Expand Down Expand Up @@ -339,7 +355,7 @@ private record GrantImpl(Type type, String accessTokenProperty, String refreshTo
private String refreshExpiresInProperty;

public GrantBuilder() {
this(OidcClientConfig.builder());
this(new OidcClientConfigBuilder());
}

public GrantBuilder(OidcClientConfigBuilder builder) {
Expand Down Expand Up @@ -405,4 +421,16 @@ public Grant build() {
return new GrantImpl(type, accessTokenProperty, refreshTokenProperty, expiresInProperty, refreshExpiresInProperty);
}
}

private static OidcClientConfig getConfigWithDefaults() {
if (configWithDefaults == null) {
final OidcClientsConfig clientsConfig = new SmallRyeConfigBuilder()
.addDiscoveredConverters()
.withMapping(OidcClientsConfig.class)
.build()
.getConfigMapping(OidcClientsConfig.class);
configWithDefaults = OidcClientsConfig.getDefaultClient(clientsConfig);
}
return configWithDefaults;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfig;
import io.quarkus.oidc.common.runtime.config.OidcCommonConfig;
import io.quarkus.runtime.annotations.ConfigDocMapKey;
import io.smallrye.config.SmallRyeConfigBuilder;
import io.smallrye.config.WithDefault;

public interface OidcClientConfig extends OidcClientCommonConfig {
Expand Down Expand Up @@ -179,12 +178,7 @@ public String getGrantType() {
* @return OidcClientConfigBuilder builder
*/
static OidcClientConfigBuilder builder() {
var clientsConfig = new SmallRyeConfigBuilder()
.addDiscoveredConverters()
.withMapping(OidcClientsConfig.class)
.build()
.getConfigMapping(OidcClientsConfig.class);
return builder(OidcClientsConfig.getDefaultClient(clientsConfig));
return new OidcClientConfigBuilder();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ public class OidcClientConfigBuilderTest {
@Test
public void testDefaultValues() {
var config = OidcClientConfig.builder().id("default-test").build();
testDefaultValues(config);
config = new OidcClientConfigBuilder().id("default-test").build();
testDefaultValues(config);
}

private static void testDefaultValues(OidcClientConfig config) {
// OidcClientConfig methods
assertEquals("default-test", config.id().orElse(null));
assertTrue(config.clientEnabled());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfig.Credentials.Provider;
import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfig.Credentials.Secret;
import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfig.Credentials.Secret.Method;
import io.smallrye.config.SmallRyeConfigBuilder;

public abstract class OidcClientCommonConfigBuilder<T> extends OidcCommonConfigBuilder<T> {

Expand Down Expand Up @@ -169,10 +170,7 @@ private record CredentialsImpl(Optional<String> secret, Secret clientSecret, Jwt
private Jwt jwt;

public CredentialsBuilder() {
this.builder = null;
this.secret = Optional.empty();
this.clientSecret = new SecretBuilder<>().build();
this.jwt = new JwtBuilder<>().build();
this(getConfigBuilderWithDefaults());
}

public CredentialsBuilder(OidcClientCommonConfigBuilder<T> builder) {
Expand Down Expand Up @@ -268,6 +266,21 @@ public T end() {
public Credentials build() {
return new CredentialsImpl(secret, clientSecret, jwt);
}

private static <T> OidcClientCommonConfigBuilder<T> getConfigBuilderWithDefaults() {
final OidcClientCommonConfig clientCommonConfig = new SmallRyeConfigBuilder()
.addDiscoveredConverters()
.withMapping(OidcClientCommonConfig.class)
.build()
.getConfigMapping(OidcClientCommonConfig.class);
return new OidcClientCommonConfigBuilder<>(clientCommonConfig) {
@Override
protected T getBuilder() {
throw new UnsupportedOperationException(
"Use the 'OidcClientCommonConfigBuilder.CredentialsBuilder#build' method instead");
}
};
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package io.quarkus.oidc.common.runtime;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.Test;

import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfig.Credentials;
import io.quarkus.oidc.common.runtime.config.OidcClientCommonConfigBuilder.CredentialsBuilder;

public class OidcClientCommonConfigBuilderTest {

@Test
public void testCredentialsBuilderDefaultValues() {
Credentials credentials = new CredentialsBuilder<>().build();
assertNotNull(credentials);
assertTrue(credentials.secret().isEmpty());
var clientSecret = credentials.clientSecret();
assertNotNull(clientSecret);
assertTrue(clientSecret.value().isEmpty());
assertTrue(clientSecret.method().isEmpty());
var provider = clientSecret.provider();
assertNotNull(provider);
assertTrue(provider.key().isEmpty());
assertTrue(provider.keyringName().isEmpty());
assertTrue(provider.name().isEmpty());
var jwt = credentials.jwt();
assertNotNull(jwt);
assertEquals(Credentials.Jwt.Source.CLIENT, jwt.source());
assertTrue(jwt.secret().isEmpty());
provider = jwt.secretProvider();
assertNotNull(provider);
assertTrue(provider.key().isEmpty());
assertTrue(provider.keyringName().isEmpty());
assertTrue(provider.name().isEmpty());
assertTrue(jwt.key().isEmpty());
assertTrue(jwt.keyFile().isEmpty());
assertTrue(jwt.keyStoreFile().isEmpty());
assertTrue(jwt.keyStorePassword().isEmpty());
assertTrue(jwt.keyId().isEmpty());
assertTrue(jwt.keyPassword().isEmpty());
assertTrue(jwt.audience().isEmpty());
assertTrue(jwt.tokenKeyId().isEmpty());
assertTrue(jwt.issuer().isEmpty());
assertTrue(jwt.subject().isEmpty());
assertTrue(jwt.claims().isEmpty());
assertTrue(jwt.signatureAlgorithm().isEmpty());
assertEquals(10, jwt.lifespan());
assertFalse(jwt.assertion());
assertFalse(jwt.tokenPath().isPresent());
}

}
Loading