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

Change password on first login #9547

Open
Tracked by #190
latin-panda opened this issue Oct 16, 2024 · 7 comments · May be fixed by #9581
Open
Tracked by #190

Change password on first login #9547

latin-panda opened this issue Oct 16, 2024 · 7 comments · May be fixed by #9581
Assignees
Labels
Type: Security Affects security
Milestone

Comments

@latin-panda
Copy link
Contributor

latin-panda commented Oct 16, 2024

Is your feature request related to a problem? Please describe.
System admin users create accounts for CHWs and then share the password with them. To enhance the security of these accounts, there should be a way to prompt a password change on the first login.

Describe the solution you'd like
On the login page, create a feature to change the password with the following considerations:

  • All UI texts should be translatable in CHT's supported languages
  • This feature is enabled based on permissions (change_password_first_login)
  • Only new users will be prompted to change their password on the first login when the permission is enabled.
  • Subsequent logins won't require a password change.
  • Changing the password in this step is an online action (they need internet)
  • If the CHW can't change the password for some reason, they can't access the app
  • If CHW logins for first login and closes the app before changing password, the next time they login (even though it is not technically first login) they are prompted to change password.

Figma design

The scope of this work is to enable changing the password on the first login. It doesn't include changing the password rules or current authentication mechanisms.

@latin-panda latin-panda added Type: Feature Add something new Type: Security Affects security labels Oct 16, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Care Teams Oct 16, 2024
@latin-panda latin-panda moved this from Todo to Next Week's Commitments in Product Team Activities Oct 16, 2024
@latin-panda latin-panda moved this from Backlog to Planned in Care Teams Oct 16, 2024
@garethbowen
Copy link
Member

This feature is enabled based on permissions

Once the MVP is proven then reset password will be mandatory for all projects so we can ensure all users on all instances are not compromised in future. Until then it's fine to use a feature flag so projects can opt-in to try it out.

Subsequent logins won't require a password change.

One addition to this, is the user loses their phone, or forgets their password and the administrator resets it for them, then the user will be required to change their password again. This is because the password has almost certainly been shared in plaintext so it's once again vulnerable to future leaks.

@Benmuiruri
Copy link
Contributor

Hi @garethbowen

I'll be starting this tomorrow (Oct 23). Probably start with conceptualizing the data flow then work on the UI first. Just checking whether you have any comment / clarification before I start ..(anything to add other that what we have in this thread) ?

cc @n-orlowski are we going with the single password input design or confirm password design in Figma?

@garethbowen
Copy link
Member

@Benmuiruri Nothing to add regarding the UX. Potentially explore ways to keep it simple for the MVP, for example, is the password strength bar strictly necessary or something we can do in a separate issue?

From an implementation point of view, I think it's a fairly simple process of flagging the user with password_change_required: true (or something). This gives us flexibility to implement further features like forgotten password and admin reset easily using the same logic. We need to make sure this flag is set when users are created or have passwords changed whether that's done using the user management app, the admin app, direct to the db, or any other way. Make sure everyone is using the API or we'll have to intercept the POST request...

Finally, because this is in the authentication system we'll have to be very careful not to accidentally break anything. For example, if the feature flag is enabled once users are already logged in they should not be forced to change their password.

@Benmuiruri Benmuiruri moved this from Next Week's Commitments to In Progress in Product Team Activities Oct 23, 2024
@Benmuiruri Benmuiruri moved this from Planned to In Progress in Care Teams Oct 23, 2024
@n-orlowski
Copy link

@Benmuiruri let's do with the confirmation input. Also happy to drop the progress bar for MVP (but we should keep the written prompts)

@latin-panda
Copy link
Contributor Author

Adding this to 4.16

@jkuester
Copy link
Contributor

Okay, sorry for the last-minute design questions here, but I am a bit concerned about how the MVP functionality is going to work from a Programs perspective. (Started this discussion in this PR thread, but wanted to continue it here at a higher level for better visibility....)

  • My primary concern is that the can_skip_password_change permission is essentially an "opt-out". By default, users will have to reset their password whenever it is changed by an admin. Unless a project updates their permissions, this password reset behavior will automatically take effect for all users.
    • This seems to be the opposite of the behavior @garethbowen described above "Until then it's fine to use a feature flag so projects can opt-in to try it out." Perhaps I missed more conversations that happened elsewhere....
  • If we are going to make this default behavior, then it really feels like our MVP needs to contain a more sophisticated "out-out" mechanism than a permission. As things are, it really feels like we could seriously complicate things for admins in the field (where getting users logged in has historically already been a problem). Other user management tools I have used (e.g. Keycloak) have included a check-box when modifying a user's password to "Require user to reset password on next login".
    • I understand the security posture is much better with the automatic password reset. However one consequence of this change that seems likely to me is that, for many user accounts, no one will actually remember the password after it gets reset by the user. (I know I struggle with this, myself, if I don't immediately take the time to record a new password that I set.) This will likely result in increased churn in passwords as users will need admins to reset their password each time they need to log in again.
    • If we have decided that we don't want programs to be able to manage passwords for users (for security purposes) it seems like token login links would be less burdensome to both the admins and the end user....
  • Have we gotten feedback from Programs folks on possible impacts that this change will have on various user management workflows? (Maybe this feature originated from talking with programs and is exactly what they need to improve their user management, but I don't want to just assume that...)

Hopefully this has all been previously discussed in threads that I have just missed. If the answer is "yeah, we took all of this into consideration already and decided the current approach is best", then great! I will be happy to move forwards with this, as is.

@jkuester
Copy link
Contributor

Another design question that has come up on the PR is how we should handle basic auth requests from a user that has "password_change_required": true.

The current implementation has added an extra lookup for the _user doc of the user in context to check the password_change_required property on every request to the server. This is needed because a simple _session check will succeed when you use the new password (the one the admin set) and is not sufficient to force you to change your password.

I am wondering if we could be content with not forcing a password reset for basic-auth requests like this. Instead, we only trigger the password-reset flow from the actual login flow. For normal users with a session cookie, when the admin resets their password, they will be forced to login again (and so will be funneled to the password-reset flow). I am just not sure it is worth adding an extra round-trip for every request just to address this basic-auth edge case....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Security Affects security
Projects
Status: In Progress
Status: In Progress
Development

Successfully merging a pull request may close this issue.

5 participants