U4-11019 - Member 'Locked out' marked as sensitive throws an error

Created by Jeffrey Schoemaker 27 Feb 2018, 14:04:32 Updated by Jacob Midtgaard-Olesen 07 May 2018, 08:10:18

Tags: Unscheduled

In Umbraco 7.9, if you mark the field 'Is locked out' as sensitive it will give an error when creating a new member or looking at a member.

An error occured

Mapping types: Member -> IEnumerable1 Umbraco.Core.Models.Member -> System.Collections.Generic.IEnumerable1[[Umbraco.Web.Models.ContentEditing.Tab`1[[Umbraco.Web.Models.ContentEditing.ContentPropertyDisplay, umbraco, Version=1.0.6631.28270, Culture=neutral, PublicKeyToken=null]], umbraco, Version=1.0.6631.28270, Culture=neutral, PublicKeyToken=null]]

Destination path: MemberDisplay.Tabs.Tabs

Source value: Umbraco.Core.Models.Member

Exception Details AutoMapper.AutoMapperMappingException:

Mapping types: Member -> IEnumerable1 Umbraco.Core.Models.Member -> System.Collections.Generic.IEnumerable1[[Umbraco.Web.Models.ContentEditing.Tab`1[[Umbraco.Web.Models.ContentEditing.ContentPropertyDisplay, umbraco, Version=1.0.6631.28270, Culture=neutral, PublicKeyToken=null]], umbraco, Version=1.0.6631.28270, Culture=neutral, PublicKeyToken=null]]

Destination path: MemberDisplay.Tabs.Tabs

Source value: Umbraco.Core.Models.Member

2 Attachments

Comments

Shannon Deminick 01 Mar 2018, 06:17:10

Fixed pushed to dev branch: ccfb64305519134afdaf6781160f2c4cb7336fe3

The error was that the value was always expected to not be null, but with sensitive values, when those are processed, the value is set to null and the property editor is changed so we just need an extra null check in 2 places.

Have tested with and without a user that has access to sensitive values and for creating and updating a member.


Shannon Deminick 01 Mar 2018, 06:28:59

However, I've found another issue. The problem is that these values are part of the IMember model but they are actually property type fields which haven't been taken into account.

There's some JS that needs to be updated and then we need to do some additional server side checks for this since currently the checks are only done against user created property types, not the built in ones.

Stay tuned.


Shannon Deminick 01 Mar 2018, 07:09:59

Ok all fixed, here's the 3x revisions to look at:

*ccfb64305519134afdaf6781160f2c4cb7336fe3 **The error was that the value was always expected to not be null, but with sensitive values, when those are processed, the value is set to null and the property editor is changed so we just need an extra null check in 2 places. *8768947519c8e732b8d16f084da711718ba30512 **This fixes the JS model formatter to deal with null values, since the values for these special properties will be null when marked as sensitive and the user doesn't have access, but a boolean (not nullable) value must be specified. This defaults to "false" **Since this defaults to "false" it is sent to the server and since the IsApproved, IsLockedOut and Comments are special fields that are bound directly to the MemberSave and IMember instances, we need to check if these have been marked as sensitive and if so and the user doesn't have access, we ensure that the already persisted values are set on the MemberSave model so they cannot actually be changed no matter what is sent in the request *78489237d9cd3b427d86c79b983902ac59dcd7f5 **This fixes the above JS model formatter when dealing with null values, i previously set both default to "false", however the IsApproved needs to be defaulted to "true". The reason for that is when a user that doesn't have access to sensitive values creates a new member, by default the member should be Approved = true. I didn't put any server side checks in for this value in the MemberController.CreateWithMembershipProvider because if someone were to modify the request manually, sure, they can hack it and change this value to false and the member would have Approved = false, but that's not really anything to worry about


Shannon Deminick 01 Mar 2018, 07:23:53

To Test:

  • Set IsApproved on the member type to be sensitive
  • Go to that member and ensure that this is checked and save the member
  • Log in with a user without sensitive data access
  • Go to that member, you will will not be able to see the IsApproved value
  • Save the member - this will actually send a "false" value to the server for IsApproved
  • Login as admin, go to that member and verify that the IsApproved value is still true

Next

  • Make sure you can create a member
  • Log in as admin and view the member, IsApproved should be true


Robert Copilau 01 Mar 2018, 09:44:00

Test Results:

*Set IsApproved on the member type to be sensitive '''(check)''' *Go to that member and ensure that this is checked and save the member '''(check)''' *Login with a user without sensitive data access '''(check)''' *Go to that member, you will not be able to see the IsApproved value '''(check)''' *Save the member - this will actually send a "false" value to the server for IsApproved '''(check)''' *Login as admin, go to that member and verify that the IsApproved value is still true '''(check)'''

Next

*Make sure you can create a member '''(check)''' *Log in as admin and view the member, IsApproved should be true '''(check)'''


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.9.0

Due in version: 7.9.1

Sprint: Sprint 79

Story Points: 1

Cycle: 8