Solidus: Remove admin ability to set a password, replace reset password email trigger.

[athal7]

In the spirit of better security. relates to https://github.com/solidusio/solidus/pull/421

[mamhoff]

@acreilly is working on this

[seand7565]

An admin would still have the ability to change the email associated with the account and then trigger the email reset - essentially giving them the ability to change the password. Should the ability to change a users email also be disabled? I can't think of too many use cases in which you'd need to change a users email. @acreilly

[ericsaupe]

The only time I could think that an admin would _need_ to be able to change the email address is if the user no longer has access to that email and would need their password changed. New account is the path there maybe?

[acreilly]

@ericsaupe Lets say the data associated with that account is important to the user though, I think keeping the ability to change the email would be helpful. It isn't a common scenario, so if we really want to take it out, I guess a developer can go into the console and change it. What do you think?

[ericsaupe]

Yeah, that's what I was thinking too. For our customer service reps I know they would be really angry if we simply told them to tell users to create a new account and lose all of their order history, saved cards, etc. Probably best to leave the ability to change emails in and let individual stores change that if they need to.

[acreilly]

It doesn't appear that we use this view: https://github.com/solidusio/solidus_auth_devise/blob/master/lib/views/backend/spree/admin/user_passwords/edit.html.erb

Does having the reset_password_token increase security? Or do we just want to stick with adding the Send Reset Password Email button

[ericsaupe]

It looks like it is required in the Devise method, https://github.com/plataformatec/devise/blob/master/app/controllers/devise/passwords_controller.rb#L32. I think it is simply for resetting the password through the controller but the issue here is to remove the change password form all together to simply send the reset email. So if it's not used it could probably be removed all together.

[acreilly]

@ericsaupe I'm not sure if you are here today... but if you are would you mind helping me for a second? I am pretty easy to spot out in the room hah..

[ericsaupe]

Of course! I'm next to that @skukx purple faced guy from that other PR. I'll come find you in a second.

[mamhoff]

Why don't you let customer support change the email address and then send out a password reset token?

[ericsaupe]

@mamhoff good idea. This task seems to be to add that ability to send the password reset using the admin.

[tvdeyen]

Why don't you let customer support change the email address and then send out a password reset token?

@mamhoff @ericsaupe This is exactly what #1942 does. Email address is already changeable and it adds a button to the users account page that allows admins to send a password reset token via email.

[BenMorganIO]

I'm :-1: on this. I think it would be better if we had a superadmin role and an admin role. At some point, store owners will be a little upset they cannot make these changes.

[seand7565]

IMO an admin being able to set any users password creates unnecessary security issues. I get that some business owners may not like not having that ability - but there are some business owners that don't like not being able to see credit card numbers on orders - at some point you have to draw a line between platform security & business needs. In this case my opinion is that the security of the platform takes precedence.

[jacobherrington]

Since there are arguments for both paths, is there a possibility this could be set up as a config option?

I tend to favor removing this ability by default.

[skukx]

Security speaking, I don't think it is ever a good idea to allow anyone other than the user themselves set the password. I think this is a good step in the right direction

[BenMorganIO]

@seand7565 I think credit card number vs. password is a whole different level of security; the former potentially leading to some business owners to court. There really is no security issue with giving a business owner access to changing passwords. They can do this with GSuite to manage employees, they can do it to their users on AWS, why not on _their own_ platform?

The decision to remove this from a business owner who already owns the data seems a bit redundant and frustrating.

The real security issue is the ability for the password to be poor or if the session is left open. Not much ado with poor passwords as that's up to the users naïvity. Simply ask for the current user's password and _then_ you've covered security threats and kept business owners not left cursing.

[seand7565]

@BenMorganIO There's a big difference between managing your employees passwords and managing your users passwords, and there is absolutely a security risk - I provided a link above that detailed a few of them (admin being able to log in and complete actions as a particular user, and setting the password usually leads to unsecured communication of the new updated password - e.g. "We've reset your password, it's now hunter2" via email in response to someone losing their password). Furthermore, the concept of password ownership as you've mentioned is a complicated subject, but I think it's safer to err on the side of the user owning the password rather than the owner - especially with everything going on in Europe right now with GDPR. Also, I imagine selling your users passwords would also lead to business owners in court, regardless of who owns them.

I admit that requiring the current password to set a new password is a big improvement over what is there currently, but I still prefer the solution proposed with the #1942 - it just seems less limiting to me. Perhaps, if the issue is employee management, we should allow the setting of passwords for admin user accounts only? (That is, super admin can set admin users passwords, but not regular users passwords)

[BenMorganIO]

I'm not educated on the requirements of GDPR (ignorant Canadian) but yes, I agree with the password ownership problem with an admin setting it.

Since an employee account, such as[email protected], is owned by Ecommerce Store, Inc. then this doesn't fall under GDPR and the password ownership is owned by the business entity.

I agree with this proposal :+1:. If a superadmin can change and takeover an employees account (or most likely an ex-employees account).

[zabrador]

It looks like this might've been solved with the above PR (https://github.com/solidusio/solidus_auth_devise/pull/146), can this issue now be closed?

[JDutil]

@zabrador this issue appears to only be half fixed. The PR you're referencing adds the ability to send a reset password email, but it does not remove the ability for an admin to set a password. I'm going to file a PR removing the password change form.

[tvdeyen]

The password fields should be still available, but protected by a permission.

All of these above mentioned arguments are correct, but it is not on us to make this decision. It is the store owners choice.

We as a framework provider should only give the tools, but not remove features we think are not compliant or problematic.