fix: Orphaned encrypted_value when Credentials are deleted

backends/credhub/src/test/java/org/cloudfoundry/credhub/handlers/DefaultCertificatesHandlerIntegrationTest.java

@@ -11,6 +11,7 @@
 import org.springframework.jdbc.support.MetaDataAccessException;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.transaction.annotation.Transactional;
 
 import org.cloudfoundry.credhub.CredhubTestApp;
 import org.cloudfoundry.credhub.certificates.DefaultCertificatesHandler;
@@ -28,6 +29,7 @@
 @RunWith(SpringRunner.class)
 @ActiveProfiles(value = "unit-test", resolver = DatabaseProfileResolver.class)
 @SpringBootTest(classes = CredhubTestApp.class)
+@Transactional
 public class DefaultCertificatesHandlerIntegrationTest {
     @Autowired
     private DefaultCertificatesHandler defaultCertificatesHandler;
@@ -77,7 +79,7 @@ private void insertTestCredentialsIntoPostgres(int count) {
 
         jdbcTemplate.update(
                 "INSERT INTO credential_version (type, uuid, version_created_at, credential_uuid) " +
-                        "SELECT 'foo', uuid, 0, uuid from credential");
+                        "SELECT 'cert', uuid, 0, uuid from credential");
 
         jdbcTemplate.update(
                 "INSERT INTO certificate_credential (uuid, transitional) " +

backends/credhub/src/test/java/org/cloudfoundry/credhub/integration/CertificateVersionDeleteTest.java

@@ -1,5 +1,7 @@
 package org.cloudfoundry.credhub.integration;
 
+import java.util.UUID;
+
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.ActiveProfiles;
@@ -13,6 +15,7 @@
 
 import com.jayway.jsonpath.JsonPath;
 import org.cloudfoundry.credhub.CredhubTestApp;
+import org.cloudfoundry.credhub.data.EncryptedValueDataService;
 import org.cloudfoundry.credhub.helpers.RequestHelper;
 import org.cloudfoundry.credhub.utils.BouncyCastleFipsConfigurer;
 import org.cloudfoundry.credhub.utils.DatabaseProfileResolver;
@@ -51,6 +54,9 @@ public class CertificateVersionDeleteTest {
   @Autowired
   private WebApplicationContext webApplicationContext;
 
+  @Autowired
+  EncryptedValueDataService encryptedValueDataService;
+
   private MockMvc mockMvc;
 
   @Rule
@@ -71,6 +77,9 @@ public void beforeEach() throws Exception {
 
   @Test
   public void deleteCertificateVersion_whenThereAreOtherVersionsOfTheCertificate_deletesTheSpecifiedVersion() throws Exception {
+    UUID aUuid = UUID.randomUUID();
+    var nEncryptedValuesPre = encryptedValueDataService.countAllByCanaryUuid(aUuid);
+
     final String credentialName = "/test-certificate";
 
     String response = generateCertificateCredential(mockMvc, credentialName, true, "test", null, ALL_PERMISSIONS_TOKEN);
@@ -81,13 +90,14 @@ public void deleteCertificateVersion_whenThereAreOtherVersionsOfTheCertificate_d
       .read("$.certificates[0].id");
 
     final String version = RequestHelper.regenerateCertificate(mockMvc, uuid, false, ALL_PERMISSIONS_TOKEN);
+    assertThat("One associated encrypted value exist for each certificate vesion",
+            encryptedValueDataService.countAllByCanaryUuid(aUuid), equalTo(nEncryptedValuesPre + 2));
+
     final String versionUuid = JsonPath.parse(version).read("$.id");
     final String versionValue = JsonPath.parse(version).read("$.value.certificate");
-
     final MockHttpServletRequestBuilder request = delete("/api/v1/certificates/" + uuid + "/versions/" + versionUuid)
       .header("Authorization", "Bearer " + ALL_PERMISSIONS_TOKEN)
       .accept(APPLICATION_JSON);
-
     response = mockMvc.perform(request)
       .andExpect(status().isOk())
       .andReturn().getResponse().getContentAsString();
@@ -103,6 +113,8 @@ public void deleteCertificateVersion_whenThereAreOtherVersionsOfTheCertificate_d
     final JSONArray jsonArray = new JSONArray(response);
     assertThat(jsonArray.length(), equalTo(1));
     assertThat(JsonPath.parse(jsonArray.get(0).toString()).read("$.value.certificate"), equalTo(nonDeletedVersion));
+    assertThat("Associated encrypted value is deleted when the certificate version is deleted",
+            encryptedValueDataService.countAllByCanaryUuid(aUuid), equalTo(nEncryptedValuesPre + 1));
   }
 
   @Test

components/credentials/src/main/kotlin/org/cloudfoundry/credhub/entity/Credential.kt

@@ -5,10 +5,13 @@ import org.cloudfoundry.credhub.audit.AuditableCredential
 import org.cloudfoundry.credhub.constants.UuidConstants.Companion.UUID_BYTES
 import org.hibernate.annotations.GenericGenerator
 import java.util.UUID
+import javax.persistence.CascadeType
 import javax.persistence.Column
 import javax.persistence.Entity
+import javax.persistence.FetchType
 import javax.persistence.GeneratedValue
 import javax.persistence.Id
+import javax.persistence.OneToMany
 import javax.persistence.Table
 
 @Entity
@@ -21,6 +24,13 @@ class Credential : AuditableCredential {
     @GenericGenerator(name = "uuid2", strategy = "uuid2")
     override var uuid: UUID? = null
 
+    @OneToMany(
+        cascade = [CascadeType.REMOVE],
+        mappedBy = "credential", fetch = FetchType.LAZY
+    )
+    var credentialVersions: MutableList<CredentialVersionData<*>> =
+        mutableListOf()
+
     @Column(nullable = false)
     override var name: String? = null
         set(name) {

components/credentials/src/main/kotlin/org/cloudfoundry/credhub/entity/CredentialVersionData.kt

@@ -43,21 +43,21 @@ abstract class CredentialVersionData<Z : CredentialVersionData<Z>>(credential: C
     @Column(length = UuidConstants.UUID_BYTES, columnDefinition = "VARBINARY")
     @GeneratedValue(generator = "uuid2")
     @GenericGenerator(name = "uuid2", strategy = "uuid2")
-    var uuid: UUID? = null
+    open var uuid: UUID? = null
 
     @OneToOne(cascade = [CascadeType.ALL])
     @NotFound(action = NotFoundAction.IGNORE)
     @JoinColumn(name = "encrypted_value_uuid")
-    var encryptedCredentialValue: EncryptedValue? = null
+    open var encryptedCredentialValue: EncryptedValue? = null
 
     @Convert(converter = InstantMillisecondsConverter::class)
     @Column(nullable = false, columnDefinition = "BIGINT NOT NULL")
     @CreatedDate
-    lateinit var versionCreatedAt: Instant
+    open lateinit var versionCreatedAt: Instant
 
     @ManyToOne
     @JoinColumn(name = "credential_uuid", nullable = false)
-    var credential: Credential? = credential
+    open var credential: Credential? = credential
 
     // this is mapped with updatable and insertable false since it's managed by the DiscriminatorColumn annotation
     // surfacing property here lets us use it in JPA queries
@@ -66,7 +66,7 @@ abstract class CredentialVersionData<Z : CredentialVersionData<Z>>(credential: C
 
     @Convert(converter = JsonNodeConverter::class)
     @Column(name = "metadata")
-    var metadata: JsonNode? = null
+    open var metadata: JsonNode? = null
 
     val nonce: ByteArray?
         get() = if (encryptedCredentialValue != null) this.encryptedCredentialValue!!.nonce else null

components/credentials/src/test/java/org/cloudfoundry/credhub/services/DefaultCredentialVersionDataServiceTest.java

@@ -7,10 +7,12 @@
 import java.util.function.Consumer;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.mock.mockito.MockBean;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
 
 import com.fasterxml.jackson.databind.JsonNode;
@@ -31,11 +33,13 @@
 import org.cloudfoundry.credhub.entity.CredentialVersionData;
 import org.cloudfoundry.credhub.entity.PasswordCredentialVersionData;
 import org.cloudfoundry.credhub.entity.SshCredentialVersionData;
+import org.cloudfoundry.credhub.entity.UserCredentialVersionData;
 import org.cloudfoundry.credhub.entity.ValueCredentialVersionData;
 import org.cloudfoundry.credhub.exceptions.MaximumSizeException;
 import org.cloudfoundry.credhub.exceptions.ParameterizedValidationException;
 import org.cloudfoundry.credhub.repositories.CredentialRepository;
 import org.cloudfoundry.credhub.repositories.CredentialVersionRepository;
+import org.cloudfoundry.credhub.repositories.EncryptedValueRepository;
 import org.cloudfoundry.credhub.util.CurrentTimeProvider;
 import org.cloudfoundry.credhub.utils.DatabaseProfileResolver;
 import org.cloudfoundry.credhub.utils.DatabaseUtilities;
@@ -60,6 +64,7 @@
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.core.IsCollectionContaining.hasItem;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
@@ -70,12 +75,18 @@
 @Transactional
 public class DefaultCredentialVersionDataServiceTest {
 
+  @Value("${spring.profiles.active}")
+  private String activeSpringProfile;
+
   @Autowired
   private CredentialVersionRepository credentialVersionRepository;
 
   @Autowired
   private CredentialRepository credentialRepository;
 
+  @Autowired
+  private EncryptedValueRepository encryptedValueRepository;
+
   @Autowired
   private EncryptionKeyCanaryDataService encryptionKeyCanaryDataService;
 
@@ -271,7 +282,9 @@ public void delete_onAnExistingCredential_returnsTrue() {
   }
 
   @Test
+  @Transactional(propagation = Propagation.NEVER)
   public void delete_onACredentialName_deletesAllCredentialsWithTheName() {
+    long nEncryptedValuesPre = encryptedValueRepository.count();
     final Credential credential = credentialDataService
       .save(new Credential("/my-credential"));
 
@@ -301,10 +314,14 @@ public void delete_onACredentialName_deletesAllCredentialsWithTheName() {
 
     assertThat(subject.findAllByName("/my-credential"), hasSize(0));
     assertNull(credentialDataService.find("/my-credential"));
+    assertEquals("Associated encryptedValues are deleted when password credential is deleted",
+            nEncryptedValuesPre, encryptedValueRepository.count());
   }
 
   @Test
+  @Transactional(propagation = Propagation.NEVER)
   public void delete_givenACredentialNameCasedDifferentlyFromTheActual_shouldBeCaseInsensitive() {
+    long nEncryptedValuesPre = encryptedValueRepository.count();
     final Credential credentialName = credentialDataService
       .save(new Credential("/my-credential"));
 
@@ -334,6 +351,47 @@ public void delete_givenACredentialNameCasedDifferentlyFromTheActual_shouldBeCas
     subject.delete("/MY-CREDENTIAL");
 
     assertThat(subject.findAllByName("/my-credential"), empty());
+    assertEquals("Associated encryptedValues are deleted when password credential is deleted",
+            nEncryptedValuesPre, encryptedValueRepository.count());
+  }
+
+  @Test
+  @Transactional(propagation = Propagation.NEVER)
+  public void delete_UserTypeCredential() {
+    long nEncryptedValuesPre = encryptedValueRepository.count();
+    final Credential credentialName = credentialDataService.save(
+            new Credential("/my-credential"));
+
+    final EncryptedValue encryptedValueA = new EncryptedValue();
+    encryptedValueA.setEncryptionKeyUuid(activeCanaryUuid);
+    encryptedValueA.setEncryptedValue("credential-password".getBytes(UTF_8));
+    encryptedValueA.setNonce(new byte[]{});
+
+    final UserCredentialVersionData credentialDataA =
+            new UserCredentialVersionData("test-user");
+    credentialDataA.setCredential(credentialName);
+    credentialDataA.setEncryptedValueData(encryptedValueA);
+    credentialDataA.setSalt("salt");
+    subject.save(credentialDataA);
+
+    final EncryptedValue encryptedValueB = new EncryptedValue();
+    encryptedValueB.setEncryptionKeyUuid(activeCanaryUuid);
+    encryptedValueB.setEncryptedValue("another password".getBytes(UTF_8));
+    encryptedValueB.setNonce(new byte[]{});
+
+    final UserCredentialVersionData credentialDataB = new UserCredentialVersionData(
+            "/my-credential");
+    credentialDataB.setCredential(credentialName);
+    credentialDataB.setEncryptedValueData(encryptedValueB);
+    credentialDataB.setSalt("salt");
+    subject.save(credentialDataB);
+
+    assertThat(subject.findAllByName("/my-credential"), hasSize(2));
+
+    subject.delete("/my-credential");
+    assertThat(subject.findAllByName("/my-credential"), empty());
+    assertEquals("Associated encryptedValues are deleted when user credential is deleted",
+            nEncryptedValuesPre, encryptedValueRepository.count());
   }
 
   @Test