Merge pull request #324 from square/jessep/remove-content-field-in-se…

…cret-response-v2 Remove content field from SecretDetailResponseV2
square · Apr 10, 2017 · c9fc7db · c9fc7db
2 parents e64a0f6 + 7980c91
commit c9fc7db
Show file tree

Hide file tree

Showing 4 changed files with 2 additions and 97 deletions.
diff --git a/api/src/main/java/keywhiz/api/automation/v2/SecretDetailResponseV2.java b/api/src/main/java/keywhiz/api/automation/v2/SecretDetailResponseV2.java
@@ -5,24 +5,18 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.primitives.UnsignedLong;
-import java.util.Base64;
 import java.util.Map;
 import javax.annotation.Nullable;
 import keywhiz.api.model.SanitizedSecret;
-import keywhiz.api.model.Secret;
-import keywhiz.api.model.SecretSeries;
 import keywhiz.api.model.SecretSeriesAndContent;
 
 import static com.google.common.base.Strings.nullToEmpty;
-import static keywhiz.api.model.Secret.decodedLength;
 
 @AutoValue public abstract class SecretDetailResponseV2 {
   SecretDetailResponseV2() {} // prevent sub-classing
 
   public static Builder builder() {
     return new AutoValue_SecretDetailResponseV2.Builder()
-        .content("")
         .checksum("")
         .description("")
         .type(null)
@@ -31,15 +25,11 @@ public static Builder builder() {
   }
 
   @AutoValue.Builder public abstract static class Builder {
-    // intended to be package-private
-    abstract String content();
-    abstract Builder size(UnsignedLong size);
     public abstract Builder metadata(ImmutableMap<String, String> metadata);
     abstract SecretDetailResponseV2 autoBuild();
 
     public abstract Builder name(String name);
     public abstract Builder description(String description);
-    public abstract Builder content(String secret);
     public abstract Builder checksum(String checksum);
     public abstract Builder createdAtSeconds(long createdAt);
     public abstract Builder createdBy(String person);
@@ -53,20 +43,6 @@ public Builder metadata(Map<String, String> metadata) {
       return metadata(ImmutableMap.copyOf(metadata));
     }
 
-    public Builder series(SecretSeries series) {
-      return this
-          .name(series.name())
-          .description(series.description())
-          .createdAtSeconds(series.createdAt().toEpochSecond())
-          .createdBy(series.createdBy())
-          .updatedAtSeconds(series.updatedAt().toEpochSecond())
-          .updatedBy(series.updatedBy())
-          .type(series.type().orElse(null))
-          .expiry(0)
-          .version(series.currentVersion().orElse(null));
-    }
-
-    // Does not save secret contents, but saves HMAC
     public Builder seriesAndContent(SecretSeriesAndContent seriesAndContent) {
       return this
           .name(seriesAndContent.series().name())
@@ -82,22 +58,6 @@ public Builder seriesAndContent(SecretSeriesAndContent seriesAndContent) {
           .version(seriesAndContent.series().currentVersion().orElse(null));
     }
 
-    public Builder secret(Secret secret) {
-      return this
-          .name(secret.getName())
-          .description(secret.getDescription())
-          .content(secret.getSecret())
-          .checksum(secret.getChecksum())
-          .createdAtSeconds(secret.getCreatedAt().toEpochSecond())
-          .createdBy(secret.getCreatedBy())
-          .updatedAtSeconds(secret.getUpdatedAt().toEpochSecond())
-          .updatedBy(secret.getUpdatedBy())
-          .metadata(secret.getMetadata())
-          .type(secret.getType().orElse(null))
-          .expiry(secret.getExpiry())
-          .version(secret.getVersion().orElse(null));
-    }
-
     public Builder sanitizedSecret(SanitizedSecret sanitizedSecret) {
       return this
           .name(sanitizedSecret.name())
@@ -114,11 +74,7 @@ public Builder sanitizedSecret(SanitizedSecret sanitizedSecret) {
     }
 
     public SecretDetailResponseV2 build() {
-      // throws IllegalArgumentException if content not base64
-      Base64.getDecoder().decode(content());
-      return this
-          .size(UnsignedLong.valueOf(decodedLength(content())))
-          .autoBuild();
+      return this.autoBuild();
     }
   }
 
@@ -129,9 +85,7 @@ public SecretDetailResponseV2 build() {
   @JsonCreator public static SecretDetailResponseV2 fromParts(
       @JsonProperty("name") String name,
       @JsonProperty("description") @Nullable String description,
-      @JsonProperty("content") String content,
       @JsonProperty("checksum") String checksum,
-      @JsonProperty("size") UnsignedLong size,
       @JsonProperty("createdAtSeconds") long createdAtSeconds,
       @JsonProperty("createdBy") String createdBy,
       @JsonProperty("updatedAtSeconds") long updatedAtSeconds,
@@ -143,9 +97,7 @@ public SecretDetailResponseV2 build() {
     return builder()
         .name(name)
         .description(nullToEmpty(description))
-        .content(content)
         .checksum(checksum)
-        .size(size)
         .createdAtSeconds(createdAtSeconds)
         .createdBy(createdBy)
         .updatedAtSeconds(updatedAtSeconds)
@@ -160,9 +112,7 @@ public SecretDetailResponseV2 build() {
   // TODO: Consider Optional values in place of Nullable.
   @JsonProperty("name") public abstract String name();
   @JsonProperty("description") public abstract String description();
-  @JsonProperty("content") public abstract String content();
   @JsonProperty("checksum") public abstract String checksum();
-  @JsonProperty("size") public abstract UnsignedLong size();
   @JsonProperty("createdAtSeconds") public abstract long createdAtSeconds();
   @JsonProperty("createdBy") public abstract String createdBy();
   @JsonProperty("updatedAtSeconds") public abstract long updatedAtSeconds();
@@ -176,9 +126,7 @@ public SecretDetailResponseV2 build() {
     return MoreObjects.toStringHelper(this)
         .add("name", name())
         .add("description", description())
-        .add("content", "[REDACTED]")
         .add("checksum", checksum())
-        .add("size", size())
         .add("createdAtSeconds", createdAtSeconds())
         .add("createdBy", createdBy())
         .add("updatedAtSeconds", updatedAtSeconds())

diff --git a/api/src/test/java/keywhiz/api/automation/v2/SecretDetailResponseV2Test.java b/api/src/test/java/keywhiz/api/automation/v2/SecretDetailResponseV2Test.java
@@ -36,7 +36,6 @@ public class SecretDetailResponseV2Test {
         .name("secret-name")
         .version(1L)
         .description("secret-description")
-        .content("YXNkZGFz")
         .checksum("checksum")
         .createdAtSeconds(OffsetDateTime.parse("2013-03-28T21:23:04.159Z").toEpochSecond())
         .createdBy("creator-user")
@@ -51,23 +50,6 @@ public class SecretDetailResponseV2Test {
         .isEqualTo(jsonFixture("fixtures/v2/secretDetailResponse.json"));
   }
 
-  @Test public void formsCorrectlyFromSecretSeries() throws Exception {
-    SecretSeries series = SecretSeries.of(1, "secret-name", "secret-description",
-        ApiDate.parse("2013-03-28T21:23:04.159Z"), "creator-user",
-        ApiDate.parse("2014-03-28T21:23:04.159Z"), "updater-user", "text/plain", null,
-        1L);
-    SecretDetailResponseV2 secretDetailResponse = SecretDetailResponseV2.builder()
-        .series(series)
-        .content("YXNkZGFz")
-        .checksum("checksum")
-        .metadata(ImmutableMap.of("owner", "root"))
-        .expiry(1136214245)
-        .build();
-
-    assertThat(asJson(secretDetailResponse))
-        .isEqualTo(jsonFixture("fixtures/v2/secretDetailResponse.json"));
-  }
-
   @Test public void formsCorrectlyFromSecretSeriesAndContent() throws Exception {
     SecretSeries series = SecretSeries.of(1, "secret-name", "secret-description",
         ApiDate.parse("2013-03-28T21:23:04.159Z"), "creator-user",
@@ -80,37 +62,20 @@ public class SecretDetailResponseV2Test {
     SecretSeriesAndContent seriesAndContent = SecretSeriesAndContent.of(series, content);
     SecretDetailResponseV2 secretDetailResponse = SecretDetailResponseV2.builder()
         .seriesAndContent(seriesAndContent)
-        .content("YXNkZGFz")
-        .build();
-
-    assertThat(asJson(secretDetailResponse))
-        .isEqualTo(jsonFixture("fixtures/v2/secretDetailResponse.json"));
-  }
-
-  @Test public void formsCorrectlyFromSecret() throws Exception {
-    Secret secret = new Secret(1, "secret-name", "secret-description", () -> "YXNkZGFz", "checksum",
-        ApiDate.parse("2013-03-28T21:23:04.159Z"), "creator-user",
-        ApiDate.parse("2014-03-28T21:23:04.159Z"), "updater-user",
-        ImmutableMap.of("owner", "root"), "text/plain", null,
-        1136214245, 1L);
-    SecretDetailResponseV2 secretDetailResponse = SecretDetailResponseV2.builder()
-        .secret(secret)
         .build();
 
     assertThat(asJson(secretDetailResponse))
         .isEqualTo(jsonFixture("fixtures/v2/secretDetailResponse.json"));
   }
 
   @Test public void formsCorrectlyFromSanitizedSecret() throws Exception {
-    Secret secret = new Secret(1, "secret-name", "secret-description", () -> "YXNkZGFz", "checksum",
+    SanitizedSecret sanitizedSecret = SanitizedSecret.of(1, "secret-name", "secret-description", "checksum",
         ApiDate.parse("2013-03-28T21:23:04.159Z"), "creator-user",
         ApiDate.parse("2014-03-28T21:23:04.159Z"), "updater-user",
         ImmutableMap.of("owner", "root"), "text/plain", null,
         1136214245, 1L);
-    SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
     SecretDetailResponseV2 secretDetailResponse = SecretDetailResponseV2.builder()
         .sanitizedSecret(sanitizedSecret)
-        .content("YXNkZGFz")
         .build();
 
     assertThat(asJson(secretDetailResponse))

diff --git a/api/src/test/resources/fixtures/v2/secretDetailResponse.json b/api/src/test/resources/fixtures/v2/secretDetailResponse.json
@@ -1,9 +1,7 @@
 {
   "name" : "secret-name",
   "description" : "secret-description",
-  "content": "YXNkZGFz",
   "checksum": "checksum",
-  "size":6,
   "createdAtSeconds": 1364505784,
   "createdBy": "creator-user",
   "updatedAtSeconds": 1396041784,

diff --git a/server/src/test/java/keywhiz/service/resources/automation/v2/SecretResourceTest.java b/server/src/test/java/keywhiz/service/resources/automation/v2/SecretResourceTest.java
@@ -9,7 +9,6 @@
 import java.io.IOException;
 import java.net.URI;
 import java.util.Base64;
-import java.util.Base64.Decoder;
 import java.util.Base64.Encoder;
 import java.util.List;
 import java.util.Optional;
@@ -49,7 +48,6 @@ public class SecretResourceTest {
   private static final ObjectMapper mapper =
       KeywhizService.customizeObjectMapper(Jackson.newObjectMapper());
   private static final Encoder encoder = Base64.getEncoder();
-  private static final Decoder decoder = Base64.getDecoder();
 
   OkHttpClient mutualSslClient;
 
@@ -200,10 +198,6 @@ public void partialUpdateSecret_notFound() throws Exception {
     assertThat(response.description()).isEqualTo("desc");
     assertThat(response.type()).isEqualTo("password");
     assertThat(response.metadata()).isEqualTo(ImmutableMap.of("owner", "root", "mode", "0440"));
-
-    // These values are left out for a series lookup as they pertain to a specific secret.
-    assertThat(response.content()).isEmpty();
-    assertThat(response.size().longValue()).isZero();
   }
 
   //---------------------------------------------------------------------------------------