natemcmaster / LettuceEncrypt

Free, automatic HTTPS certificate generation for ASP.NET Core web apps
https://nuget.org/packages/LettuceEncrypt
Apache License 2.0
1.55k stars 153 forks source link

Support additional issuers for generated certificates #207

Closed MxFr closed 1 year ago

MxFr commented 2 years ago

These changes allowed me mitigate #194 and use the LetsEncrypt staging environment.

Adding the new service IAdditionalIssuersSource looked like the most flexible solution opposed to a list of certificates in the LettuceEncryptOptions and allows to combine different approaches.

codecov[bot] commented 2 years ago

Codecov Report

Merging #207 (efc2186) into main (cd68f74) will decrease coverage by 0.24%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
- Coverage   45.94%   45.69%   -0.25%     
==========================================
  Files          44       44              
  Lines        1110     1116       +6     
==========================================
  Hits          510      510              
- Misses        600      606       +6     
Impacted Files Coverage Δ
.../LettuceEncrypt/Internal/AcmeCertificateFactory.cs 0.00% <0.00%> (ø)
src/LettuceEncrypt/LettuceEncryptOptions.cs 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cd68f74...efc2186. Read the comment docs.

natemcmaster commented 2 years ago

Changes look fine to me. Did you consider writing a unit test for the new code?

MxFr commented 2 years ago

A unit test will probably be a good idea, but I will need some time to think on how to test it properly.

I think since the PfxBuilder is created by the CertificateChainExtensions in a extension method I dont think I can mock it. So I'd need to use a mock client returning a certificate with an issuer that is not supported and a IAdditionalIssuersSource that supplies that issuer.

I might manage it in the next few days.

natemcmaster commented 2 years ago

Any progress on unit tests? Writing tests on this code can be hard. Did you at least do some manual testing on this?

github-actions[bot] commented 1 year ago

This pull request appears to be stale. Please comment if you believe this should remain open and reviewed. If there are no updates, it will be closed in 14 days.

github-actions[bot] commented 1 year ago

Thank you for your contributions to this project. This pull request has been closed due to inactivity.