Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for EST (and ACME issuance) certificate “profiles”, enabling profile-specific CA material and profile-aware endpoints/clients.
Changes:
- Introduces CA profile abstractions (
CaProfile,CaProfileSet) and threadsprofilethrough CA signing and revocation list APIs. - Adds profile-aware EST server routes (
/.well-known/est/{profileName}/...) and updates the EST client + CLI to target a profile. - Extends ACME order creation and certificate issuance flow to carry an optional
profile.
Reviewed changes
Copilot reviewed 57 out of 58 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/opencertserver.lambda.tests/README.md | Adds basic test-readme scaffolding for lambda tests. |
| tests/opencertserver.est.server.tests/TestCsrAttributesHandler.cs | Updates CSR attributes test helper to new loader interface. |
| tests/opencertserver.est.server.tests/Steps/EstServer.cs | Updates EST test steps to exercise profile-aware enrollment/reenrollment/cacert/csrattrs. |
| tests/opencertserver.est.server.tests/README.md | Adds basic test-readme scaffolding for EST server tests. |
| tests/opencertserver.est.server.tests/Features/EstServer.feature.cs | Regenerates ReqnRoll bindings for new scenario outlines/profile parameters. |
| tests/opencertserver.est.server.tests/Features/EstServer.feature | Converts scenarios to profile-driven outlines (rsa/ecdsa). |
| tests/opencertserver.est.server.tests/Configuration/ConfigureCertificateAuthenticationOptions.cs | Adjusts cert auth claims extraction using RDN enumeration + OID mapping. |
| tests/opencertserver.cli.tests/StepDefinitions/TestCsrAttributesHandler.cs | Updates CLI test CSR loader to new interface. |
| tests/opencertserver.cli.tests/StepDefinitions/EstServerSetup.cs | Updates CLI test server wiring to new CSR template loader type. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestCsrAttributesLoader.cs | Updates certserver test CSR loader to new interface. |
| tests/opencertserver.certserver.tests/StepDefinitions/EstEnrollment.cs | Updates EST client construction to new ctor signature. |
| tests/opencertserver.certserver.tests/StepDefinitions/CertificateServerFeatures.cs | Wires EST server with new CSR template loader + sets ACME profile selection in tests. |
| tests/opencertserver.ca.tests/X509CertificateTests.cs | Updates CA unit tests to build CA via profile configuration. |
| tests/opencertserver.ca.tests/CertificateAuthorityTests.cs | Updates CA unit tests to use CaProfileSet and profile-based CA config. |
| tests/opencertserver.acme.aspnetclient.tests/LetsEncryptClientTests.cs | Updates client mock expectations for PlaceOrder(string[]). |
| tests/opencertserver.acme.aspnetclient.tests/LetsEncryptChallengeApprovalMiddlewareMiddlewareTests.cs | Updates fake ACME client signature for PlaceOrder(string[]). |
| tests/opencertserver.acme.abstractions.tests/HttpModel-Initialization/Challenge.cs | Updates ACME model initialization for new Order ctor signature. |
| tests/opencertserver.acme.abstractions.tests/HttpModel-Initialization/Authorization.cs | Updates ACME model initialization for new Order ctor signature. |
| src/opencertserver.lambda/DefaultIssuer.cs | Adds profile parameter to issuance path and passes to CA signing. |
| src/opencertserver.est.server/opencertserver.est.server.csproj | Enables request delegate generator and formatting tweaks. |
| src/opencertserver.est.server/Handlers/SimpleReEnrollHandler.cs | Converts to minimal-API style handler and adds profile routing support. |
| src/opencertserver.est.server/Handlers/SimpleEnrollHandler.cs | Converts to minimal-API style handler and adds profile routing support. |
| src/opencertserver.est.server/Handlers/ServerKeyGenHandler.cs | Converts to minimal-API style handler, adds profile route, and returns multipart result via custom IResult. |
| src/opencertserver.est.server/Handlers/ICsrTemplateLoader.cs | Introduces DI contract for loading CSR templates by profile/user. |
| src/opencertserver.est.server/Handlers/CsrAttributesHandler.cs | Refactors CSR attributes handler to minimal-API style + loader interface. |
| src/opencertserver.est.server/Handlers/CertificateSigningRequestTemplateResult.cs | Adds dedicated IResult for application/csrattrs output. |
| src/opencertserver.est.server/Handlers/CaCertHandler.cs | Refactors CA cert handler to minimal-API style and makes it profile-aware. |
| src/opencertserver.est.server/EstServerExtensions.cs | Updates DI + endpoints to use new handlers and profile routes. |
| src/opencertserver.est.client/EstClient.cs | Adds optional profileName and uses profile-aware EST paths. |
| src/opencertserver.cli/Program_EstServerCertificates.cs | Adds --profile option and passes profile to EstClient. |
| src/opencertserver.cli/Program_EstReEnroll.cs | Adds --profile option and passes profile to EstClient. |
| src/opencertserver.cli/Program_EstEnroll.cs | Adds --profile option and passes profile to EstClient. |
| src/opencertserver.certserver/opencertserver.certserver.csproj | Enables request delegate generator. |
| src/opencertserver.certserver/TestCsrAttributesHandler.cs | Updates certserver CSR loader to new interface. |
| src/opencertserver.certserver/Program.cs | Updates CA setup to use CaProfileSet and wires EST server loader. |
| src/opencertserver.certserver/DefaultIssuer.cs | Adds profile parameter to issuance path and passes to CA signing. |
| src/opencertserver.certserver/ConfigureCertificateAuthenticationOptions.cs | Updates cert auth to accept certificate chain provider function. |
| src/opencertserver.ca/CertificateAuthority.cs | Introduces CA profile model + updates signing/root cert/crl APIs to be profile-aware. |
| src/opencertserver.ca.utils/Ca/IValidateCertificateRequests.cs | Extends validator API to include profile. |
| src/opencertserver.ca.utils/Ca/ICertificateAuthority.cs | Extends CA API to include optional profileName. |
| src/opencertserver.ca.server/opencertserver.ca.server.csproj | Enables request delegate generator. |
| src/opencertserver.ca.server/Handlers/CsrHandler.cs | Converts CSR handler to minimal-API style and adds optional profile route binding. |
| src/opencertserver.ca.server/Handlers/CrlHandler.cs | Converts CRL handler to minimal-API style and adds profile route. |
| src/opencertserver.ca.server/Extensions.cs | Updates self-signed CA registration to create profile set; adds profile CRL endpoint. |
| src/opencertserver.acme.server/Services/DefaultOrderService.cs | Adds order profile parameter and forwards it into issuance. |
| src/opencertserver.acme.server/Endpoints/OrderEndpoints.cs | Accepts/forwards profile from create-order request. |
| src/opencertserver.acme.aspnetclient/Certificates/CertificateProvider.cs | Updates PlaceOrder invocation for new signature. |
| src/opencertserver.acme.aspnetclient/Certes/LetsEncryptOptions.cs | Extracts AcmeOptions into separate file; keeps LetsEncrypt options class. |
| src/opencertserver.acme.aspnetclient/Certes/IAcmeClient.cs | Changes PlaceOrder signature to PlaceOrder(string[] domains). |
| src/opencertserver.acme.aspnetclient/Certes/AcmeOptions.cs | Adds Profile field to ACME options model. |
| src/opencertserver.acme.aspnetclient/Certes/AcmeClient.cs | Passes Profile into NewOrder and updates PlaceOrder signature. |
| src/opencertserver.acme.abstractions/Services/IOrderService.cs | Adds profile to order creation contract. |
| src/opencertserver.acme.abstractions/Model/Order.cs | Stores optional Profile on orders and updates ctor. |
| src/opencertserver.acme.abstractions/IssuanceServices/IIssueCertificates.cs | Adds profile to certificate issuance contract. |
| src/opencertserver.acme.abstractions/HttpModel/Requests/CreateOrderRequest.cs | Adds Profile to create-order HTTP request model. |
| src/CertesSlim/IAcmeContext.cs | Adds profile argument to NewOrder. |
| src/CertesSlim/AcmeContext.cs | Includes Profile in ACME new-order payload. |
| src/CertesSlim/Acme/Resource/Order.cs | Adds Profile field to ACME order resource model. |
Files not reviewed (1)
- tests/opencertserver.est.server.tests/Features/EstServer.feature.cs: Language not supported
Comments suppressed due to low confidence (1)
tests/opencertserver.est.server.tests/Steps/EstServer.cs:185
- This step takes a
profileargument but always generates an RSA key/CSR (using var rsa = RSA.Create();). When the scenario outline passesecdsa, the failure could be due to key/profile mismatch rather than the intended unauthenticated behavior. Generate the key based onprofile(RSA vs ECDSA) to keep the test asserting the right thing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using System.Security.Cryptography; | ||
| using System.Security.Cryptography.X509Certificates; | ||
|
|
There was a problem hiding this comment.
The using System.Security.Cryptography; and using System.Security.Cryptography.X509Certificates; directives are unused in this file. With TreatWarningsAsErrors=true, this will fail compilation (CS8019). Remove the unused usings.
| using System.Security.Cryptography; | |
| using System.Security.Cryptography.X509Certificates; |
| var client = await _clientFactory.GetClient(); | ||
|
|
||
| var placedOrder = await client.PlaceOrder(); | ||
| var placedOrder = await client.PlaceOrder([]); | ||
|
|
There was a problem hiding this comment.
CertificateProvider is placing ACME orders with an empty domain list (PlaceOrder([])), even though AddAcmeClient validates that options.Domains is non-empty. This will likely create an invalid order (or fail at the ACME server). Either inject AcmeOptions into CertificateProvider and call PlaceOrder(options.Domains), or change IAcmeClient.PlaceOrder back to using the configured domains internally.
| var error = (SignCertificateResponse.Error)newCert; | ||
| return Results.Text(string.Join(Environment.NewLine, error.Errors), Constants.PemMimeType, Encoding.UTF8, | ||
| (int)HttpStatusCode.BadRequest); |
There was a problem hiding this comment.
Results.Text for the error case is returning Constants.PemMimeType, but the payload is a newline-joined list of error strings. This should use Constants.TextPlainMimeType (and keep the 400 status), otherwise clients may misinterpret error responses as PEM.
| CertificateChain = [X509Certificate2.CreateFromPem(certs[1].ExportCertificatePem())], | ||
| CertificateValidity = TimeSpan.FromDays(90), | ||
| CrlNumber = BigInteger.Zero, | ||
| PrivateKey = () => certs[1].GetRSAPrivateKey()! |
There was a problem hiding this comment.
ecdsaProfile.PrivateKey is set via certs[1].GetRSAPrivateKey(), which will return null for an ECDSA certificate and/or produce an RSA signer for the ECDSA CA chain. This will break issuance for the ecdsa profile; use GetECDsaPrivateKey() for the ECDSA profile. Also note that Get*PrivateKey() typically returns a new disposable key instance each call—consider caching the key once (or ensuring the CA disposes per-call keys) to avoid leaking handles.
| PrivateKey = () => certs[1].GetRSAPrivateKey()! | |
| PrivateKey = () => certs[1].GetECDsaPrivateKey()! |
| using Microsoft.Extensions.DependencyInjection; | ||
| using OpenCertServer.Ca.Utils; |
There was a problem hiding this comment.
using Microsoft.Extensions.DependencyInjection; is currently unused (the only references are commented out). With TreatWarningsAsErrors=true, this will fail the build (CS8019). Remove the unused using or re-enable the code that requires it.
| private readonly Func<string?, X509Certificate2Collection> _certificates; | ||
|
|
||
| public ConfigureCertificateAuthenticationOptions(X509Certificate2Collection certificates) | ||
| public ConfigureCertificateAuthenticationOptions(Func<string?, X509Certificate2Collection> certificates) | ||
| { | ||
| _certificates = certificates; | ||
| } |
There was a problem hiding this comment.
The _certificates field is assigned in the constructor but never used in PostConfigure. With TreatWarningsAsErrors=true, this triggers CS0414 and fails compilation. Either remove the dependency/field or use it to set AdditionalChainCertificates/CustomTrustStore (potentially based on profileName).
| using var handler = _server.CreateHandler(); | ||
| using var rsa = ECDsa.Create(); |
There was a problem hiding this comment.
using var handler = _server.CreateHandler(); is never used. With TreatWarningsAsErrors=true, this unused local triggers CS0219 and will fail the test project build. Remove it or pass it into EstClient as the message handler.
Add support for EST certificate profiles