This is an automated email from the ASF dual-hosted git repository.
emaynard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 45df8ac4c Improve the parsing and validation of
UserSecretReferenceUrns (#1840)
45df8ac4c is described below
commit 45df8ac4cc284d9c238d8d896e6656372822cbb0
Author: Pooja Nilangekar <poo...@umd.edu>
AuthorDate: Wed Jun 18 16:05:59 2025 -0400
Improve the parsing and validation of UserSecretReferenceUrns (#1840)
This change addresses all the TODOs found the org.polaris.core.secrets
package.
Main changes:
- Create a helper to parse, validate and build the URN strings.
- Use Regex instead of `String.split()`.
- Add Precondition checks to ensure that the URN is valid and the
UserSecretManager matches the expected type.
- Remove the now unused `GLOBAL_INSTANCE` of the
UnsafeInMemorySecretsManager.
Testing
- Existing `UnsafeInMemorySecretsManagerTest` captures most of the
functional changes.
- Added `UserSecretReferenceUrnHelperTest` to capture the utilities exposed.
---
.../core/secrets/UnsafeInMemorySecretsManager.java | 21 ++--
.../polaris/core/secrets/UserSecretReference.java | 112 +++++++++++++++++++--
.../polaris/core/secrets/UserSecretsManager.java | 13 +++
.../core/secrets/UserSecretReferenceTest.java | 83 +++++++++++++++
4 files changed, 213 insertions(+), 16 deletions(-)
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java
b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java
index 38cf00e22..bb690a158 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java
@@ -18,6 +18,7 @@
*/
package org.apache.polaris.core.secrets;
+import com.google.common.base.Preconditions;
import jakarta.annotation.Nonnull;
import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
@@ -34,9 +35,6 @@ import org.apache.polaris.core.entity.PolarisEntityCore;
* development purposes.
*/
public class UnsafeInMemorySecretsManager implements UserSecretsManager {
- // TODO: Remove this and wire into QuarkusProducers; just a placeholder for
now to get the
- // rest of the logic working.
- public static final UserSecretsManager GLOBAL_INSTANCE = new
UnsafeInMemorySecretsManager();
private final Map<String, String> rawSecretStore = new ConcurrentHashMap<>();
private final SecureRandom rand = new SecureRandom();
@@ -45,6 +43,8 @@ public class UnsafeInMemorySecretsManager implements
UserSecretsManager {
private static final String CIPHERTEXT_HASH = "ciphertext-hash";
private static final String ENCRYPTION_KEY = "encryption-key";
+ public static final String SECRET_MANAGER_TYPE = "unsafe-in-memory";
+
/** {@inheritDoc} */
@Override
@Nonnull
@@ -73,10 +73,8 @@ public class UnsafeInMemorySecretsManager implements
UserSecretsManager {
String secretUrn;
for (int secretOrdinal = 0; ; ++secretOrdinal) {
- secretUrn =
- String.format(
- "urn:polaris-secret:unsafe-in-memory:%d:%d", forEntity.getId(),
secretOrdinal);
-
+ String typeSpecificIdentifier = forEntity.getId() + ":" + secretOrdinal;
+ secretUrn = buildUrn(SECRET_MANAGER_TYPE, typeSpecificIdentifier);
// Store the base64-encoded encrypted ciphertext in the simulated
"secret store".
String existingSecret =
rawSecretStore.putIfAbsent(secretUrn,
encryptedSecretCipherTextBase64);
@@ -107,7 +105,14 @@ public class UnsafeInMemorySecretsManager implements
UserSecretsManager {
@Override
@Nonnull
public String readSecret(@Nonnull UserSecretReference secretReference) {
- // TODO: Precondition checks and/or wire in PolarisDiagnostics
+ String secretManagerType = secretReference.getUserSecretManagerType();
+ Preconditions.checkState(
+ secretManagerType.equals(SECRET_MANAGER_TYPE),
+ "Invalid secret manager type, expected: "
+ + SECRET_MANAGER_TYPE
+ + " got: "
+ + secretManagerType);
+
String encryptedSecretCipherTextBase64 =
rawSecretStore.get(secretReference.getUrn());
if (encryptedSecretCipherTextBase64 == null) {
// Secret at this URN no longer exists.
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretReference.java
b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretReference.java
index 7181acb04..77a69dc6f 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretReference.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretReference.java
@@ -27,6 +27,8 @@ import jakarta.annotation.Nullable;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/**
* Represents a "wrapped reference" to a user-owned secret that holds an
identifier to retrieve
@@ -56,6 +58,43 @@ public class UserSecretReference {
@JsonProperty(value = "referencePayload")
private final Map<String, String> referencePayload;
+ private static final String URN_SCHEME = "urn";
+ private static final String URN_NAMESPACE = "polaris-secret";
+ private static final String SECRET_MANAGER_TYPE_REGEX = "([a-zA-Z0-9_-]+)";
+ private static final String TYPE_SPECIFIC_IDENTIFIER_REGEX =
+ "([a-zA-Z0-9_-]+(?::[a-zA-Z0-9_-]+)*)";
+
+ /**
+ * Precompiled regex pattern for validating the secret manager type and
type-specific identifier.
+ */
+ private static final Pattern SECRET_MANAGER_TYPE_PATTERN =
+ Pattern.compile("^" + SECRET_MANAGER_TYPE_REGEX + "$");
+
+ private static final Pattern TYPE_SPECIFIC_IDENTIFIER_PATTERN =
+ Pattern.compile("^" + TYPE_SPECIFIC_IDENTIFIER_REGEX + "$");
+
+ /**
+ * Precompiled regex pattern for validating and parsing UserSecretReference
URNs. Expected format:
+ *
urn:polaris-secret:<secret-manager-type>:<identifier1>(:<identifier2>:...).
+ *
+ * <p>Groups:
+ *
+ * <p>Group 1: secret-manager-type (alphanumeric, hyphens, underscores).
+ *
+ * <p>Group 2: type-specific-identifier (one or more colon-separated
alphanumeric components).
+ */
+ private static final Pattern URN_PATTERN =
+ Pattern.compile(
+ "^"
+ + URN_SCHEME
+ + ":"
+ + URN_NAMESPACE
+ + ":"
+ + SECRET_MANAGER_TYPE_REGEX
+ + ":"
+ + TYPE_SPECIFIC_IDENTIFIER_REGEX
+ + "$");
+
/**
* @param urn A string which should be self-sufficient to retrieve whatever
secret material that
* is stored in the remote secret store and also to identify an
implementation of the
@@ -70,16 +109,60 @@ public class UserSecretReference {
public UserSecretReference(
@JsonProperty(value = "urn", required = true) @Nonnull String urn,
@JsonProperty(value = "referencePayload") @Nullable Map<String, String>
referencePayload) {
- // TODO: Add better/standardized parsing and validation of URN syntax
Preconditions.checkArgument(
- urn.startsWith("urn:polaris-secret:") && urn.split(":").length >= 4,
- "Invalid secret URN '%s'; must be of the form "
- +
"'urn:polaris-secret:<secret-manager-type>:<type-specific-identifier>'",
- urn);
+ urnIsValid(urn),
+ "Invalid secret URN: " + urn + "; must be of the form: " +
URN_PATTERN.toString());
this.urn = urn;
this.referencePayload = Objects.requireNonNullElse(referencePayload, new
HashMap<>());
}
+ /**
+ * Validates whether the given URN string matches the expected format for
UserSecretReference
+ * URNs.
+ *
+ * @param urn The URN string to validate.
+ * @return true if the URN is valid, false otherwise.
+ */
+ private static boolean urnIsValid(@Nonnull String urn) {
+ return urn.trim().isEmpty() ? false : URN_PATTERN.matcher(urn).matches();
+ }
+
+ /**
+ * Builds a URN string from the given secret manager type and type-specific
identifier. Validates
+ * the inputs to ensure they conform to the expected pattern.
+ *
+ * @param secretManagerType The secret manager type (alphanumeric, hyphens,
underscores).
+ * @param typeSpecificIdentifier The type-specific identifier
(colon-separated alphanumeric
+ * components).
+ * @return The constructed URN string.
+ */
+ @Nonnull
+ public static String buildUrnString(
+ @Nonnull String secretManagerType, @Nonnull String
typeSpecificIdentifier) {
+
+ Preconditions.checkArgument(
+ !secretManagerType.trim().isEmpty(), "Secret manager type cannot be
empty");
+ Preconditions.checkArgument(
+ SECRET_MANAGER_TYPE_PATTERN.matcher(secretManagerType).matches(),
+ "Invalid secret manager type '%s'; must contain only alphanumeric
characters, hyphens, and underscores",
+ secretManagerType);
+
+ Preconditions.checkArgument(
+ !typeSpecificIdentifier.trim().isEmpty(), "Type-specific identifier
cannot be empty");
+ Preconditions.checkArgument(
+
TYPE_SPECIFIC_IDENTIFIER_PATTERN.matcher(typeSpecificIdentifier).matches(),
+ "Invalid type-specific identifier '%s'; must be colon-separated
alphanumeric components (hyphens and underscores allowed)",
+ typeSpecificIdentifier);
+
+ return URN_SCHEME
+ + ":"
+ + URN_NAMESPACE
+ + ":"
+ + secretManagerType
+ + ":"
+ + typeSpecificIdentifier;
+ }
+
/**
* Since UserSecretReference objects are specific to UserSecretManager
implementations, the
* "secret-manager-type" portion of the URN should be used to validate that
a URN is valid for a
@@ -87,9 +170,22 @@ public class UserSecretReference {
* concurrent implementations are possible in a given runtime environment.
*/
@JsonIgnore
- public String getUserSecretManagerTypeFromUrn() {
- // TODO: Add better/standardized parsing and validation of URN syntax
- return urn.split(":")[2];
+ public String getUserSecretManagerType() {
+ Matcher matcher = URN_PATTERN.matcher(urn);
+ Preconditions.checkState(matcher.matches(), "Invalid secret URN: " + urn);
+ return matcher.group(1);
+ }
+
+ /**
+ * Returns the type-specific identifier from the URN. Since the format is
specific to the
+ * UserSecretManager implementation, this method does not validate the
identifier. It is the
+ * responsibility of the caller to validate it.
+ */
+ @JsonIgnore
+ public String getTypeSpecificIdentifier() {
+ Matcher matcher = URN_PATTERN.matcher(urn);
+ Preconditions.checkState(matcher.matches(), "Invalid secret URN: " + urn);
+ return matcher.group(2);
}
public @Nonnull String getUrn() {
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretsManager.java
b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretsManager.java
index b1418efc9..194223053 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretsManager.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UserSecretsManager.java
@@ -63,4 +63,17 @@ public interface UserSecretsManager {
* @param secretReference Reference object for retrieving the original secret
*/
void deleteSecret(@Nonnull UserSecretReference secretReference);
+
+ /**
+ * Builds a URN string from the given secret manager type and type-specific
identifier.
+ *
+ * @param typeSpecificIdentifier The type-specific identifier
(colon-separated alphanumeric
+ * components with underscores and hyphens).
+ * @return The constructed URN string.
+ */
+ @Nonnull
+ default String buildUrn(
+ @Nonnull String secretManagerType, @Nonnull String
typeSpecificIdentifier) {
+ return UserSecretReference.buildUrnString(secretManagerType,
typeSpecificIdentifier);
+ }
}
diff --git
a/polaris-core/src/test/java/org/apache/polaris/core/secrets/UserSecretReferenceTest.java
b/polaris-core/src/test/java/org/apache/polaris/core/secrets/UserSecretReferenceTest.java
new file mode 100644
index 000000000..e05bd947b
--- /dev/null
+++
b/polaris-core/src/test/java/org/apache/polaris/core/secrets/UserSecretReferenceTest.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.core.secrets;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class UserSecretReferenceTest {
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "urn:polaris-secret:unsafe-in-memory:key1",
+ "urn:polaris-secret:unsafe-in-memory:key1:value1",
+ "urn:polaris-secret:aws_secrets-manager:my-key_123",
+ "urn:polaris-secret:vault:project:env:service:key"
+ })
+ public void testValidUrns(String validUrn) {
+ assertThat(new UserSecretReference(validUrn, null)).isNotNull();
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "",
+ " ",
+ "not-a-urn",
+ "urn:",
+ "urn:polaris-secret:",
+ "urn:polaris-secret:type:",
+ "wrong:polaris-secret:type:key:",
+ "urn:polaris-secret:type with spaces:key",
+ "urn:polaris-secret:type@invalid:key",
+ "urn:polaris-secret:unsafe-in-memory:key::"
+ })
+ public void testInvalidUrns(String invalidUrn) {
+ assertThatThrownBy(() -> new UserSecretReference(invalidUrn, null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Invalid secret URN: " + invalidUrn);
+ }
+
+ @Test
+ public void tesGetUrnComponents() {
+ String urn = "urn:polaris-secret:unsafe-in-memory:key1:value1";
+ UserSecretReference reference = new UserSecretReference(urn, null);
+
+
assertThat(reference.getUserSecretManagerType()).isEqualTo("unsafe-in-memory");
+ assertThat(reference.getTypeSpecificIdentifier()).isEqualTo("key1:value1");
+ }
+
+ @Test
+ public void testBuildUrn() {
+ String urn = UserSecretReference.buildUrnString("aws-secrets", "my-key");
+ assertThat(urn).isEqualTo("urn:polaris-secret:aws-secrets:my-key");
+
+ String urnWithMultipleIdentifiers =
+ UserSecretReference.buildUrnString("vault", "project:service");
+
assertThat(urnWithMultipleIdentifiers).isEqualTo("urn:polaris-secret:vault:project:service");
+
+ String urnWithNumbers = UserSecretReference.buildUrnString("type_123",
"456:789");
+
assertThat(urnWithNumbers).isEqualTo("urn:polaris-secret:type_123:456:789");
+ }
+}
