yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

User endpoint additions and corrections (#15773)

Browse Source 
* Make create user endpoint work with the supplied id

Return 201 instead of 200 with correct resource identifier

* Add ResetPassword endpoint

* Bring changepassword route inline with other resource actions

* Fixed User endpoints not advertising all their possible response codes/ models

Fixed certain endpoints not authorizing targeted user(s) versus the admin needs admin authorization requirement
Fixed a user not found response bug for the update flow
Fix spacing

* Fixed CurrentUser endpoints not advertising all their possible response codes/ models

Fix incorrect responseStatus in UserService.GetPermissionsAsync

* Update OpenApi definition

Fix smal model oversights in previous commits

* Update incorrect Response type

* Check for duplicate id's in user create validation

* Remove unnecasary returnmodel from changepassword

Renamed the model to it's remaining usage

* rename bad constructor parameter

* Renamed method parameters for better readability and usage

* Fixed wrong userkey being passed down because of (refactored) bad naming

Technically doesn't change anything as the two id's should be the same in this case (reset with token is always for self)

* Fixed resetpassword bug

* Update openapi

* Update src/Umbraco.Core/Services/UserService.cs

Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>

* Remove old password from change user password request model

Only makes sense when doing it for the logged in user => current endpoint

---------

Co-authored-by: Sven Geusens <sge@umbraco.dk>
Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>
This commit is contained in:
Sven Geusens
2024-02-29 10:40:48 +01:00
committed by GitHub
parent 1e043cbcfb
commit 393d178b58
36 changed files with 1263 additions and 244 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/Umbraco.Core/Models/UserCreateModel.cs
View File

@@ -4,6 +4,8 @@ namespace Umbraco.Cms.Core.Models;
public class UserCreateModel
{
public Guid? Id { get; set; }
public string Email { get; set; } = string.Empty;
public string UserName { get; set; } = string.Empty;

5
src/Umbraco.Core/Security/ICoreBackOfficeUserManager.cs
View File

@@ -17,6 +17,7 @@ public interface ICoreBackOfficeUserManager
Task<IdentityCreationResult> CreateForInvite(UserCreateModel createModel);
Task<Attempt<string, UserOperationStatus>> GenerateEmailConfirmationTokenAsync(IUser user);
Task<Attempt<string, UserOperationStatus>> GeneratePasswordResetTokenAsync(IUser user);
Task<Attempt<UserUnlockResult, UserOperationStatus>> UnlockUser(IUser user);
@@ -24,6 +25,10 @@ public interface ICoreBackOfficeUserManager
Task<Attempt<ICollection<IIdentityUserLogin>, UserOperationStatus>> GetLoginsAsync(IUser user);
Task<bool> IsEmailConfirmationTokenValidAsync(IUser user, string token);
Task<bool> IsResetPasswordTokenValidAsync(IUser user, string token);
void NotifyForgotPasswordRequested(IPrincipal user, string toString);
public string GeneratePassword();
}

26
src/Umbraco.Core/Services/IUserService.cs
View File

@@ -51,33 +51,33 @@ public interface IUserService : IMembershipUserService
/// <remarks>
/// This creates both the Umbraco user and the identity user.
/// </remarks>
/// <param name="userKey">The key of the user performing the operation.</param>
/// <param name="performingUserKey">The key of the user performing the operation.</param>
/// <param name="model">Model to create the user from.</param>
/// <param name="approveUser">Specifies if the user should be enabled be default. Defaults to false.</param>
/// <returns>An attempt indicating if the operation was a success as well as a more detailed <see cref="UserOperationStatus"/>.</returns>
Task<Attempt<UserCreationResult, UserOperationStatus>> CreateAsync(Guid userKey, UserCreateModel model, bool approveUser = false);
Task<Attempt<UserCreationResult, UserOperationStatus>> CreateAsync(Guid performingUserKey, UserCreateModel model, bool approveUser = false);
Task<Attempt<UserInvitationResult, UserOperationStatus>> InviteAsync(Guid userKey, UserInviteModel model);
Task<Attempt<UserInvitationResult, UserOperationStatus>> InviteAsync(Guid performingUserKey, UserInviteModel model);
Task<Attempt<UserOperationStatus>> VerifyInviteAsync(Guid userKey, string token);
Task<Attempt<PasswordChangedModel, UserOperationStatus>> CreateInitialPasswordAsync(Guid userKey, string token, string password);
Task<Attempt<IUser?, UserOperationStatus>> UpdateAsync(Guid userKey, UserUpdateModel model);
Task<Attempt<IUser?, UserOperationStatus>> UpdateAsync(Guid performingUserKey, UserUpdateModel model);
Task<UserOperationStatus> SetAvatarAsync(Guid userKey, Guid temporaryFileKey);
Task<UserOperationStatus> DeleteAsync(Guid userKey, ISet<Guid> keys);
Task<UserOperationStatus> DeleteAsync(Guid performingUserKey, ISet<Guid> keys);
Task<UserOperationStatus> DeleteAsync(Guid userKey, Guid key) => DeleteAsync(userKey, new HashSet<Guid> { key });
Task<UserOperationStatus> DeleteAsync(Guid performingUserKey, Guid key) => DeleteAsync(performingUserKey, new HashSet<Guid> { key });
Task<UserOperationStatus> DisableAsync(Guid userKey, ISet<Guid> keys);
Task<UserOperationStatus> DisableAsync(Guid performingUserKey, ISet<Guid> keys);
Task<UserOperationStatus> EnableAsync(Guid userKey, ISet<Guid> keys);
Task<UserOperationStatus> EnableAsync(Guid performingUserKey, ISet<Guid> keys);
Task<Attempt<UserUnlockResult, UserOperationStatus>> UnlockAsync(Guid userKey, params Guid[] keys);
Task<Attempt<UserUnlockResult, UserOperationStatus>> UnlockAsync(Guid performingUserKey, params Guid[] keys);
Task<Attempt<PasswordChangedModel, UserOperationStatus>> ChangePasswordAsync(Guid userKey, ChangeUserPasswordModel model);
Task<Attempt<PasswordChangedModel, UserOperationStatus>> ChangePasswordAsync(Guid performingUserKey, ChangeUserPasswordModel model);
Task<UserOperationStatus> ClearAvatarAsync(Guid userKey);
@@ -86,11 +86,11 @@ public interface IUserService : IMembershipUserService
/// <summary>
/// Gets all users that the requesting user is allowed to see.
/// </summary>
/// <param name="userKey">The Key of the user requesting the users.</param>
/// <param name="performingUserKey">The Key of the user requesting the users.</param>
/// <param name="skip">Amount to skip.</param>
/// <param name="take">Amount to take.</param>
/// <returns>All users that the user is allowed to see.</returns>
Task<Attempt<PagedModel<IUser>?, UserOperationStatus>> GetAllAsync(Guid userKey, int skip, int take) => throw new NotImplementedException();
Task<Attempt<PagedModel<IUser>?, UserOperationStatus>> GetAllAsync(Guid performingUserKey, int skip, int take) => throw new NotImplementedException();
public Task<Attempt<PagedModel<IUser>, UserOperationStatus>> FilterAsync(
Guid userKey,
@@ -406,4 +406,6 @@ public interface IUserService : IMembershipUserService
Task<Attempt<UserOperationStatus>> SendResetPasswordEmailAsync(string userEmail);
Task<Attempt<UserInvitationResult, UserOperationStatus>> ResendInvitationAsync(Guid performingUserKey, UserResendInviteModel model);
Task<Attempt<PasswordChangedModel, UserOperationStatus>> ResetPasswordAsync(Guid performingUserKey, Guid userKey);
}

6
src/Umbraco.Core/Services/OperationStatus/UserOperationStatus.cs
View File

@@ -24,7 +24,7 @@ public enum UserOperationStatus
CannotDisableSelf,
CannotDeleteSelf,
CannotDisableInvitedUser,
OldPasswordRequired,
SelfOldPasswordRequired,
InvalidAvatar,
InvalidIsoCode,
InvalidInviteToken,
@@ -35,5 +35,7 @@ public enum UserOperationStatus
MediaNodeNotFound,
UnknownFailure,
CannotPasswordReset,
NotInInviteState
NotInInviteState,
SelfPasswordResetNotAllowed,
DuplicateId,
}

2
src/Umbraco.Core/Services/TwoFactorLoginServiceBase.cs
View File

@@ -37,7 +37,7 @@ internal abstract class TwoFactorLoginServiceBase
public virtual async Task<Attempt<IEnumerable<UserTwoFactorProviderModel>, TwoFactorOperationStatus>> GetProviderNamesAsync(Guid userKey)
{
IEnumerable<string> allProviders = _twoFactorLoginService.GetAllProviderNames();
var userProviders =(await _twoFactorLoginService.GetEnabledTwoFactorProviderNamesAsync(userKey)).ToHashSet();
var userProviders = (await _twoFactorLoginService.GetEnabledTwoFactorProviderNamesAsync(userKey)).ToHashSet();
IEnumerable<UserTwoFactorProviderModel> result = allProviders.Select(x => new UserTwoFactorProviderModel(x, userProviders.Contains(x)));
return Attempt.SucceedWithStatus(TwoFactorOperationStatus.Success, result);

90
src/Umbraco.Core/Services/UserService.cs
View File

@@ -603,12 +603,12 @@ internal class UserService : RepositoryService, IUserService
}
/// <inheritdoc/>
public async Task<Attempt<UserCreationResult, UserOperationStatus>> CreateAsync(Guid userKey, UserCreateModel model, bool approveUser = false)
public async Task<Attempt<UserCreationResult, UserOperationStatus>> CreateAsync(Guid performingUserKey, UserCreateModel model, bool approveUser = false)
{
using ICoreScope scope = ScopeProvider.CreateCoreScope();
using IServiceScope serviceScope = _serviceScopeFactory.CreateScope();
IUser? performingUser = await GetAsync(userKey);
IUser? performingUser = await GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -622,7 +622,7 @@ internal class UserService : RepositoryService, IUserService
return Attempt.FailWithStatus(UserOperationStatus.MissingUserGroup, new UserCreationResult());
}
UserOperationStatus result = ValidateUserCreateModel(model);
UserOperationStatus result = await ValidateUserCreateModel(model);
if (result != UserOperationStatus.Success)
{
return Attempt.FailWithStatus(result, new UserCreationResult());
@@ -724,12 +724,12 @@ internal class UserService : RepositoryService, IUserService
return Attempt.Succeed(UserOperationStatus.Success);
}
public async Task<Attempt<UserInvitationResult, UserOperationStatus>> InviteAsync(Guid userKey, UserInviteModel model)
public async Task<Attempt<UserInvitationResult, UserOperationStatus>> InviteAsync(Guid performingUserKey, UserInviteModel model)
{
using ICoreScope scope = ScopeProvider.CreateCoreScope();
using IServiceScope serviceScope = _serviceScopeFactory.CreateScope();
IUser? performingUser = await GetAsync(userKey);
IUser? performingUser = await GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -743,7 +743,7 @@ internal class UserService : RepositoryService, IUserService
return Attempt.FailWithStatus(UserOperationStatus.MissingUserGroup, new UserInvitationResult());
}
UserOperationStatus validationResult = ValidateUserCreateModel(model);
UserOperationStatus validationResult = await ValidateUserCreateModel(model);
if (validationResult is not UserOperationStatus.Success)
{
@@ -858,7 +858,7 @@ internal class UserService : RepositoryService, IUserService
return Attempt.SucceedWithStatus(UserOperationStatus.Success, new UserInvitationResult { InvitedUser = invitedUser });
}
private UserOperationStatus ValidateUserCreateModel(UserCreateModel model)
private async Task<UserOperationStatus> ValidateUserCreateModel(UserCreateModel model)
{
if (_securitySettings.UsernameIsEmail && model.UserName != model.Email)
{
@@ -869,6 +869,11 @@ internal class UserService : RepositoryService, IUserService
return UserOperationStatus.InvalidEmail;
}
if (model.Id is not null && await GetAsync(model.Id.Value) is not null)
{
return UserOperationStatus.DuplicateId;
}
if (GetByEmail(model.Email) is not null)
{
return UserOperationStatus.DuplicateEmail;
@@ -887,7 +892,7 @@ internal class UserService : RepositoryService, IUserService
return UserOperationStatus.Success;
}
public async Task<Attempt<IUser?, UserOperationStatus>> UpdateAsync(Guid userKey, UserUpdateModel model)
public async Task<Attempt<IUser?, UserOperationStatus>> UpdateAsync(Guid performingUserKey, UserUpdateModel model)
{
using ICoreScope scope = ScopeProvider.CreateCoreScope();
using IServiceScope serviceScope = _serviceScopeFactory.CreateScope();
@@ -897,10 +902,10 @@ internal class UserService : RepositoryService, IUserService
if (existingUser is null)
{
return Attempt.FailWithStatus(UserOperationStatus.MissingUser, existingUser);
return Attempt.FailWithStatus(UserOperationStatus.UserNotFound, existingUser);
}
IUser? performingUser = await userStore.GetAsync(userKey);
IUser? performingUser = await userStore.GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -1091,7 +1096,7 @@ internal class UserService : RepositoryService, IUserService
return keys;
}
public async Task<Attempt<PasswordChangedModel, UserOperationStatus>> ChangePasswordAsync(Guid userKey, ChangeUserPasswordModel model)
public async Task<Attempt<PasswordChangedModel, UserOperationStatus>> ChangePasswordAsync(Guid performingUserKey, ChangeUserPasswordModel model)
{
IServiceScope serviceScope = _serviceScopeFactory.CreateScope();
using ICoreScope scope = ScopeProvider.CreateCoreScope();
@@ -1103,15 +1108,16 @@ internal class UserService : RepositoryService, IUserService
return Attempt.FailWithStatus(UserOperationStatus.UserNotFound, new PasswordChangedModel());
}
IUser? performingUser = await userStore.GetAsync(userKey);
IUser? performingUser = await userStore.GetAsync(performingUserKey);
if (performingUser is null)
{
return Attempt.FailWithStatus(UserOperationStatus.MissingUser, new PasswordChangedModel());
}
// require old password for self change when outside of invite or resetByToken flows
if (performingUser.UserState != UserState.Invited && performingUser.Username == user.Username && string.IsNullOrEmpty(model.OldPassword) && string.IsNullOrEmpty(model.ResetPasswordToken))
{
return Attempt.FailWithStatus(UserOperationStatus.OldPasswordRequired, new PasswordChangedModel());
return Attempt.FailWithStatus(UserOperationStatus.SelfOldPasswordRequired, new PasswordChangedModel());
}
if (performingUser.IsAdmin() is false && user.IsAdmin())
@@ -1121,7 +1127,7 @@ internal class UserService : RepositoryService, IUserService
if (string.IsNullOrEmpty(model.ResetPasswordToken) is false)
{
Attempt<UserOperationStatus> verifyPasswordResetAsync = await VerifyPasswordResetAsync(userKey, model.ResetPasswordToken);
Attempt<UserOperationStatus> verifyPasswordResetAsync = await VerifyPasswordResetAsync(model.UserKey, model.ResetPasswordToken);
if (verifyPasswordResetAsync.Result != UserOperationStatus.Success)
{
return Attempt.FailWithStatus(verifyPasswordResetAsync.Result, new PasswordChangedModel());
@@ -1147,11 +1153,11 @@ internal class UserService : RepositoryService, IUserService
return Attempt.SucceedWithStatus(UserOperationStatus.Success, result.Result ?? new PasswordChangedModel());
}
public async Task<Attempt<PagedModel<IUser>?, UserOperationStatus>> GetAllAsync(Guid userKey, int skip, int take)
public async Task<Attempt<PagedModel<IUser>?, UserOperationStatus>> GetAllAsync(Guid performingUserKey, int skip, int take)
{
using ICoreScope scope = ScopeProvider.CreateCoreScope();
IUser? requestingUser = await GetAsync(userKey);
IUser? requestingUser = await GetAsync(performingUserKey);
if (requestingUser is null)
{
@@ -1364,7 +1370,7 @@ internal class UserService : RepositoryService, IUserService
};
}
public async Task<UserOperationStatus> DeleteAsync(Guid userKey, ISet<Guid> keys)
public async Task<UserOperationStatus> DeleteAsync(Guid performingUserKey, ISet<Guid> keys)
{
if(keys.Any() is false)
{
@@ -1372,7 +1378,7 @@ internal class UserService : RepositoryService, IUserService
}
using ICoreScope scope = ScopeProvider.CreateCoreScope();
IUser? performingUser = await GetAsync(userKey);
IUser? performingUser = await GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -1412,7 +1418,7 @@ internal class UserService : RepositoryService, IUserService
return UserOperationStatus.Success;
}
public async Task<UserOperationStatus> DisableAsync(Guid userKey, ISet<Guid> keys)
public async Task<UserOperationStatus> DisableAsync(Guid performingUserKey, ISet<Guid> keys)
{
if(keys.Any() is false)
{
@@ -1420,7 +1426,7 @@ internal class UserService : RepositoryService, IUserService
}
using ICoreScope scope = ScopeProvider.CreateCoreScope();
IUser? performingUser = await GetAsync(userKey);
IUser? performingUser = await GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -1458,7 +1464,7 @@ internal class UserService : RepositoryService, IUserService
return UserOperationStatus.Success;
}
public async Task<UserOperationStatus> EnableAsync(Guid userKey, ISet<Guid> keys)
public async Task<UserOperationStatus> EnableAsync(Guid performingUserKey, ISet<Guid> keys)
{
if(keys.Any() is false)
{
@@ -1466,7 +1472,7 @@ internal class UserService : RepositoryService, IUserService
}
using ICoreScope scope = ScopeProvider.CreateCoreScope();
IUser? performingUser = await GetAsync(userKey);
IUser? performingUser = await GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -1528,7 +1534,7 @@ internal class UserService : RepositoryService, IUserService
return UserOperationStatus.Success;
}
public async Task<Attempt<UserUnlockResult, UserOperationStatus>> UnlockAsync(Guid userKey, params Guid[] keys)
public async Task<Attempt<UserUnlockResult, UserOperationStatus>> UnlockAsync(Guid performingUserKey, params Guid[] keys)
{
if (keys.Length == 0)
{
@@ -1536,7 +1542,7 @@ internal class UserService : RepositoryService, IUserService
}
using ICoreScope scope = ScopeProvider.CreateCoreScope();
IUser? performingUser = await GetAsync(userKey);
IUser? performingUser = await GetAsync(performingUserKey);
if (performingUser is null)
{
@@ -2102,6 +2108,40 @@ internal class UserService : RepositoryService, IUserService
return changePasswordAttempt;
}
public async Task<Attempt<PasswordChangedModel, UserOperationStatus>> ResetPasswordAsync(Guid performingUserKey, Guid userKey)
{
if (performingUserKey.Equals(userKey))
{
return Attempt.FailWithStatus(UserOperationStatus.SelfPasswordResetNotAllowed, new PasswordChangedModel());
}
using ICoreScope scope = ScopeProvider.CreateCoreScope();
using IServiceScope serviceScope = _serviceScopeFactory.CreateScope();
ICoreBackOfficeUserManager backOfficeUserManager = serviceScope.ServiceProvider.GetRequiredService<ICoreBackOfficeUserManager>();
var generatedPassword = backOfficeUserManager.GeneratePassword();
Attempt<PasswordChangedModel, UserOperationStatus> changePasswordAttempt =
await ChangePasswordAsync(performingUserKey, new ChangeUserPasswordModel
{
NewPassword = generatedPassword,
UserKey = userKey,
});
scope.Complete();
// todo tidy this up
// this should be part of the result of the ChangePasswordAsync() method
// but the model requires NewPassword
// and the passwordChanger does not have a codePath that deals with generating
if (changePasswordAttempt.Success)
{
changePasswordAttempt.Result.ResetPassword = generatedPassword;
}
return changePasswordAttempt;
}
/// <summary>
@@ -2234,7 +2274,7 @@ internal class UserService : RepositoryService, IUserService
results.Add(new NodePermissions { NodeKey = idKeyMap[nodeId], Permissions = permissions });
}
return Attempt.SucceedWithStatus<IEnumerable<NodePermissions>, UserOperationStatus>(UserOperationStatus.UserNotFound, results);
return Attempt.SucceedWithStatus<IEnumerable<NodePermissions>, UserOperationStatus>(UserOperationStatus.Success, results);
}
/// <summary>