modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

Remove the "Show password on screen" and optimize handling with admin provided passwords #12162

Closed wuuti closed 6 years ago

wuuti commented 9 years ago

If you edit a user password in user management, you have to choose between 2 following-up actions: send the password by email or show the password on screen. At least hey should not be radio options, because this forces you to choose one. If you do not want to either send it or show it, you have no chance.

And, why is the password (which has to be confirmed!) shown on the screen at all? I believe that is totally unneccessary, but instead reveals a secret in clear text, which sould be avoided if possible. A better way would be to have 3 actions:

I can't see a reason why a password which is specified by a user and double confirmed should be sent or shown anywhere.

OptimusCrime commented 9 years ago

+1!

electrickite commented 9 years ago

+1. This has always seemed very strange to me.

There should honestly be an option to create a user with no password. (For users authenticated using an external identity provider)

adamwintle commented 9 years ago

This has always baffled me.

In my opinion once a user has entered the password it should never be shown in plain text again, it should be hashed and thats it forever.

The fact that the system is saving the password to the database (with hash and salt) then retrieving it to be displayed in plain text is obviously a security flaw; the users http data could be being intercepted (public wifi, etc) and their password would be revealed.

Also, the "growl" style notification also displays the new password in plain text and it remains as a sticky notification, which to me is insane.

Lastly, and maybe should be logged on another issue; all of the above applies to the plain text password in the emails too.

rtripault commented 9 years ago

:+1: on allowing password to be changed/created without displaying it back.

@adamwintle just to clarify, the system is not "retrieving" the hashed password (that is/should not be possible), it's just keeping the plain password (and displaying it back). That's the only moment the clear password could be displayed (because that's the only moment we still have it plain text).

About the plain password being emailed, i somehow agree, but i still see it as a viable option, use case being : you create the account on the behalf of the user. To make it more "secure", we should think of a "mechanism" to create an account (without password), and email the user which will then be prompted to define its own password.

@electrickite if you bridge Revo to a third party authentication system, i would say you should not have to manually create the modUser. Your bridge should take care of that on first logging attempt (but i might be missing some use case ^^)

alroniks commented 6 years ago

I guess it is already fixed in MODX 3 by this PR https://github.com/modxcms/revolution/pull/13786 I am closing this one and we can continue a discussion here https://github.com/modxcms/revolution/issues/13973