Skip to content

Commit

Permalink
Merge pull request #39035 from sberyozkin/minor_oidc_strava_updates
Browse files Browse the repository at this point in the history
Make OIDC scope separator configurable to support multiple Strava scopes
  • Loading branch information
sberyozkin authored Feb 27, 2024
2 parents 36f75be + eaa429e commit 8b567ac
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,13 @@ public enum ResponseMode {
@ConfigItem
public Optional<List<String>> scopes = Optional.empty();

/**
* The separator which is used when more than one scope is configured.
* A single space is used by default.
*/
@ConfigItem
public Optional<String> scopeSeparator = Optional.empty();

/**
* Require that ID token includes a `nonce` claim which must match `nonce` authentication request query parameter.
* Enabling this property can help mitigate replay attacks.
Expand Down Expand Up @@ -1342,6 +1349,14 @@ public Optional<String> getStateSecret() {
public void setStateSecret(Optional<String> stateSecret) {
this.stateSecret = stateSecret;
}

public Optional<String> getScopeSeparator() {
return scopeSeparator;
}

public void setScopeSeparator(String scopeSeparator) {
this.scopeSeparator = Optional.of(scopeSeparator);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public final class OidcUtils {
public static final String STATE_COOKIE_NAME = "q_auth";
public static final Integer MAX_COOKIE_VALUE_LENGTH = 4096;
public static final String POST_LOGOUT_COOKIE_NAME = "q_post_logout";
public static final String DEFAULT_SCOPE_SEPARATOR = " ";
static final String UNDERSCORE = "_";
static final String CODE_ACCESS_TOKEN_RESULT = "code_flow_access_token_result";
static final String COMMA = ",";
Expand Down Expand Up @@ -552,6 +553,9 @@ static OidcTenantConfig mergeTenantConfig(OidcTenantConfig tenant, OidcTenantCon
if (tenant.authentication.scopes.isEmpty()) {
tenant.authentication.scopes = provider.authentication.scopes;
}
if (tenant.authentication.scopeSeparator.isEmpty()) {
tenant.authentication.scopeSeparator = provider.authentication.scopeSeparator;
}
if (tenant.authentication.addOpenidScope.isEmpty()) {
tenant.authentication.addOpenidScope = provider.authentication.addOpenidScope;
}
Expand Down Expand Up @@ -661,7 +665,8 @@ public void handle(Void event) {
}

public static String encodeScopes(OidcTenantConfig oidcConfig) {
return OidcCommonUtils.urlEncode(String.join(" ", getAllScopes(oidcConfig)));
return OidcCommonUtils.urlEncode(String.join(oidcConfig.authentication.scopeSeparator.orElse(DEFAULT_SCOPE_SEPARATOR),
getAllScopes(oidcConfig)));
}

public static List<String> getAllScopes(OidcTenantConfig oidcConfig) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ private static OidcTenantConfig strava() {

ret.getToken().setVerifyAccessTokenWithUserInfo(true);
ret.getCredentials().getClientSecret().setMethod(Method.QUERY);
ret.getAuthentication().setScopeSeparator(",");

return ret;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ public void testAcceptStravaProperties() {
assertFalse(config.getAuthentication().idTokenRequired.get());
assertEquals(Method.QUERY, config.credentials.clientSecret.method.get());
assertEquals("/strava", config.authentication.redirectPath.get());
assertEquals(",", config.authentication.scopeSeparator.get());
}

@Test
Expand All @@ -472,6 +473,7 @@ public void testOverrideStravaProperties() {
tenant.token.setVerifyAccessTokenWithUserInfo(false);
tenant.credentials.clientSecret.setMethod(Method.BASIC);
tenant.authentication.setRedirectPath("/fitness-app");
tenant.authentication.setScopeSeparator(" ");

OidcTenantConfig config = OidcUtils.mergeTenantConfig(tenant, KnownOidcProviders.provider(Provider.STRAVA));

Expand All @@ -485,6 +487,7 @@ public void testOverrideStravaProperties() {
assertFalse(config.token.verifyAccessTokenWithUserInfo.get());
assertEquals(Method.BASIC, config.credentials.clientSecret.method.get());
assertEquals("/fitness-app", config.authentication.redirectPath.get());
assertEquals(" ", config.authentication.scopeSeparator.get());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,15 @@ public void testEncodeAllScopes() throws Exception {
assertEquals("openid+a%3A1+b%3A2+c+d", OidcUtils.encodeScopes(config));
}

@Test
public void testEncodeAllScopesWithCustomSeparator() throws Exception {
OidcTenantConfig config = new OidcTenantConfig();
config.authentication.setScopeSeparator(",");
config.authentication.setScopes(List.of("a:1", "b:2"));
config.authentication.setExtraParams(Map.of("scope", "c,d"));
assertEquals("openid%2Ca%3A1%2Cb%3A2%2Cc%2Cd", OidcUtils.encodeScopes(config));
}

public static JsonObject read(InputStream input) throws IOException {
try (BufferedReader buffer = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8))) {
return new JsonObject(buffer.lines().collect(Collectors.joining("\n")));
Expand Down

0 comments on commit 8b567ac

Please sign in to comment.