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 152 forks source link

feat: allow configuring delay for validation #259

Closed h3rmanj closed 2 years ago

h3rmanj commented 2 years ago

Fixes #254

This PR adds an optional ChallengeValidationDelayMs to the configuration, allowing consumers to specify a delay before the challenge is ready for validation.

The delay happens in the AcmeClient.ValidateChallengeAsync, which is only used by the HTTP01 and TLSAlpn01 validators, as far as I can see. I don't know if it should rather be in the Http01DomainValidator.PrepareHttpChallengeAsync / TlsAlpn01DomainValidator.PrepareTlsAlpnChallengeResponseAsync, but there I don't have access to the configuration without changing the AcmeCertificateFactory.

h3rmanj commented 2 years ago

Apparently had some extra whitespace. Ran the build script locally which automatically fixed it for me and passed locally 🙂

codecov[bot] commented 2 years ago

Codecov Report

Merging #259 (c515a70) into main (1084963) will decrease coverage by 0.07%. The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main     #259      +/-   ##
==========================================
- Coverage   46.23%   46.16%   -0.08%     
==========================================
  Files          45       45              
  Lines        1116     1120       +4     
==========================================
+ Hits          516      517       +1     
- Misses        600      603       +3     
Impacted Files Coverage Δ
src/LettuceEncrypt/Internal/AcmeClient.cs 0.00% <0.00%> (ø)
src/LettuceEncrypt/LettuceEncryptOptions.cs 100.00% <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 1084963...c515a70. Read the comment docs.

h3rmanj commented 2 years ago

I've looked at the failing build for macos, but I can't find it's related to anything i made.

While delaying x amount could work in my scenario, it is a workaround and probably not an optimal solution to add to the library. I think a better solution would be to have a shared validation challenge store, which instances of my application could read and write to, no matter which one requested the domain validation.

After looking through the internals of this library, I think IHttpChallengeResponseStore might be a good place to hook into. It's internal right now, but making it public (and async) would allow consumers to implement their custom store, and also let it be implemented in the Azure integration.

I'll copy this text in the original issue where we can continue the discussion if you have feedback, and I'll try to make another PR. Sorry for taking your time with this PR.

natemcmaster commented 2 years ago

Thanks for taking a look at the issue!