Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MODX 3: Users - Password notification method missing. Bug or by design? #13973

Open
jonleverrier opened this issue Jul 4, 2018 · 20 comments
Open
Labels
proposal Proposal about improvement aka RFC. Need to be discussed before start implementation.
Milestone

Comments

@jonleverrier
Copy link
Contributor

jonleverrier commented Jul 4, 2018

Summary

There is a large gap where the Password notification method use to be. Also, the Password notification method is missing from a users profile - is this a bug or by design?

When "Let MODX generate a password" or "Let me specify the password" is selected on save, the password is presented on screen - no email is sent.

screen shot 2018-07-04 at 21 15 24

Step to reproduce

  1. Edit a users profile
  2. Go to the new password area
  3. Try to specify a password and email the user

Observed behavior

Password notification method missing

Expected behavior

To show Password notification method options

screen shot 2018-07-04 at 21 15 15

Environment

MODX3 Alpha v 3.0.0-dev
From https://github.com/Sterc/modx3builds/raw/master/latest/modx3-alpha-regular.zip
Chrome Version 67.0.3396.99

@Mark-H
Copy link
Collaborator

Mark-H commented Jul 4, 2018

I believe that to be by design; doesn't it send the user an email to change their pass always now? Maybe the text needs updating.

@jonleverrier
Copy link
Contributor Author

I thought that also @Mark-H. But I didn't receive the email when I selected "Let MODX generate a password" and hit save. Plus there is a gap in the design where the password notifications use to be.

@JoshuaLuckers
Copy link
Contributor

There are a lot of times I don't want an email to be send when a (new) password is made for a user.
I really don't hope this will be the default behaviour.

@Mark-H
Copy link
Collaborator

Mark-H commented Jul 5, 2018

I may be wrong and mixing up the manager reset password feature and changing the password from the back-end. Stuff happened in that department, but I don't immediately have it clear what did and didn't change, so this issue would warrant double checking at the least.

@OptimusCrime
Copy link
Contributor

Actually, we should consider removing the option for sending the password over email. It is very insecure and considered a terrible practice.

@jonleverrier
Copy link
Contributor Author

jonleverrier commented Jul 5, 2018

42292998-f7f14342-7fd6-11e8-90a4-92fc19ea590f

How about adding a 3rd option "Let the user choose their own password via email", which would work like the forgotten password tool on the manager login page (it sends them a hashed link that takes them to the login screen to reset their password)

That way, we have the same functionality but is more secure (no plain text passwords over email)?

@jonleverrier
Copy link
Contributor Author

Or was the plan for new users to just reset their password on the login manager page?

@OptimusCrime
Copy link
Contributor

I do not know if there are any plans about what to do next really. But a reset password link the a token that expire is a much better approach than sending the actual password in clear text.

@JoshuaLuckers
Copy link
Contributor

There should always be a possibility to view the generated link. Certainly for situations where it's not possible to send an e-mail (new install without the e-mail settings being configured).

@jonleverrier
Copy link
Contributor Author

@JoshuaLuckers Nice idea!

@jonleverrier jonleverrier changed the title MODX 3: Users - Password notification method missing MODX 3: Users - Password notification method missing. Bug or by design? Jul 6, 2018
@electrickite
Copy link
Contributor

In #12162 @rtripault said:

@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 ^^)

In some cases, the external authentication system does not handle authorization. A user account is "pre-created" in MODX and given appropriate permissions. The external provider then matches the authenticated user to a MODX account.

@alroniks alroniks added the MODX3 label Jul 24, 2018
@alroniks alroniks added the proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. label Aug 2, 2018
@alroniks alroniks added this to the v3.0.0-alpha milestone Aug 17, 2018
@SnowCreative
Copy link
Contributor

I use "show on screen" ALL THE TIME! Please do not remove it. What is the problem with this when I'm sitting here all by myself? In a crowded public place, maybe you don't want to show the password on screen, so just add a third option to neither email nor show on screen.

Some of my clients are very tech un-savvy, so I set up passwords, then call them and give them the password (or email the password by itself with nothing else at all in the email, so that no one intercepting it could have any clue what it's for). I don't want to have to require clients to use a password reset at the outset. Also, at times (for these tech-unsavvy people), I need to have a record of their password so that if anything goes wrong I can log in as them to troubleshoot a problem.

@OptimusCrime
Copy link
Contributor

I do not think removing the show on screen option has been brought up here?

@JoshuaLuckers
Copy link
Contributor

This issue is about the "show on screen option" not being there in 3.x.

@OptimusCrime
Copy link
Contributor

Oh, I am so sorry. The discussion derailed a bit in the later comments. Then I agree, the show on screen should be added back, despite the fact that this approach is horrible from a security standpoint.

@Rainbowtiger One concern is network security, as the password now is sent both to and from the server in clear text.

@Jako
Copy link
Collaborator

Jako commented Aug 26, 2018

@OptimusCrime SSL encrypted these days.

@SnowCreative
Copy link
Contributor

Yes, I make sure all my sites use SSL only.

@opengeek opengeek modified the milestones: v3.0.0-alpha3, v3.0.0-beta1 Oct 27, 2021
@opengeek opengeek modified the milestones: v3.0.0-beta1, v3.0.0-beta2 Nov 9, 2021
@alroniks alroniks modified the milestones: v3.0.0-beta2, v3.1.0 Nov 10, 2021
@SnowCreative
Copy link
Contributor

What's the current thinking on this? In MODX 3 now, there is just "show the new password on the screen" as an option, with no way to turn it off or select something else. If the plan is to only show it on the screen, why do we need this at all?
Screen Shot 2021-11-14 at 10 49 48 AM

@Mark-H
Copy link
Collaborator

Mark-H commented Nov 14, 2021

Please note #15461 proposed some additions but requested changes were never made, so that's not been merged.

@Ruslan-Aleev
Copy link
Collaborator

@SnowCreative We had to return the radio option, without it there was a bug and the password was not shown on the screen, see in #15629

opengeek pushed a commit that referenced this issue Aug 30, 2024
### What does it do?

This is a re-up of #15461 originally by @sdrenth back in 2021, which has
gone stale waiting for some minor changes. I've rebased it, tweaked it,
tested it, so we can include it in 3.1.

This adds a new option for setting the password when creating/updating a
user: send the user a link to set their password. That's more secure and
builds upon improvements to the password reset flow that was done in
3.0.

### Why is it needed?

Showing the password on screen or manually setting a password is kinda
outdated and insecure.

### How to test

Create and/or edit a user, and choose "Let the user choose their own
password via email" for the password method. Look for the email (make
sure you have email delivery set up beforehand) and attempt to set the
new password.

### Related issue(s)/PR(s)

This PR replaces the stale PR #15461

Sterc#22
#13973
Sterc#31

Co-authored-by: sander <sander@sterc.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposal about improvement aka RFC. Need to be discussed before start implementation.
Projects
None yet
Development

No branches or pull requests

10 participants