gplazma: oidc add support for suppress option

Motivation: Some OPs should be handled in a particular way. To concrete examples are disabling offline verification (always use the userinfo endpoint) and disabling audience (`aud`) claim verification. These both are enhancements that come when uprading from an earlier version of dCache to v8.2. Although both provide benefits (in the right circumstances), they also both risk disrupting existing use-cases. Therefore, we should provide admins with a way to disable disruptive new features, at least until users or OPs have "caught up". Modification: Add support for the `-suppress` option when defining an OP. This option takes a comma-separated list of keywords. The option may also be repeated, in which case keywords from all options are merged. Result: No user or admin observable changes; however, this patch layes the basis for subsequent changes. Target: master Request: 9.0 Request: 8.2 Requires-notes: yes Requires-book: no Patch: https://rb.dcache.org/r/13901/ Acked-by: Tigran Mkrtchyan
dCache · Mar 14, 2023 · f13812e · f13812e
1 parent ecc1b2b
commit f13812e
Show file tree

Hide file tree

Showing 9 changed files with 197 additions and 54 deletions.
diff --git a/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/IdentityProvider.java b/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/IdentityProvider.java
@@ -25,11 +25,16 @@
 import java.io.IOException;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import com.google.common.base.Splitter;
 import static java.util.Objects.requireNonNull;
 
 import java.net.URI;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import org.apache.http.client.HttpClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -57,12 +62,13 @@ public class IdentityProvider {
     private final Profile profile;
     private final HttpClient client;
     private final Duration cacheDurationWhenSuccessful;
+    private final List<String> suppress;
 
     private Instant nextDiscoveryFetch = Instant.now();
     private JsonNode discoveryDocument = MissingNode.getInstance();
 
     public IdentityProvider(String name, URI endpoint, Profile profile, HttpClient client,
-            Duration discoveryCacheDuration) {
+            Duration discoveryCacheDuration, List<String> suppress) {
         checkArgument(!name.isEmpty(), "Empty name not allowed");
         this.name = name;
         this.issuer = requireNonNull(endpoint);
@@ -72,6 +78,7 @@ public IdentityProvider(String name, URI endpoint, Profile profile, HttpClient c
         configuration = issuer.resolve(
               withTrailingSlash(issuer.getPath()) + ".well-known/openid-configuration");
         cacheDurationWhenSuccessful = requireNonNull(discoveryCacheDuration);
+        this.suppress = requireNonNull(suppress);
     }
 
     private static String withTrailingSlash(String path) {
@@ -90,6 +97,10 @@ public Profile getProfile() {
         return profile;
     }
 
+    public boolean isSuppressed(String value) {
+        return suppress.stream().anyMatch(v -> v.equals(value));
+    }
+
     /**
      */
     @VisibleForTesting

diff --git a/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/OidcAuthPlugin.java b/modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/OidcAuthPlugin.java
@@ -91,7 +91,12 @@ private static IdentityProvider createIdentityProvider(String name, String descr
         try {
             URI issuer = new URI(endpoint);
 
-            return new IdentityProvider(name, issuer, profile, client, discoveryCacheDuration);
+            List<String> suppress = args.getOptions("suppress").stream()
+                .flatMap(v -> Splitter.on(',').trimResults().splitToStream(v))
+                .collect(Collectors.toList());
+
+            return new IdentityProvider(name, issuer, profile, client, discoveryCacheDuration,
+                suppress);
         } catch (URISyntaxException e) {
             throw new IllegalArgumentException(
                   "Invalid endpoint " + endpoint + ": " + e.getMessage());
@@ -104,7 +109,7 @@ private static Profile buildProfile(Args args) {
         ProfileFactory factory = PROFILES.get(profileName);
         checkArgument(factory != null, "profile '%s' is not supported", profileName);
 
-        return factory.create(args.optionsAsMap());
+        return factory.create(args.removeOptions("suppress").optionsAsMap());
     }
 
     @Override

diff --git a/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/ChainedTokenProcessorTest.java b/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/ChainedTokenProcessorTest.java
@@ -24,6 +24,7 @@
 import java.time.Duration;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import org.dcache.gplazma.AuthenticationException;
 import org.junit.Test;
@@ -44,7 +45,7 @@ private class ResultBuilder {
         public ResultBuilder withIP(String name) {
             provider = new IdentityProvider(name, URI.create("https://" + name + ".example.org/"),
                     (idp,c) -> new ProfileResult(Collections.emptySet()),
-                    MockHttpClientBuilder.aClient().build(), Duration.ofSeconds(1));
+                    MockHttpClientBuilder.aClient().build(), Duration.ofSeconds(1), List.of());
             return this;
         }
 

diff --git a/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/ExtractResultTest.java b/modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/ExtractResultTest.java
@@ -21,6 +21,7 @@
 import java.net.URI;
 import java.time.Duration;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.junit.Test;
 
@@ -32,6 +33,7 @@
 
 public class ExtractResultTest {
     private static final Profile IGNORE_ALL = (i,c) -> new ProfileResult(Collections.emptySet());
+    private static final List<String> NO_SUPPRESSION = List.of();
     private final ObjectMapper mapper = new ObjectMapper();
 
 
@@ -43,13 +45,13 @@ public void shouldThrowNpeOnNullIp() {
     @Test(expected=NullPointerException.class)
     public void shouldThrowNpeOnNullMap() {
         new ExtractResult(new IdentityProvider("test", URI.create("https://example.org/"), IGNORE_ALL,
-                aClient().build(), Duration.ofSeconds(2)), null);
+                aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION), null);
     }
 
     @Test
     public void shouldMatchIp() {
         var idp = new IdentityProvider("test", URI.create("https://example.org"), IGNORE_ALL,
-                aClient().build(), Duration.ofSeconds(2));
+                aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION);
 
         var result = new ExtractResult(idp, Collections.emptyMap());
 
@@ -59,7 +61,7 @@ public void shouldMatchIp() {
     @Test
     public void shouldMatchClaims() throws Exception {
         var result = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         assertThat(result.claims(), aMapWithSize(1));
@@ -69,11 +71,11 @@ IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
     @Test
     public void twoSameResultsShouldBeEqual() throws Exception {
         var result1 = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         var result2 = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         assertTrue(result1.equals(result2));
@@ -82,11 +84,11 @@ IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
     @Test
     public void twoResultsWithDifferentIpShouldNotBeEqual() throws Exception {
         var result1 = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         var result2 = new ExtractResult(new IdentityProvider("test2", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         assertFalse(result1.equals(result2));
@@ -95,11 +97,11 @@ IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
     @Test
     public void twoResultsWithDifferentClaimsShouldNotBeEqual() throws Exception {
         var result1 = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         var result2 = new ExtractResult(new IdentityProvider("test2", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"some-other-value\"")));
 
         assertFalse(result1.equals(result2));
@@ -108,11 +110,11 @@ IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
     @Test
     public void twoSameResultsShouldHaveSameHash() throws Exception {
         var result1 = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         var result2 = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")));
 
         assertThat(result2.hashCode(), equalTo(result1.hashCode()));
@@ -121,7 +123,7 @@ IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
     @Test
     public void shouldHaveElementsInToString() throws Exception {
         var description = new ExtractResult(new IdentityProvider("test", URI.create("https://example.org"),
-                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2)),
+                IGNORE_ALL, aClient().build(), Duration.ofSeconds(2), NO_SUPPRESSION),
                 Map.of("sub", mapper.readTree("\"abcdefg012345\"")))
                 .toString();