refactor: cleanup implementation of ACME server selection

pangfd · Jan 6, 2020 · 3c0e143 · 3c0e143
1 parent 452f25e
commit 3c0e143
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 36 deletions.
diff --git a/src/LetsEncrypt/Internal/AcmeCertificateLoader.cs b/src/LetsEncrypt/Internal/AcmeCertificateLoader.cs
@@ -155,8 +155,12 @@ private async Task<X509Certificate2> GetOrCreateCertificate(CertificateFactory f
 
             try
             {
-                _logger.LogInformation("Creating certificate for {hostname} using ACME server {acmeServer}", domainName, _options.Value.GetAcmeServer(_hostEnvironment));
+                _logger.LogInformation("Creating certificate for {hostname} using ACME server {acmeServer}",
+                    domainName,
+                    factory.AcmeServer);
+
                 cert = await factory.CreateCertificateAsync(cancellationToken);
+
                 _logger.LogInformation("Created certificate {subjectName} ({thumbprint})", cert.Subject, cert.Thumbprint);
                 return cert;
             }

diff --git a/src/LetsEncrypt/Internal/CertificateFactory.cs b/src/LetsEncrypt/Internal/CertificateFactory.cs
@@ -37,10 +37,12 @@ public CertificateFactory(
             _options = options;
             _challengeStore = challengeStore;
             _logger = logger;
-            var acmeUrl = _options.Value.GetAcmeServer(env);
-            _context = new AcmeContext(acmeUrl);
+            AcmeServer = GetAcmeServer(_options.Value, env);
+            _context = new AcmeContext(AcmeServer);
         }
 
+        public Uri AcmeServer { get; }
+
         public async Task RegisterUserAsync(CancellationToken cancellationToken)
         {
             var options = _options.Value;
@@ -73,6 +75,20 @@ public async Task<X509Certificate2> CreateCertificateAsync(CancellationToken can
             return await CompleteCertificateRequestAsync(order, cancellationToken);
         }
 
+        /// <summary>
+        /// The uri to the server that implements the ACME protocol for certificate generation.
+        /// </summary>
+        internal static Uri GetAcmeServer(LetsEncryptOptions options, IHostEnvironment env)
+        {
+            var useStaging = options.UseStagingServerExplicitlySet
+                ? options.UseStagingServer
+                : env.IsDevelopment();
+
+            return useStaging
+                ? WellKnownServers.LetsEncryptStagingV2
+                : WellKnownServers.LetsEncryptV2;
+        }
+
         private IEnumerable<Task> BeginValidateAllAuthorizations(IEnumerable<IAuthorizationContext> authorizations, CancellationToken cancellationToken)
         {
             foreach (var authorization in authorizations)

diff --git a/src/LetsEncrypt/LetsEncryptOptions.cs b/src/LetsEncrypt/LetsEncryptOptions.cs
@@ -3,8 +3,6 @@
 
 using System;
 using System.Security.Cryptography.X509Certificates;
-using Certes.Acme;
-using Microsoft.Extensions.Hosting;
 
 #if NETSTANDARD2_0
 using IHostEnvironment = Microsoft.Extensions.Hosting.IHostingEnvironment;
@@ -17,8 +15,8 @@ namespace McMaster.AspNetCore.LetsEncrypt
     /// </summary>
     public class LetsEncryptOptions
     {
-        private Uri? _acmeServer;
         private string[] _domainNames = Array.Empty<string>();
+        private bool? _useStagingServer;
 
         /// <summary>
         /// The domain names for which to generate certificates.
@@ -51,12 +49,12 @@ public string[] DomainNames
         /// </summary>
         public bool UseStagingServer
         {
-            get => _acmeServer == WellKnownServers.LetsEncryptStaging;
-            set => _acmeServer = value
-                   ? WellKnownServers.LetsEncryptStagingV2
-                   : WellKnownServers.LetsEncryptV2;
+            get => _useStagingServer ?? false;
+            set => _useStagingServer = value;
         }
 
+        internal bool UseStagingServerExplicitlySet => _useStagingServer.HasValue;
+
         /// <summary>
         /// A certificate to use if a certifcates cannot be created automatically.
         /// <para>
@@ -69,21 +67,5 @@ public bool UseStagingServer
         /// Asymetric encryption algorithm: RS256, ES256, ES384, ES512
         /// </summary>
         public KeyAlgorithm KeyAlgorithm { get; set; } = KeyAlgorithm.ES256;
-
-        /// <summary>
-        /// The uri to the server that implements the ACME protocol for certificate generation.
-        /// </summary>
-        /// <param name="env"></param>
-        internal Uri GetAcmeServer(IHostEnvironment env)
-        {
-            if (_acmeServer != null)
-            {
-                return _acmeServer;
-            }
-
-            return env.IsDevelopment()
-                ? WellKnownServers.LetsEncryptStagingV2
-                : WellKnownServers.LetsEncryptV2;
-        }
     }
 }
diff --git a/src/LetsEncrypt/LetsEncryptServiceCollectionExtensions.cs b/src/LetsEncrypt/LetsEncryptServiceCollectionExtensions.cs
@@ -55,16 +55,7 @@ public static ILetsEncryptServiceBuilder AddLetsEncrypt(this IServiceCollection
             services.AddSingleton<IConfigureOptions<LetsEncryptOptions>>(services =>
             {
                 var config = services.GetService<IConfiguration?>();
-                var hostEnv = services.GetService<IHostEnvironment?>();
-                return new ConfigureOptions<LetsEncryptOptions>(options =>
-                    {
-                        if (hostEnv != null)
-                        {
-                            options.UseStagingServer = hostEnv.IsDevelopment();
-                        }
-
-                        config?.Bind("LetsEncrypt", options);
-                    });
+                return new ConfigureOptions<LetsEncryptOptions>(options => config?.Bind("LetsEncrypt", options));
             });
 
             services.Configure(configure);

diff --git a/test/LetsEncrypt.UnitTests/CertificateFactoryTests.cs b/test/LetsEncrypt.UnitTests/CertificateFactoryTests.cs
@@ -0,0 +1,74 @@
+using System;
+using Certes.Acme;
+using McMaster.AspNetCore.LetsEncrypt;
+using McMaster.AspNetCore.LetsEncrypt.Internal;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Hosting.Internal;
+using Xunit;
+
+#if NETCOREAPP2_1
+using IHostEnvironment = Microsoft.Extensions.Hosting.IHostingEnvironment;
+using Environments = Microsoft.Extensions.Hosting.EnvironmentName;
+#endif
+
+namespace LetsEncrypt.UnitTests
+{
+    public class CertificateFactoryTests
+    {
+        public static TheoryData<string, Uri> EnvironmentToDefaultAcmeServer()
+        {
+            return new TheoryData<string, Uri>
+            {
+                { Environments.Development,  WellKnownServers.LetsEncryptStagingV2 },
+                { Environments.Staging,  WellKnownServers.LetsEncryptV2 },
+                { Environments.Production,  WellKnownServers.LetsEncryptV2 },
+                { null,  WellKnownServers.LetsEncryptV2 },
+            };
+        }
+
+        [Theory]
+        [MemberData(nameof(EnvironmentToDefaultAcmeServer))]
+
+        public void UsesDefaultAcmeServerBasedOnEnvironmentName(string environmentName, Uri acmeServer)
+        {
+            var env = new HostingEnvironment
+            {
+                EnvironmentName = environmentName
+            };
+
+            Assert.Equal(
+                acmeServer,
+                CertificateFactory.GetAcmeServer(new LetsEncryptOptions(), env));
+        }
+
+
+        [Theory]
+        [InlineData("Development")]
+        [InlineData("Production")]
+        public void OverridesDefaultAcmeServer(string environmentName)
+        {
+            var env = new HostingEnvironment
+            {
+                EnvironmentName = environmentName
+            };
+
+            var useStaging = new LetsEncryptOptions
+            {
+                UseStagingServer = true,
+            };
+
+            Assert.Equal(
+                WellKnownServers.LetsEncryptStagingV2,
+                CertificateFactory.GetAcmeServer(useStaging, env));
+
+            var useProduction = new LetsEncryptOptions
+            {
+                UseStagingServer = false,
+            };
+
+            Assert.Equal(
+                WellKnownServers.LetsEncryptV2,
+                CertificateFactory.GetAcmeServer(useProduction, env));
+        }
+    }
+}