Installer: Fix issues with newsletter signup (#20705)

* Add setter to allow handling of requests to subscribe to newsletter on install.

* Correct serialization of newsletter subscription request.

* Fix serialization and use the Umbraco.EmailMarketing service for newsletter signup.

* Remove logging of user when setting telemetry level.

* Applied suggestions from code review.

Author: AndyButland

src/Umbraco.Cms.Api.Management/ViewModels/Installer/UserInstallRequestModel.cs

@@ -1,4 +1,4 @@
-using System.ComponentModel;
+using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
 
 namespace Umbraco.Cms.Api.Management.ViewModels.Installer;
@@ -17,5 +17,5 @@ public class UserInstallRequestModel
     [PasswordPropertyText]
     public string Password { get; set; } = string.Empty;
 
-    public bool SubscribeToNewsletter { get; }
+    public bool SubscribeToNewsletter { get; set; }
 }

src/Umbraco.Core/Services/MetricsConsentService.cs

@@ -39,11 +39,10 @@ public TelemetryLevel GetConsentLevel()
         return analyticsLevel;
     }
 
-    public async Task SetConsentLevelAsync(TelemetryLevel telemetryLevel)
+    public Task SetConsentLevelAsync(TelemetryLevel telemetryLevel)
     {
-        IUser? currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser ?? await _userService.GetAsync(Constants.Security.SuperUserKey);
-
-        _logger.LogInformation("Telemetry level set to {telemetryLevel} by {username}", telemetryLevel, currentUser?.Username);
+        _logger.LogInformation("Telemetry level set to {telemetryLevel}", telemetryLevel);
         _keyValueService.SetValue(Key, telemetryLevel.ToString());
+        return Task.CompletedTask;
     }
 }

src/Umbraco.Infrastructure/Installer/Steps/CreateUserStep.cs

@@ -1,10 +1,11 @@
-using System.Collections.Specialized;
 using System.Data.Common;
-using System.Text;
 using Microsoft.AspNetCore.Identity;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Installer;
 using Umbraco.Cms.Core.Models.Installer;
 using Umbraco.Cms.Core.Models.Membership;
@@ -32,7 +33,9 @@ public class CreateUserStep : StepBase, IInstallStep
     private readonly IDbProviderFactoryCreator _dbProviderFactoryCreator;
     private readonly IMetricsConsentService _metricsConsentService;
     private readonly IJsonSerializer _jsonSerializer;
+    private readonly ILogger<CreateUserStep> _logger;
 
+    [Obsolete("Please use the constructor that takes all parameters. Scheduled for removal in Umbraco 19.")]
     public CreateUserStep(
         IUserService userService,
         DatabaseBuilder databaseBuilder,
@@ -44,71 +47,132 @@ public CreateUserStep(
         IDbProviderFactoryCreator dbProviderFactoryCreator,
         IMetricsConsentService metricsConsentService,
         IJsonSerializer jsonSerializer)
+        : this(
+            userService,
+            databaseBuilder,
+            httpClientFactory,
+            securitySettings,
+            connectionStrings,
+            cookieManager,
+            userManager,
+            dbProviderFactoryCreator,
+            metricsConsentService,
+            jsonSerializer,
+            StaticServiceProvider.Instance.GetRequiredService<ILogger<CreateUserStep>>())
     {
-        _userService = userService ?? throw new ArgumentNullException(nameof(userService));
-        _databaseBuilder = databaseBuilder ?? throw new ArgumentNullException(nameof(databaseBuilder));
+    }
+
+    public CreateUserStep(
+        IUserService userService,
+        DatabaseBuilder databaseBuilder,
+        IHttpClientFactory httpClientFactory,
+        IOptions<SecuritySettings> securitySettings,
+        IOptionsMonitor<ConnectionStrings> connectionStrings,
+        ICookieManager cookieManager,
+        IBackOfficeUserManager userManager,
+        IDbProviderFactoryCreator dbProviderFactoryCreator,
+        IMetricsConsentService metricsConsentService,
+        IJsonSerializer jsonSerializer,
+        ILogger<CreateUserStep> logger)
+    {
+        _userService = userService;
+        _databaseBuilder = databaseBuilder;
         _httpClientFactory = httpClientFactory;
-        _securitySettings = securitySettings.Value ?? throw new ArgumentNullException(nameof(securitySettings));
+        _securitySettings = securitySettings.Value;
         _connectionStrings = connectionStrings;
         _cookieManager = cookieManager;
-        _userManager = userManager ?? throw new ArgumentNullException(nameof(userManager));
-        _dbProviderFactoryCreator = dbProviderFactoryCreator ?? throw new ArgumentNullException(nameof(dbProviderFactoryCreator));
+        _userManager = userManager;
+        _dbProviderFactoryCreator = dbProviderFactoryCreator;
         _metricsConsentService = metricsConsentService;
         _jsonSerializer = jsonSerializer;
+        _logger = logger;
     }
 
     public async Task<Attempt<InstallationResult>> ExecuteAsync(InstallData model)
     {
-            IUser? admin = _userService.GetAsync(Constants.Security.SuperUserKey).GetAwaiter().GetResult();
-            if (admin is null)
-            {
-                return FailWithMessage("Could not find the super user");
-            }
+        IUser? admin = await _userService.GetAsync(Constants.Security.SuperUserKey);
+        if (admin is null)
+        {
+            return FailWithMessage("Could not find the super user");
+        }
 
-            UserInstallData user = model.User;
-            admin.Email = user.Email.Trim();
-            admin.Name = user.Name.Trim();
-            admin.Username = user.Email.Trim();
+        UserInstallData user = model.User;
+        admin.Email = user.Email.Trim();
+        admin.Name = user.Name.Trim();
+        admin.Username = user.Email.Trim();
 
-            _userService.Save(admin);
+        _userService.Save(admin);
 
-            BackOfficeIdentityUser? membershipUser = await _userManager.FindByIdAsync(Constants.Security.SuperUserIdAsString);
-            if (membershipUser == null)
-            {
-                return FailWithMessage(
-                    $"No user found in membership provider with id of {Constants.Security.SuperUserIdAsString}.");
-            }
+        BackOfficeIdentityUser? membershipUser = await _userManager.FindByIdAsync(Constants.Security.SuperUserIdAsString);
+        if (membershipUser == null)
+        {
+            return FailWithMessage(
+                $"No user found in membership provider with id of {Constants.Security.SuperUserIdAsString}.");
+        }
 
-            // To change the password here we actually need to reset it since we don't have an old one to use to change
-            var resetToken = await _userManager.GeneratePasswordResetTokenAsync(membershipUser);
-            if (string.IsNullOrWhiteSpace(resetToken))
+        // To change the password here we actually need to reset it since we don't have an old one to use to change
+        var resetToken = await _userManager.GeneratePasswordResetTokenAsync(membershipUser);
+        if (string.IsNullOrWhiteSpace(resetToken))
+        {
+            return FailWithMessage("Could not reset password: unable to generate internal reset token");
+        }
+
+        IdentityResult resetResult = await _userManager.ChangePasswordWithResetAsync(membershipUser.Id, resetToken, user.Password.Trim());
+        if (!resetResult.Succeeded)
+        {
+            return FailWithMessage("Could not reset password: " + string.Join(", ", resetResult.Errors.ToErrorMessage()));
+        }
+
+        await _metricsConsentService.SetConsentLevelAsync(model.TelemetryLevel);
+
+        if (model.User.SubscribeToNewsletter)
+        {
+            const string EmailCollectorUrl = "https://emailcollector.umbraco.io/api/EmailProxy";
+
+            var emailModel = new EmailModel
+            {
+                Name = admin.Name,
+                Email = admin.Email,
+                UserGroup = [Constants.Security.AdminGroupAlias],
+            };
+
+            HttpClient httpClient = _httpClientFactory.CreateClient();
+            using var content = new StringContent(_jsonSerializer.Serialize(emailModel), System.Text.Encoding.UTF8, "application/json");
+            try
             {
-                return FailWithMessage("Could not reset password: unable to generate internal reset token");
+                // Set a reasonable timeout of 5 seconds for web request to save subscriber.
+                using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
+                HttpResponseMessage response = await httpClient.PostAsync(EmailCollectorUrl, content, cts.Token);
+                if (response.IsSuccessStatusCode)
+                {
+                    _logger.LogInformation("Successfully subscribed the user created on installation to the Umbraco newsletter.");
+                }
+                else
+                {
+                    _logger.LogWarning("Failed to subscribe the user created on installation to the Umbraco newsletter. Status code: {StatusCode}", response.StatusCode);
+                }
             }
-
-            IdentityResult resetResult = await _userManager.ChangePasswordWithResetAsync(membershipUser.Id, resetToken, user.Password.Trim());
-            if (!resetResult.Succeeded)
+            catch (Exception ex)
             {
-                return FailWithMessage("Could not reset password: " + string.Join(", ", resetResult.Errors.ToErrorMessage()));
+                // Log and move on if a failure occurs, we don't want to block installation for this.
+                _logger.LogError(ex, "Exception occurred while trying to subscribe the user created on installation to the Umbraco newsletter.");
             }
 
-            await _metricsConsentService.SetConsentLevelAsync(model.TelemetryLevel);
+        }
 
-            if (model.User.SubscribeToNewsletter)
-            {
-                var values = new NameValueCollection { { "name", admin.Name }, { "email", admin.Email } };
-                var content = new StringContent(_jsonSerializer.Serialize(values), Encoding.UTF8, "application/json");
+        return Success();
+    }
 
-                HttpClient httpClient = _httpClientFactory.CreateClient();
+    /// <summary>
+    /// Model used to subscribe to the newsletter. Aligns with EmailModel defined in Umbraco.EmailMarketing.
+    /// </summary>
+    private class EmailModel
+    {
+        public required string Name { get; init; }
 
-                try
-                {
-                    HttpResponseMessage response = httpClient.PostAsync("https://shop.umbraco.com/base/Ecom/SubmitEmail/installer.aspx", content).Result;
-                }
-                catch { /* fail in silence */ }
-            }
+        public required string Email { get; init; }
 
-            return Success();
+        public required List<string> UserGroup { get; init; }
     }
 
     /// <inheritdoc/>