Skip to content

Commit

Permalink
OAuth2Client: refactor scopes
Browse files Browse the repository at this point in the history
This change refactors the API around OAuth scopes and turns this
attribute into a list of scopes, instead of a space-separated
string list. This makes the programmatic configuration slightly
more natural.

This change has no effect on configuration, which remains the same
(scopes must be passed in one single option as a space-separated
string list).
  • Loading branch information
adutra committed Jun 13, 2024
1 parent 4b6c2b3 commit a1c6735
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ private static OAuth2ClientConfig.Builder clientConfig(String clientId) throws E
.clientSecret("s3cr3t")
// offline_access is required to get a refresh token when using authorization code flow
// note: Authelia has issues when more than one scope is requested
.scope(clientId.equals("nessie-private-cc") ? "profile" : "offline_access")
.addScope(clientId.equals("nessie-private-cc") ? "profile" : "offline_access")
.authorizationCodeFlowWebServerPort(NESSIE_CALLBACK_PORT)
.issuerUrl(issuerUrl)
.sslContext(insecureSslContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ void testOAuth2ClientWithBackgroundRefresh() throws Exception {
.clientSecret("s3cr3t")
.issuerUrl(issuerUrl)
.audience("Private1")
.scope("exchange")
.addScope("exchange")
.build())
.build();
OAuth2ClientConfig config2 =
Expand Down Expand Up @@ -548,7 +548,7 @@ private static OAuth2ClientConfig.Builder clientConfig(
.username("Alice")
.password("s3cr3t")
// Otherwise Keycloak complains about missing scope, but still issues tokens
.scope("openid")
.addScope("openid")
.defaultAccessTokenLifespan(Duration.ofSeconds(10))
.defaultRefreshTokenLifespan(Duration.ofSeconds(15))
.refreshSafetyWindow(Duration.ofSeconds(5))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ DeviceCodeResponse invokeDeviceAuthEndpoint() {
}

Optional<String> getScope() {
return config.getScope();
return config.getScopes().stream().reduce((a, b) -> a + " " + b);
}

URI getResolvedTokenEndpoint() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ class AuthorizationCodeFlow extends AbstractFlow {
.path(authEndpoint.getPath())
.queryParam("response_type", "code")
.queryParam("client_id", config.getClientId())
.queryParam("scope", config.getScope().orElse(null))
.queryParam(
"scope", config.getScopes().stream().reduce((a, b) -> a + " " + b).orElse(null))
.queryParam("redirect_uri", redirectUri)
.queryParam("state", state)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.net.URI;
import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;
Expand Down Expand Up @@ -91,7 +93,10 @@ static OAuth2AuthenticatorConfig fromConfigSupplier(Function<String, String> con
config, CONF_NESSIE_OAUTH2_GRANT_TYPE, builder::grantType, GrantType::fromConfigName);
applyConfigOption(config, CONF_NESSIE_OAUTH2_USERNAME, builder::username);
applyConfigOption(config, CONF_NESSIE_OAUTH2_PASSWORD, builder::password);
applyConfigOption(config, CONF_NESSIE_OAUTH2_CLIENT_SCOPES, builder::scope);
applyConfigOption(
config,
CONF_NESSIE_OAUTH2_CLIENT_SCOPES,
scope -> Arrays.stream(scope.split(" ")).forEach(builder::addScope));
applyConfigOption(
config,
CONF_NESSIE_OAUTH2_DEFAULT_ACCESS_TOKEN_LIFESPAN,
Expand Down Expand Up @@ -215,12 +220,18 @@ default GrantType getGrantType() {
*/
Optional<Secret> getPassword();

@Value.Derived
@Deprecated
default Optional<String> getScope() {
return getScopes().stream().reduce((a, b) -> a + " " + b);
}

/**
* The OAuth2 scope. Optional.
* The OAuth2 scopes. Optional.
*
* @see NessieConfigConstants#CONF_NESSIE_OAUTH2_CLIENT_SCOPES
*/
Optional<String> getScope();
List<String> getScopes();

@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated
Expand Down Expand Up @@ -411,7 +422,20 @@ default Builder password(String password) {
}

@CanIgnoreReturnValue
Builder scope(String scope);
@Deprecated
default Builder scope(String scope) {
Arrays.stream(scope.split(" ")).forEach(this::addScope);
return this;
}

@CanIgnoreReturnValue
Builder addScope(String scope);

@CanIgnoreReturnValue
Builder addScopes(String... scopes);

@CanIgnoreReturnValue
Builder scopes(Iterable<String> scopes);

@Deprecated
@CanIgnoreReturnValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,15 @@ URI getResolvedTokenEndpointForTokenExchange() {
}

@Value.Lazy
Optional<String> getScopeForTokenExchange() {
List<String> getScopesForTokenExchange() {
if (!getTokenExchangeConfig().isEnabled()) {
return getScope();
return getScopes();
}
String scope = getTokenExchangeConfig().getScope();
if (scope == null || scope.isEmpty()) {
return Optional.empty();
List<String> scopes = getTokenExchangeConfig().getScopes();
if (scopes.equals(TokenExchangeConfig.SCOPES_INHERIT)) {
return getScopes();
}
if (TokenExchangeConfig.SCOPES_INHERIT.equals(scope)) {
return getScope();
}
return Optional.of(scope);
return scopes;
}

/**
Expand Down Expand Up @@ -527,7 +524,13 @@ default Builder password(String password) {
}

@Override
Builder scope(String scope);
Builder addScope(String scope);

@Override
Builder addScopes(String... scopes);

@Override
Builder scopes(Iterable<String> scopes);

@Override
Builder tokenExchangeConfig(TokenExchangeConfig tokenExchangeConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.net.URI;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiFunction;
Expand All @@ -47,9 +50,9 @@
@Value.Immutable
public interface TokenExchangeConfig {

TokenExchangeConfig DISABLED = builder().enabled(false).build();
List<String> SCOPES_INHERIT = Collections.singletonList("\\inherit\\");

String SCOPES_INHERIT = "\\inherit\\";
TokenExchangeConfig DISABLED = builder().enabled(false).build();

static TokenExchangeConfig fromConfigSupplier(Function<String, String> config) {
String enabled = config.apply(CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_ENABLED);
Expand All @@ -69,7 +72,10 @@ static TokenExchangeConfig fromConfigSupplier(Function<String, String> config) {
URI::create);
applyConfigOption(
config, CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_RESOURCE, builder::resource, URI::create);
applyConfigOption(config, CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_SCOPES, builder::scope);
applyConfigOption(
config,
CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_SCOPES,
scope -> Arrays.stream(scope.split(" ")).forEach(builder::addScope));
applyConfigOption(config, CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_AUDIENCE, builder::audience);
String subjectToken = config.apply(CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_SUBJECT_TOKEN);
String actorToken = config.apply(CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_ACTOR_TOKEN);
Expand Down Expand Up @@ -192,15 +198,15 @@ default URI getRequestedTokenType() {
Optional<String> getAudience();

/**
* The OAuth2 scope. Optional.
* The OAuth2 scopes. Optional.
*
* <p>The special value {@link #SCOPES_INHERIT} (default) means that the scope will be inherited
* <p>The special value {@link #SCOPES_INHERIT} (default) means that the scopes will be inherited
* from the global OAuth2 configuration.
*
* @see NessieConfigConstants#CONF_NESSIE_OAUTH2_TOKEN_EXCHANGE_SCOPES
*/
@Value.Default
default String getScope() {
default List<String> getScopes() {
return SCOPES_INHERIT;
}

Expand Down Expand Up @@ -282,7 +288,13 @@ default Builder clientSecret(String clientSecret) {
Builder audience(String audience);

@CanIgnoreReturnValue
Builder scope(String scope);
Builder addScope(String scope);

@CanIgnoreReturnValue
Builder addScopes(String... scopes);

@CanIgnoreReturnValue
Builder scopes(Iterable<String> scopes);

@CanIgnoreReturnValue
Builder subjectTokenProvider(BiFunction<AccessToken, RefreshToken, TypedToken> provider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public Tokens fetchNewTokens(Tokens currentTokens) {

@Override
Optional<String> getScope() {
return config.getScopeForTokenExchange();
return config.getScopesForTokenExchange().stream().reduce((a, b) -> a + " " + b);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ void testExchangeTokens() throws Exception {
.issuerUrl(server2.getUri().resolve("/"))
.resource(URI.create("urn:resource"))
.audience("audience")
.scope("test-exchanged")
.addScope("test-exchanged")
.clientId("Client1")
.clientSecret("s3cr3t")
.actorToken(TypedToken.of("actor-token", TypedToken.URN_ID_TOKEN))
Expand Down Expand Up @@ -537,7 +537,7 @@ void testBadRequest() throws Exception {

try (HttpTestServer server = new HttpTestServer(handler(), true)) {

OAuth2ClientConfig config = configBuilder(server, false).scope("invalid-scope").build();
OAuth2ClientConfig config = configBuilder(server, false).addScope("invalid-scope").build();

try (OAuth2Client client = new OAuth2Client(config)) {

Expand Down Expand Up @@ -587,7 +587,7 @@ private void handleTokenEndpoint(HttpServletRequest req, HttpServletResponse res
soft.assertThat(req.getContentType()).isEqualTo("application/x-www-form-urlencoded");
soft.assertThat(req.getHeader("Authorization")).isEqualTo("Basic Q2xpZW50MTpzM2NyM3Q=");
Map<String, String> data = BaseTestHttpClient.decodeFormData(req.getInputStream());
if (data.containsKey("scope") && data.get("scope").equals("invalid-scope")) {
if (data.containsKey("scope") && data.get("scope").contains("invalid-scope")) {
ErrorResponse response =
ImmutableErrorResponse.builder()
.errorCode("invalid_request")
Expand Down Expand Up @@ -849,7 +849,7 @@ private OAuth2ClientConfig.Builder configBuilder(HttpTestServer server, boolean
OAuth2ClientConfig.builder()
.clientId("Client1")
.clientSecret("s3cr3t")
.scope("test")
.addScope("test")
.clock(() -> now);
if (server.getSslContext() != null) {
builder.sslContext(server.getSslContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ static Stream<Arguments> testFromConfig() {
.clientSecret("w00t")
.username("Alice")
.password("s3cr3t")
.scope("test")
.addScope("test")
.defaultAccessTokenLifespan(Duration.ofSeconds(30))
.defaultRefreshTokenLifespan(Duration.ofSeconds(30))
.refreshSafetyWindow(Duration.ofSeconds(10))
Expand All @@ -386,7 +386,7 @@ static Stream<Arguments> testFromConfig() {
.clientId("token-exchange-client-id")
.clientSecret("token-exchange-client-secret")
.audience("audience")
.scope("scope1 scope2")
.addScopes("scope1", "scope2")
.resource(URI.create("https://token-exchange.com/resource"))
.subjectToken(TypedToken.of("subject-token", TypedToken.URN_ID_TOKEN))
.actorToken(TypedToken.of("actor-token", TypedToken.URN_JWT))
Expand Down

0 comments on commit a1c6735

Please sign in to comment.