Add validation to prevent update of a user or member to an invalid username. (#18263)

Avoid password manager updates of user name field on user details screen.

Author: AndyButland

src/Umbraco.Cms.Api.Management/Controllers/Member/MemberControllerBase.cs

@@ -1,4 +1,4 @@
-using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Umbraco.Cms.Api.Common.Builders;
@@ -58,7 +58,8 @@ protected IActionResult MemberEditingOperationStatusResult(MemberEditingOperatio
                 .WithTitle("Invalid name supplied")
                 .Build()),
             MemberEditingOperationStatus.InvalidUsername => BadRequest(problemDetailsBuilder
-                .WithTitle("Invalid username supplied")
+                .WithTitle("Invalid username")
+                .WithDetail("The username is either empty or contains one or more invalid characters.")
                 .Build()),
             MemberEditingOperationStatus.InvalidEmail => BadRequest(problemDetailsBuilder
                 .WithTitle("Invalid email supplied")

src/Umbraco.Cms.Api.Management/Controllers/User/UserOrCurrentUserControllerBase.cs

@@ -35,6 +35,10 @@ protected IActionResult UserOperationStatusResult(UserOperationStatus status, Er
                 .WithTitle("Email Cannot be changed")
                 .WithDetail("Local login is disabled, so the email cannot be changed.")
                 .Build()),
+            UserOperationStatus.InvalidUserName => BadRequest(problemDetailsBuilder
+                .WithTitle("Invalid username")
+                .WithDetail("The username contains one or more invalid characters.")
+                .Build()),
             UserOperationStatus.DuplicateUserName => BadRequest(problemDetailsBuilder
                 .WithTitle("Duplicate Username")
                 .WithDetail("The username is already in use.")

src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs

@@ -1,4 +1,4 @@
-namespace Umbraco.Cms.Core.Services.OperationStatus;
+namespace Umbraco.Cms.Core.Services.OperationStatus;
 
 /// <summary>
 /// Used to signal a user operation succeeded or an atomic failure reason
@@ -41,4 +41,5 @@ public enum UserOperationStatus
     SelfPasswordResetNotAllowed,
     DuplicateId,
     InvalidUserType,
+    InvalidUserName,
 }

src/Umbraco.Core/Services/UserService.cs

@@ -1,13 +1,11 @@
 using System.ComponentModel.DataAnnotations;
-using System.Globalization;
 using System.Linq.Expressions;
-using Microsoft.Extensions.DependencyInjection;
 using System.Security.Claims;
 using System.Security.Cryptography;
 using System.Text.RegularExpressions;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
-using Umbraco.Cms.Core.Cache;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Editors;
@@ -40,7 +38,6 @@ internal partial class UserService : RepositoryService, IUserService
 {
     private readonly GlobalSettings _globalSettings;
     private readonly SecuritySettings _securitySettings;
-    private readonly ILogger<UserService> _logger;
     private readonly IUserGroupRepository _userGroupRepository;
     private readonly UserEditorAuthorizationHelper _userEditorAuthorizationHelper;
     private readonly IServiceScopeFactory _serviceScopeFactory;
@@ -137,7 +134,6 @@ public UserService(
         _globalSettings = globalSettings.Value;
         _securitySettings = securitySettings.Value;
         _contentSettings = contentSettings.Value;
-        _logger = loggerFactory.CreateLogger<UserService>();
     }
 
     /// <summary>
@@ -966,6 +962,15 @@ private async Task<UserOperationStatus> ValidateUserCreateModel(UserCreateModel
             return Attempt.FailWithStatus<IUser?, UserOperationStatus>(UserOperationStatus.MissingUser, existingUser);
         }
 
+        // User names can only contain the configured allowed characters. This is validated by ASP.NET Identity on create
+        // as the setting is applied to the BackOfficeIdentityOptions, but we need to check ourselves for updates.
+        var allowedUserNameCharacters = _securitySettings.AllowedUserNameCharacters;
+        if (model.UserName.Any(c => allowedUserNameCharacters.Contains(c) == false))
+        {
+            scope.Complete();
+            return Attempt.FailWithStatus<IUser?, UserOperationStatus>(UserOperationStatus.InvalidUserName, existingUser);
+        }
+
         IEnumerable<IUserGroup> allUserGroups = _userGroupRepository.GetMany().ToArray();
         var userGroups = allUserGroups.Where(x => model.UserGroupKeys.Contains(x.Key)).ToHashSet();
 

src/Umbraco.Infrastructure/Services/MemberEditingService.cs

@@ -1,5 +1,9 @@
-using Microsoft.AspNetCore.Identity;
+using Microsoft.AspNetCore.Identity;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentEditing;
 using Umbraco.Cms.Core.Models.Membership;
@@ -19,7 +23,9 @@ internal sealed class MemberEditingService : IMemberEditingService
     private readonly IPasswordChanger<MemberIdentityUser> _passwordChanger;
     private readonly ILogger<MemberEditingService> _logger;
     private readonly IMemberGroupService _memberGroupService;
+    private readonly SecuritySettings _securitySettings;
 
+    [Obsolete("Use the constructor that takes all parameters. Scheduled for removal in V16.")]
     public MemberEditingService(
         IMemberService memberService,
         IMemberTypeService memberTypeService,
@@ -29,6 +35,29 @@ public MemberEditingService(
         IPasswordChanger<MemberIdentityUser> passwordChanger,
         ILogger<MemberEditingService> logger,
         IMemberGroupService memberGroupService)
+        : this(
+              memberService,
+              memberTypeService,
+              memberContentEditingService,
+              memberManager,
+              twoFactorLoginService,
+              passwordChanger,
+              logger,
+              memberGroupService,
+              StaticServiceProvider.Instance.GetRequiredService<IOptions<SecuritySettings>>())
+    {
+    }
+
+    public MemberEditingService(
+        IMemberService memberService,
+        IMemberTypeService memberTypeService,
+        IMemberContentEditingService memberContentEditingService,
+        IMemberManager memberManager,
+        ITwoFactorLoginService twoFactorLoginService,
+        IPasswordChanger<MemberIdentityUser> passwordChanger,
+        ILogger<MemberEditingService> logger,
+        IMemberGroupService memberGroupService,
+        IOptions<SecuritySettings> securitySettings)
     {
         _memberService = memberService;
         _memberTypeService = memberTypeService;
@@ -38,6 +67,7 @@ public MemberEditingService(
         _passwordChanger = passwordChanger;
         _logger = logger;
         _memberGroupService = memberGroupService;
+        _securitySettings = securitySettings.Value;
     }
 
     public async Task<IMember?> GetAsync(Guid key)
@@ -225,6 +255,14 @@ private async Task<MemberEditingOperationStatus> ValidateMemberDataAsync(MemberE
             return MemberEditingOperationStatus.InvalidUsername;
         }
 
+        // User names can only contain the configured allowed characters. This is validated by ASP.NET Identity on create
+        // as the setting is applied to the BackOfficeIdentityOptions, but we need to check ourselves for updates.
+        var allowedUserNameCharacters = _securitySettings.AllowedUserNameCharacters;
+        if (model.Username.Any(c => allowedUserNameCharacters.Contains(c) == false))
+        {
+            return MemberEditingOperationStatus.InvalidUsername;
+        }
+
         if (model.Email.IsEmail() is false)
         {
             return MemberEditingOperationStatus.InvalidEmail;

src/Umbraco.Web.UI.Client/src/packages/user/user/workspace/user/components/user-workspace-profile-settings/user-workspace-profile-settings.element.ts

@@ -97,6 +97,7 @@ export class UmbUserWorkspaceProfileSettingsElement extends UmbLitElement {
 				<uui-input
 					slot="editor"
 					name="username"
+					autocomplete="off"
 					label="${this.localize.term('user_loginname')}"
 					@change="${this.#onUsernameChange}"
 					required