Skip to content

Commit

Permalink
Add duration_used field to generate/regenerate API
Browse files Browse the repository at this point in the history
The new duration_used field contains the duration (either the requested
or minimum) that was actually used to generate or regenerate the
certificate.

Co-authored-by: Brian Upton <bupton@vmware.com>
Co-authored-by: Pablo Rodas <prodas@vmware.com>
Co-authored-by: Preethi Varambally <pvarambally@pivotal.io>
  • Loading branch information
3 people authored and bruce-ricard committed Sep 22, 2021
1 parent 43c5eaa commit a64374f
Show file tree
Hide file tree
Showing 14 changed files with 42 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ See https://github.com/pivotal-cf/credhub-release/blob/master/docs/ca-rotation.m

A minimum duration can be configured for leaf and CA certificates using the `certificates.leaf_minimum_duration_in_days` and `certificates.ca_minimum_duration_in_days` server-level configuration fields. When these fields are configured, if a request to generate or regenerate a certificate has a duration lower than the minimum, then the minimum duration is used instead.

The API response will include a `duration_overridden` field that is `true` when the minimum duration was used instead, or `false` if the requested duration was used.
The API response will include two fields:

* A `duration_overridden` field that is `true` when the minimum duration was used instead, or `false` if the requested duration was used.
* A `duration_used` field that is the duration (in days) used when the certificate was generated.

---

Expand All @@ -35,7 +38,7 @@ operation::POST__certificates_uuid_regenerate__returns_certificate[]
Note:

* If a certificate credential only has one version and it is marked as transitional the credential cannot be regenerated using this endpoint.
* If the duration used to generate the currently active version of the certificate is lower than the minimum duration, the regenerated certificate will use the minimum duration instead and the response will contain the `duration_overridden` flag set to true.
* If the duration used to generate the currently active version of the certificate is lower than the minimum duration, the regenerated certificate will use the minimum duration instead and the response will contain the `duration_overridden` flag set to true. The duration value used to regenerate the certificate is included in the `duration_used` field of the response.

---

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ operation::POST__generate_certificate_returns__certificate_credential[]

Notes:

* If the duration is overridden by the minimum duration, the response will contain the `duration_overridden` flag set to true.
* If the duration is overridden by the minimum duration, the response will contain the `duration_overridden` flag set to true. It will also include the actual duration used to generate the certificate in the `duration_used` field.
* When the mode is set to converge, certificates are no longer regenerated if the duration doesn't match the existing certificate's duration.

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public void certificateGeneration_shouldGenerateCorrectCertificate() throws Exce
assertThat(result.getBoolean("self_signed"), equalTo(true));
assertThat(result.getBoolean("generated"), equalTo(true));
assertThat(result.getBoolean("duration_overridden"), equalTo(false));
assertThat(result.getInt("duration_used"), equalTo(1));

assertThat(picardCert, notNullValue());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public void generatingCA_usesTheMinimumDuration() throws Exception {
.getContentAsString());

assertThat(generateResponse.read("$.duration_overridden").toString(), is("true"));
assertThat(generateResponse.read("$.duration_used"), is(1825));
Instant expiryDate = Instant.parse(generateResponse.read("$.expiry_date").toString());
assertThat(expiryDate, is(equalTo(mockCurrentTimeProvider.getInstant().plus(1825L, ChronoUnit.DAYS))));
}
Expand Down Expand Up @@ -157,9 +158,11 @@ public void regeneratingALeafCertificateThatAlreadyHasMinimumDuration_doesNotHav
List<Object> versions = getVersionsForCertificate(CREDENTIAL_NAME);

assertThat(generateResponse.read("$.duration_overridden"), is(true));
assertThat(generateResponse.read("$.duration_used"), is(1460));
assertThat(regeneratedExpiryDate, is(equalTo(mockCurrentTimeProvider.getInstant().plus(1460L, ChronoUnit.DAYS))));
assertThat(versions.size(), is(equalTo(2)));
assertThat(regenerateResponse.read("$.duration_overridden"), is(false));
assertThat(regenerateResponse.read("$.duration_used"), is(1460));
}

@Test
Expand Down Expand Up @@ -195,6 +198,7 @@ public void regeneratingAPreexistingLeafCertificateUsingCertificatesController_u

assertThat(regeneratedExpiryDate, is(equalTo(mockCurrentTimeProvider.getInstant().plus(1460L, ChronoUnit.DAYS))));
assertThat(regenerateResponse.read("$.duration_overridden"), is(equalTo(true)));
assertThat(regenerateResponse.read("$.duration_used"), is(equalTo(1460)));
assertThat(versions.size(), is(equalTo(2)));
}

Expand Down Expand Up @@ -223,6 +227,7 @@ public void regeneratingAPreexistingLeafCertificateUsingCredentialsController_us

assertThat(regeneratedExpiryDate, is(equalTo(mockCurrentTimeProvider.getInstant().plus(1460L, ChronoUnit.DAYS))));
assertThat(regenerateResponse.read("$.duration_overridden"), is(equalTo(true)));
assertThat(regenerateResponse.read("$.duration_used"), is(equalTo(1460)));
assertThat(versions.size(), is(equalTo(2)));
}

Expand All @@ -249,6 +254,7 @@ public void regeneratingAPreexistingLeafCertificateUsingRegenerateController_use

assertThat(regeneratedExpiryDate, is(equalTo(mockCurrentTimeProvider.getInstant().plus(1460L, ChronoUnit.DAYS))));
assertThat(regenerateResponse.read("$.duration_overridden"), is(equalTo(true)));
assertThat(regenerateResponse.read("$.duration_used"), is(equalTo(1460)));
assertThat(versions.size(), is(equalTo(2)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class CertificatesControllerTest {
""".trimIndent()
certificateView = CertificateGenerationView(certificateCredentialVersion, false)
(certificateView as CertificateGenerationView).durationOverridden = true
(certificateView as CertificateGenerationView).durationUsed = 1234
spyCertificatesHandler.handleRegenerate__returns_credentialView = certificateView

val mvcResult = mockMvc
Expand Down Expand Up @@ -167,6 +168,7 @@ class CertificatesControllerTest {
"certificate_authority": false,
"self_signed": false,
"duration_overridden": true,
"duration_used": 1234,
"value": {
"ca": "${TestConstants.TEST_CA}",
"certificate": "${TestConstants.TEST_CERTIFICATE}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ class CredentialsControllerGenerateTest {
false,
true,
false,
true
true,
1460
)

val encryptor = Mockito.mock(Encryptor::class.java)
Expand Down Expand Up @@ -385,7 +386,6 @@ class CredentialsControllerGenerateTest {

val actualResponseBody = mvcResult.response.contentAsString

print(actualResponseBody)
// language=json
val expectedResponseBody =
"""
Expand All @@ -401,6 +401,7 @@ class CredentialsControllerGenerateTest {
"self_signed": false,
"generated": true,
"duration_overridden": true,
"duration_used": 1460,
"value": {
"ca": "${TestConstants.TEST_CA}",
"certificate": "${TestConstants.TEST_CERTIFICATE}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class CredentialsPostIntegrationTest {
true,
true,
true,
false
false,
730
)

Mockito.doReturn(caCredentialValue).`when`<CertificateAuthorityService>(certificateAuthorityService).findActiveVersion("/myCA")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public CertificateCredentialValue generateCredential(final GenerationParameters
if (params.isSelfSigned()) {
try {
final String cert = CertificateFormatter.pemOf(signedCertificateGenerator.getSelfSigned(keyPair, params));
return new CertificateCredentialValue(cert, cert, privatePem, null, null, params.isCa(), params.isSelfSigned(), true, false, durationOverridden);
return new CertificateCredentialValue(cert, cert, privatePem, null, null, params.isCa(), params.isSelfSigned(), true, false, durationOverridden, params.getDuration());
} catch (final Exception e) {
throw new RuntimeException(e);
}
Expand Down Expand Up @@ -102,7 +102,7 @@ public CertificateCredentialValue generateCredential(final GenerationParameters
certificateReader.getCertificate(),
PrivateKeyReader.getPrivateKey(signingCaPrivateKey)
);
return new CertificateCredentialValue(signingCaCertificate, CertificateFormatter.pemOf(cert), privatePem, caName, trustedCaCertificate, params.isCa(), params.isSelfSigned(), true, false, durationOverridden);
return new CertificateCredentialValue(signingCaCertificate, CertificateFormatter.pemOf(cert), privatePem, caName, trustedCaCertificate, params.isCa(), params.isSelfSigned(), true, false, durationOverridden, params.getDuration());
} catch (final Exception e) {
throw new RuntimeException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class CertificateCredentialValue : CredentialValue {
@JsonIgnore
var durationOverridden: Boolean = false

@JsonIgnore
var durationUsed: Int = 0

constructor() : super() {}

constructor(
Expand All @@ -70,7 +73,8 @@ class CertificateCredentialValue : CredentialValue {
selfSigned: Boolean,
generated: Boolean?,
transitional: Boolean,
durationOverridden: Boolean
durationOverridden: Boolean,
durationUsed: Int
) : super() {
this.ca = ca
this.trustedCa = trustedCa
Expand All @@ -82,6 +86,7 @@ class CertificateCredentialValue : CredentialValue {
this.generated = generated
this.caName = caName
this.durationOverridden = durationOverridden
this.durationUsed = durationUsed
}

constructor(
Expand All @@ -103,7 +108,8 @@ class CertificateCredentialValue : CredentialValue {
selfSigned,
generated,
transitional,
false
false,
0
) {
}

Expand All @@ -126,7 +132,8 @@ class CertificateCredentialValue : CredentialValue {
selfSigned,
generated,
transitional,
false
false,
0
) {
this.versionCreatedAt = versionCreatedAt
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class CertificateCredentialVersion(delegate: CertificateCredentialVersionData) :
private set

var durationOverridden: Boolean = false
var durationUsed: Int = 0

var ca: String?
get() = (delegate as CertificateCredentialVersionData).ca
Expand Down Expand Up @@ -95,6 +96,7 @@ class CertificateCredentialVersion(delegate: CertificateCredentialVersionData) :
this.isSelfSigned = certificate.selfSigned
this.generated = certificate.generated
this.durationOverridden = certificate.durationOverridden
this.durationUsed = certificate.durationUsed
}

override fun getCredentialType(): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class DefaultCredentialService(
val savedVersion = credentialVersionDataService.save(newVersion)
if (savedVersion is CertificateCredentialVersion) {
savedVersion.durationOverridden = (newVersion as CertificateCredentialVersion).durationOverridden
savedVersion.durationUsed = newVersion.durationUsed
}
return savedVersion
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.cloudfoundry.credhub.domain.CertificateCredentialVersion

class CertificateGenerationView : CertificateView {
var durationOverridden: Boolean = false
var durationUsed: Int = 0

internal constructor() : super() /* Jackson */ {}

Expand All @@ -12,5 +13,6 @@ class CertificateGenerationView : CertificateView {
concatenateCas
) {
durationOverridden = version.durationOverridden
durationUsed = version.durationUsed
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public void createsAViewFromEntity() throws Exception {
@Test
public void createsAViewFromEntityIncludesDurationOverriddenWhenSet() throws Exception {
entity.setDurationOverridden(true);
entity.setDurationUsed(1234);
final CredentialView subject = CertificateView.fromEntity(entity, true, true);
final String actualJson = serializeToString(subject);

Expand All @@ -121,6 +122,7 @@ public void createsAViewFromEntityIncludesDurationOverriddenWhenSet() throws Exc
+ "\"name\":\"" + credentialName + "\","
+ "\"metadata\":{\"name\":\"test\"},"
+ "\"duration_overridden\":true,"
+ "\"duration_used\":1234,"
+ "\"value\":{"
+ "\"ca\":\"" + CertificateStringConstants.SELF_SIGNED_CA_CERT + "\","
+ "\"certificate\":\"" + CertificateStringConstants.SIMPLE_SELF_SIGNED_TEST_CERT + "\","
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ class DefaultCredentialServiceTest {
}

@Test
fun save_whenThereIsACertificateOverriddenDurationAndIsSet_ReturnsDurationOverridden() {
fun save_whenThereIsACertificateOverriddenDurationAndIsSet_ReturnsDurationOverriddenAndDurationUsed() {
val generateRequest = mock(BaseCredentialGenerateRequest::class.java)
`when`(generateRequest.name).thenReturn(CREDENTIAL_NAME)
`when`(generateRequest.type).thenReturn("certificate")
Expand All @@ -472,6 +472,7 @@ class DefaultCredentialServiceTest {

val certificateCredentialVersion = CertificateCredentialVersion(CREDENTIAL_NAME)
certificateCredentialVersion.durationOverridden = true
certificateCredentialVersion.durationUsed = 1234

val savedCertificateCredentialVersion = CertificateCredentialVersion(CREDENTIAL_NAME)
savedCertificateCredentialVersion.durationOverridden = false
Expand All @@ -495,6 +496,7 @@ class DefaultCredentialServiceTest {

val returnedCertificateCredentialVersion = this.subject.save(originalCredentialVersion, certificateCredentialValue, generateRequest) as CertificateCredentialVersion
assertThat(returnedCertificateCredentialVersion.durationOverridden).isEqualTo(true)
assertThat(returnedCertificateCredentialVersion.durationUsed).isEqualTo(1234)
}

companion object {
Expand Down

0 comments on commit a64374f

Please sign in to comment.