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

Revert rather than prevent updates to sensitive properties on members without sensitive data access (#18794)

Browse Source 
* Revert rather than prevent updates to sensitive properties on members without sensitive data access.

* Added suppression for integration test updates.
This commit is contained in:
Andy Butland
2025-03-31 15:16:41 +02:00
committed by GitHub
parent c5a3778ce7
commit f200da48d8
4 changed files with 50 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/Umbraco.Infrastructure/Services/MemberEditingService.cs
View File

@@ -149,12 +149,11 @@ internal sealed class MemberEditingService : IMemberEditingService
if (user.HasAccessToSensitiveData() is false)
{
// handle sensitive data. certain member properties (IsApproved, IsLockedOut) are subject to "sensitive data" rules.
if (member.IsLockedOut != updateModel.IsLockedOut || member.IsApproved != updateModel.IsApproved)
{
status.ContentEditingOperationStatus = ContentEditingOperationStatus.NotAllowed;
return Attempt.FailWithStatus(status, new MemberUpdateResult());
}
// Handle sensitive data. Certain member properties (IsApproved, IsLockedOut) are subject to "sensitive data" rules.
// The client won't have received these, so will always be false.
// We should reset them back to their original values before proceeding with the update.
updateModel.IsApproved = member.IsApproved;
updateModel.IsLockedOut = member.IsLockedOut;
}
MemberIdentityUser? identityMember = await _memberManager.FindByIdAsync(member.Id.ToString());

45
src/Umbraco.Web.UI.Client/src/packages/members/member/workspace/member/views/member/member-workspace-view-member.element.ts
View File

@@ -9,6 +9,7 @@ import type { UUIBooleanInputEvent } from '@umbraco-cms/backoffice/external/uui'
import './member-workspace-view-member-info.element.js';
import type { UmbInputMemberGroupElement } from '@umbraco-cms/backoffice/member-group';
import { UMB_CURRENT_USER_CONTEXT } from '@umbraco-cms/backoffice/current-user';
@customElement('umb-member-workspace-view-member')
export class UmbMemberWorkspaceViewMemberElement extends UmbLitElement implements UmbWorkspaceViewElement {
@@ -24,6 +25,12 @@ export class UmbMemberWorkspaceViewMemberElement extends UmbLitElement implement
this._isNew = !!isNew;
});
});
this.consumeContext(UMB_CURRENT_USER_CONTEXT, (context) => {
this.observe(context.hasAccessToSensitiveData, (hasAccessToSensitiveData) => {
this._hasAccessToSensitiveData = hasAccessToSensitiveData === true;
});
});
}
@state()
@@ -35,6 +42,9 @@ export class UmbMemberWorkspaceViewMemberElement extends UmbLitElement implement
@state()
private _isNew = true;
@state()
private _hasAccessToSensitiveData = false;
#onChange(propertyName: keyof UmbMemberDetailModel, value: UmbMemberDetailModel[keyof UmbMemberDetailModel]) {
if (!this._workspaceContext) return;
@@ -172,23 +182,26 @@ export class UmbMemberWorkspaceViewMemberElement extends UmbLitElement implement
.selection=${this._workspaceContext.memberGroups}></umb-input-member-group>
</umb-property-layout>
<umb-property-layout label=${this.localize.term('user_stateApproved')}>
<uui-toggle
slot="editor"
.checked=${this._workspaceContext.isApproved}
@change=${(e: UUIBooleanInputEvent) => this.#onChange('isApproved', e.target.checked)}>
</uui-toggle>
</umb-property-layout>
<umb-property-layout label=${this.localize.term('user_stateLockedOut')}>
<uui-toggle
slot="editor"
?disabled=${this._isNew || !this._workspaceContext.isLockedOut}
.checked=${this._workspaceContext.isLockedOut}
@change=${(e: UUIBooleanInputEvent) => this.#onChange('isLockedOut', e.target.checked)}>
</uui-toggle>
</umb-property-layout>
${when(this._hasAccessToSensitiveData,
() => html`
<umb-property-layout label=${this.localize.term('user_stateApproved')}>
<uui-toggle
slot="editor"
.checked=${this._workspaceContext!.isApproved}
@change=${(e: UUIBooleanInputEvent) => this.#onChange('isApproved', e.target.checked)}>
</uui-toggle>
</umb-property-layout>
<umb-property-layout label=${this.localize.term('user_stateLockedOut')}>
<uui-toggle
slot="editor"
?disabled=${this._isNew || !this._workspaceContext!.isLockedOut}
.checked=${this._workspaceContext!.isLockedOut}
@change=${(e: UUIBooleanInputEvent) => this.#onChange('isLockedOut', e.target.checked)}>
</uui-toggle>
</umb-property-layout>
`)
}
<umb-property-layout label=${this.localize.term('member_2fa')}>
<uui-toggle
slot="editor"

14
tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml
View File

@@ -92,6 +92,20 @@
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.MemberEditingServiceTests.Cannot_Change_IsApproved_Without_Access</Target>
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.MemberEditingServiceTests.Cannot_Change_IsLockedOut_Without_Access</Target>
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.TemplateServiceTests.Deleting_Master_Template_Also_Deletes_Children</Target>

63
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberEditingServiceTests.cs
View File

@@ -1,4 +1,4 @@
using NUnit.Framework;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
@@ -341,7 +341,6 @@ public class MemberEditingServiceTests : UmbracoIntegrationTest
{
Email = "test-updated@test.com",
Username = "test-updated",
IsApproved = member.IsApproved,
InvariantName = "T. Est Updated",
InvariantProperties = new[]
{
@@ -363,67 +362,9 @@ public class MemberEditingServiceTests : UmbracoIntegrationTest
Assert.AreEqual("test-updated@test.com", member.Email);
Assert.AreEqual("test-updated", member.Username);
Assert.AreEqual("T. Est Updated", member.Name);
}
[Test]
public async Task Cannot_Change_IsApproved_Without_Access()
{
// this user does NOT have access to sensitive data
var user = UserBuilder.CreateUser();
UserService.Save(user);
var member = await CreateMemberAsync();
var updateModel = new MemberUpdateModel
{
Email = member.Email,
Username = member.Username,
IsApproved = false,
InvariantName = member.Name,
InvariantProperties = member.Properties.Select(property => new PropertyValueModel
{
Alias = property.Alias,
Value = property.GetValue()
})
};
var result = await MemberEditingService.UpdateAsync(member.Key, updateModel, user);
Assert.IsFalse(result.Success);
Assert.AreEqual(ContentEditingOperationStatus.NotAllowed, result.Status.ContentEditingOperationStatus);
member = await MemberEditingService.GetAsync(member.Key);
Assert.IsNotNull(member);
// IsApproved and IsLockedOut are always sensitive properties.
Assert.IsTrue(member.IsApproved);
}
[Test]
public async Task Cannot_Change_IsLockedOut_Without_Access()
{
// this user does NOT have access to sensitive data
var user = UserBuilder.CreateUser();
UserService.Save(user);
var member = await CreateMemberAsync();
var updateModel = new MemberUpdateModel
{
Email = member.Email,
Username = member.Username,
IsLockedOut = true,
InvariantName = member.Name,
InvariantProperties = member.Properties.Select(property => new PropertyValueModel
{
Alias = property.Alias,
Value = property.GetValue()
})
};
var result = await MemberEditingService.UpdateAsync(member.Key, updateModel, user);
Assert.IsFalse(result.Success);
Assert.AreEqual(ContentEditingOperationStatus.NotAllowed, result.Status.ContentEditingOperationStatus);
member = await MemberEditingService.GetAsync(member.Key);
Assert.IsNotNull(member);
Assert.IsFalse(member.IsLockedOut);
}