We have moved to GitHub Issues
Created by Jeffrey Schoemaker 08 Aug 2017, 08:49:24 Updated by Sebastiaan Janssen 08 Sep 2017, 22:15:28
Subtask of: UAASSCRUM-1073
In my opinion changing the password should be as follows:
Click on your avatar, click change password. Always specify your old password and then put in the new password (twice). In that way there's no option that if your still logged in on Umbraco but away from your computer anybody can change your password. Currently by default you only have to specify the new password twice.
User-section, change password. You would always only add in the new password twice. I don't see why there should be a button "Reset password". In my opinion there's no use for that.
I know in previous versions of Umbraco we had all kinds of settings possible on the UsersMemberProvider like "allowManuallyChangingPassword", "enablePasswordReset", "enablePasswordRetrieval", but I think this wouldn't have to be an configuration setting for the user section. It should be just like this :)
Most of those settings are still relevant and it all comes down to backward compatibility since we still have active Membership providers which ASP.NET Identity exists alongside and extends. Both auth mechanisms share the exact same logic ... because backwards compatibility.
FYI, you do have to specify your old password to change your own, see screenshot If that is not the case for you, then you have allowManuallyChangingPassword set to true. This is another compat setting because many admins want the ability to manually change a user's password. By default in both ASP.NET Identity and Membership providers, you cannot just do that so this is a special switch to allow that.
If you don't want to have the password reset thing, then set enablePasswordReset = false
I've had a look though and yes both AllowManuallyChangingPassword and EnablePasswordReset is true by default, so that can be changed to false, i'll have a look
There's also this special case too: https://github.com/umbraco/Umbraco-CMS/blob/dev-v7.7/src/Umbraco.Core/Security/MembershipProviderExtensions.cs#L29
@Shandem A question:
I have created a new user with Editor rights and extra access to the user section. When editing the user I see the rest password checkbox but in the user dialog, it is gone. The model I get from the "GetMembershipProbiderConfig" endpoint tells me that "enableReset" is true in both cases.
I found out that we hardcode this to false for the user dialog. Do you know the reason other than what the comment says? It doesn't make much sense when we have a setting.
maybe I'm getting crazy from these settings, but IMHO you can't get it to work like it 'supposed' to be. You can only change your own password when you specify the old password.
If you have access to the User section you can always changes everyone's password (except your own) by only specifying the new password twice.
If tried all kinds of combinations but didn't get this output.
@email@example.com To be clear, you expect:
And this doesn't currently work for you?
Exactly. And I can't get a combination of settings where this works
Cool, I'll look at it today!
OK, here's the info.
AllowManuallyChangingPassword is now false by default, i thought i had changed it but I didn't. This should be false otherwise it allows for less secure API usage. This setting has always existed as a hack for legacy purposes. I've also fixed editing your own user in the user section so that you cannot reset your own password (just like in the change password dialog)
In both ASP.NET Identity and ASP.NET MembershipProvider APIs there is no API that allows you to arbitrarily change a user's password without first knowing their existing password. (hence the need for the AllowManuallyChangingPassword hack mentioned above). The only APIs that exist in these 2 frameworks to do something similar is to reset the password - which is a hack in itself since this is meant to be used for emails to reset their own passwords.
By default, the user's membership provider config will simply be:
<add name="UsersMembershipProvider" type="Umbraco.Web.Security.Providers.UsersMembershipProvider, Umbraco" />
Which will mean:
These settings are what control the UI password changing functionality. Therefore this means that
Regarding the "weird checkbox-reset thing" - our options for this are:
It should be a button saying: Do you want to
- Reset password (press this button) or
- provide the old password and a new password (click here to show the three input fields to do that)
What do you guys think?
Righto, the changes were easier than i thought so here's what you get now which is what Seb mentioned before:
I've tested these scenarios and all works. The changes can be found here:
Should be good now :)
angular.min.js?cdv=1:63 TypeError: Cannot read property 'oldPassword' of undefined
at c.save (http://localhost:7700/umbraco/js/umbraco.controllers.js?cdv=1:20849:68)
at Object.d.(anonymous function) [as action] (http://localhost:7700/umbraco/lib/angular/1.1.5/angular.min.js?cdv=1:44:33)
at Object.$eval (http://localhost:7700/umbraco/lib/angular/1.1.5/angular.min.js?cdv=1:92:272)
at Object.$apply (http://localhost:7700/umbraco/lib/angular/1.1.5/angular.min.js?cdv=1:92:379)
I think #1 is okay for now, we can update in 7.7.1, it generates a new password and shows it. If it's easy to fix then it would be great to fix of course!
Should we still merge https://github.com/umbraco/Umbraco-CMS/pull/2140 ?
I couldn't get a breakpoint to trigger in that code, so not sure what it does. If you want to merge then go ahead, seems harmless.
Righto... i think this should be good now :)
Thanks for all the testing as this revealed a few other odd quirks with password changing (that have existed for some time too)
I've pushed all code changes and merged that branch in. There was some issues with (for example), resetting a members password and then pressing save again without changing anything would cause an error. Also when you changed your own password, the password values weren't cleared from the text boxes. The 1 character thing was actually caused by a different bug/quirk with the reset password switch in js so that's fixed too.
Whoops! Commented on the wrong issue!
Works great now!
Backwards Compatible: True
Affected versions: 7.7.0
Due in version: 7.7.0
Sprint: Sprint 67
Story Points: 1