Revert "Simplify credentials format when bootstrapping (#806)"

This reverts commit 4eee4fe.

Author: eric-maynard

polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrap.java

@@ -25,8 +25,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Optional;
-import org.apache.polaris.core.entity.PolarisEntityConstants;
 import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
 
 /**
@@ -54,7 +54,7 @@ public static PolarisCredentialsBootstrap fromEnvironment() {
    * Parse a string of credentials in the format:
    *
    * <pre>
-   * realm1,client1,secret1;realm2,client2,secret2;...
+   * realm1,user1a,client1a,secret1a;realm1,user1b,client1b,secret1b;realm2,user2a,client2a,secret2a;...
    * </pre>
    */
   public static PolarisCredentialsBootstrap fromString(@Nullable String credentialsString) {
@@ -65,51 +65,52 @@ public static PolarisCredentialsBootstrap fromString(@Nullable String credential
 
   /**
    * Parse a list of credentials; each element should be in the format: {@code
-   * realm,clientId,clientSecret}.
+   * realm,principal,clientId,clientSecret}.
    */
   public static PolarisCredentialsBootstrap fromList(List<String> credentialsList) {
-    Map<String, Map.Entry<String, String>> credentials = new HashMap<>();
-    for (String triplet : credentialsList) {
-      if (!triplet.isBlank()) {
-        List<String> parts = Splitter.on(',').trimResults().splitToList(triplet);
-        if (parts.size() != 3) {
-          throw new IllegalArgumentException("Invalid credentials format: " + triplet);
+    Map<String, Map<String, Map.Entry<String, String>>> credentials = new HashMap<>();
+    for (String quadruple : credentialsList) {
+      if (!quadruple.isBlank()) {
+        List<String> parts = Splitter.on(',').trimResults().splitToList(quadruple);
+        if (parts.size() != 4) {
+          throw new IllegalArgumentException("Invalid credentials format: " + quadruple);
         }
         String realmName = parts.get(0);
-        String clientId = parts.get(1);
-        String clientSecret = parts.get(2);
-
-        if (credentials.containsKey(realmName)) {
-          throw new IllegalArgumentException("Duplicate realm: " + realmName);
-        }
-        credentials.put(realmName, new SimpleEntry<>(clientId, clientSecret));
+        String principalName = parts.get(1);
+        String clientId = parts.get(2);
+        String clientSecret = parts.get(3);
+        credentials
+            .computeIfAbsent(realmName, k -> new HashMap<>())
+            .merge(
+                principalName,
+                new SimpleEntry<>(clientId, clientSecret),
+                (a, b) -> {
+                  throw new IllegalArgumentException("Duplicate principal: " + principalName);
+                });
       }
     }
     return credentials.isEmpty() ? EMPTY : new PolarisCredentialsBootstrap(credentials);
   }
 
-  @VisibleForTesting final Map<String, Map.Entry<String, String>> credentials;
+  @VisibleForTesting final Map<String, Map<String, Map.Entry<String, String>>> credentials;
 
-  private PolarisCredentialsBootstrap(Map<String, Map.Entry<String, String>> credentials) {
+  private PolarisCredentialsBootstrap(Map<String, Map<String, Entry<String, String>>> credentials) {
     this.credentials = credentials;
   }
 
   /**
    * Get the secrets for the specified principal in the specified realm, if available among the
-   * credentials that were supplied for bootstrap. Only credentials for the root principal are
-   * supported.
+   * credentials that were supplied for bootstrap.
    */
   public Optional<PolarisPrincipalSecrets> getSecrets(
       String realmName, long principalId, String principalName) {
-    if (principalName.equals(PolarisEntityConstants.getRootPrincipalName())) {
-      return Optional.ofNullable(credentials.get(realmName))
-          .map(
-              credentials -> {
-                String clientId = credentials.getKey();
-                String secret = credentials.getValue();
-                return new PolarisPrincipalSecrets(principalId, clientId, secret, secret);
-              });
-    }
-    return Optional.empty();
+    return Optional.ofNullable(credentials.get(realmName))
+        .flatMap(principals -> Optional.ofNullable(principals.get(principalName)))
+        .map(
+            credentials -> {
+              String clientId = credentials.getKey();
+              String secret = credentials.getValue();
+              return new PolarisPrincipalSecrets(principalId, clientId, secret, secret);
+            });
   }
 }

polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrapTest.java

@@ -21,11 +21,22 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import java.util.Comparator;
 import java.util.List;
+import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
 import org.junit.jupiter.api.Test;
 
 class PolarisCredentialsBootstrapTest {
 
+  private final Comparator<PolarisPrincipalSecrets> comparator =
+      (a, b) ->
+          a.getPrincipalId() == b.getPrincipalId()
+                  && a.getPrincipalClientId().equals(b.getPrincipalClientId())
+                  && a.getMainSecret().equals(b.getMainSecret())
+                  && a.getSecondarySecret().equals(b.getSecondarySecret())
+              ? 0
+              : 1;
+
   @Test
   void nullString() {
     PolarisCredentialsBootstrap credentials = PolarisCredentialsBootstrap.fromString(null);
@@ -51,63 +62,67 @@ void invalidString() {
   }
 
   @Test
-  void duplicateRealm() {
+  void duplicatePrincipal() {
     assertThatThrownBy(
             () ->
                 PolarisCredentialsBootstrap.fromString(
-                    "realm1,client1a,secret1a;realm1,client1b,secret1b"))
-        .hasMessage("Duplicate realm: realm1");
+                    "realm1,user1a,client1a,secret1a;realm1,user1a,client1b,secret1b"))
+        .hasMessage("Duplicate principal: user1a");
   }
 
   @Test
   void getSecretsValidString() {
     PolarisCredentialsBootstrap credentials =
         PolarisCredentialsBootstrap.fromString(
-            " ; realm1 , client1 , secret1 ; realm2 , client2 , secret2 ; ");
-    assertCredentials(credentials);
+            " ; realm1 , user1a , client1a , secret1a ; realm1 , user1b , client1b , secret1b ; realm2 , user2a , client2a , secret2a ; ");
+    assertThat(credentials.getSecrets("realm1", 123, "nonexistent")).isEmpty();
+    assertThat(credentials.getSecrets("nonexistent", 123, "user1a")).isEmpty();
+    assertThat(credentials.getSecrets("realm1", 123, "user1a"))
+        .usingValueComparator(comparator)
+        .contains(new PolarisPrincipalSecrets(123, "client1a", "secret1a", "secret1a"));
+    assertThat(credentials.getSecrets("realm1", 123, "user1b"))
+        .usingValueComparator(comparator)
+        .contains(new PolarisPrincipalSecrets(123, "client1b", "secret1b", "secret1b"));
+    assertThat(credentials.getSecrets("realm2", 123, "user2a"))
+        .usingValueComparator(comparator)
+        .contains(new PolarisPrincipalSecrets(123, "client2a", "secret2a", "secret2a"));
   }
 
   @Test
   void getSecretsValidList() {
     PolarisCredentialsBootstrap credentials =
         PolarisCredentialsBootstrap.fromList(
-            List.of("realm1,client1,secret1", "realm2,client2,secret2"));
-    assertCredentials(credentials);
+            List.of(
+                "realm1,user1a,client1a,secret1a",
+                "realm1,user1b,client1b,secret1b",
+                "realm2,user2a,client2a,secret2a"));
+    assertThat(credentials.getSecrets("realm1", 123, "nonexistent")).isEmpty();
+    assertThat(credentials.getSecrets("nonexistent", 123, "user1a")).isEmpty();
+    assertThat(credentials.getSecrets("realm1", 123, "user1a"))
+        .usingValueComparator(comparator)
+        .contains(new PolarisPrincipalSecrets(123, "client1a", "secret1a", "secret1a"));
+    assertThat(credentials.getSecrets("realm1", 123, "user1b"))
+        .usingValueComparator(comparator)
+        .contains(new PolarisPrincipalSecrets(123, "client1b", "secret1b", "secret1b"));
+    assertThat(credentials.getSecrets("realm2", 123, "user2a"))
+        .usingValueComparator(comparator)
+        .contains(new PolarisPrincipalSecrets(123, "client2a", "secret2a", "secret2a"));
   }
 
   @Test
   void getSecretsValidSystemProperty() {
     PolarisCredentialsBootstrap credentials = PolarisCredentialsBootstrap.fromEnvironment();
     assertThat(credentials.credentials).isEmpty();
     try {
-      System.setProperty(
-          "polaris.bootstrap.credentials", "realm1,client1,secret1;realm2,client2,secret2");
+      System.setProperty("polaris.bootstrap.credentials", "realm1,user1a,client1a,secret1a");
       credentials = PolarisCredentialsBootstrap.fromEnvironment();
-      assertCredentials(credentials);
+      assertThat(credentials.getSecrets("realm1", 123, "nonexistent")).isEmpty();
+      assertThat(credentials.getSecrets("nonexistent", 123, "user1a")).isEmpty();
+      assertThat(credentials.getSecrets("realm1", 123, "user1a"))
+          .usingValueComparator(comparator)
+          .contains(new PolarisPrincipalSecrets(123, "client1a", "secret1a", "secret1a"));
     } finally {
       System.clearProperty("polaris.bootstrap.credentials");
     }
   }
-
-  private void assertCredentials(PolarisCredentialsBootstrap credentials) {
-    assertThat(credentials.getSecrets("realm3", 123, "root")).isEmpty();
-    assertThat(credentials.getSecrets("nonexistent", 123, "root")).isEmpty();
-    assertThat(credentials.getSecrets("realm1", 123, "non-root")).isEmpty();
-    assertThat(credentials.getSecrets("realm1", 123, "root"))
-        .hasValueSatisfying(
-            secrets -> {
-              assertThat(secrets.getPrincipalId()).isEqualTo(123);
-              assertThat(secrets.getPrincipalClientId()).isEqualTo("client1");
-              assertThat(secrets.getMainSecret()).isEqualTo("secret1");
-              assertThat(secrets.getSecondarySecret()).isEqualTo("secret1");
-            });
-    assertThat(credentials.getSecrets("realm2", 123, "root"))
-        .hasValueSatisfying(
-            secrets -> {
-              assertThat(secrets.getPrincipalId()).isEqualTo(123);
-              assertThat(secrets.getPrincipalClientId()).isEqualTo("client2");
-              assertThat(secrets.getMainSecret()).isEqualTo("secret2");
-              assertThat(secrets.getSecondarySecret()).isEqualTo("secret2");
-            });
-  }
 }

polaris-core/src/test/java/org/apache/polaris/core/persistence/PrincipalSecretsGeneratorTest.java

@@ -39,8 +39,9 @@ void testRandomSecrets() {
   @Test
   void testSecretOverride() {
     PrincipalSecretsGenerator gen =
-        bootstrap("test-Realm", PolarisCredentialsBootstrap.fromString("test-Realm,client1,sec2"));
-    PolarisPrincipalSecrets s = gen.produceSecrets("root", 123);
+        bootstrap(
+            "test-Realm", PolarisCredentialsBootstrap.fromString("test-Realm,user1,client1,sec2"));
+    PolarisPrincipalSecrets s = gen.produceSecrets("user1", 123);
     assertThat(s).isNotNull();
     assertThat(s.getPrincipalId()).isEqualTo(123);
     assertThat(s.getPrincipalClientId()).isEqualTo("client1");

quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java

@@ -39,7 +39,7 @@ public class BootstrapCommand extends BaseCommand {
   @CommandLine.Option(
       names = {"-c", "--credential"},
       description =
-          "Root principal credentials to bootstrap. Must be of the form 'realm,clientId,clientSecret'.")
+          "Principal credentials to bootstrap. Must be of the form 'realm,userName,clientId,clientSecret'.")
   List<String> credentials;
 
   @Override

quarkus/admin/src/test/java/org/apache/polaris/admintool/BootstrapCommandTest.java

@@ -37,9 +37,9 @@ class BootstrapCommandTest {
         "-r",
         "realm2",
         "-c",
-        "realm1,root,s3cr3t",
+        "realm1,root,root,s3cr3t",
         "-c",
-        "realm2,root,s3cr3t"
+        "realm2,root,root,s3cr3t"
       })
   public void testBootstrap(LaunchResult result) {
     assertThat(result.getOutput()).contains("Bootstrap completed successfully.");

quarkus/service/build.gradle.kts

@@ -150,9 +150,8 @@ tasks.withType(Test::class.java).configureEach {
   if (System.getenv("AWS_REGION") == null) {
     environment("AWS_REGION", "us-west-2")
   }
-  // Note: the test secrets are referenced in
-  // org.apache.polaris.service.quarkus.it.QuarkusServerManager
-  environment("POLARIS_BOOTSTRAP_CREDENTIALS", "POLARIS,test-admin,test-secret")
+  // Note: the test secrets are referenced in DropwizardServerManager
+  environment("POLARIS_BOOTSTRAP_CREDENTIALS", "POLARIS,root,test-admin,test-secret")
   jvmArgs("--add-exports", "java.base/sun.nio.ch=ALL-UNNAMED")
   // Need to allow a java security manager after Java 21, for Subject.getSubject to work
   // "getSubject is supported only if a security manager is allowed".

site/content/in-dev/unreleased/configuring-polaris-for-production.md

@@ -75,19 +75,21 @@ Before using Polaris when using a metastore manager other than `in-memory`, you
 By default, Polaris will create randomised `CLIENT_ID` and `CLIENT_SECRET` for the `root` principal and store their hashes in the metastore backend. In order to provide your own credentials for `root` principal (so you can request tokens via `api/catalog/v1/oauth/tokens`), set the `POLARIS_BOOTSTRAP_CREDENTIALS` environment variable as follows:
 
 ```
-export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,my-client-id,my-client-secret
+export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,root,my-client-id,my-client-secret
 ```
 
-The format of the environment variable is `realm,client_id,client_secret`. You can provide multiple credentials separated by `;`. For example, to provide credentials for two realms `my_realm` and `my_realm2`:
+The format of the environment variable is `realm,principal,client_id,client_secret`. You can provide multiple credentials separated by `;`. For example, to provide credentials for two realms `my_realm` and `my_realm2`:
 
 ```
-export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,my-client-id,my-client-secret;my_realm2,my-client-id2,my-client-secret2
+export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,root,my-client-id,my-client-secret;my_realm2,root,my-client-id2,my-client-secret2
 ```
 
+You can also provide credentials for other users too. 
+
 It is also possible to use system properties to provide the credentials:
 
 ```
-java -Dpolaris.bootstrap.credentials=my_realm,my-client-id,my-client-secret -jar /path/to/jar/polaris-service-all.jar bootstrap polaris-server.yml
+java -Dpolaris.bootstrap.credentials=my_realm,root,my-client-id,my-client-secret -jar /path/to/jar/polaris-service-all.jar bootstrap polaris-server.yml
 ```
 
 Now, to bootstrap Polaris, run: