OAuth client: avoid recomputing HTTP headers for static secrets (#9411)

projectnessie · Aug 27, 2024 · a5f9894 · a5f9894
1 parent fe65f23
commit a5f9894
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 120 deletions.
diff --git a/api/client/src/main/java/org/projectnessie/client/auth/oauth2/ImpersonationConfig.java b/api/client/src/main/java/org/projectnessie/client/auth/oauth2/ImpersonationConfig.java
@@ -87,24 +87,21 @@ default boolean isEnabled() {
   Optional<String> getClientId();
 
   /**
-   * An alternate client secret to use for impersonations only. Required if the alternate client
-   * obtained from {@link #getClientId()} is confidential.
+   * An alternate, static client secret to use for impersonations only. If the alternate client
+   * obtained from {@link #getClientId()} is confidential, either this attribute or {@link
+   * #getClientSecretSupplier()} must be set.
    *
-   * @deprecated Use {@link #getClientSecretSupplier()} instead.
+   * @see NessieConfigConstants#CONF_NESSIE_OAUTH2_IMPERSONATION_CLIENT_SECRET
    */
-  @SuppressWarnings("DeprecatedIsStillUsed")
-  @Deprecated
-  @Value.Derived
-  default Optional<Secret> getClientSecret() {
-    return getClientSecretSupplier().map(Supplier::get).map(Secret::new);
-  }
+  @Value.Redacted
+  Optional<Secret> getClientSecret();
 
   /**
-   * An alternate client secret to use for impersonations only. Required if the alternate client
-   * obtained from {@link #getClientId()} is confidential.
-   *
-   * @see NessieConfigConstants#CONF_NESSIE_OAUTH2_IMPERSONATION_CLIENT_SECRET
+   * An alternate client secret supplier to use for impersonations only. If the alternate client
+   * obtained from {@link #getClientId()} is confidential, either this attribute or {@link
+   * #getClientSecret()} must be set.
    */
+  @Value.Redacted
   Optional<Supplier<String>> getClientSecretSupplier();
 
   /**
@@ -160,18 +157,15 @@ interface Builder {
     Builder clientId(String clientId);
 
     @CanIgnoreReturnValue
-    Builder clientSecretSupplier(Supplier<String> clientSecret);
+    Builder clientSecret(Secret clientSecret);
 
     @CanIgnoreReturnValue
-    @SuppressWarnings("deprecation")
-    default Builder clientSecret(Secret clientSecret) {
-      return clientSecretSupplier(clientSecret::getString);
+    default Builder clientSecret(String clientSecret) {
+      return clientSecret(new Secret(clientSecret));
     }
 
     @CanIgnoreReturnValue
-    default Builder clientSecret(String clientSecret) {
-      return clientSecretSupplier(() -> clientSecret);
-    }
+    Builder clientSecretSupplier(Supplier<String> clientSecret);
 
     @CanIgnoreReturnValue
     Builder issuerUrl(URI issuerUrl);

diff --git a/api/client/src/main/java/org/projectnessie/client/auth/oauth2/ImpersonationFlow.java b/api/client/src/main/java/org/projectnessie/client/auth/oauth2/ImpersonationFlow.java
@@ -48,34 +48,23 @@ Optional<String> getScope() {
 
   @Override
   URI getResolvedTokenEndpoint() {
-    if (impersonationConfig.getIssuerUrl().isPresent()
-        || impersonationConfig.getTokenEndpoint().isPresent()) {
-      return config.getResolvedImpersonationTokenEndpoint();
-    }
-    return super.getResolvedTokenEndpoint();
+    return config
+        .getResolvedImpersonationTokenEndpoint()
+        .orElseGet(super::getResolvedTokenEndpoint);
   }
 
   @Override
   String getClientId() {
-    if (impersonationConfig.getClientId().isPresent()) {
-      return impersonationConfig.getClientId().get();
-    }
-    return super.getClientId();
+    return impersonationConfig.getClientId().orElseGet(super::getClientId);
   }
 
   @Override
   boolean isPublicClient() {
-    if (impersonationConfig.getClientId().isPresent()) {
-      return !impersonationConfig.getClientSecret().isPresent();
-    }
-    return super.isPublicClient();
+    return config.isImpersonationPublicClient().orElseGet(super::isPublicClient);
   }
 
   @Override
   Optional<HttpAuthentication> getBasicAuthentication() {
-    if (impersonationConfig.getClientId().isPresent()) {
-      return config.getImpersonationBasicAuthentication();
-    }
-    return super.getBasicAuthentication();
+    return config.getImpersonationBasicAuthentication().or(super::getBasicAuthentication);
   }
 }
diff --git a/api/client/src/main/java/org/projectnessie/client/auth/oauth2/OAuth2AuthenticatorConfig.java b/api/client/src/main/java/org/projectnessie/client/auth/oauth2/OAuth2AuthenticatorConfig.java
@@ -201,22 +201,19 @@ default GrantType getGrantType() {
   String getClientId();
 
   /**
-   * The OAuth2 client secret. Must be set, if required by the IdP.
+   * The OAuth2 static client secret. Either this attribute or {@link #getClientSecretSupplier()}
+   * must be set, if a client secret is required by the IdP.
    *
-   * @deprecated Use {@link #getClientSecretSupplier()} instead.
+   * @see NessieConfigConstants#CONF_NESSIE_OAUTH2_CLIENT_SECRET
    */
-  @SuppressWarnings("DeprecatedIsStillUsed")
-  @Deprecated
-  @Value.Derived
-  default Optional<Secret> getClientSecret() {
-    return getClientSecretSupplier().map(Supplier::get).map(Secret::new);
-  }
+  @Value.Redacted
+  Optional<Secret> getClientSecret();
 
   /**
-   * The OAuth2 client secret supplier. Must be set, if required by the IdP.
-   *
-   * @see NessieConfigConstants#CONF_NESSIE_OAUTH2_CLIENT_SECRET
+   * The OAuth2 client secret supplier. Either this attribute or {@link #getClientSecret()} must be
+   * set, if a client secret is required by the IdP.
    */
+  @Value.Redacted
   Optional<Supplier<String>> getClientSecretSupplier();
 
   /**
@@ -227,22 +224,19 @@ default Optional<Secret> getClientSecret() {
   Optional<String> getUsername();
 
   /**
-   * The OAuth2 password. Only relevant for {@link GrantType#PASSWORD} grant type.
+   * The OAuth2 static password. Only relevant for {@link GrantType#PASSWORD} grant type. Either
+   * this attribute or {@link #getPasswordSupplier()} must be set if a password is required.
    *
-   * @deprecated Use {@link #getPasswordSupplier()} instead.
+   * @see NessieConfigConstants#CONF_NESSIE_OAUTH2_PASSWORD
    */
-  @SuppressWarnings("DeprecatedIsStillUsed")
-  @Deprecated
-  @Value.Derived
-  default Optional<Secret> getPassword() {
-    return getPasswordSupplier().map(Supplier::get).map(Secret::new);
-  }
+  @Value.Redacted
+  Optional<Secret> getPassword();
 
   /**
-   * The OAuth2 password supplier. Only relevant for {@link GrantType#PASSWORD} grant type.
-   *
-   * @see NessieConfigConstants#CONF_NESSIE_OAUTH2_PASSWORD
+   * The OAuth2 password supplier. Only relevant for {@link GrantType#PASSWORD} grant type. Either
+   * this attribute or {@link #getPassword()} must be set if a password is required.
    */
+  @Value.Redacted
   Optional<Supplier<String>> getPasswordSupplier();
 
   @Value.Derived
@@ -436,37 +430,29 @@ interface Builder {
     Builder clientId(String clientId);
 
     @CanIgnoreReturnValue
-    Builder clientSecretSupplier(Supplier<String> clientSecret);
+    Builder clientSecret(Secret clientSecret);
 
     @CanIgnoreReturnValue
-    @Deprecated
-    default Builder clientSecret(Secret clientSecret) {
-      return clientSecretSupplier(clientSecret::getString);
+    default Builder clientSecret(String clientSecret) {
+      return clientSecret(new Secret(clientSecret));
     }
 
     @CanIgnoreReturnValue
-    default Builder clientSecret(String clientSecret) {
-      char[] chars = clientSecret.toCharArray();
-      return clientSecretSupplier(() -> new String(chars));
-    }
+    Builder clientSecretSupplier(Supplier<String> clientSecret);
 
     @CanIgnoreReturnValue
     Builder username(String username);
 
     @CanIgnoreReturnValue
-    Builder passwordSupplier(Supplier<String> password);
+    Builder password(Secret password);
 
     @CanIgnoreReturnValue
-    @Deprecated
-    default Builder password(Secret password) {
-      return passwordSupplier(password::getString);
+    default Builder password(String password) {
+      return password(new Secret(password));
     }
 
     @CanIgnoreReturnValue
-    default Builder password(String password) {
-      char[] chars = password.toCharArray();
-      return passwordSupplier(() -> new String(chars));
-    }
+    Builder passwordSupplier(Supplier<String> password);
 
     @CanIgnoreReturnValue
     @Deprecated

diff --git a/api/client/src/main/java/org/projectnessie/client/auth/oauth2/OAuth2ClientConfig.java b/api/client/src/main/java/org/projectnessie/client/auth/oauth2/OAuth2ClientConfig.java
@@ -130,7 +130,14 @@ Supplier<Instant> getClock() {
 
   @Value.Derived
   boolean isPublicClient() {
-    return !getClientSecretSupplier().isPresent();
+    return getClientSecret().isEmpty() && getClientSecretSupplier().isEmpty();
+  }
+
+  @Value.Derived
+  Optional<Boolean> isImpersonationPublicClient() {
+    return getImpersonationConfig()
+        .getClientId()
+        .map(id -> getClientSecret().isEmpty() && getClientSecretSupplier().isEmpty());
   }
 
   @Value.Lazy
@@ -140,12 +147,10 @@ JsonNode getOpenIdProviderMetadata() {
   }
 
   @Value.Lazy
-  JsonNode getImpersonationOpenIdProviderMetadata() {
-    URI issuerUrl =
-        getImpersonationConfig()
-            .getIssuerUrl()
-            .orElseThrow(() -> new IllegalStateException("No issuer-URL"));
-    return OAuth2Utils.fetchOpenIdProviderMetadata(getHttpClient(), issuerUrl);
+  Optional<JsonNode> getImpersonationOpenIdProviderMetadata() {
+    return getImpersonationConfig()
+        .getIssuerUrl()
+        .map(url -> OAuth2Utils.fetchOpenIdProviderMetadata(getHttpClient(), url));
   }
 
   @Value.Lazy
@@ -187,15 +192,20 @@ URI getResolvedDeviceAuthEndpoint() {
   }
 
   @Value.Lazy
-  URI getResolvedImpersonationTokenEndpoint() {
-    if (getImpersonationConfig().getTokenEndpoint().isPresent()) {
-      return getImpersonationConfig().getTokenEndpoint().get();
-    }
-    JsonNode json = getImpersonationOpenIdProviderMetadata();
-    if (json.has("token_endpoint")) {
-      return URI.create(json.get("token_endpoint").asText());
-    }
-    throw new IllegalStateException("OpenID provider metadata does not contain a token endpoint");
+  Optional<URI> getResolvedImpersonationTokenEndpoint() {
+    return getImpersonationConfig()
+        .getTokenEndpoint()
+        .or(
+            () ->
+                getImpersonationOpenIdProviderMetadata()
+                    .map(
+                        json -> {
+                          if (json.has("token_endpoint")) {
+                            return URI.create(json.get("token_endpoint").asText());
+                          }
+                          throw new IllegalStateException(
+                              "OpenID provider metadata does not contain a token endpoint");
+                        }));
   }
 
   /**
@@ -204,8 +214,12 @@ URI getResolvedImpersonationTokenEndpoint() {
    */
   @Value.Lazy
   Optional<HttpAuthentication> getBasicAuthentication() {
-    return getClientSecretSupplier()
-        .map(secretSupplier -> BasicAuthenticationProvider.create(getClientId(), secretSupplier));
+    return getClientSecret()
+        .map(s -> BasicAuthenticationProvider.create(getClientId(), s.getString()))
+        .or(
+            () ->
+                getClientSecretSupplier()
+                    .map(s -> BasicAuthenticationProvider.create(getClientId(), s)));
   }
 
   /**
@@ -218,11 +232,12 @@ Optional<HttpAuthentication> getImpersonationBasicAuthentication() {
         .getClientId()
         .flatMap(
             clientId ->
-                getImpersonationConfig()
-                    .getClientSecretSupplier()
-                    .map(
-                        secretSupplier ->
-                            BasicAuthenticationProvider.create(clientId, secretSupplier)));
+                getClientSecret()
+                    .map(s -> BasicAuthenticationProvider.create(clientId, s.getString()))
+                    .or(
+                        () ->
+                            getClientSecretSupplier()
+                                .map(s -> BasicAuthenticationProvider.create(clientId, s))));
   }
 
   /**
@@ -265,7 +280,9 @@ void check() {
     check(
         violations,
         CONF_NESSIE_OAUTH2_GRANT_TYPE + " / " + CONF_NESSIE_OAUTH2_CLIENT_SECRET,
-        getClientSecretSupplier().isPresent() || getGrantType() != GrantType.CLIENT_CREDENTIALS,
+        getClientSecret().isPresent()
+            || getClientSecretSupplier().isPresent()
+            || getGrantType() != GrantType.CLIENT_CREDENTIALS,
         "client secret must not be empty when grant type is '%s'",
         CONF_NESSIE_OAUTH2_GRANT_TYPE_CLIENT_CREDENTIALS);
     check(
@@ -318,7 +335,7 @@ void check() {
       check(
           violations,
           CONF_NESSIE_OAUTH2_PASSWORD,
-          getPasswordSupplier().isPresent(),
+          getPassword().isPresent() || getPasswordSupplier().isPresent(),
           "password must be set if grant type is '%s'",
           CONF_NESSIE_OAUTH2_GRANT_TYPE_PASSWORD);
     }
@@ -448,36 +465,30 @@ interface Builder extends OAuth2AuthenticatorConfig.Builder {
     Builder clientId(String clientId);
 
     @CanIgnoreReturnValue
-    Builder clientSecretSupplier(Supplier<String> clientSecret);
-
-    @CanIgnoreReturnValue
-    @SuppressWarnings("deprecation")
-    default Builder clientSecret(Secret clientSecret) {
-      return clientSecretSupplier(clientSecret::getString);
-    }
+    Builder clientSecret(Secret clientSecret);
 
     @CanIgnoreReturnValue
     default Builder clientSecret(String clientSecret) {
       return clientSecretSupplier(() -> clientSecret);
     }
 
+    @CanIgnoreReturnValue
+    Builder clientSecretSupplier(Supplier<String> clientSecret);
+
     @Override
     Builder username(String username);
 
     @CanIgnoreReturnValue
-    Builder passwordSupplier(Supplier<String> password);
-
-    @CanIgnoreReturnValue
-    @SuppressWarnings("deprecation")
-    default Builder password(Secret password) {
-      return passwordSupplier(password::getString);
-    }
+    Builder password(Secret password);
 
     @CanIgnoreReturnValue
     default Builder password(String password) {
       return passwordSupplier(() -> password);
     }
 
+    @CanIgnoreReturnValue
+    Builder passwordSupplier(Supplier<String> password);
+
     @Override
     Builder addScope(String scope);
 

diff --git a/api/client/src/main/java/org/projectnessie/client/auth/oauth2/Secret.java b/api/client/src/main/java/org/projectnessie/client/auth/oauth2/Secret.java
@@ -15,12 +15,7 @@
  */
 package org.projectnessie.client.auth.oauth2;
 
-/**
- * A secret value.
- *
- * @deprecated will be removed in a future release.
- */
-@Deprecated
+/** A secret value. */
 public final class Secret {
 
   private final char[] value;