Skip to content

Commit

Permalink
fix: IDP supported scopes - Turn the exception into a warning message #…
Browse files Browse the repository at this point in the history
  • Loading branch information
idirze committed Jun 3, 2024
1 parent 6a9b35a commit 9275de1
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 36 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

<groupId>io.okdp</groupId>
<artifactId>okdp-spark-auth-filter</artifactId>
<version>1.2.0</version>
<version>1.2.1-SNAPSHOT</version>

<name>OIDC authentication filter for Apache spark</name>
<description>OIDC authentication filter for Apache spark web UIs (Spark app and History Web UIs)
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/okdp/spark/authc/OidcAuthFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import static io.okdp.spark.authc.utils.HttpAuthenticationUtils.sendError;
import static io.okdp.spark.authc.utils.PreconditionsUtils.assertCookieSecure;
import static io.okdp.spark.authc.utils.PreconditionsUtils.assertSupportePKCE;
import static io.okdp.spark.authc.utils.PreconditionsUtils.assertSupportedScopes;
import static io.okdp.spark.authc.utils.PreconditionsUtils.checkAuthLogin;
import static io.okdp.spark.authc.utils.PreconditionsUtils.warnUnsupportedScopes;
import static java.lang.String.format;
import static java.util.Optional.ofNullable;

Expand Down Expand Up @@ -156,7 +156,7 @@ public void init(FilterConfig filterConfig) {
oidcConfig.wellKnownConfiguration().scopesSupported(),
oidcConfig.wellKnownConfiguration().supportedPKCECodeChallengeMethods());

assertSupportedScopes(
warnUnsupportedScopes(
oidcConfig.wellKnownConfiguration().scopesSupported(),
scope,
format("%s|env: %s", AUTH_SCOPE, "AUTH_SCOPE"));
Expand Down
17 changes: 10 additions & 7 deletions src/main/java/io/okdp/spark/authc/utils/PreconditionsUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
import java.util.List;
import java.util.Optional;
import javax.servlet.ServletRequest;
import lombok.extern.slf4j.Slf4j;

/** Preconditions check utility methods */
@Slf4j
public class PreconditionsUtils {

/** Ensures the given string is not null. */
Expand All @@ -55,17 +57,18 @@ public static void checkState(String provided, String expected, Object errorMess
* @param supported
* @param provided
*/
public static void assertSupportedScopes(List<String> supported, String provided, String label) {
public static void warnUnsupportedScopes(List<String> supported, String provided, String label) {
List<String> unsupported =
Arrays.stream(provided.split("\\+"))
.filter(element -> !supported.contains(element))
.collect(toList());
checkArgument(
unsupported.isEmpty(),
format(
"The parameter '%s' contains an unsupported scopes '%s' by your oidc provider.\n"
+ "The supported scopes are: %s",
label, unsupported, supported));
if (!unsupported.isEmpty()) {
log.warn(
"The parameter '{}' contains an unsupported scopes '{}' by your oidc provider. The supported scopes are: {}",
label,
unsupported,
supported);
}
}

/** The OIDC provider should support PKCE for public clients */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package io.okdp.spark.authc.utils;

import static io.okdp.spark.authc.utils.PreconditionsUtils.assertSupportePKCE;
import static io.okdp.spark.authc.utils.PreconditionsUtils.assertSupportedScopes;
import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
Expand Down Expand Up @@ -73,31 +72,6 @@ public void should_not_throw_any_exception() {
assertThatCode(validValue).doesNotThrowAnyException();
}

@Test
public void should_assert_valid_scopes() {
// when
ThrowingCallable validScopes =
() ->
assertSupportedScopes(
asList("openid", "profile", "email", "roles", "offline_access"),
"openid+profile+email",
"scope");

ThrowingCallable unsupportedScopes =
() ->
assertSupportedScopes(
asList("openid", "profile", "email", "roles", "offline_access"),
"openid+profile+email+groups+roles+offline_access",
"scope");

// Then
assertThatCode(validScopes).doesNotThrowAnyException();
assertThatCode(unsupportedScopes)
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("'[groups]'")
.hasMessageContaining("scope");
}

@Test
public void should_support_pkce_for_public_clients() {

Expand Down

0 comments on commit 9275de1

Please sign in to comment.