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

Merge remote-tracking branch 'origin/netcore/dev' into netcore/members-roles-save

Browse Source 
# Conflicts:
#	src/Umbraco.Infrastructure/Security/MemberUserStore.cs
#	src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs
#	src/Umbraco.Web.BackOffice/Controllers/MemberController.cs
This commit is contained in:
Bjarke Berg
2021-03-18 08:27:17 +01:00
parent ed931a9629 8c66470d65
commit 6ce3a5f2a9
64 changed files with 987 additions and 374 deletions
Download Patch File Download Diff File Expand all files Collapse all files

165
src/Umbraco.Web.BackOffice/Controllers/MemberController.cs
View File

@@ -20,6 +20,7 @@ using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
@@ -58,6 +59,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
private readonly IJsonSerializer _jsonSerializer;
private readonly IShortStringHelper _shortStringHelper;
private readonly IPasswordChanger<MemberIdentityUser> _passwordChanger;
private readonly IScopeProvider _scopeProvider;
/// <summary>
/// Initializes a new instance of the <see cref="MemberController"/> class.
@@ -90,7 +92,8 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
IDataTypeService dataTypeService,
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IJsonSerializer jsonSerializer,
IPasswordChanger<MemberIdentityUser> passwordChanger)
IPasswordChanger<MemberIdentityUser> passwordChanger,
IScopeProvider scopeProvider)
: base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, jsonSerializer)
{
_propertyEditors = propertyEditors;
@@ -104,6 +107,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
_jsonSerializer = jsonSerializer;
_shortStringHelper = shortStringHelper;
_passwordChanger = passwordChanger;
_scopeProvider = scopeProvider;
}
/// <summary>
@@ -261,9 +265,13 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
return new ValidationErrorResult(forDisplay);
}
// Create a scope here which will wrap all child data operations in a single transaction.
// We'll complete this at the end of this method if everything succeeeds, else
// all data operations will roll back.
using IScope scope = _scopeProvider.CreateScope();
// Depending on the action we need to first do a create or update using the membership manager
// this ensures that passwords are formatted correctly and also performs the validation on the provider itself.
switch (contentItem.Action)
{
case ContentSaveAction.Save:
@@ -287,9 +295,6 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
return NotFound();
}
// TODO: There's 3 things saved here and we should do this all in one transaction, which we can do here by wrapping in a scope
// but it would be nicer to have this taken care of within the Save method itself
// return the updated model
MemberDisplay display = _umbracoMapper.Map<MemberDisplay>(contentItem.PersistedContent);
@@ -300,19 +305,20 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
return new ValidationErrorResult(display, StatusCodes.Status403Forbidden);
}
ILocalizedTextService localizedTextService = _localizedTextService;
// put the correct messages in
switch (contentItem.Action)
{
case ContentSaveAction.Save:
case ContentSaveAction.SaveNew:
display.AddSuccessNotification(
localizedTextService.Localize("speechBubbles/editMemberSaved"),
localizedTextService.Localize("speechBubbles/editMemberSaved"));
_localizedTextService.Localize("speechBubbles/editMemberSaved"),
_localizedTextService.Localize("speechBubbles/editMemberSaved"));
break;
}
// Mark transaction to commit all changes
scope.Complete();
return display;
}
@@ -329,7 +335,8 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
}
// map the custom properties - this will already be set for new entities in our member binder
contentItem.PersistedContent.Email = contentItem.Email;
contentItem.PersistedContent.IsApproved = contentItem.IsApproved;
contentItem.PersistedContent.Email = contentItem.Email.Trim();
contentItem.PersistedContent.Username = contentItem.Username;
// use the base method to map the rest of the properties
@@ -395,15 +402,29 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
_memberService.Save(member);
contentItem.PersistedContent = member;
await AddOrUpdateRoles(contentItem, identityMember);
ActionResult<bool> rolesChanged = await AddOrUpdateRoles(contentItem.Groups, identityMember);
if (!rolesChanged.Value && rolesChanged.Result != null)
{
return rolesChanged.Result;
}
return true;
}
/// <summary>
/// Update the member security data
/// If the password has been reset then this method will return the reset/generated password, otherwise will return null.
/// Update existing member data
/// </summary>
/// <param name="contentItem">The member to save</param>
/// <remarks>
/// We need to use both IMemberService and ASP.NET Identity to do our updates because Identity is responsible for passwords/security.
/// When this method is called, the IMember will already have updated/mapped values from the http POST.
/// So then we do this in order:
/// 1. Deal with sensitive property values on IMember
/// 2. Use IMemberService to persist all changes
/// 3. Use ASP.NET and MemberUserManager to deal with lockouts
/// 4. Use ASP.NET, MemberUserManager and password changer to deal with passwords
/// 5. Deal with groups/roles
/// </remarks>
private async Task<ActionResult<bool>> UpdateMemberAsync(MemberSave contentItem)
{
contentItem.PersistedContent.WriterId = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id;
@@ -421,6 +442,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
foreach (IPropertyType sensitiveProperty in sensitiveProperties)
{
// TODO: This logic seems to deviate from the logic that is in v8 where we are explitly checking
// against 3 properties: Comments, IsApproved, IsLockedOut, is the v8 version incorrect?
ContentPropertyBasic destProp = contentItem.Properties.FirstOrDefault(x => x.Alias == sensitiveProperty.Alias);
if (destProp != null)
{
@@ -431,27 +455,37 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
}
}
bool isLockedOut = contentItem.IsLockedOut;
// First save the IMember with mapped values before we start updating data with aspnet identity
_memberService.Save(contentItem.PersistedContent);
// if they were locked but now they are trying to be unlocked
if (contentItem.PersistedContent.IsLockedOut && isLockedOut == false)
{
contentItem.PersistedContent.IsLockedOut = false;
contentItem.PersistedContent.FailedPasswordAttempts = 0;
}
else if (!contentItem.PersistedContent.IsLockedOut && isLockedOut)
{
// NOTE: This should not ever happen unless someone is mucking around with the request data.
// An admin cannot simply lock a user, they get locked out by password attempts, but an admin can un-approve them
ModelState.AddModelError("custom", "An admin cannot lock a user");
}
bool needsResync = false;
MemberIdentityUser identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString());
if (identityMember == null)
{
return new ValidationErrorResult("Identity member was not found");
return new ValidationErrorResult("Member was not found");
}
// Handle unlocking with the member manager (takes care of other nuances)
if (identityMember.IsLockedOut && contentItem.IsLockedOut == false)
{
IdentityResult unlockResult = await _memberManager.SetLockoutEndDateAsync(identityMember, DateTimeOffset.Now.AddMinutes(-1));
if (unlockResult.Succeeded == false)
{
return new ValidationErrorResult(
$"Could not unlock for member {contentItem.Id} - error {unlockResult.Errors.ToErrorMessage()}");
}
needsResync = true;
}
else if (identityMember.IsLockedOut == false && contentItem.IsLockedOut)
{
// NOTE: This should not ever happen unless someone is mucking around with the request data.
// An admin cannot simply lock a user, they get locked out by password attempts, but an admin can unlock them
return new ValidationErrorResult("An admin cannot lock a member");
}
// If we're changing the password...
// Handle changing with the member manager & password changer (takes care of other nuances)
if (contentItem.Password != null)
{
IdentityResult validatePassword = await _memberManager.ValidatePasswordAsync(contentItem.Password.NewPassword);
@@ -466,18 +500,6 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
return new ValidationErrorResult("Member ID was not valid");
}
IMember foundMember = _memberService.GetById(intId.Result);
if (foundMember == null)
{
return new ValidationErrorResult("Member was not found");
}
IUser currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser;
// if the current user has access to reset/manually change the password
if (currentUser.HasSectionAccess(Constants.Applications.Members) == false)
{
return new ValidationErrorResult("The current user is not authorized");
}
var changingPasswordModel = new ChangingPasswordModel
{
Id = intId.Result,
@@ -485,44 +507,39 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
NewPassword = contentItem.Password.NewPassword,
};
// Change and persist the password
Attempt<PasswordChangedModel> passwordChangeResult = await _passwordChanger.ChangePasswordWithIdentityAsync(changingPasswordModel, _memberManager);
if (passwordChangeResult.Result.ChangeError?.MemberNames != null)
if (!passwordChangeResult.Success)
{
foreach (string memberName in passwordChangeResult.Result.ChangeError?.MemberNames)
foreach (string memberName in passwordChangeResult.Result?.ChangeError?.MemberNames ?? Enumerable.Empty<string>())
{
ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError?.ErrorMessage ?? string.Empty);
}
return new ValidationErrorResult(new SimpleValidationModel(ModelState.ToErrorDictionary()));
}
if (!passwordChangeResult.Success)
{
return new ValidationErrorResult("The password could not be changed");
}
// get the identity member now the password and dates have changed
identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString());
//TODO: confirm this is correct
contentItem.PersistedContent.RawPasswordValue = identityMember.PasswordHash;
if (identityMember.LastPasswordChangeDateUtc != null)
{
contentItem.PersistedContent.LastPasswordChangeDate = (DateTime)identityMember.LastPasswordChangeDateUtc;
}
needsResync = true;
}
IdentityResult updatedResult = await _memberManager.UpdateAsync(identityMember);
if (updatedResult.Succeeded == false)
// Update the roles and check for changes
ActionResult<bool> rolesChanged = await AddOrUpdateRoles(contentItem.Groups, identityMember);
if (!rolesChanged.Value && rolesChanged.Result != null)
{
return new ValidationErrorResult(updatedResult.Errors.ToErrorMessage());
return rolesChanged.Result;
}
else
{
needsResync = true;
}
_memberService.Save(contentItem.PersistedContent);
// If there have been underlying changes made by ASP.NET Identity, then we need to resync the
// IMember on the PersistedContent with what is stored since it will be mapped to display.
if (needsResync)
{
contentItem.PersistedContent = _memberService.GetById(contentItem.PersistedContent.Id);
}
await AddOrUpdateRoles(contentItem, identityMember);
return true;
}
@@ -585,10 +602,12 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
/// <summary>
/// Add or update the identity roles
/// </summary>
/// <param name="contentItem">The member content item</param>
/// <param name="groups">The groups to updates</param>
/// <param name="identityMember">The member as an identity user</param>
private async Task AddOrUpdateRoles(MemberSave contentItem, MemberIdentityUser identityMember)
private async Task<ActionResult<bool>> AddOrUpdateRoles(IEnumerable<string> groups, MemberIdentityUser identityMember)
{
var hasChanges = false;
// We're gonna look up the current roles now because the below code can cause
// events to be raised and developers could be manually adding roles to members in
// their handlers. If we don't look this up now there's a chance we'll just end up
@@ -597,22 +616,34 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
// find the ones to remove and remove them
IEnumerable<string> roles = currentRoles.ToList();
string[] rolesToRemove = roles.Except(contentItem.Groups).ToArray();
string[] rolesToRemove = roles.Except(groups).ToArray();
// Now let's do the role provider stuff - now that we've saved the content item (that is important since
// if we are changing the username, it must be persisted before looking up the member roles).
if (rolesToRemove.Any())
{
IdentityResult rolesIdentityResult = await _memberManager.RemoveFromRolesAsync(identityMember, rolesToRemove);
IdentityResult identityResult = await _memberManager.RemoveFromRolesAsync(identityMember, rolesToRemove);
if (!identityResult.Succeeded)
{
return ValidationErrorResult.CreateNotificationValidationErrorResult(identityResult.Errors.ToErrorMessage());
}
hasChanges = true;
}
// find the ones to add and add them
string[] toAdd = contentItem.Groups.Except(roles).ToArray();
string[] toAdd = groups.Except(roles).ToArray();
if (toAdd.Any())
{
// add the ones submitted
IdentityResult identityResult = await _memberManager.AddToRolesAsync(identityMember, toAdd);
if (!identityResult.Succeeded)
{
return ValidationErrorResult.CreateNotificationValidationErrorResult(identityResult.Errors.ToErrorMessage());
}
hasChanges = true;
}
return hasChanges;
}
/// <summary>