Fix bug when fetching secrets by name.

The code was returning a random (oldest in practice) secret when multiple secrets existed. Made sure test fails without the fix.
square · Jun 21, 2016 · 7b240ee · 7b240ee
1 parent b0a3a1f
commit 7b240ee
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 8 deletions.
diff --git a/server/src/main/java/keywhiz/service/daos/SecretDAO.java b/server/src/main/java/keywhiz/service/daos/SecretDAO.java
@@ -158,14 +158,15 @@ public Optional<SecretSeriesAndContent> getSecretByNameOne(String name) {
     SecretContentDAO secretContentDAO = secretContentDAOFactory.using(dslContext.configuration());
     SecretSeriesDAO secretSeriesDAO = secretSeriesDAOFactory.using(dslContext.configuration());
 
-    Optional<SecretSeries> secretSeries = secretSeriesDAO.getSecretSeriesByName(name);
+    Optional<SecretSeries> series = secretSeriesDAO.getSecretSeriesByName(name);
-    if (secretSeries.isPresent() && secretSeries.get().currentVersion().isPresent()) {
+    if (series.isPresent() && series.get().currentVersion().isPresent()) {
-      Optional<SecretContent> secretContent = secretContentDAO.getSecretContentBySecretIdOne(secretSeries.get().id());
+      long secretContentId = series.get().currentVersion().get();
+      Optional<SecretContent> secretContent = secretContentDAO.getSecretContentById(secretContentId);
       if (!secretContent.isPresent()) {
         return Optional.empty();
       }
 
-      return Optional.of(SecretSeriesAndContent.of(secretSeries.get(), secretContent.get()));
+      return Optional.of(SecretSeriesAndContent.of(series.get(), secretContent.get()));
     }
     return Optional.empty();
   }

diff --git a/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java b/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java
@@ -57,7 +57,7 @@ public class SecretDAOTest {
   SecretContent content1 = SecretContent.of(101, 1, encryptedContent, date, "creator", date, "updater", emptyMetadata);
   SecretSeriesAndContent secret1 = SecretSeriesAndContent.of(series1, content1);
 
-  SecretSeries series2 = SecretSeries.of(2, "secret2", "desc2", date, "creator", date, "updater", null, null, 102L);
+  SecretSeries series2 = SecretSeries.of(2, "secret2", "desc2", date, "creator", date, "updater", null, null, 103L);
   SecretContent content2a = SecretContent.of(102, 2, encryptedContent, date, "creator", date, "updater", emptyMetadata);
   SecretSeriesAndContent secret2a = SecretSeriesAndContent.of(series2, content2a);
 
@@ -251,8 +251,8 @@ public void createSecretFailsIfSecretExists() {
   //---------------------------------------------------------------------------------------
 
   @Test public void getSecretByNameOne() {
-    String name = secret1.series().name();
+    String name = secret2b.series().name();
-    assertThat(secretDAO.getSecretByNameOne(name)).contains(secret1);
+    assertThat(secretDAO.getSecretByNameOne(name)).contains(secret2b);
   }
 
   @Test public void getSecretByNameOneReturnsEmptyWhenCurrentVersionIsNull() {
@@ -282,7 +282,7 @@ public void createSecretFailsIfSecretExists() {
   }
 
   @Test public void getSecretByIdOne() {
-    assertThat(secretDAO.getSecretByIdOne(series2.id())).isEqualTo(Optional.of(secret2a));
+    assertThat(secretDAO.getSecretByIdOne(series2.id())).isEqualTo(Optional.of(secret2b));
   }
 
   @Test public void getSecretByIdOneReturnsEmptyWhenCurrentVersionIsNull() {