diff --git a/src/Umbraco.Cms.Api.Management/Controllers/ManagementApiControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/ManagementApiControllerBase.cs index 42edc572cb..971d3faa63 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/ManagementApiControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/ManagementApiControllerBase.cs @@ -39,5 +39,8 @@ public class ManagementApiControllerBase : Controller } protected static Guid CurrentUserKey(IBackOfficeSecurityAccessor backOfficeSecurityAccessor) - => backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Key ?? Core.Constants.Security.SuperUserKey; + { + //FIXME - Throw if no current user, when we are able to get the current user + return backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Key ?? Core.Constants.Security.SuperUserKey; + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/ByKeyUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/ByKeyUsersController.cs index aa3e48fa5e..29432dbe6c 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/ByKeyUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/ByKeyUsersController.cs @@ -4,6 +4,7 @@ using Umbraco.Cms.Api.Management.Factories; using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Api.Management.Controllers.Users; @@ -30,7 +31,7 @@ public class ByKeyUsersController : UsersControllerBase if (user is null) { - return NotFound(); + return UserOperationStatusResult(UserOperationStatus.UserNotFound); } UserResponseModel responseModel = _userPresentationFactory.CreateResponseModel(user); diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/ChangePasswordUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/ChangePasswordUsersController.cs index 1cc01fcb4f..f273b7cfef 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/ChangePasswordUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/ChangePasswordUsersController.cs @@ -1,10 +1,10 @@ -using System.ComponentModel.DataAnnotations; -using Microsoft.AspNetCore.Mvc; -using Umbraco.Cms.Api.Common.Builders; +using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -13,48 +13,33 @@ namespace Umbraco.Cms.Api.Management.Controllers.Users; public class ChangePasswordUsersController : UsersControllerBase { private readonly IUserService _userService; + private readonly IUmbracoMapper _mapper; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; - public ChangePasswordUsersController(IUserService userService) + public ChangePasswordUsersController( + IUserService userService, + IUmbracoMapper mapper, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) { _userService = userService; + _mapper = mapper; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; } [HttpPost("change-password/{id:guid}")] [MapToApiVersion("1.0")] public async Task ChangePassword(Guid id, ChangePasswordUserRequestModel model) { - IUser? existingUser = await _userService.GetAsync(id); - - if (existingUser is null) - { - return NotFound(); - } - - var passwordModel = new ChangeBackofficeUserPasswordModel + var passwordModel = new ChangeUserPasswordModel { NewPassword = model.NewPassword, OldPassword = model.OldPassword, - User = existingUser, + UserKey = id, }; - // FIXME: use the actual currently logged in user key - Attempt response = await _userService.ChangePasswordAsync(Constants.Security.SuperUserKey, passwordModel); + Attempt response = await _userService.ChangePasswordAsync(CurrentUserKey(_backOfficeSecurityAccessor), passwordModel); - if (response.Success) - { - return Ok(new ChangePasswordUserResponseModel { ResetPassword = response.Result.ResetPassword }); - } - - if (response.Result.ChangeError is not null) - { - ValidationResult validationError = response.Result.ChangeError; - - return BadRequest(new ProblemDetailsBuilder() - .WithTitle("Password change failed") - .WithDetail(validationError.ErrorMessage!) - .Build()); - } - - return UserOperationStatusResult(response.Status); + return response.Success + ? Ok(_mapper.Map(response.Result)) + : UserOperationStatusResult(response.Status, response.Result); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/CreateUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/CreateUsersController.cs index 16ab9633e9..618f6c1ea4 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/CreateUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/CreateUsersController.cs @@ -3,8 +3,10 @@ using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Api.Management.Factories; using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -14,13 +16,19 @@ public class CreateUsersController : UsersControllerBase { private readonly IUserService _userService; private readonly IUserPresentationFactory _presentationFactory; + private readonly IUmbracoMapper _mapper; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; public CreateUsersController( IUserService userService, - IUserPresentationFactory presentationFactory) + IUserPresentationFactory presentationFactory, + IUmbracoMapper mapper, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor) { _userService = userService; _presentationFactory = presentationFactory; + _mapper = mapper; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; } [HttpPost] @@ -31,19 +39,10 @@ public class CreateUsersController : UsersControllerBase { UserCreateModel createModel = await _presentationFactory.CreateCreationModelAsync(model); - // FIXME: use the actual currently logged in user key - Attempt result = await _userService.CreateAsync(Constants.Security.SuperUserKey, createModel, true); + Attempt result = await _userService.CreateAsync(CurrentUserKey(_backOfficeSecurityAccessor), createModel, true); - if (result.Success) - { - return Ok(_presentationFactory.CreateCreationResponseModel(result.Result)); - } - - if (result.Status is UserOperationStatus.UnknownFailure) - { - return FormatErrorMessageResult(result.Result); - } - - return UserOperationStatusResult(result.Status); + return result.Success + ? Ok(_mapper.Map(result.Result)) + : UserOperationStatusResult(result.Status, result.Result); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/DisableUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/DisableUsersController.cs index ae37ff9416..f44d43c567 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/DisableUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/DisableUsersController.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -10,8 +11,13 @@ namespace Umbraco.Cms.Api.Management.Controllers.Users; public class DisableUsersController : UsersControllerBase { private readonly IUserService _userService; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; - public DisableUsersController(IUserService userService) => _userService = userService; + public DisableUsersController(IUserService userService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + { + _userService = userService; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + } [HttpPost("disable")] [MapToApiVersion("1.0")] @@ -19,8 +25,7 @@ public class DisableUsersController : UsersControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] public async Task DisableUsers(DisableUserRequestModel model) { - // FIXME: use the actual currently logged in user key - UserOperationStatus result = await _userService.DisableAsync(Constants.Security.SuperUserKey, model.UserIds); + UserOperationStatus result = await _userService.DisableAsync(CurrentUserKey(_backOfficeSecurityAccessor), model.UserIds); return result is UserOperationStatus.Success ? Ok() diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/EnableUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/EnableUsersController.cs index 7678471865..aed2c1baa8 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/EnableUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/EnableUsersController.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -10,8 +11,13 @@ namespace Umbraco.Cms.Api.Management.Controllers.Users; public class EnableUsersController : UsersControllerBase { private readonly IUserService _userService; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; - public EnableUsersController(IUserService userService) => _userService = userService; + public EnableUsersController(IUserService userService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + { + _userService = userService; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + } [HttpPost("enable")] [MapToApiVersion("1.0")] @@ -19,8 +25,7 @@ public class EnableUsersController : UsersControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] public async Task EnableUsers(EnableUserRequestModel model) { - // FIXME: use the actual currently logged in user key - UserOperationStatus result = await _userService.EnableAsync(Constants.Security.SuperUserKey, model.UserIds); + UserOperationStatus result = await _userService.EnableAsync(CurrentUserKey(_backOfficeSecurityAccessor), model.UserIds); return result is UserOperationStatus.Success ? Ok() diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/FilterUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/FilterUsersController.cs index b646abe02f..1b3fff5e5c 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/FilterUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/FilterUsersController.cs @@ -56,9 +56,8 @@ public class FilterUsersController : UsersControllerBase NameFilters = string.IsNullOrEmpty(filter) ? null : new SortedSet { filter } }; - // FIXME: use the actual currently logged in user key Attempt, UserOperationStatus> filterAttempt = - await _userService.FilterAsync(Constants.Security.SuperUserKey, userFilter, skip, take, orderBy, orderDirection); + await _userService.FilterAsync(CurrentUserKey(_backOfficeSecurityAccessor), userFilter, skip, take, orderBy, orderDirection); if (filterAttempt.Success is false) { diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/GetAllUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/GetAllUsersController.cs index ee033d841f..fcd353a47a 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/GetAllUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/GetAllUsersController.cs @@ -34,8 +34,7 @@ public class GetAllUsersController : UsersControllerBase [ProducesResponseType(typeof(PagedViewModel), StatusCodes.Status200OK)] public async Task GetAll(int skip = 0, int take = 100) { - // FIXME: use the actual currently logged in user key - Attempt?, UserOperationStatus> attempt = await _userService.GetAllAsync(Constants.Security.SuperUserKey, skip, take); + Attempt?, UserOperationStatus> attempt = await _userService.GetAllAsync(CurrentUserKey(_backOfficeSecurityAccessor), skip, take); if (attempt.Success is false) { diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/InviteUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/InviteUsersController.cs index fc504a7120..5b20b7d961 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/InviteUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/InviteUsersController.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -14,13 +15,15 @@ public class InviteUsersController : UsersControllerBase { private readonly IUserService _userService; private readonly IUserPresentationFactory _userPresentationFactory; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; public InviteUsersController( IUserService userService, - IUserPresentationFactory userPresentationFactory) + IUserPresentationFactory userPresentationFactory, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) { _userService = userService; _userPresentationFactory = userPresentationFactory; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; } @@ -31,16 +34,10 @@ public class InviteUsersController : UsersControllerBase { UserInviteModel userInvite = await _userPresentationFactory.CreateInviteModelAsync(model); - // FIXME: use the actual currently logged in user key - Attempt result = await _userService.InviteAsync(Constants.Security.SuperUserKey, userInvite); + Attempt result = await _userService.InviteAsync(CurrentUserKey(_backOfficeSecurityAccessor), userInvite); - if (result.Success) - { - return Ok(); - } - - return result.Status is UserOperationStatus.UnknownFailure - ? FormatErrorMessageResult(result.Result) - : UserOperationStatusResult(result.Status); + return result.Success + ? Ok() + : UserOperationStatusResult(result.Status, result.Result); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/SetAvatarUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/SetAvatarUsersController.cs index e57b0fa578..e35867a8d4 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/SetAvatarUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/SetAvatarUsersController.cs @@ -21,14 +21,7 @@ public class SetAvatarUsersController : UsersControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] public async Task SetAvatar(Guid id, SetAvatarRequestModel model) { - IUser? user = await _userService.GetAsync(id); - - if (user is null) - { - return NotFound(); - } - - UserOperationStatus result = await _userService.SetAvatarAsync(user, model.FileId); + UserOperationStatus result = await _userService.SetAvatarAsync(id, model.FileId); return result is UserOperationStatus.Success ? Ok() diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/UnlockUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/UnlockUsersController.cs index 41d70ae3e1..be189c6866 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/UnlockUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/UnlockUsersController.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -11,8 +12,13 @@ namespace Umbraco.Cms.Api.Management.Controllers.Users; public class UnlockUsersController : UsersControllerBase { private readonly IUserService _userService; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; - public UnlockUsersController(IUserService userService) => _userService = userService; + public UnlockUsersController(IUserService userService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + { + _userService = userService; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + } [HttpPost("unlock")] [MapToApiVersion("1.0")] @@ -20,16 +26,10 @@ public class UnlockUsersController : UsersControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] public async Task UnlockUsers(UnlockUsersRequestModel model) { - // FIXME: use the actual currently logged in user key - Attempt attempt = await _userService.UnlockAsync(Constants.Security.SuperUserKey, model.UserIds.ToArray()); + Attempt attempt = await _userService.UnlockAsync(CurrentUserKey(_backOfficeSecurityAccessor), model.UserIds.ToArray()); - if (attempt.Success) - { - return Ok(); - } - - return attempt.Status is UserOperationStatus.UnknownFailure - ? FormatErrorMessageResult(attempt.Result) - : UserOperationStatusResult(attempt.Status); + return attempt.Success + ? Ok() + : UserOperationStatusResult(attempt.Status, attempt.Result); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/UpdateUsersController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/UpdateUsersController.cs index 6daca51b62..cff5556388 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/UpdateUsersController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/UpdateUsersController.cs @@ -4,6 +4,7 @@ using Umbraco.Cms.Api.Management.ViewModels.Users; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; @@ -13,32 +14,26 @@ public class UpdateUsersController : UsersControllerBase { private readonly IUserService _userService; private readonly IUserPresentationFactory _userPresentationFactory; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; public UpdateUsersController( IUserService userService, - IUserPresentationFactory userPresentationFactory) + IUserPresentationFactory userPresentationFactory, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) { _userService = userService; _userPresentationFactory = userPresentationFactory; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; } [HttpPut("{id:guid}")] [MapToApiVersion("1.0")] public async Task Update(Guid id, UpdateUserRequestModel model) { - IUser? existingUser = await _userService.GetAsync(id); - - if (existingUser is null) - { - return NotFound(); - } - - // We have to use and intermediate save model, and cannot map it directly to an IUserModel + // We have to use an intermediate save model, and cannot map it directly to an IUserModel // This is because we need to compare the updated values with what the user already has, for audit purposes. - UserUpdateModel updateModel = await _userPresentationFactory.CreateUpdateModelAsync(existingUser, model); + UserUpdateModel updateModel = await _userPresentationFactory.CreateUpdateModelAsync(id, model); - // FIXME: use the actual currently logged in user key - Attempt result = await _userService.UpdateAsync(Constants.Security.SuperUserKey, updateModel); + Attempt result = await _userService.UpdateAsync(CurrentUserKey(_backOfficeSecurityAccessor), updateModel); return result.Success ? Ok() diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Users/UsersControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Users/UsersControllerBase.cs index 4c2c88bb84..12b4173f0e 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Users/UsersControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Users/UsersControllerBase.cs @@ -13,7 +13,7 @@ namespace Umbraco.Cms.Api.Management.Controllers.Users; [ApiVersion("1.0")] public abstract class UsersControllerBase : ManagementApiControllerBase { - protected IActionResult UserOperationStatusResult(UserOperationStatus status) => + protected IActionResult UserOperationStatusResult(UserOperationStatus status, ErrorMessageResult? errorMessageResult = null) => status switch { UserOperationStatus.Success => Ok(), @@ -64,17 +64,39 @@ public abstract class UsersControllerBase : ManagementApiControllerBase .WithTitle("Invalid avatar") .WithDetail("The selected avatar is invalid") .Build()), - UserOperationStatus.NotFound => NotFound(), + UserOperationStatus.InvalidEmail => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Invalid email") + .WithDetail("The email is invalid") + .Build()), + UserOperationStatus.AvatarFileNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Avatar file not found") + .WithDetail("The file key did not resolve in to a file") + .Build()), + UserOperationStatus.ContentStartNodeNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Content Start Node not found") + .WithDetail("Some of the provided content start nodes was not found.") + .Build()), + UserOperationStatus.MediaStartNodeNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Media Start Node not found") + .WithDetail("Some of the provided media start nodes was not found.") + .Build()), + UserOperationStatus.UserNotFound => NotFound(new ProblemDetailsBuilder() + .WithTitle("The was not found") + .WithDetail("The specified user was not found.") + .Build()), UserOperationStatus.CannotDisableInvitedUser => BadRequest(new ProblemDetailsBuilder() .WithTitle("Cannot disable invited user") .WithDetail("An invited user cannot be disabled.") .Build()), + UserOperationStatus.UnknownFailure => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Unknown failure") + .WithDetail(errorMessageResult?.Error?.ErrorMessage ?? "The error was unknown") + .Build()), + UserOperationStatus.InvalidIsoCode => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Invalid ISO code") + .WithDetail("The specified ISO code is invalid.") + .Build()), + UserOperationStatus.Forbidden => Forbid(), _ => StatusCode(StatusCodes.Status500InternalServerError, "Unknown user operation status."), }; - - protected IActionResult FormatErrorMessageResult(IErrorMessageResult errorMessageResult) => - BadRequest(new ProblemDetailsBuilder() - .WithTitle("An error occured.") - .WithDetail(errorMessageResult.ErrorMessage ?? "The error was unknown") - .Build()); } diff --git a/src/Umbraco.Cms.Api.Management/DependencyInjection/UsersBuilderExtensions.cs b/src/Umbraco.Cms.Api.Management/DependencyInjection/UsersBuilderExtensions.cs index fec138aca0..65ee9d4c44 100644 --- a/src/Umbraco.Cms.Api.Management/DependencyInjection/UsersBuilderExtensions.cs +++ b/src/Umbraco.Cms.Api.Management/DependencyInjection/UsersBuilderExtensions.cs @@ -1,6 +1,8 @@ using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Api.Management.Factories; +using Umbraco.Cms.Api.Management.Mapping.Users; using Umbraco.Cms.Core.DependencyInjection; +using Umbraco.Cms.Core.Mapping; namespace Umbraco.Cms.Api.Management.DependencyInjection; @@ -9,6 +11,10 @@ internal static class UsersBuilderExtensions internal static IUmbracoBuilder AddUsers(this IUmbracoBuilder builder) { builder.Services.AddTransient(); + + builder.WithCollectionBuilder() + .Add(); + return builder; } } diff --git a/src/Umbraco.Cms.Api.Management/Factories/IUserPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/IUserPresentationFactory.cs index a8e091bb1a..22d28d25d8 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/IUserPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/IUserPresentationFactory.cs @@ -12,7 +12,5 @@ public interface IUserPresentationFactory Task CreateInviteModelAsync(InviteUserRequestModel requestModel); - Task CreateUpdateModelAsync(IUser existingUser, UpdateUserRequestModel updateModel); - - CreateUserResponseModel CreateCreationResponseModel(UserCreationResult creationResult); + Task CreateUpdateModelAsync(Guid existingUserKey, UpdateUserRequestModel updateModel); } diff --git a/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs index d2aaba7335..8371df7604 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs @@ -14,20 +14,17 @@ public class UserPresentationFactory : IUserPresentationFactory private readonly AppCaches _appCaches; private readonly MediaFileManager _mediaFileManager; private readonly IImageUrlGenerator _imageUrlGenerator; - private readonly IUserGroupService _userGroupService; public UserPresentationFactory( IEntityService entityService, AppCaches appCaches, MediaFileManager mediaFileManager, - IImageUrlGenerator imageUrlGenerator, - IUserGroupService userGroupService) + IImageUrlGenerator imageUrlGenerator) { _entityService = entityService; _appCaches = appCaches; _mediaFileManager = mediaFileManager; _imageUrlGenerator = imageUrlGenerator; - _userGroupService = userGroupService; } public UserResponseModel CreateResponseModel(IUser user) @@ -57,14 +54,12 @@ public class UserPresentationFactory : IUserPresentationFactory public async Task CreateCreationModelAsync(CreateUserRequestModel requestModel) { - IEnumerable groups = await _userGroupService.GetAsync(requestModel.UserGroupIds); - var createModel = new UserCreateModel { Email = requestModel.Email, Name = requestModel.Name, UserName = requestModel.UserName, - UserGroups = new HashSet(groups), + UserGroupKeys = requestModel.UserGroupIds, }; return createModel; @@ -72,46 +67,36 @@ public class UserPresentationFactory : IUserPresentationFactory public async Task CreateInviteModelAsync(InviteUserRequestModel requestModel) { - IEnumerable groups = await _userGroupService.GetAsync(requestModel.UserGroupIds); - var inviteModel = new UserInviteModel { Email = requestModel.Email, Name = requestModel.Name, UserName = requestModel.UserName, - UserGroups = new HashSet(groups), + UserGroupKeys = requestModel.UserGroupIds, Message = requestModel.Message, }; return inviteModel; } - public async Task CreateUpdateModelAsync(IUser existingUser, UpdateUserRequestModel updateModel) + public async Task CreateUpdateModelAsync(Guid existingUserKey, UpdateUserRequestModel updateModel) { var model = new UserUpdateModel { - ExistingUser = existingUser, + ExistingUserKey = existingUserKey, Email = updateModel.Email, Name = updateModel.Name, UserName = updateModel.UserName, - Language = updateModel.LanguageIsoCode, + LanguageIsoCode = updateModel.LanguageIsoCode, ContentStartNodeKeys = updateModel.ContentStartNodeIds, MediaStartNodeKeys = updateModel.MediaStartNodeIds, }; - IEnumerable userGroups = await _userGroupService.GetAsync(updateModel.UserGroupIds); - model.UserGroups = userGroups; + model.UserGroupKeys = updateModel.UserGroupIds; return model; } - public CreateUserResponseModel CreateCreationResponseModel(UserCreationResult creationResult) - => new() - { - UserId = creationResult.CreatedUser?.Key ?? Guid.Empty, - InitialPassword = creationResult.InitialPassword, - }; - private SortedSet GetKeysFromIds(IEnumerable? ids, UmbracoObjectTypes type) { IEnumerable? keys = ids? diff --git a/src/Umbraco.Cms.Api.Management/Mapping/Users/UsersViewModelsMapDefinition.cs b/src/Umbraco.Cms.Api.Management/Mapping/Users/UsersViewModelsMapDefinition.cs new file mode 100644 index 0000000000..f0561551c4 --- /dev/null +++ b/src/Umbraco.Cms.Api.Management/Mapping/Users/UsersViewModelsMapDefinition.cs @@ -0,0 +1,26 @@ +using Umbraco.Cms.Api.Management.ViewModels.Users; +using Umbraco.Cms.Core.Mapping; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; + +namespace Umbraco.Cms.Api.Management.Mapping.Users; + +public class UsersViewModelsMapDefinition : IMapDefinition +{ + public void DefineMaps(IUmbracoMapper mapper) + { + mapper.Define((_, _) => new ChangePasswordUserResponseModel(), Map); + mapper.Define((_, _) => new CreateUserResponseModel(), Map); + } + + private void Map(UserCreationResult source, CreateUserResponseModel target, MapperContext context) + { + target.UserId = source.CreatedUser?.Key ?? Guid.Empty; + target.InitialPassword = source.InitialPassword; + } + + private void Map(PasswordChangedModel source, ChangePasswordUserResponseModel target, MapperContext context) + { + target.ResetPassword = source.ResetPassword; + } +} diff --git a/src/Umbraco.Core/CompatibilitySuppressions.xml b/src/Umbraco.Core/CompatibilitySuppressions.xml index 2f97b7ca10..b11593e5d0 100644 --- a/src/Umbraco.Core/CompatibilitySuppressions.xml +++ b/src/Umbraco.Core/CompatibilitySuppressions.xml @@ -518,6 +518,20 @@ lib/net7.0/Umbraco.Core.dll true + + CP0002 + M:Umbraco.Cms.Core.Models.PasswordChangedModel.get_ChangeError + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + + + CP0002 + M:Umbraco.Cms.Core.Models.PasswordChangedModel.set_ChangeError(System.ComponentModel.DataAnnotations.ValidationResult) + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + CP0002 M:Umbraco.Cms.Core.Models.RelationType.#ctor(System.String,System.String,System.Boolean,System.Nullable{System.Guid},System.Nullable{System.Guid},System.Boolean) @@ -1218,6 +1232,13 @@ lib/net7.0/Umbraco.Core.dll true + + CP0006 + M:Umbraco.Cms.Core.Services.IUserService.ChangePasswordAsync(System.Guid,Umbraco.Cms.Core.Models.Membership.ChangeUserPasswordModel) + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + CP0006 M:Umbraco.Cms.Core.Services.IUserService.ClearAvatarAsync(System.Guid) @@ -1274,6 +1295,13 @@ lib/net7.0/Umbraco.Core.dll true + + CP0006 + M:Umbraco.Cms.Core.Services.IUserService.SetAvatarAsync(System.Guid,System.Guid) + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + CP0006 M:Umbraco.Cms.Core.Services.IUserService.SetAvatarAsync(Umbraco.Cms.Core.Models.Membership.IUser,System.Guid) diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 80f298d2e8..a269e4681f 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -286,6 +286,7 @@ namespace Umbraco.Cms.Core.DependencyInjection Services.AddUnique(); Services.AddUnique(); Services.AddUnique(); + Services.AddUnique(); Services.AddUnique(); Services.AddUnique(); Services.AddUnique(); diff --git a/src/Umbraco.Core/Models/Membership/ChangeBackofficeUserPasswordModel.cs b/src/Umbraco.Core/Models/Membership/ChangeBackOfficeUserPasswordModel.cs similarity index 89% rename from src/Umbraco.Core/Models/Membership/ChangeBackofficeUserPasswordModel.cs rename to src/Umbraco.Core/Models/Membership/ChangeBackOfficeUserPasswordModel.cs index 5723d57fe4..d2b9219e2c 100644 --- a/src/Umbraco.Core/Models/Membership/ChangeBackofficeUserPasswordModel.cs +++ b/src/Umbraco.Core/Models/Membership/ChangeBackOfficeUserPasswordModel.cs @@ -1,6 +1,6 @@ namespace Umbraco.Cms.Core.Models.Membership; -public class ChangeBackofficeUserPasswordModel +public class ChangeBackOfficeUserPasswordModel { public required string NewPassword { get; set; } diff --git a/src/Umbraco.Core/Models/Membership/ChangeUserPasswordModel.cs b/src/Umbraco.Core/Models/Membership/ChangeUserPasswordModel.cs new file mode 100644 index 0000000000..ac0dbefc6a --- /dev/null +++ b/src/Umbraco.Core/Models/Membership/ChangeUserPasswordModel.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Cms.Core.Models.Membership; + +public class ChangeUserPasswordModel +{ + public required string NewPassword { get; set; } + + public string? OldPassword { get; set; } + + public Guid UserKey { get; set; } +} diff --git a/src/Umbraco.Core/Models/Membership/IErrorMessageResult.cs b/src/Umbraco.Core/Models/Membership/IErrorMessageResult.cs index ca90b54bb3..9dcd15fafa 100644 --- a/src/Umbraco.Core/Models/Membership/IErrorMessageResult.cs +++ b/src/Umbraco.Core/Models/Membership/IErrorMessageResult.cs @@ -1,6 +1,8 @@ -namespace Umbraco.Cms.Core.Models.Membership; +using System.ComponentModel.DataAnnotations; -public interface IErrorMessageResult +namespace Umbraco.Cms.Core.Models.Membership; + +public abstract class ErrorMessageResult { - public string? ErrorMessage { get; } + public ValidationResult? Error { get; set; } } diff --git a/src/Umbraco.Core/Models/Membership/UserCreationResult.cs b/src/Umbraco.Core/Models/Membership/UserCreationResult.cs index 9476492905..7efdddbede 100644 --- a/src/Umbraco.Core/Models/Membership/UserCreationResult.cs +++ b/src/Umbraco.Core/Models/Membership/UserCreationResult.cs @@ -1,10 +1,8 @@ namespace Umbraco.Cms.Core.Models.Membership; -public class UserCreationResult : IErrorMessageResult +public class UserCreationResult : ErrorMessageResult { public IUser? CreatedUser { get; init; } public string? InitialPassword { get; init; } - - public string? ErrorMessage { get; init; } } diff --git a/src/Umbraco.Core/Models/Membership/UserInvitationResult.cs b/src/Umbraco.Core/Models/Membership/UserInvitationResult.cs index bb5de140a3..4a614a01ab 100644 --- a/src/Umbraco.Core/Models/Membership/UserInvitationResult.cs +++ b/src/Umbraco.Core/Models/Membership/UserInvitationResult.cs @@ -1,8 +1,6 @@ namespace Umbraco.Cms.Core.Models.Membership; -public class UserInvitationResult: IErrorMessageResult +public class UserInvitationResult: ErrorMessageResult { public IUser? InvitedUser { get; init; } - - public string? ErrorMessage { get; init; } } diff --git a/src/Umbraco.Core/Models/Membership/UserUnlockResult.cs b/src/Umbraco.Core/Models/Membership/UserUnlockResult.cs index a40860efda..a37cb3587c 100644 --- a/src/Umbraco.Core/Models/Membership/UserUnlockResult.cs +++ b/src/Umbraco.Core/Models/Membership/UserUnlockResult.cs @@ -1,6 +1,5 @@ namespace Umbraco.Cms.Core.Models.Membership; -public class UserUnlockResult : IErrorMessageResult +public class UserUnlockResult : ErrorMessageResult { - public string? ErrorMessage { get; init; } } diff --git a/src/Umbraco.Core/Models/PasswordChangedModel.cs b/src/Umbraco.Core/Models/PasswordChangedModel.cs index 0cd405e604..5cf47d1bc7 100644 --- a/src/Umbraco.Core/Models/PasswordChangedModel.cs +++ b/src/Umbraco.Core/Models/PasswordChangedModel.cs @@ -1,17 +1,13 @@ using System.ComponentModel.DataAnnotations; +using Umbraco.Cms.Core.Models.Membership; namespace Umbraco.Cms.Core.Models; /// /// A model representing an attempt at changing a password /// -public class PasswordChangedModel +public class PasswordChangedModel : ErrorMessageResult { - /// - /// The error affiliated with the failing password changes, null if changing was successful - /// - public ValidationResult? ChangeError { get; set; } - /// /// If the password was reset, this is the value it has been changed to /// diff --git a/src/Umbraco.Core/Models/UserCreateModel.cs b/src/Umbraco.Core/Models/UserCreateModel.cs index bb446fdfd8..fafd47946c 100644 --- a/src/Umbraco.Core/Models/UserCreateModel.cs +++ b/src/Umbraco.Core/Models/UserCreateModel.cs @@ -10,5 +10,5 @@ public class UserCreateModel public string Name { get; set; } = string.Empty; - public HashSet UserGroups { get; set; } = new(); + public ISet UserGroupKeys { get; set; } = new HashSet(); } diff --git a/src/Umbraco.Core/Models/UserUpdateModel.cs b/src/Umbraco.Core/Models/UserUpdateModel.cs index e5e7e8b490..2046bdc4a7 100644 --- a/src/Umbraco.Core/Models/UserUpdateModel.cs +++ b/src/Umbraco.Core/Models/UserUpdateModel.cs @@ -1,10 +1,8 @@ -using Umbraco.Cms.Core.Models.Membership; - -namespace Umbraco.Cms.Core.Models; +namespace Umbraco.Cms.Core.Models; public class UserUpdateModel { - public required IUser ExistingUser { get; set; } + public required Guid ExistingUserKey { get; set; } public string Email { get; set; } = string.Empty; @@ -12,11 +10,11 @@ public class UserUpdateModel public string Name { get; set; } = string.Empty; - public string Language { get; set; } = string.Empty; + public string LanguageIsoCode { get; set; } = string.Empty; public SortedSet ContentStartNodeKeys { get; set; } = new(); public SortedSet MediaStartNodeKeys { get; set; } = new(); - public IEnumerable UserGroups { get; set; } = Enumerable.Empty(); + public ISet UserGroupKeys { get; set; } = new HashSet(); } diff --git a/src/Umbraco.Core/Security/IBackOfficePasswordChanger.cs b/src/Umbraco.Core/Security/IBackOfficePasswordChanger.cs new file mode 100644 index 0000000000..d0e2254f68 --- /dev/null +++ b/src/Umbraco.Core/Security/IBackOfficePasswordChanger.cs @@ -0,0 +1,9 @@ +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; + +namespace Umbraco.Cms.Core.Security; + +public interface IBackOfficePasswordChanger +{ + Task> ChangeBackOfficePassword(ChangeBackOfficeUserPasswordModel model); +} diff --git a/src/Umbraco.Core/Security/IBackofficeUserStore.cs b/src/Umbraco.Core/Security/IBackOfficeUserStore.cs similarity index 98% rename from src/Umbraco.Core/Security/IBackofficeUserStore.cs rename to src/Umbraco.Core/Security/IBackOfficeUserStore.cs index 53d7b9623b..a89c97c75d 100644 --- a/src/Umbraco.Core/Security/IBackofficeUserStore.cs +++ b/src/Umbraco.Core/Security/IBackOfficeUserStore.cs @@ -6,7 +6,7 @@ namespace Umbraco.Cms.Core.Security; /// /// Manages persistence of users. /// -public interface IBackofficeUserStore +public interface IBackOfficeUserStore { /// /// Saves an diff --git a/src/Umbraco.Core/Security/IBackofficePasswordChanger.cs b/src/Umbraco.Core/Security/IBackofficePasswordChanger.cs deleted file mode 100644 index 0c6813a638..0000000000 --- a/src/Umbraco.Core/Security/IBackofficePasswordChanger.cs +++ /dev/null @@ -1,9 +0,0 @@ -using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.Membership; - -namespace Umbraco.Cms.Core.Security; - -public interface IBackofficePasswordChanger -{ - Task> ChangeBackofficePassword(ChangeBackofficeUserPasswordModel model); -} diff --git a/src/Umbraco.Core/Security/ICoreBackofficeUserManager.cs b/src/Umbraco.Core/Security/ICoreBackOfficeUserManager.cs similarity index 94% rename from src/Umbraco.Core/Security/ICoreBackofficeUserManager.cs rename to src/Umbraco.Core/Security/ICoreBackOfficeUserManager.cs index 2ce3260925..b38dfea345 100644 --- a/src/Umbraco.Core/Security/ICoreBackofficeUserManager.cs +++ b/src/Umbraco.Core/Security/ICoreBackOfficeUserManager.cs @@ -4,7 +4,7 @@ using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Security; -public interface ICoreBackofficeUserManager +public interface ICoreBackOfficeUserManager { Task CreateAsync(UserCreateModel createModel); diff --git a/src/Umbraco.Core/Services/IIsoCodeValidator.cs b/src/Umbraco.Core/Services/IIsoCodeValidator.cs new file mode 100644 index 0000000000..28de55de2c --- /dev/null +++ b/src/Umbraco.Core/Services/IIsoCodeValidator.cs @@ -0,0 +1,14 @@ +namespace Umbraco.Cms.Core.Services; + +/// +/// A validator for validating if an ISO code string can be is valid. +/// +public interface IIsoCodeValidator +{ + /// + /// Validates that a string is a valid ISO code. + /// + /// The string to validate. + /// True if the string is a valid ISO code. + bool IsValid(string isoCode); +} diff --git a/src/Umbraco.Core/Services/IUserIdKeyResolver.cs b/src/Umbraco.Core/Services/IUserIdKeyResolver.cs index 85008c28b5..de963567e3 100644 --- a/src/Umbraco.Core/Services/IUserIdKeyResolver.cs +++ b/src/Umbraco.Core/Services/IUserIdKeyResolver.cs @@ -2,7 +2,17 @@ public interface IUserIdKeyResolver { + /// + /// Tries to resolve a user key to a user id without fetching the entire user. + /// + /// The key of the user. + /// The id of the user, null if the user doesn't exist. public Task GetAsync(Guid key); + /// + /// Tries to resolve a user id to a user key without fetching the entire user. + /// + /// The id of the user. + /// The key of the user, null if the user doesn't exist. public Task GetAsync(int id); } diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index 033c1db977..cc61ef895a 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -59,9 +59,9 @@ public interface IUserService : IMembershipUserService Task> InviteAsync(Guid performingUserKey, UserInviteModel model); - Task> UpdateAsync(Guid performingUserKey, UserUpdateModel model); + Task> UpdateAsync(Guid performingUserKey, UserUpdateModel model); - Task SetAvatarAsync(IUser user, Guid temporaryFileKey); + Task SetAvatarAsync(Guid userKey, Guid temporaryFileKey); Task DeleteAsync(Guid key); @@ -71,7 +71,7 @@ public interface IUserService : IMembershipUserService Task> UnlockAsync(Guid performingUserKey, params Guid[] keys); - Task> ChangePasswordAsync(Guid performingUserKey, ChangeBackofficeUserPasswordModel model); + Task> ChangePasswordAsync(Guid performingUserKey, ChangeUserPasswordModel model); Task ClearAvatarAsync(Guid userKey); diff --git a/src/Umbraco.Core/Services/IsoCodeValidator.cs b/src/Umbraco.Core/Services/IsoCodeValidator.cs new file mode 100644 index 0000000000..2ba12d58b0 --- /dev/null +++ b/src/Umbraco.Core/Services/IsoCodeValidator.cs @@ -0,0 +1,21 @@ +using System.Globalization; + +namespace Umbraco.Cms.Core.Services; + +/// +public class IsoCodeValidator : IIsoCodeValidator +{ + /// + public bool IsValid(string isoCode) + { + try + { + var culture = CultureInfo.GetCultureInfo(isoCode); + return culture.Name == isoCode && culture.CultureTypes.HasFlag(CultureTypes.UserCustomCulture) == false; + } + catch (CultureNotFoundException) + { + return false; + } + } +} diff --git a/src/Umbraco.Core/Services/LanguageService.cs b/src/Umbraco.Core/Services/LanguageService.cs index 48e5b2ddd3..ab7f181ba3 100644 --- a/src/Umbraco.Core/Services/LanguageService.cs +++ b/src/Umbraco.Core/Services/LanguageService.cs @@ -15,6 +15,7 @@ internal sealed class LanguageService : RepositoryService, ILanguageService private readonly ILanguageRepository _languageRepository; private readonly IAuditRepository _auditRepository; private readonly IUserIdKeyResolver _userIdKeyResolver; + private readonly IIsoCodeValidator _isoCodeValidator; public LanguageService( ICoreScopeProvider provider, @@ -22,12 +23,14 @@ internal sealed class LanguageService : RepositoryService, ILanguageService IEventMessagesFactory eventMessagesFactory, ILanguageRepository languageRepository, IAuditRepository auditRepository, - IUserIdKeyResolver userIdKeyResolver) + IUserIdKeyResolver userIdKeyResolver, + IIsoCodeValidator isoCodeValidator) : base(provider, loggerFactory, eventMessagesFactory) { _languageRepository = languageRepository; _auditRepository = auditRepository; _userIdKeyResolver = userIdKeyResolver; + _isoCodeValidator = isoCodeValidator; } /// @@ -153,11 +156,12 @@ internal sealed class LanguageService : RepositoryService, ILanguageService string auditMessage, Guid userKey) { - if (IsValidIsoCode(language.IsoCode) == false) + if (_isoCodeValidator.IsValid(language.IsoCode) == false) { return Attempt.FailWithStatus(LanguageOperationStatus.InvalidIsoCode, language); } - if (language.FallbackIsoCode is not null && IsValidIsoCode(language.FallbackIsoCode) == false) + + if (language.FallbackIsoCode is not null && _isoCodeValidator.IsValid(language.FallbackIsoCode) == false) { return Attempt.FailWithStatus(LanguageOperationStatus.InvalidFallbackIsoCode, language); } @@ -256,17 +260,4 @@ internal sealed class LanguageService : RepositoryService, ILanguageService isoCode = languagesByIsoCode[isoCode].FallbackIsoCode; // else keep chaining } } - - private static bool IsValidIsoCode(string isoCode) - { - try - { - var culture = CultureInfo.GetCultureInfo(isoCode); - return culture.Name == isoCode && culture.CultureTypes.HasFlag(CultureTypes.UserCustomCulture) == false; - } - catch (CultureNotFoundException) - { - return false; - } - } } diff --git a/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs index 0de3548683..e0d012bc8f 100644 --- a/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs @@ -9,16 +9,21 @@ public enum UserOperationStatus EmailCannotBeChanged, NoUserGroup, DuplicateUserName, + InvalidEmail, DuplicateEmail, Unauthorized, Forbidden, CancelledByNotification, - NotFound, + UserNotFound, + AvatarFileNotFound, CannotInvite, CannotDelete, CannotDisableSelf, CannotDisableInvitedUser, OldPasswordRequired, InvalidAvatar, + InvalidIsoCode, + ContentStartNodeNotFound, + MediaStartNodeNotFound, UnknownFailure, } diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index ccc1511cc3..9a365ecef6 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -1,4 +1,4 @@ -using System.Formats.Asn1; +using System.ComponentModel.DataAnnotations; using System.Globalization; using System.Linq.Expressions; using System.Security.Cryptography; @@ -23,7 +23,6 @@ using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Core.Strings; using Umbraco.Extensions; using Umbraco.New.Cms.Core.Models; -using Umbraco.Extensions; using UserProfile = Umbraco.Cms.Core.Models.Membership.UserProfile; namespace Umbraco.Cms.Core.Services; @@ -46,6 +45,7 @@ internal class UserService : RepositoryService, IUserService private readonly MediaFileManager _mediaFileManager; private readonly ITemporaryFileService _temporaryFileService; private readonly IShortStringHelper _shortStringHelper; + private readonly IIsoCodeValidator _isoCodeValidator; private readonly IUserRepository _userRepository; private readonly ContentSettings _contentSettings; @@ -65,7 +65,8 @@ internal class UserService : RepositoryService, IUserService MediaFileManager mediaFileManager, ITemporaryFileService temporaryFileService, IShortStringHelper shortStringHelper, - IOptions contentSettings) + IOptions contentSettings, + IIsoCodeValidator isoCodeValidator) : base(provider, loggerFactory, eventMessagesFactory) { _userRepository = userRepository; @@ -78,6 +79,7 @@ internal class UserService : RepositoryService, IUserService _mediaFileManager = mediaFileManager; _temporaryFileService = temporaryFileService; _shortStringHelper = shortStringHelper; + _isoCodeValidator = isoCodeValidator; _globalSettings = globalSettings.Value; _securitySettings = securitySettings.Value; _contentSettings = contentSettings.Value; @@ -282,8 +284,8 @@ internal class UserService : RepositoryService, IUserService public IUser? GetByEmail(string email) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetByEmailAsync(email).GetAwaiter().GetResult(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); + return backOfficeUserStore.GetByEmailAsync(email).GetAwaiter().GetResult(); } /// @@ -300,9 +302,9 @@ internal class UserService : RepositoryService, IUserService return null; } using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetByUserNameAsync(username).GetAwaiter().GetResult(); + return backOfficeUserStore.GetByUserNameAsync(username).GetAwaiter().GetResult(); } /// @@ -312,9 +314,9 @@ internal class UserService : RepositoryService, IUserService public void Delete(IUser membershipUser) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - backofficeUserStore.DisableAsync(membershipUser).GetAwaiter().GetResult(); + backOfficeUserStore.DisableAsync(membershipUser).GetAwaiter().GetResult(); } /// @@ -357,9 +359,9 @@ internal class UserService : RepositoryService, IUserService public void Save(IUser entity) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - backofficeUserStore.SaveAsync(entity).GetAwaiter().GetResult(); + backOfficeUserStore.SaveAsync(entity).GetAwaiter().GetResult(); } /// @@ -369,9 +371,9 @@ internal class UserService : RepositoryService, IUserService public async Task SaveAsync(IUser entity) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return await backofficeUserStore.SaveAsync(entity); + return await backOfficeUserStore.SaveAsync(entity); } /// @@ -609,6 +611,13 @@ internal class UserService : RepositoryService, IUserService return Attempt.FailWithStatus(UserOperationStatus.MissingUser, new UserCreationResult()); } + var userGroups = _userGroupRepository.GetMany().Where(x=>model.UserGroupKeys.Contains(x.Key)).ToArray(); + + if (userGroups.Length != model.UserGroupKeys.Count) + { + return Attempt.FailWithStatus(UserOperationStatus.MissingUserGroup, new UserCreationResult()); + } + UserOperationStatus result = ValidateUserCreateModel(model); if (result != UserOperationStatus.Success) { @@ -620,16 +629,16 @@ internal class UserService : RepositoryService, IUserService null, null, null, - model.UserGroups.Select(x => x.Alias)); + userGroups.Select(x => x.Alias)); if (authorizationAttempt.Success is false) { return Attempt.FailWithStatus(UserOperationStatus.Unauthorized, new UserCreationResult()); } - ICoreBackofficeUserManager backofficeUserManager = serviceScope.ServiceProvider.GetRequiredService(); + ICoreBackOfficeUserManager backOfficeUserManager = serviceScope.ServiceProvider.GetRequiredService(); - IdentityCreationResult identityCreationResult = await backofficeUserManager.CreateAsync(model); + IdentityCreationResult identityCreationResult = await backOfficeUserManager.CreateAsync(model); if (identityCreationResult.Succeded is false) { @@ -637,13 +646,13 @@ internal class UserService : RepositoryService, IUserService // But there should be more information in the message. return Attempt.FailWithStatus( UserOperationStatus.UnknownFailure, - new UserCreationResult { ErrorMessage = identityCreationResult.ErrorMessage }); + new UserCreationResult { Error = new ValidationResult(identityCreationResult.ErrorMessage) }); } // The user is now created, so we can fetch it to map it to a result model with our generated password. // and set it to being approved - IBackofficeUserStore backofficeUserStore = serviceScope.ServiceProvider.GetRequiredService(); - IUser? createdUser = await backofficeUserStore.GetByEmailAsync(model.Email); + IBackOfficeUserStore backOfficeUserStore = serviceScope.ServiceProvider.GetRequiredService(); + IUser? createdUser = await backOfficeUserStore.GetByEmailAsync(model.Email); if (createdUser is null) { @@ -653,12 +662,12 @@ internal class UserService : RepositoryService, IUserService createdUser.IsApproved = approveUser; - foreach (IUserGroup userGroup in model.UserGroups) + foreach (IUserGroup userGroup in userGroups) { createdUser.AddGroup(userGroup.ToReadOnlyGroup()); } - await backofficeUserStore.SaveAsync(createdUser); + await backOfficeUserStore.SaveAsync(createdUser); scope.Complete(); @@ -683,6 +692,13 @@ internal class UserService : RepositoryService, IUserService return Attempt.FailWithStatus(UserOperationStatus.MissingUser, new UserInvitationResult()); } + var userGroups = _userGroupRepository.GetMany().Where(x=>model.UserGroupKeys.Contains(x.Key)).ToArray(); + + if (userGroups.Length != model.UserGroupKeys.Count) + { + return Attempt.FailWithStatus(UserOperationStatus.MissingUserGroup, new UserInvitationResult()); + } + UserOperationStatus validationResult = ValidateUserCreateModel(model); if (validationResult is not UserOperationStatus.Success) @@ -695,7 +711,7 @@ internal class UserService : RepositoryService, IUserService null, null, null, - model.UserGroups.Select(x => x.Alias)); + userGroups.Select(x => x.Alias)); if (authorizationAttempt.Success is false) { @@ -707,8 +723,8 @@ internal class UserService : RepositoryService, IUserService return Attempt.FailWithStatus(UserOperationStatus.CannotInvite, new UserInvitationResult()); } - ICoreBackofficeUserManager userManager = serviceScope.ServiceProvider.GetRequiredService(); - IBackofficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); + ICoreBackOfficeUserManager userManager = serviceScope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); IdentityCreationResult creationResult = await userManager.CreateForInvite(model); if (creationResult.Succeded is false) @@ -717,7 +733,7 @@ internal class UserService : RepositoryService, IUserService // But there should be more information in the message. return Attempt.FailWithStatus( UserOperationStatus.UnknownFailure, - new UserInvitationResult { ErrorMessage = creationResult.ErrorMessage }); + new UserInvitationResult { Error = new ValidationResult(creationResult.ErrorMessage) }); } IUser? invitedUser = await userStore.GetByEmailAsync(model.Email); @@ -730,7 +746,7 @@ internal class UserService : RepositoryService, IUserService invitedUser.InvitedDate = DateTime.Now; invitedUser.ClearGroups(); - foreach(IUserGroup userGroup in model.UserGroups) + foreach(IUserGroup userGroup in userGroups) { invitedUser.AddGroup(userGroup.ToReadOnlyGroup()); } @@ -764,6 +780,10 @@ internal class UserService : RepositoryService, IUserService { return UserOperationStatus.UserNameIsNotEmail; } + if (!IsEmailValid(model.Email)) + { + return UserOperationStatus.InvalidEmail; + } if (GetByEmail(model.Email) is not null) { @@ -775,7 +795,7 @@ internal class UserService : RepositoryService, IUserService return UserOperationStatus.DuplicateUserName; } - if(model.UserGroups.Count == 0) + if(model.UserGroupKeys.Count == 0) { return UserOperationStatus.NoUserGroup; } @@ -783,59 +803,100 @@ internal class UserService : RepositoryService, IUserService return UserOperationStatus.Success; } - public async Task> UpdateAsync(Guid performingUserKey, UserUpdateModel model) + public async Task> UpdateAsync(Guid performingUserKey, UserUpdateModel model) { - IUser? performingUser = await GetAsync(performingUserKey); + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + using IServiceScope serviceScope = _serviceScopeFactory.CreateScope(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); + + IUser? existingUser = await userStore.GetAsync(model.ExistingUserKey); + + if (existingUser is null) + { + return Attempt.FailWithStatus(UserOperationStatus.MissingUser, existingUser); + } + + IUser? performingUser = await userStore.GetAsync(performingUserKey); if (performingUser is null) { - return Attempt.FailWithStatus(UserOperationStatus.MissingUser, model.ExistingUser); + return Attempt.FailWithStatus(UserOperationStatus.MissingUser, existingUser); + } + + var userGroups = _userGroupRepository.GetMany().Where(x=>model.UserGroupKeys.Contains(x.Key)).ToHashSet(); + + if (userGroups.Count != model.UserGroupKeys.Count) + { + return Attempt.FailWithStatus(UserOperationStatus.MissingUserGroup, existingUser); } // We have to resolve the keys to ids to be compatible with the repository, this could be done in the factory, // but I'd rather keep the ids out of the service API as much as possible. int[]? startContentIds = GetIdsFromKeys(model.ContentStartNodeKeys, UmbracoObjectTypes.Document); + + if (startContentIds is null || startContentIds.Length != model.ContentStartNodeKeys.Count) + { + return Attempt.FailWithStatus(UserOperationStatus.ContentStartNodeNotFound, existingUser); + } + int[]? startMediaIds = GetIdsFromKeys(model.MediaStartNodeKeys, UmbracoObjectTypes.Media); + if (startMediaIds is null || startMediaIds.Length != model.MediaStartNodeKeys.Count) + { + return Attempt.FailWithStatus(UserOperationStatus.MediaStartNodeNotFound, existingUser); + } + Attempt isAuthorized = _userEditorAuthorizationHelper.IsAuthorized( performingUser, - model.ExistingUser, + existingUser, startContentIds, startMediaIds, - model.UserGroups.Select(x => x.Alias)); + userGroups.Select(x => x.Alias)); if (isAuthorized.Success is false) { - return Attempt.FailWithStatus(UserOperationStatus.Unauthorized, model.ExistingUser); + return Attempt.FailWithStatus(UserOperationStatus.Unauthorized, existingUser); } - UserOperationStatus validationStatus = ValidateUserUpdateModel(model); + UserOperationStatus validationStatus = ValidateUserUpdateModel(existingUser, model); if (validationStatus is not UserOperationStatus.Success) { - return Attempt.FailWithStatus(validationStatus, model.ExistingUser); + return Attempt.FailWithStatus(validationStatus, existingUser); } // Now that we're all authorized and validated we can actually map over changes and update the user // TODO: This probably shouldn't live here, once we have user content start nodes as keys this can be moved to a mapper // Alternatively it should be a map definition, but then we need to use entity service to resolve the IDs // TODO: Add auditing - IUser updated = MapUserUpdate(model, model.ExistingUser, startContentIds, startMediaIds); - UserOperationStatus saveStatus = await SaveAsync(updated); + IUser updated = MapUserUpdate(model, userGroups, existingUser, startContentIds, startMediaIds); + UserOperationStatus saveStatus = await userStore.SaveAsync(updated); + + if (saveStatus is not UserOperationStatus.Success) + { + return Attempt.FailWithStatus(saveStatus, existingUser); + } + + scope.Complete(); + return Attempt.SucceedWithStatus(UserOperationStatus.Success, updated); - return saveStatus is UserOperationStatus.Success - ? Attempt.SucceedWithStatus(UserOperationStatus.Success, updated) - : Attempt.FailWithStatus(saveStatus, model.ExistingUser); } - public async Task SetAvatarAsync(IUser user, Guid temporaryFileKey) + public async Task SetAvatarAsync(Guid userKey, Guid temporaryFileKey) { using ICoreScope scope = ScopeProvider.CreateCoreScope(); + + IUser? user = await GetAsync(userKey); + if (user is null) + { + return UserOperationStatus.UserNotFound; + } + TemporaryFileModel? avatarTemporaryFile = await _temporaryFileService.GetAsync(temporaryFileKey); _temporaryFileService.EnlistDeleteIfScopeCompletes(temporaryFileKey, ScopeProvider); if (avatarTemporaryFile is null) { - return UserOperationStatus.NotFound; + return UserOperationStatus.AvatarFileNotFound; } const string allowedAvatarFileTypes = "jpeg,jpg,gif,bmp,png,tiff,tif,webp"; @@ -866,19 +927,20 @@ internal class UserService : RepositoryService, IUserService private IUser MapUserUpdate( UserUpdateModel source, + ISet sourceUserGroups, IUser target, int[]? startContentIds, int[]? startMediaIds) { target.Name = source.Name; - target.Language = source.Language; + target.Language = source.LanguageIsoCode; target.Email = source.Email; target.Username = source.UserName; target.StartContentIds = startContentIds; target.StartMediaIds = startMediaIds; target.ClearGroups(); - foreach (IUserGroup group in source.UserGroups) + foreach (IUserGroup group in sourceUserGroups) { target.AddGroup(group.ToReadOnlyGroup()); } @@ -886,10 +948,15 @@ internal class UserService : RepositoryService, IUserService return target; } - private UserOperationStatus ValidateUserUpdateModel(UserUpdateModel model) + private UserOperationStatus ValidateUserUpdateModel(IUser existingUser, UserUpdateModel model) { + if (_isoCodeValidator.IsValid(model.LanguageIsoCode) is false) + { + return UserOperationStatus.InvalidIsoCode; + } + // We need to check if there's any Deny Local login providers present, if so we need to ensure that the user's email address cannot be changed. - if (_localLoginSettingProvider.HasDenyLocalLogin() && model.Email != model.ExistingUser.Email) + if (_localLoginSettingProvider.HasDenyLocalLogin() && model.Email != existingUser.Email) { return UserOperationStatus.EmailCannotBeChanged; } @@ -899,8 +966,13 @@ internal class UserService : RepositoryService, IUserService return UserOperationStatus.UserNameIsNotEmail; } + if (!IsEmailValid(model.Email)) + { + return UserOperationStatus.InvalidEmail; + } + IUser? existing = GetByEmail(model.Email); - if (existing is not null && existing.Key != model.ExistingUser.Key) + if (existing is not null && existing.Key != existingUser.Key) { return UserOperationStatus.DuplicateEmail; } @@ -908,13 +980,13 @@ internal class UserService : RepositoryService, IUserService // In case the user has updated their username to be a different email, but not their actually email // we have to try and get the user by email using their username, and ensure we don't get any collisions. existing = GetByEmail(model.UserName); - if (existing is not null && existing.Key != model.ExistingUser.Key) + if (existing is not null && existing.Key != existingUser.Key) { return UserOperationStatus.DuplicateUserName; } existing = GetByUsername(model.UserName); - if (existing is not null && existing.Key != model.ExistingUser.Key) + if (existing is not null && existing.Key != existingUser.Key) { return UserOperationStatus.DuplicateUserName; } @@ -922,6 +994,8 @@ internal class UserService : RepositoryService, IUserService return UserOperationStatus.Success; } + private static bool IsEmailValid(string email) => new EmailAddressAttribute().IsValid(email); + private int[]? GetIdsFromKeys(IEnumerable? guids, UmbracoObjectTypes type) { int[]? keys = guids? @@ -933,29 +1007,41 @@ internal class UserService : RepositoryService, IUserService return keys; } - public async Task> ChangePasswordAsync(Guid performingUserKey, ChangeBackofficeUserPasswordModel model) + public async Task> ChangePasswordAsync(Guid performingUserKey, ChangeUserPasswordModel model) { IServiceScope serviceScope = _serviceScopeFactory.CreateScope(); using ICoreScope scope = ScopeProvider.CreateCoreScope(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); - IUser? performingUser = await GetAsync(performingUserKey); + IUser? user = await userStore.GetAsync(model.UserKey); + if (user is null) + { + return Attempt.FailWithStatus(UserOperationStatus.UserNotFound, new PasswordChangedModel()); + } + + IUser? performingUser = await userStore.GetAsync(performingUserKey); if (performingUser is null) { return Attempt.FailWithStatus(UserOperationStatus.MissingUser, new PasswordChangedModel()); } - if (performingUser.Username == model.User.Username && string.IsNullOrEmpty(model.OldPassword)) + if (performingUser.Username == user.Username && string.IsNullOrEmpty(model.OldPassword)) { return Attempt.FailWithStatus(UserOperationStatus.OldPasswordRequired, new PasswordChangedModel()); } - if (performingUser.IsAdmin() is false && model.User.IsAdmin()) + if (performingUser.IsAdmin() is false && user.IsAdmin()) { return Attempt.FailWithStatus(UserOperationStatus.Forbidden, new PasswordChangedModel()); } - IBackofficePasswordChanger passwordChanger = serviceScope.ServiceProvider.GetRequiredService(); - Attempt result = await passwordChanger.ChangeBackofficePassword(model); + IBackOfficePasswordChanger passwordChanger = serviceScope.ServiceProvider.GetRequiredService(); + Attempt result = await passwordChanger.ChangeBackOfficePassword(new ChangeBackOfficeUserPasswordModel + { + NewPassword = model.NewPassword, + OldPassword = model.OldPassword, + User = user, + }); if (result.Success is false) { @@ -1175,7 +1261,7 @@ internal class UserService : RepositoryService, IUserService if (user is null) { - return UserOperationStatus.NotFound; + return UserOperationStatus.UserNotFound; } // Check user hasn't logged in. If they have they may have made content changes which will mean @@ -1212,12 +1298,12 @@ internal class UserService : RepositoryService, IUserService } IServiceScope serviceScope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); IUser[] usersToDisable = (await userStore.GetUsersAsync(keys.ToArray())).ToArray(); if (usersToDisable.Length != keys.Count) { - return UserOperationStatus.NotFound; + return UserOperationStatus.UserNotFound; } foreach (IUser user in usersToDisable) @@ -1253,12 +1339,12 @@ internal class UserService : RepositoryService, IUserService } IServiceScope serviceScope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); IUser[] usersToEnable = (await userStore.GetUsersAsync(keys.ToArray())).ToArray(); if (usersToEnable.Length != keys.Count) { - return UserOperationStatus.NotFound; + return UserOperationStatus.UserNotFound; } foreach (IUser user in usersToEnable) @@ -1278,7 +1364,7 @@ internal class UserService : RepositoryService, IUserService if (user is null) { - return UserOperationStatus.NotFound; + return UserOperationStatus.UserNotFound; } if (string.IsNullOrWhiteSpace(user.Avatar)) @@ -1288,11 +1374,11 @@ internal class UserService : RepositoryService, IUserService } using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); string filePath = user.Avatar; user.Avatar = null; - UserOperationStatus result = await backofficeUserStore.SaveAsync(user); + UserOperationStatus result = await backOfficeUserStore.SaveAsync(user); if (result is not UserOperationStatus.Success) { @@ -1323,8 +1409,8 @@ internal class UserService : RepositoryService, IUserService } IServiceScope serviceScope = _serviceScopeFactory.CreateScope(); - ICoreBackofficeUserManager manager = serviceScope.ServiceProvider.GetRequiredService(); - IBackofficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); + ICoreBackOfficeUserManager manager = serviceScope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore userStore = serviceScope.ServiceProvider.GetRequiredService(); IEnumerable usersToUnlock = await userStore.GetUsersAsync(keys); @@ -1447,9 +1533,9 @@ internal class UserService : RepositoryService, IUserService } using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetAllInGroupAsync(groupId.Value).GetAwaiter().GetResult(); + return backOfficeUserStore.GetAllInGroupAsync(groupId.Value).GetAwaiter().GetResult(); } /// @@ -1516,9 +1602,9 @@ internal class UserService : RepositoryService, IUserService public IUser? GetUserById(int id) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetAsync(id).GetAwaiter().GetResult(); + return backOfficeUserStore.GetAsync(id).GetAwaiter().GetResult(); } /// @@ -1529,25 +1615,25 @@ internal class UserService : RepositoryService, IUserService public Task GetAsync(Guid key) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetAsync(key); + return backOfficeUserStore.GetAsync(key); } public Task> GetAsync(IEnumerable keys) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetUsersAsync(keys.ToArray()); + return backOfficeUserStore.GetUsersAsync(keys.ToArray()); } public IEnumerable GetUsersById(params int[]? ids) { using IServiceScope scope = _serviceScopeFactory.CreateScope(); - IBackofficeUserStore backofficeUserStore = scope.ServiceProvider.GetRequiredService(); + IBackOfficeUserStore backOfficeUserStore = scope.ServiceProvider.GetRequiredService(); - return backofficeUserStore.GetUsersAsync(ids).GetAwaiter().GetResult(); + return backOfficeUserStore.GetUsersAsync(ids).GetAwaiter().GetResult(); } /// diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs index 50358a917c..5a97ee377b 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs @@ -27,7 +27,7 @@ namespace Umbraco.Cms.Core.Security; public class BackOfficeUserStore : UmbracoUserStore>, IUserSessionStore, - IBackofficeUserStore + IBackOfficeUserStore { private readonly AppCaches _appCaches; private readonly IEntityService _entityService; diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs b/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs index 6153c21646..a4054808d9 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs @@ -1,4 +1,5 @@ -using NPoco; +using System.Collections.Concurrent; +using NPoco; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.Dtos; @@ -7,45 +8,95 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Services.Implement; -// This could be made better with caching and stuff, but it's really a stop gap measure -// So for now we'll just use the database to resolve the key/id every time. // It's okay that we never clear this, since you can never change a user's key/id // and it'll be caught by the services if it doesn't exist. internal sealed class UserIdKeyResolver : IUserIdKeyResolver { private readonly IScopeProvider _scopeProvider; + private readonly ConcurrentDictionary _keyToId = new(); + private readonly ConcurrentDictionary _idToKey = new(); + private readonly SemaphoreSlim _keytToIdLock = new(1, 1); + private readonly SemaphoreSlim _idToKeyLock = new(1, 1); - public UserIdKeyResolver(IScopeProvider scopeProvider) + public UserIdKeyResolver(IScopeProvider scopeProvider) => _scopeProvider = scopeProvider; + + /// + public async Task GetAsync(Guid key) { - _scopeProvider = scopeProvider; - } - - public Task GetAsync(Guid key) - { - using IScope scope = _scopeProvider.CreateScope(autoComplete: true); - ISqlContext sqlContext = scope.SqlContext; - - Sql query = sqlContext.Sql() - .Select(x => x.Id) - .From() - .Where(x => x.Key == key); - - - return scope.Database.ExecuteScalarAsync(query); + if (_keyToId.TryGetValue(key, out int? id)) + { + return id; + } + + // We don't have it in the cache, so we'll need to look it up in the database + // We'll lock, and then recheck, just to make sure the value wasn't added between the initial check and now. + await _keytToIdLock.WaitAsync(); + try + { + if (_keyToId.TryGetValue(key, out int? recheckedId)) + { + // It was added while we were waiting, so we'll just return it + return recheckedId; + } + + // Still not here, so actually fetch it now + using IScope scope = _scopeProvider.CreateScope(autoComplete: true); + ISqlContext sqlContext = scope.SqlContext; + + Sql query = sqlContext.Sql() + .Select(x => x.Id) + .From() + .Where(x => x.Key == key); + + int? fetchedId = await scope.Database.ExecuteScalarAsync(query); + + _keyToId[key] = fetchedId; + return fetchedId; + } + finally + { + _keytToIdLock.Release(); + } } + /// public async Task GetAsync(int id) { - using IScope scope = _scopeProvider.CreateScope(autoComplete: true); - ISqlContext sqlContext = scope.SqlContext; + if (_idToKey.TryGetValue(id, out Guid? key)) + { + return key; + } - Sql query = sqlContext.Sql() - .Select(x => x.Key) - .From() - .Where(x => x.Id == id); + await _idToKeyLock.WaitAsync(); + try + { + if (_idToKey.TryGetValue(id, out Guid? recheckedKey)) + { + return recheckedKey; + } - string? guidString = await scope.Database.ExecuteScalarAsync(query); + using IScope scope = _scopeProvider.CreateScope(autoComplete: true); + ISqlContext sqlContext = scope.SqlContext; - return guidString is null ? null : new Guid(guidString); + Sql query = sqlContext.Sql() + .Select(x => x.Key) + .From() + .Where(x => x.Id == id); + + string? guidString = await scope.Database.ExecuteScalarAsync(query); + Guid? fetchedKey = guidString is null ? null : new Guid(guidString); + + // For ids we don't want to cache the null value, since unlike keys, it's pretty likely that we'll see collision + if (fetchedKey is not null) + { + _idToKey[id] = fetchedKey; + } + + return fetchedKey; + } + finally + { + _idToKeyLock.Release(); + } } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs b/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs index fb1666e522..5269d3d0d7 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs @@ -288,11 +288,11 @@ public class CurrentUserController : UmbracoAuthorizedJsonController return result; } - if (passwordChangeResult.Result?.ChangeError?.MemberNames is not null) + if (passwordChangeResult.Result?.Error?.MemberNames is not null) { - foreach (var memberName in passwordChangeResult.Result.ChangeError.MemberNames) + foreach (var memberName in passwordChangeResult.Result.Error.MemberNames) { - ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError.ErrorMessage ?? string.Empty); + ModelState.AddModelError(memberName, passwordChangeResult.Result.Error.ErrorMessage ?? string.Empty); } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index f06ae05f86..1c2d9f8039 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -627,10 +627,10 @@ public class MemberController : ContentControllerBase if (!passwordChangeResult.Success) { - foreach (var memberName in passwordChangeResult.Result?.ChangeError?.MemberNames ?? + foreach (var memberName in passwordChangeResult.Result?.Error?.MemberNames ?? Enumerable.Empty()) { - ModelState.AddModelError(memberName, passwordChangeResult.Result?.ChangeError?.ErrorMessage ?? string.Empty); + ModelState.AddModelError(memberName, passwordChangeResult.Result?.Error?.ErrorMessage ?? string.Empty); } return ValidationProblem(ModelState); diff --git a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs index 02eb9cda8e..9b400af00d 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs @@ -769,12 +769,12 @@ public class UsersController : BackOfficeNotificationsController return result; } - if (passwordChangeResult.Result?.ChangeError is not null) + if (passwordChangeResult.Result?.Error is not null) { - foreach (var memberName in passwordChangeResult.Result.ChangeError.MemberNames) + foreach (var memberName in passwordChangeResult.Result.Error.MemberNames) { ModelState.AddModelError(memberName, - passwordChangeResult.Result.ChangeError.ErrorMessage ?? string.Empty); + passwordChangeResult.Result.Error.ErrorMessage ?? string.Empty); } } diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeAuth.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeAuth.cs index 8909bca018..7275daf810 100644 --- a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeAuth.cs +++ b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeAuth.cs @@ -56,7 +56,7 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddUnique(); builder.Services.AddUnique, PasswordChanger>(); builder.Services.AddUnique, PasswordChanger>(); - builder.Services.AddScoped(); + builder.Services.AddScoped(); builder.AddNotificationHandler(); builder.AddNotificationHandler(); diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeIdentity.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeIdentity.cs index 2132cdf608..3bf4b0b291 100644 --- a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeIdentity.cs +++ b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeIdentity.cs @@ -57,8 +57,8 @@ public static partial class UmbracoBuilderExtensions .AddErrorDescriber(); // We also need to register the store as a core-friendly interface that doesn't leak technology. - services.AddScoped(); - services.AddScoped(); + services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddSingleton(); ; diff --git a/src/Umbraco.Web.BackOffice/Security/BackofficePasswordChanger.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordChanger.cs similarity index 84% rename from src/Umbraco.Web.BackOffice/Security/BackofficePasswordChanger.cs rename to src/Umbraco.Web.BackOffice/Security/BackOfficePasswordChanger.cs index ca9764efee..3e6551e8ca 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackofficePasswordChanger.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordChanger.cs @@ -6,12 +6,12 @@ using Umbraco.Cms.Web.Common.Security; namespace Umbraco.Cms.Web.BackOffice.Security; -public class BackofficePasswordChanger : IBackofficePasswordChanger +public class BackOfficePasswordChanger : IBackOfficePasswordChanger { private readonly IPasswordChanger _passwordChanger; private readonly IBackOfficeUserManager _userManager; - public BackofficePasswordChanger( + public BackOfficePasswordChanger( IPasswordChanger passwordChanger, IBackOfficeUserManager userManager) { @@ -19,8 +19,8 @@ public class BackofficePasswordChanger : IBackofficePasswordChanger _userManager = userManager; } - public async Task> ChangeBackofficePassword( - ChangeBackofficeUserPasswordModel model) + public async Task> ChangeBackOfficePassword( + ChangeBackOfficeUserPasswordModel model) { var mappedModel = new ChangingPasswordModel { diff --git a/src/Umbraco.Web.BackOffice/Security/InviteUriProvider.cs b/src/Umbraco.Web.BackOffice/Security/InviteUriProvider.cs index dd3632b895..7bee6768dc 100644 --- a/src/Umbraco.Web.BackOffice/Security/InviteUriProvider.cs +++ b/src/Umbraco.Web.BackOffice/Security/InviteUriProvider.cs @@ -15,13 +15,13 @@ namespace Umbraco.Cms.Web.BackOffice.Security; public class InviteUriProvider : IInviteUriProvider { private readonly LinkGenerator _linkGenerator; - private readonly ICoreBackofficeUserManager _userManager; + private readonly ICoreBackOfficeUserManager _userManager; private readonly IHttpContextAccessor _httpContextAccessor; private readonly WebRoutingSettings _webRoutingSettings; public InviteUriProvider( LinkGenerator linkGenerator, - ICoreBackofficeUserManager userManager, + ICoreBackOfficeUserManager userManager, IHttpContextAccessor httpContextAccessor, IOptions webRoutingSettings) { diff --git a/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs b/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs index 25f0548386..27956cb544 100644 --- a/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs +++ b/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs @@ -47,7 +47,7 @@ internal class PasswordChanger : IPasswordChanger where TUser : Um { return Attempt.Fail(new PasswordChangedModel { - ChangeError = new ValidationResult("Cannot set an empty password", new[] { "value" }) + Error = new ValidationResult("Cannot set an empty password", new[] { "value" }) }); } @@ -58,7 +58,7 @@ internal class PasswordChanger : IPasswordChanger where TUser : Um // this really shouldn't ever happen... but just in case return Attempt.Fail(new PasswordChangedModel { - ChangeError = new ValidationResult("Password could not be verified", new[] { "oldPassword" }) + Error = new ValidationResult("Password could not be verified", new[] { "oldPassword" }) }); } @@ -77,7 +77,7 @@ internal class PasswordChanger : IPasswordChanger where TUser : Um _logger.LogWarning("Could not reset user password {PasswordErrors}", errors); return Attempt.Fail(new PasswordChangedModel { - ChangeError = new ValidationResult(errors, new[] { "value" }) + Error = new ValidationResult(errors, new[] { "value" }) }); } @@ -91,7 +91,7 @@ internal class PasswordChanger : IPasswordChanger where TUser : Um // no, fail with an error message for "oldPassword" return Attempt.Fail(new PasswordChangedModel { - ChangeError = new ValidationResult("Incorrect password", new[] { "oldPassword" }) + Error = new ValidationResult("Incorrect password", new[] { "oldPassword" }) }); } @@ -104,7 +104,7 @@ internal class PasswordChanger : IPasswordChanger where TUser : Um _logger.LogWarning("Could not change user password {PasswordErrors}", errors); return Attempt.Fail(new PasswordChangedModel { - ChangeError = new ValidationResult(errors, new[] { "password" }) + Error = new ValidationResult(errors, new[] { "password" }) }); } diff --git a/src/Umbraco.Web.Common/Security/BackOfficeUserManager.cs b/src/Umbraco.Web.Common/Security/BackOfficeUserManager.cs index 4dd7db9e74..0be734b62c 100644 --- a/src/Umbraco.Web.Common/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Web.Common/Security/BackOfficeUserManager.cs @@ -1,3 +1,4 @@ +using System.ComponentModel.DataAnnotations; using System.Security.Claims; using System.Security.Principal; using Microsoft.AspNetCore.Http; @@ -20,7 +21,7 @@ namespace Umbraco.Cms.Web.Common.Security; public class BackOfficeUserManager : UmbracoUserManager, IBackOfficeUserManager, - ICoreBackofficeUserManager + ICoreBackOfficeUserManager { private readonly IBackOfficeUserPasswordChecker _backOfficeUserPasswordChecker; private readonly GlobalSettings _globalSettings; @@ -153,14 +154,14 @@ public class BackOfficeUserManager : UmbracoUserManager ResetAccessFailedCountAsync(BackOfficeIdentityUser user) @@ -322,7 +323,7 @@ public class BackOfficeUserManager : UmbracoUserManager { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var result = await userService.CreateAsync(Constants.Security.SuperUserKey, creationModel, true); @@ -62,7 +62,7 @@ public partial class UserServiceCrudTests UserName = "Test1", Email = email, Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var userService = CreateUserService(new SecuritySettings { UsernameIsEmail = false }); @@ -74,7 +74,7 @@ public partial class UserServiceCrudTests UserName = "Test2", Email = email, Name = "Duplicate Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var secondResult = await userService.CreateAsync(Constants.Security.SuperUserKey, duplicateUserCreateModel, true); @@ -92,7 +92,7 @@ public partial class UserServiceCrudTests UserName = userName, Email = "test@email.com", Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var userService = CreateUserService(new SecuritySettings { UsernameIsEmail = false }); @@ -104,7 +104,7 @@ public partial class UserServiceCrudTests UserName = userName, Email = "another@email.com", Name = "Duplicate Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var secondResult = await userService.CreateAsync(Constants.Security.SuperUserKey, duplicateUserCreateModel, true); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs index 1ba02a1b05..8936b93e90 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Delete.cs @@ -13,7 +13,7 @@ public partial class UserServiceCrudTests { var userService = CreateUserService(); var result = await userService.DeleteAsync(Guid.NewGuid()); - Assert.AreEqual(UserOperationStatus.NotFound, result); + Assert.AreEqual(UserOperationStatus.UserNotFound, result); } [Test] @@ -25,7 +25,7 @@ public partial class UserServiceCrudTests Email = "test@test.com", UserName = "test@test.com", Name = "test@test.com", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var userService = CreateUserService(); @@ -54,7 +54,7 @@ public partial class UserServiceCrudTests Email = "test@test.com", UserName = "test@test.com", Name = "test@test.com", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var userService = CreateUserService(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs index add38f9f40..b62f235aa4 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs @@ -23,7 +23,7 @@ public partial class UserServiceCrudTests UserName = "editor@mail.com", Email = "editor@mail.com", Name = "Editor", - UserGroups = new HashSet {editorGroup!} + UserGroupKeys = new HashSet { editorGroup.Key } }; var createAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, createModel, true); @@ -51,7 +51,7 @@ public partial class UserServiceCrudTests { Email = "not@super.com", UserName = "not@super.com", - UserGroups = new HashSet {editorGroup!, adminGroup!}, + UserGroupKeys = new HashSet { editorGroup.Key, adminGroup.Key }, Name = "Not A Super User" }; @@ -84,7 +84,7 @@ public partial class UserServiceCrudTests { Email = "not@super.com", UserName = "not@super.com", - UserGroups = new HashSet {editorGroup!}, + UserGroupKeys = new HashSet { editorGroup.Key }, Name = "Not A Super User" }; @@ -104,7 +104,7 @@ public partial class UserServiceCrudTests } [Test] - public async Task Only_Admins_Can_Filter_Admins() + public async Task Non_Admins_Cannot_Filter_Admins() { var userService = CreateUserService(); var adminGroup = await UserGroupService.GetAsync(Constants.Security.AdminGroupAlias); @@ -115,7 +115,7 @@ public partial class UserServiceCrudTests UserName = "editor@mail.com", Email = "editor@mail.com", Name = "Editor Mc. Gee", - UserGroups = new HashSet {editorGroup!} + UserGroupKeys = new HashSet { editorGroup.Key }, }; var adminCreateModel = new UserCreateModel @@ -123,7 +123,7 @@ public partial class UserServiceCrudTests UserName = "admin@mail.com", Email = "admin@mail.com", Name = "Admin Mc. Gee", - UserGroups = new HashSet {adminGroup!, editorGroup} + UserGroupKeys = new HashSet { editorGroup.Key, adminGroup.Key }, }; var createEditorAttempt = @@ -140,7 +140,40 @@ public partial class UserServiceCrudTests Assert.IsTrue(editorFilterAttempt.Success); var editorAllUsers = editorFilterAttempt.Result.Items.ToList(); Assert.AreEqual(0, editorAllUsers.Count); + } + [Test] + public async Task Admins_Can_Filter_Admins() + { + var userService = CreateUserService(); + var adminGroup = await UserGroupService.GetAsync(Constants.Security.AdminGroupAlias); + var editorGroup = await UserGroupService.GetAsync(Constants.Security.EditorGroupAlias); + + var editorCreateModel = new UserCreateModel + { + UserName = "editor@mail.com", + Email = "editor@mail.com", + Name = "Editor Mc. Gee", + UserGroupKeys = new HashSet { editorGroup.Key }, + }; + + var adminCreateModel = new UserCreateModel + { + UserName = "admin@mail.com", + Email = "admin@mail.com", + Name = "Admin Mc. Gee", + UserGroupKeys = new HashSet { editorGroup.Key, adminGroup.Key }, + }; + + var createEditorAttempt = + await userService.CreateAsync(Constants.Security.SuperUserKey, editorCreateModel, true); + var createAdminAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, adminCreateModel, true); + + Assert.IsTrue(createEditorAttempt.Success); + Assert.IsTrue(createAdminAttempt.Success); + + var filter = new UserFilter {IncludedUserGroups = new SortedSet {adminGroup!.Key}}; + var adminFilterAttempt = await userService.FilterAsync(createAdminAttempt.Result.CreatedUser!.Key, filter, 0, 10000); Assert.IsTrue(adminFilterAttempt.Success); @@ -149,6 +182,7 @@ public partial class UserServiceCrudTests Assert.IsNotNull(adminAllUsers.FirstOrDefault(x => x.Key == createAdminAttempt.Result.CreatedUser!.Key)); } + private async Task CreateTestUsers(IUserService userService) { var editorGroup = await UserGroupService.GetAsync(Constants.Security.EditorGroupAlias); @@ -163,35 +197,35 @@ public partial class UserServiceCrudTests UserName = "editor@email.com", Email = "editor@email.com", Name = "Editor", - UserGroups = new HashSet {editorGroup!} + UserGroupKeys = new HashSet { editorGroup.Key }, }, new() { UserName = "admin@email.com", Email = "admin@email.com", Name = "Admin", - UserGroups = new HashSet {adminGroup!} + UserGroupKeys = new HashSet { adminGroup.Key }, }, new() { UserName = "write@email.com", Email = "write@email.com", Name = "Write", - UserGroups = new HashSet {writerGroup} + UserGroupKeys = new HashSet { writerGroup.Key }, }, new() { UserName = "translator@email.com", Email = "translator@email.com", Name = "Translator", - UserGroups = new HashSet {translatorGroup} + UserGroupKeys = new HashSet { translatorGroup.Key }, }, new() { UserName = "EverythingButAdmin@email.com", Email = "EverythingButAdmin@email.com", Name = "Everything But Admin", - UserGroups = new HashSet {editorGroup, writerGroup, translatorGroup} + UserGroupKeys = new HashSet { editorGroup.Key, writerGroup.Key, translatorGroup.Key }, } }; diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Get.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Get.cs index 25faf96f61..69263159e6 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Get.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Get.cs @@ -20,7 +20,7 @@ public partial class UserServiceCrudTests { Email = "not@super.com", UserName = "not@super.com", - UserGroups = new HashSet { editorGroup!, adminGroup! }, + UserGroupKeys = new HashSet { editorGroup.Key, adminGroup.Key }, Name = "Not A Super User" }; @@ -49,7 +49,7 @@ public partial class UserServiceCrudTests { Email = "not@super.com", UserName = "not@super.com", - UserGroups = new HashSet { editorGroup! }, + UserGroupKeys = new HashSet { editorGroup.Key }, Name = "Not A Super User" }; @@ -68,7 +68,7 @@ public partial class UserServiceCrudTests } [Test] - public async Task Only_Admins_Can_See_Admins() + public async Task Non_Admins_Cannot_Get_admins() { var userService = CreateUserService(); var adminGroup = await UserGroupService.GetAsync(Constants.Security.AdminGroupAlias); @@ -79,7 +79,7 @@ public partial class UserServiceCrudTests UserName = "editor@mail.com", Email = "editor@mail.com", Name = "Editor Mc. Gee", - UserGroups = new HashSet { editorGroup! } + UserGroupKeys = new HashSet { editorGroup.Key }, }; var adminCreateModel = new UserCreateModel @@ -87,7 +87,7 @@ public partial class UserServiceCrudTests UserName = "admin@mail.com", Email = "admin@mail.com", Name = "Admin Mc. Gee", - UserGroups = new HashSet { adminGroup!, editorGroup } + UserGroupKeys = new HashSet { editorGroup.Key, adminGroup.Key }, }; var createEditorAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, editorCreateModel, true); @@ -101,6 +101,36 @@ public partial class UserServiceCrudTests var editorAllUsers = editorAllUsersAttempt.Result.Items.ToList(); Assert.AreEqual(1, editorAllUsers.Count); Assert.AreEqual(createEditorAttempt.Result.CreatedUser!.Key, editorAllUsers.First().Key); + } + + [Test] + public async Task Admins_Can_See_Admins() + { + var userService = CreateUserService(); + var adminGroup = await UserGroupService.GetAsync(Constants.Security.AdminGroupAlias); + var editorGroup = await UserGroupService.GetAsync(Constants.Security.EditorGroupAlias); + + var editorCreateModel = new UserCreateModel + { + UserName = "editor@mail.com", + Email = "editor@mail.com", + Name = "Editor Mc. Gee", + UserGroupKeys = new HashSet { editorGroup.Key }, + }; + + var adminCreateModel = new UserCreateModel + { + UserName = "admin@mail.com", + Email = "admin@mail.com", + Name = "Admin Mc. Gee", + UserGroupKeys = new HashSet { editorGroup.Key, adminGroup.Key }, + }; + + var createEditorAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, editorCreateModel, true); + var createAdminAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, adminCreateModel, true); + + Assert.IsTrue(createEditorAttempt.Success); + Assert.IsTrue(createAdminAttempt.Success); var adminAllUsersAttempt = await userService.GetAllAsync(createAdminAttempt.Result.CreatedUser!.Key, 0, 10000); Assert.IsTrue(adminAllUsersAttempt.Success); @@ -121,7 +151,7 @@ public partial class UserServiceCrudTests UserName = "firstEditor@mail.com", Email = "firstEditor@mail.com", Name = "First Editor", - UserGroups = new HashSet { editorGroup! } + UserGroupKeys = new HashSet { editorGroup.Key }, }; var firstEditorResult = await userService.CreateAsync(Constants.Security.SuperUserKey, firstEditorCreateModel, true); @@ -132,7 +162,7 @@ public partial class UserServiceCrudTests UserName = "secondEditor@mail.com", Email = "secondEditor@mail.com", Name = "Second Editor", - UserGroups = new HashSet {editorGroup} + UserGroupKeys = new HashSet { editorGroup.Key }, }; var secondEditorResult = await userService.CreateAsync(Constants.Security.SuperUserKey, secondEditorCreateModel, true); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Invite.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Invite.cs index 9f03a49f74..538c260e2f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Invite.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Invite.cs @@ -32,7 +32,7 @@ public partial class UserServiceCrudTests UserName = username, Email = email, Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key }, }; var result = await userService.InviteAsync(Constants.Security.SuperUserKey, inviteModel); @@ -62,7 +62,7 @@ public partial class UserServiceCrudTests UserName = "Test1", Email = email, Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key }, }; var userService = CreateUserService(new SecuritySettings { UsernameIsEmail = false }); @@ -74,7 +74,7 @@ public partial class UserServiceCrudTests UserName = "Test2", Email = email, Name = "Duplicate Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key }, }; var secondResult = await userService.InviteAsync(Constants.Security.SuperUserKey, duplicateUserInviteModel); @@ -92,7 +92,7 @@ public partial class UserServiceCrudTests UserName = userName, Email = "test@email.com", Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key }, }; var userService = CreateUserService(new SecuritySettings { UsernameIsEmail = false }); @@ -104,7 +104,7 @@ public partial class UserServiceCrudTests UserName = userName, Email = "another@email.com", Name = "Duplicate Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key }, }; var secondResult = await userService.InviteAsync(Constants.Security.SuperUserKey, duplicateUserInviteModelModel); @@ -150,7 +150,7 @@ public partial class UserServiceCrudTests UserName = "some@email.com", Email = "some@email.com", Name = "Bob", - UserGroups = new HashSet {userGroup!}, + UserGroupKeys = new HashSet { userGroup.Key }, }; IUserService userService = CreateUserService(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.PartialUpdates.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.PartialUpdates.cs index 8645a9273e..013e53057e 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.PartialUpdates.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.PartialUpdates.cs @@ -18,7 +18,7 @@ public partial class UserServiceCrudTests UserName = "test@email.com", Email = "test@email.com", Name = "Test User", - UserGroups = new HashSet { editorUserGroup } + UserGroupKeys = new HashSet { editorUserGroup.Key }, }; var userService = CreateUserService(); @@ -46,7 +46,7 @@ public partial class UserServiceCrudTests UserName = "test@email.com", Email = "test@email.com", Name = "Test User", - UserGroups = new HashSet { editorUserGroup } + UserGroupKeys = new HashSet { editorUserGroup.Key }, }; var userService = CreateUserService(); @@ -70,7 +70,7 @@ public partial class UserServiceCrudTests UserName = "test@email.com", Email = "test@email.com", Name = "Test User", - UserGroups = new HashSet { adminUserGroup } + UserGroupKeys = new HashSet { adminUserGroup.Key }, }; var userService = CreateUserService(); @@ -92,7 +92,7 @@ public partial class UserServiceCrudTests UserName = "test@email.com", Email = "test@email.com", Name = "Test User", - UserGroups = new HashSet { editorUserGroup } + UserGroupKeys = new HashSet { editorUserGroup.Key }, }; var userService = CreateUserService(); @@ -104,4 +104,51 @@ public partial class UserServiceCrudTests var disableStatus = await userService.DisableAsync(Constants.Security.SuperUserKey, new HashSet { invitedUser!.Key }); Assert.AreEqual(UserOperationStatus.CannotDisableInvitedUser, disableStatus); } + + [Test] + public async Task Enable_Missing_User_Fails_With_Not_Found() + { + var editorUserGroup = await UserGroupService.GetAsync(Constants.Security.EditorGroupAlias); + + var userCreateModel = new UserCreateModel + { + UserName = "test@email.com", + Email = "test@email.com", + Name = "Test User", + UserGroupKeys = new HashSet { editorUserGroup.Key }, + }; + + var userService = CreateUserService(); + var createAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, userCreateModel, false); + + Assert.IsTrue(createAttempt.Success); + var user = createAttempt.Result.CreatedUser; + Assert.AreEqual(UserState.Disabled, user!.UserState); + + var enableStatus = await userService.EnableAsync(Constants.Security.SuperUserKey, new HashSet { user.Key, Guid.NewGuid() }); + Assert.AreEqual(UserOperationStatus.UserNotFound, enableStatus); + } + + [Test] + public async Task Disable_Missing_User_Fails_With_Not_Found() + { + var editorUserGroup = await UserGroupService.GetAsync(Constants.Security.EditorGroupAlias); + + var userCreateModel = new UserCreateModel + { + UserName = "test@email.com", + Email = "test@email.com", + Name = "Test User", + UserGroupKeys = new HashSet { editorUserGroup.Key }, + }; + + var userService = CreateUserService(); + var createAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, userCreateModel, true); + + Assert.IsTrue(createAttempt.Success); + var user = createAttempt.Result.CreatedUser; + + var enableStatus = await userService.DisableAsync(Constants.Security.SuperUserKey, new HashSet { user.Key, Guid.NewGuid() }); + Assert.AreEqual(UserOperationStatus.UserNotFound, enableStatus); + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Update.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Update.cs index 9e24581169..565433885b 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Update.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Update.cs @@ -29,14 +29,14 @@ public partial class UserServiceCrudTests var groups = await UserGroupService.GetAsync(user.Groups.Select(x => x.Id).ToArray()); return new UserUpdateModel { - ExistingUser = user, + ExistingUserKey = user.Key, Email = user.Email, Name = user.Name, UserName = user.Username, - Language = user.Language, + LanguageIsoCode = user.Language, ContentStartNodeKeys = GetKeysFromIds(user.StartContentIds, UmbracoObjectTypes.Document), MediaStartNodeKeys = GetKeysFromIds(user.StartMediaIds, UmbracoObjectTypes.Media), - UserGroups = groups, + UserGroupKeys = groups.Select(x=>x.Key).ToHashSet(), }; } @@ -51,7 +51,7 @@ public partial class UserServiceCrudTests Email = email, UserName = userName, Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var createExistingUser = await userService.CreateAsync(Constants.Security.SuperUserKey, createUserModel, true); @@ -138,7 +138,7 @@ public partial class UserServiceCrudTests Email = email, UserName = email, Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var createExisting = await userService.CreateAsync(Constants.Security.SuperUserKey, createModel, true); @@ -171,7 +171,7 @@ public partial class UserServiceCrudTests Email = existingEmail, UserName = existingUserName, Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var createExisting = await userService.CreateAsync(Constants.Security.SuperUserKey, createModel, true); @@ -186,4 +186,32 @@ public partial class UserServiceCrudTests Assert.IsFalse(updateAttempt.Success); Assert.AreEqual(UserOperationStatus.DuplicateUserName, updateAttempt.Status); } + + [Test] + [TestCase("en-US", true)] + [TestCase("Very much not an ISO Code (:", false)] + [TestCase("da-ZA", false)] + public async Task Iso_Code_Is_Validated(string isoCode, bool shouldSucceed) + { + var userService = CreateUserService(); + + var (updateModel, _) = await CreateUserForUpdate(userService); + + updateModel.LanguageIsoCode = isoCode; + + var result = await userService.UpdateAsync(Constants.Security.SuperUserKey, updateModel); + + if (shouldSucceed is false) + { + Assert.IsFalse(result.Success); + Assert.AreEqual(UserOperationStatus.InvalidIsoCode, result.Status); + return; + } + + Assert.IsTrue(result.Success); + // We'll get the user again to ensure that the changes has been persisted + var updatedUser = await userService.GetAsync(result.Result.Key); + Assert.IsNotNull(updatedUser); + Assert.AreEqual(isoCode, updatedUser.Language); + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.cs index e7e7b49eac..77c3b55e58 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.cs @@ -80,7 +80,8 @@ public partial class UserServiceCrudTests : UmbracoIntegrationTest GetRequiredService(), GetRequiredService(), GetRequiredService(), - GetRequiredService>()); + GetRequiredService>(), + GetRequiredService()); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs index 9690b77a7c..7ad9b83b80 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs @@ -27,7 +27,7 @@ public class UserIdKeyResolverTests : UmbracoIntegrationTest UserName = "test@test.com", Email = "test@test.com", Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var creationResult = await UserService.CreateAsync(Constants.Security.SuperUserKey, userCreateModel); @@ -48,7 +48,7 @@ public class UserIdKeyResolverTests : UmbracoIntegrationTest UserName = "test@test.com", Email = "test@test.com", Name = "Test Mc. Gee", - UserGroups = new HashSet { userGroup! } + UserGroupKeys = new HashSet { userGroup.Key } }; var creationResult = await UserService.CreateAsync(Constants.Security.SuperUserKey, userCreateModel); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index 83d39a4245..b0ad8955ef 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -352,7 +352,7 @@ public class MemberControllerUnitTests IPasswordChanger passwordChanger, bool successful = true) { - var passwordChanged = new PasswordChangedModel { ChangeError = null, ResetPassword = null }; + var passwordChanged = new PasswordChangedModel { Error = null, ResetPassword = null }; if (!successful) { var attempt = Attempt.Fail(passwordChanged);