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

Fix master password hint update not working. #2834

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

BlackDex
Copy link
Collaborator

@BlackDex BlackDex commented Oct 17, 2022

  • The Master Password Hint input has changed it's location to the password update form. This PR updates the code to process this.

  • Also changed the ProfileData struct to exclude Culture and MasterPasswordHint, since both are not used at all, and when not defined they will also not be allocated.

Fixes #2833

- The Master Password Hint input has changed it's location to the
password update form. This PR updates the the code to process this.

- Also changed the `ProfileData` struct to exclude `Culture` and
`MasterPasswordHint`, since both are not used at all, and when not
defined they will also not be allocated.

Fixes dani-garcia#2833
@tessus
Copy link
Contributor

tessus commented Oct 17, 2022

Just out of curosity, this means that people have to change their password to change the password hint, correct? If yes, it's a bit of a strange workflow. ;-)

Update: I know this is coming from bitwarden, but I am just wondering what their SW designers are thinking. Very odd design choices.

This just came to mind, because I told my dad back then not to worry about the password hint, since he could change it afterwards. Hmm, I guess not true anymore.

@BlackDex
Copy link
Collaborator Author

Well, the flow is kinda ok i think.
You probably only set the password hint when you want to change the password, because if you change the password, the hint needs to be changed to.

Also, you can still provide the old-password as the new-password and that would work just fine.

@tessus
Copy link
Contributor

tessus commented Oct 17, 2022

In most cases you are correct. I just find it strange, not being able to edit the current one. e.g. when there's a typo.

I know, this is not important at all, it's just that after almost 30 years in IT, I still sometimes wonder... haha.

Copy link
Contributor

@stefan0xC stefan0xC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. But you can set your password as the hint when changing this in this form (not while signing up). Is this something we can/should prevent or something the client is missing?

@BlackDex
Copy link
Collaborator Author

Looks good to me. But you can set your password as the hint when changing this in this form (not while signing up). Is this something we can/should prevent or something the client is missing?

If there is a check right now during registering and not during change, then you need to report this upstream. We can't check if the password and hint match on the server side.

@stefan0xC
Copy link
Contributor

Done: bitwarden/clients#3821

@dani-garcia dani-garcia merged commit f41ba2a into dani-garcia:main Oct 19, 2022
@BlackDex BlackDex deleted the fix-password-hint branch October 21, 2022 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

password hint not changed
5 participants