Skip to content

Commit

Permalink
Certificate API - Follow Key Vault behavior more accurately (#505)
Browse files Browse the repository at this point in the history
- Sets new defaults for key usages
- Sets new defaults for extended key usages
- Sets new defaults for certificate lifetime actions
- Renames ips to upns in SANS to correct a mistake
- Clears up confusion between certificate policy and issuance policy
- Makes sure renewal can change certificate content type
- Splits certificate controller into two smaller controllers
- Fixes get certificate policy controller to return issuance policy
- Fixes JSON mapping of key usage enum
- Removes unintended issuance policy change behavior at certificate import
- Adds new tests

Resolves #503
{minor}

Signed-off-by: Esta Nagy <[email protected]>
  • Loading branch information
nagyesta authored Mar 15, 2023
1 parent 0ca6e6d commit 7532958
Show file tree
Hide file tree
Showing 44 changed files with 1,095 additions and 444 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
import javax.validation.Valid;
import javax.validation.constraints.Pattern;
import java.net.URI;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;

import static com.github.nagyesta.lowkeyvault.controller.common.util.CertificateRequestMapperUtil.createCertificateWithAttributes;
import static com.github.nagyesta.lowkeyvault.controller.common.util.CertificateRequestMapperUtil.importCertificateWithAttributes;
Expand Down Expand Up @@ -66,31 +64,6 @@ public ResponseEntity<KeyVaultPendingCertificateModel> create(
return ResponseEntity.accepted().body(pendingModelConverter.convert(readOnlyEntity, baseUri));
}

public ResponseEntity<KeyVaultPendingCertificateModel> pendingCreate(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
final URI baseUri) {
log.info("Received request to {} get pending create certificate: {} using API version: {}",
baseUri.toString(), certificateName, apiVersion());
final CertificateVaultFake vaultFake = getVaultByUri(baseUri);
final VersionedCertificateEntityId entityId = vaultFake
.getEntities().getLatestVersionOfEntity(entityId(baseUri, certificateName));
final ReadOnlyKeyVaultCertificateEntity readOnlyEntity = vaultFake
.getEntities().getReadOnlyEntity(entityId);
return ResponseEntity.ok(pendingModelConverter.convert(readOnlyEntity, baseUri));
}

public ResponseEntity<KeyVaultPendingCertificateModel> pendingDelete(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
final URI baseUri) {
log.info("Received request to {} get pending delete certificate: {} using API version: {}",
baseUri.toString(), certificateName, apiVersion());
final CertificateVaultFake vaultFake = getVaultByUri(baseUri);
final VersionedCertificateEntityId entityId = vaultFake.getDeletedEntities()
.getLatestVersionOfEntity(entityId(baseUri, certificateName));
final ReadOnlyKeyVaultCertificateEntity readOnlyEntity = vaultFake
.getDeletedEntities().getReadOnlyEntity(entityId);
return ResponseEntity.ok(pendingModelConverter.convert(readOnlyEntity, baseUri));
}

public ResponseEntity<KeyVaultCertificateModel> get(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
Expand All @@ -101,15 +74,6 @@ public ResponseEntity<KeyVaultCertificateModel> get(
return ResponseEntity.ok(getLatestEntityModel(baseUri, certificateName));
}

public ResponseEntity<CertificatePolicyModel> getPolicy(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
final URI baseUri) {
log.info("Received request to {} get certificate policy: {} with version: -LATEST- using API version: {}",
baseUri.toString(), certificateName, apiVersion());

return ResponseEntity.ok(getLatestEntityModel(baseUri, certificateName).getPolicy());
}

public ResponseEntity<KeyVaultCertificateModel> getWithVersion(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
@Valid @Pattern(regexp = VERSION_NAME_PATTERN) final String certificateVersion,
Expand Down Expand Up @@ -223,7 +187,7 @@ protected KeyVaultCertificateModel getModelById(
final URI baseUri,
final boolean includeDisabled) {
final KeyVaultCertificateModel model = super.getModelById(entityVaultFake, entityId, baseUri, includeDisabled);
setLifetimeActionModels(entityId, baseUri, model.getPolicy()::setLifetimeActions);
lifetimeActionsModelConverter.populateLifetimeActions(entityVaultFake, entityId, model.getPolicy()::setLifetimeActions);
return model;
}

Expand All @@ -234,19 +198,10 @@ protected DeletedKeyVaultCertificateModel getDeletedModelById(
final URI baseUri,
final boolean includeDisabled) {
final DeletedKeyVaultCertificateModel model = super.getDeletedModelById(entityVaultFake, entityId, baseUri, includeDisabled);
setLifetimeActionModels(entityId, baseUri, model.getPolicy()::setLifetimeActions);
lifetimeActionsModelConverter.populateLifetimeActions(entityVaultFake, entityId, model.getPolicy()::setLifetimeActions);
return model;
}

private void setLifetimeActionModels(
final VersionedCertificateEntityId entityId,
final URI baseUri,
final Consumer<List<CertificateLifetimeActionModel>> consumer) {
Optional.ofNullable(getVaultByUri(baseUri).lifetimeActionPolicy(entityId))
.map(lifetimeActionsModelConverter::convert)
.ifPresent(consumer);
}

private KeyVaultItemListModel<KeyVaultCertificateItemModel> getPageOfItems(
final URI baseUri, final int limit, final int offset, final boolean includePending) {
final KeyVaultItemListModel<KeyVaultCertificateItemModel> page =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.github.nagyesta.lowkeyvault.controller.common;

import com.github.nagyesta.lowkeyvault.mapper.v7_3.certificate.CertificateEntityToV73IssuancePolicyModelConverter;
import com.github.nagyesta.lowkeyvault.mapper.v7_3.certificate.CertificateEntityToV73PendingCertificateOperationModelConverter;
import com.github.nagyesta.lowkeyvault.mapper.v7_3.certificate.LifetimeActionsPolicyToV73ModelConverter;
import com.github.nagyesta.lowkeyvault.model.v7_3.certificate.CertificatePolicyModel;
import com.github.nagyesta.lowkeyvault.model.v7_3.certificate.KeyVaultPendingCertificateModel;
import com.github.nagyesta.lowkeyvault.service.certificate.CertificateVaultFake;
import com.github.nagyesta.lowkeyvault.service.certificate.ReadOnlyKeyVaultCertificateEntity;
import com.github.nagyesta.lowkeyvault.service.certificate.id.CertificateEntityId;
import com.github.nagyesta.lowkeyvault.service.certificate.id.VersionedCertificateEntityId;
import com.github.nagyesta.lowkeyvault.service.vault.VaultFake;
import com.github.nagyesta.lowkeyvault.service.vault.VaultService;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.NonNull;

import javax.validation.Valid;
import javax.validation.constraints.Pattern;
import java.net.URI;
import java.util.function.Function;

@Slf4j
public abstract class CommonCertificatePolicyController
extends BaseEntityReadController<CertificateEntityId, VersionedCertificateEntityId,
ReadOnlyKeyVaultCertificateEntity, CertificateVaultFake> {

private final CertificateEntityToV73PendingCertificateOperationModelConverter pendingOperationConverter;
private final CertificateEntityToV73IssuancePolicyModelConverter issuancePolicyConverter;
private final LifetimeActionsPolicyToV73ModelConverter lifetimeActionsConverter;

protected CommonCertificatePolicyController(
@lombok.NonNull final CertificateEntityToV73PendingCertificateOperationModelConverter pendingOperationConverter,
@lombok.NonNull final CertificateEntityToV73IssuancePolicyModelConverter issuancePolicyConverter,
@lombok.NonNull final LifetimeActionsPolicyToV73ModelConverter lifetimeActionsConverter,
@NonNull final VaultService vaultService,
@NonNull final Function<VaultFake, CertificateVaultFake> toEntityVault) {
super(vaultService, toEntityVault);
this.pendingOperationConverter = pendingOperationConverter;
this.issuancePolicyConverter = issuancePolicyConverter;
this.lifetimeActionsConverter = lifetimeActionsConverter;
}

public ResponseEntity<KeyVaultPendingCertificateModel> pendingCreate(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
final URI baseUri) {
log.info("Received request to {} get pending create certificate: {} using API version: {}",
baseUri.toString(), certificateName, apiVersion());
final CertificateVaultFake vaultFake = getVaultByUri(baseUri);
final VersionedCertificateEntityId entityId = vaultFake
.getEntities().getLatestVersionOfEntity(entityId(baseUri, certificateName));
final ReadOnlyKeyVaultCertificateEntity readOnlyEntity = vaultFake
.getEntities().getReadOnlyEntity(entityId);
return ResponseEntity.ok(pendingOperationConverter.convert(readOnlyEntity, baseUri));
}

public ResponseEntity<KeyVaultPendingCertificateModel> pendingDelete(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
final URI baseUri) {
log.info("Received request to {} get pending delete certificate: {} using API version: {}",
baseUri.toString(), certificateName, apiVersion());
final CertificateVaultFake vaultFake = getVaultByUri(baseUri);
final VersionedCertificateEntityId entityId = vaultFake.getDeletedEntities()
.getLatestVersionOfEntity(entityId(baseUri, certificateName));
final ReadOnlyKeyVaultCertificateEntity readOnlyEntity = vaultFake
.getDeletedEntities().getReadOnlyEntity(entityId);
return ResponseEntity.ok(pendingOperationConverter.convert(readOnlyEntity, baseUri));
}

public ResponseEntity<CertificatePolicyModel> getPolicy(
@Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
final URI baseUri) {
log.info("Received request to {} get certificate policy: {} with version: -LATEST- using API version: {}",
baseUri.toString(), certificateName, apiVersion());
final CertificateVaultFake vaultFake = getVaultByUri(baseUri);
final VersionedCertificateEntityId latest = vaultFake.getEntities().getLatestVersionOfEntity(entityId(baseUri, certificateName));
final ReadOnlyKeyVaultCertificateEntity entity = vaultFake.getEntities().getReadOnlyEntity(latest);
final CertificatePolicyModel model = issuancePolicyConverter.convert(entity, baseUri);
lifetimeActionsConverter.populateLifetimeActions(vaultFake, latest, model::setLifetimeActions);
return ResponseEntity.ok(model);
}

@Override
protected VersionedCertificateEntityId versionedEntityId(final URI baseUri, final String name, final String version) {
return new VersionedCertificateEntityId(baseUri, name, version);
}

@Override
protected CertificateEntityId entityId(final URI baseUri, final String name) {
return new CertificateEntityId(baseUri, name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import java.util.*;
import java.util.stream.Collectors;

import static com.github.nagyesta.lowkeyvault.service.certificate.impl.CertificateCreationInput.*;

public final class CertificateRequestMapperUtil {
private CertificateRequestMapperUtil() {
throw new IllegalCallerException("Utility cannot be instantiated.");
Expand All @@ -28,7 +30,7 @@ public static VersionedCertificateEntityId createCertificateWithAttributes(
//no need to set expiry, the generation should take care of it based on the X509 properties
certificateVaultFake.setEnabled(certificateEntityId, properties.isEnabled());
//no need to set managed property as this endpoint cannot create managed entities by definition
setLifetimeActions(certificateVaultFake, request.getPolicy(), certificateEntityId);
setLifetimeActions(certificateVaultFake, request.getPolicy(), certificateEntityId, CertAuthorityType.SELF_SIGNED);
return certificateEntityId;
}

Expand All @@ -42,8 +44,10 @@ public static VersionedCertificateEntityId importCertificateWithAttributes(
//no need to set expiry, the generation should take care of it based on the X509 properties
certificateVaultFake.setEnabled(certificateEntityId, properties.isEnabled());
//no need to set managed property as this endpoint cannot create managed entities by definition
Optional.ofNullable(request.getPolicy())
.ifPresent(policyModel -> setLifetimeActions(certificateVaultFake, policyModel, certificateEntityId));
final CertAuthorityType certAuthorityType = certificateVaultFake.getEntities()
.getReadOnlyEntity(certificateEntityId).getOriginalCertificatePolicy().getCertAuthorityType();
final CertificatePolicyModel policyModel = Objects.requireNonNullElse(request.getPolicy(), new CertificatePolicyModel());
setLifetimeActions(certificateVaultFake, policyModel, certificateEntityId, certAuthorityType);
return certificateEntityId;
}

Expand All @@ -53,18 +57,20 @@ private static CertificatePropertiesModel defaultIfNull(final CertificatePropert

private static void validateLifetimeActions(final CertificatePolicyModel policy) {
final Integer validityMonths = Objects.requireNonNullElse(policy.getX509Properties().getValidityMonths(),
CertificateCreationInput.DEFAULT_VALIDITY_MONTHS);
DEFAULT_VALIDITY_MONTHS);
Optional.ofNullable(policy.getLifetimeActions())
.ifPresent(actions -> actions.forEach(a -> a.getTrigger().validate(validityMonths)));
}

private static void setLifetimeActions(
final CertificateVaultFake certificateVaultFake,
final CertificatePolicyModel policy,
final VersionedCertificateEntityId certificateEntityId) {
Optional.ofNullable(policy.getLifetimeActions())
.ifPresent(actions -> certificateVaultFake.setLifetimeActionPolicy(
new CertificateLifetimeActionPolicy(certificateEntityId, convertActivityMap(actions))));
final VersionedCertificateEntityId certificateEntityId,
final CertAuthorityType certAuthorityType) {
final CertificateLifetimeActionPolicy lifetimeActionPolicy = Optional.ofNullable(policy.getLifetimeActions())
.map(actions -> new CertificateLifetimeActionPolicy(certificateEntityId, convertActivityMap(actions)))
.orElse(new DefaultCertificateLifetimeActionPolicy(certificateEntityId, certAuthorityType));
certificateVaultFake.setLifetimeActionPolicy(lifetimeActionPolicy);
}

private static Map<CertificateLifetimeActionActivity, CertificateLifetimeActionTrigger> convertActivityMap(
Expand All @@ -85,11 +91,10 @@ private static CertificateCreationInput toCertificateCreationInput(
.subject(x509Properties.getSubject())
.dnsNames(Objects.requireNonNullElse(x509Properties.getSubjectAlternativeNames().getDnsNames(), Set.of()))
.emails(Objects.requireNonNullElse(x509Properties.getSubjectAlternativeNames().getEmails(), Set.of()))
.ips(Objects.requireNonNullElse(x509Properties.getSubjectAlternativeNames().getUpns(), Set.of()))
.keyUsage(Objects.requireNonNullElse(x509Properties.getKeyUsage(), Set.of()))
.extendedKeyUsage(Objects.requireNonNullElse(x509Properties.getExtendedKeyUsage(), Set.of()))
.validityMonths(
Objects.requireNonNullElse(x509Properties.getValidityMonths(), CertificateCreationInput.DEFAULT_VALIDITY_MONTHS))
.upns(Objects.requireNonNullElse(x509Properties.getSubjectAlternativeNames().getUpns(), Set.of()))
.keyUsage(Objects.requireNonNullElse(x509Properties.getKeyUsage(), DEFAULT_KEY_USAGES))
.extendedKeyUsage(Objects.requireNonNullElse(x509Properties.getExtendedKeyUsage(), DEFAULT_EXT_KEY_USAGES))
.validityMonths(Objects.requireNonNullElse(x509Properties.getValidityMonths(), DEFAULT_VALIDITY_MONTHS))
.validityStart(OffsetDateTime.now())
//issuer
.certificateType(issuer.getCertType())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,6 @@ public ResponseEntity<KeyVaultPendingCertificateModel> create(
return super.create(certificateName, baseUri, request);
}

@Override
@GetMapping(value = "/certificates/{certificateName}/pending",
params = API_VERSION_7_3,
produces = APPLICATION_JSON_VALUE)
public ResponseEntity<KeyVaultPendingCertificateModel> pendingCreate(
@PathVariable @Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
@RequestAttribute(name = ApiConstants.REQUEST_BASE_URI) final URI baseUri) {
return super.pendingCreate(certificateName, baseUri);
}

@Override
@DeleteMapping(value = "/certificates/{certificateName}/pending",
params = API_VERSION_7_3,
produces = APPLICATION_JSON_VALUE)
public ResponseEntity<KeyVaultPendingCertificateModel> pendingDelete(
@PathVariable @Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
@RequestAttribute(name = ApiConstants.REQUEST_BASE_URI) final URI baseUri) {
return super.pendingDelete(certificateName, baseUri);
}

@Override
@PostMapping(value = "/certificates/{certificateName}/import",
params = API_VERSION_7_3,
Expand All @@ -95,16 +75,6 @@ public ResponseEntity<KeyVaultCertificateModel> get(
return super.get(certificateName, baseUri);
}

@Override
@GetMapping(value = "/certificates/{certificateName}/policy",
params = API_VERSION_7_3,
produces = APPLICATION_JSON_VALUE)
public ResponseEntity<CertificatePolicyModel> getPolicy(
@PathVariable @Valid @Pattern(regexp = NAME_PATTERN) final String certificateName,
@RequestAttribute(name = ApiConstants.REQUEST_BASE_URI) final URI baseUri) {
return super.getPolicy(certificateName, baseUri);
}

@Override
@GetMapping(value = "/certificates/{certificateName}/{certificateVersion}",
params = API_VERSION_7_3,
Expand Down
Loading

0 comments on commit 7532958

Please sign in to comment.