Don't allow slashes in secret names

square · Jul 31, 2023 · 69b0069 · 69b0069
1 parent 1e94a68
commit 69b0069
Show file tree

Hide file tree

Showing 5 changed files with 5 additions and 41 deletions.
diff --git a/cli/src/main/java/keywhiz/cli/Utilities.java b/cli/src/main/java/keywhiz/cli/Utilities.java
@@ -17,7 +17,7 @@
 
 public class Utilities {
 
-  public static final String VALID_NAME_PATTERN = "^[a-zA-Z_0-9\\-.:/]+$";
+  public static final String VALID_NAME_PATTERN = "^[a-zA-Z_0-9\\-.:]+$";
 
   public static boolean validName(String name) {
     // "." is allowed at any position but the first.

diff --git a/cli/src/test/java/keywhiz/cli/commands/AddActionTest.java b/cli/src/test/java/keywhiz/cli/commands/AddActionTest.java
@@ -67,9 +67,6 @@ public class AddActionTest {
       new Secret(15, "newSecret", null, null, () -> "c2VjcmV0MQ==", "checksum", NOW, null, NOW, null,
           null, null, ImmutableMap.of(), 0, 1L, NOW, null);
 
-  Secret secretWithSpecialName =
-      new Secret(16, "sp:ns/owner/secret-name", null, null, () -> "c2VjcmV0MQ==", "checksum", NOW, null, NOW, null,
-          null, null, ImmutableMap.of(), 0, 1L, NOW, null);
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
   SecretDetailResponse secretDetailResponse = SecretDetailResponse.fromSecret(secret, null, null);
 
@@ -289,33 +286,4 @@ public void addValidatesSecretName() throws Exception {
 
     addAction.run();
   }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void addValidatesSecretNameNoDollarSign() throws Exception {
-    addActionConfig.addType = Arrays.asList("secret");
-    addActionConfig.name = "Invalid$Name";
-
-    addAction.run();
-  }
-
-  @Test
-  public void addValidatesSecretNameSpecialName() throws Exception {
-    addActionConfig.addType = Arrays.asList("secret");
-    addActionConfig.name = secretWithSpecialName.getDisplayName();
-    addActionConfig.expiry = "2006-01-02T15:04:05Z";
-
-    byte[] content = base64Decoder.decode(secretWithSpecialName.getSecret());
-    addAction.stream = new ByteArrayInputStream(content);
-    when(keywhizClient.getSanitizedSecretByName(secretWithSpecialName.getName()))
-        .thenThrow(new NotFoundException()); // Call checks for existence.
-
-    when(
-        keywhizClient.createSecret(secretWithSpecialName.getName(), null, "", content,
-            secretWithSpecialName.getMetadata(), 1136214245))
-        .thenReturn(secretDetailResponse);
-
-    addAction.run();
-    verify(keywhizClient, times(1)).createSecret(secretWithSpecialName.getName(), null, "", content,
-        secretWithSpecialName.getMetadata(), 1136214245);
-  }
 }
diff --git a/client/src/main/java/keywhiz/client/KeywhizClient.java b/client/src/main/java/keywhiz/client/KeywhizClient.java
@@ -210,11 +210,7 @@ public SecretDetailResponse createSecret(
             .metadata(metadata)
             .expiry(expiry)
             .build();
-    HttpUrl url = baseUrl.newBuilder()
-        .addPathSegment("admin")
-        .addPathSegment("secrets")
-        .build();
-    String response = httpPost(url, request);
+    String response = httpPost(baseUrl.resolve("/admin/secrets"), request);
     return mapper.readValue(response, SecretDetailResponse.class);
   }
 

diff --git a/server/src/test/java/keywhiz/KeywhizConfigTest.java b/server/src/test/java/keywhiz/KeywhizConfigTest.java
@@ -73,7 +73,7 @@ public void parseNewSecretOwnershipStrategyInfer() {
   }
 
   @Test
-  public void parseReservedPrefixes() {
+  public void handleReservedPrefixes() {
     KeywhizConfig config = loadConfig("with-reserved-prefixes.yaml");
     assertThat(config.canCreateSecretWithName("any-secret-name", "any-owner-name")).isTrue();
 

diff --git a/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java b/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java
@@ -281,7 +281,7 @@ public void createOrUpdateExistingSecretUpdatesOwner() {
 
   @Test public void createSecretWithReservedPrefix() {
     groupDAO.createGroup("specialOwner", "creator", "description", NO_METADATA);
-    String name = "sp:namespace/owner/secretName";
+    String name = "sp:namespace:owner:key_name";
     String content = "c2VjcmV0MQ==";
     String hmac = cryptographer.computeHmac(content.getBytes(UTF_8), "hmackey");
     String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content);
@@ -295,7 +295,7 @@ public void createOrUpdateExistingSecretUpdatesOwner() {
   public void createSecretFailsIfPrefixReservedByDifferentOwner() {
     groupDAO.createGroup("specialOwner", "creator", "description", NO_METADATA);
     groupDAO.createGroup("regularOwner", "creator", "description", NO_METADATA);
-    String name = "sp:namespace/owner/secretName";
+    String name = "sp:namespace:owner:key_name";
     String content = "c2VjcmV0MQ==";
     String hmac = cryptographer.computeHmac(content.getBytes(UTF_8), "hmackey");
     String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content);