eric-maynard commented on code in PR #461:
URL: https://github.com/apache/polaris/pull/461#discussion_r1860357874
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/secrets/PrincipalSecretsGenerator.java:
##########
@@ -38,48 +37,48 @@
* <li>{@code POLARIS_BOOTSTRAP_<REALM-NAME>_<PRINCIPAL-NAME>_CLIENT_SECRET}
* </ul>
*
- * For example: {@code POLARIS_BOOTSTRAP_DEFAULT-REALM_ROOT_CLIENT_ID} and
{@code
- * POLARIS_BOOTSTRAP_DEFAULT-REALM_ROOT_CLIENT_SECRET}.
+ * For example: {@code POLARIS_BOOTSTRAP_default-realm_root_CLIENT_ID} and
{@code
+ * POLARIS_BOOTSTRAP_default-realm_root_CLIENT_SECRET}.
*/
-@FunctionalInterface
-public interface PrincipalSecretsGenerator {
+public abstract class PrincipalSecretsGenerator {
- /**
- * A secret generator that produces cryptographically random client ID and
client secret values.
- */
- PrincipalSecretsGenerator RANDOM_SECRETS = (name, id) -> new
PolarisPrincipalSecrets(id);
+ protected final String realmName;
+
+ public PrincipalSecretsGenerator() {
+ this.realmName = null;
+ }
+
+ public PrincipalSecretsGenerator(@Nullable String realmName) {
+ this.realmName = realmName;
+ }
/**
* Produces a new {@link PolarisPrincipalSecrets} object for the given
principal ID. The returned
- * secrets may or may not be random, depending on context. In bootstrapping
contexts, the returned
- * secrets can be predefined. After bootstrapping, the returned secrets can
be expected to be
- * cryptographically random.
+ * secrets may or may not be random, depending on context. The returned
secrets can be predefined.
*
* @param principalName the name of the related principal. This parameter is
a hint for
* pre-defined secrets lookup during bootstrapping it is not included in
the returned data.
* @param principalId the ID of the related principal. This ID is part of
the returned data.
* @return a new {@link PolarisPrincipalSecrets} instance for the specified
principal.
*/
- PolarisPrincipalSecrets produceSecrets(@NotNull String principalName, long
principalId);
-
- static PrincipalSecretsGenerator bootstrap(String realmName) {
- return bootstrap(realmName, System.getenv()::get);
- }
+ public abstract PolarisPrincipalSecrets produceSecrets(
+ @NotNull String principalName, long principalId);
- static PrincipalSecretsGenerator bootstrap(String realmName,
Function<String, String> config) {
- return (principalName, principalId) -> {
- String propId = String.format("POLARIS_BOOTSTRAP_%s_%s_CLIENT_ID",
realmName, principalName);
- String propSecret =
- String.format("POLARIS_BOOTSTRAP_%s_%s_CLIENT_SECRET", realmName,
principalName);
+ /**
+ * @param principalName the name of the related principal. This parameter is
a hint for
+ * pre-defined secrets lookup during bootstrapping it is not included in
the returned data.
+ * @return true if the secrets generated by this {@link
PrincipalSecretsGenerator} are
+ * Polaris-generated as opposed to being provided by the user or another
system.
+ */
+ public abstract boolean systemGeneratedSecrets(@NotNull String
principalName);
- String clientId = config.apply(propId.toUpperCase(Locale.ROOT));
Review Comment:
Yeah, I actually think this is wrong isn't it? It seems like you can have
both a `dimas` and a `DIMAS` user, so how would you differentiate them in the
env variables?
This is assuming we now allow the use of env variables for non-root users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
