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

More honest reset password request response #37603

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Apr 5, 2023

Summary

(Addresses feedback in additional comments of #37408)

Makes the response provided to users requesting a password reset more honest. Also reminds users to verify the provided account name or email address. Removes redundant copy. Continues to do so without leaking information.

TODO

  • Review

Checklist

@szaimen szaimen added this to the Nextcloud 27 milestone Apr 5, 2023
@szaimen szaimen added the 3. to review Waiting for reviews label Apr 5, 2023
@szaimen szaimen requested review from Valdnet, a team, artonge, Pytal, szaimen and rakekniven and removed request for a team April 5, 2023 18:19
Copy link
Contributor

@Valdnet Valdnet left a comment

Choose a reason for hiding this comment

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

Change "administrator" to "administration".

szaimen
szaimen previously requested changes Apr 6, 2023
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Nitpick, but nice otherwise 👍

@szaimen
Copy link
Contributor

szaimen commented Apr 6, 2023

Hi @joshtrichards please squash, rebase, run npm ci and npm run build and commit the changes here which will allow us to merge this. Thank you!

Copy link
Contributor

@jtrees jtrees left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@joshtrichards joshtrichards force-pushed the jr-reset-pw-honest-msg branch from 52691cb to 1a5700d Compare April 13, 2023 15:09
Addresses request in additional comments of nextcloud#37408

* Honest but still won't leak information
* Also reminds user to verify the provided user id email address/user

Signed-off-by: Josh Richards <josh.t.richards@gmail.com>

Refine reset password response copy

Co-authored-by: Louis <6653109+artonge@users.noreply.github.com>
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>

Wrap honest password reset request response
@joshtrichards joshtrichards force-pushed the jr-reset-pw-honest-msg branch from 1a5700d to fb25cd4 Compare April 13, 2023 21:32
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
@joshtrichards joshtrichards force-pushed the jr-reset-pw-honest-msg branch from 3f00e28 to 330c9cf Compare April 14, 2023 01:50
@joshtrichards
Copy link
Member Author

Hi @joshtrichards please squash, rebase, run npm ci and npm run build and commit the changes here which will allow us to merge this. Thank you!

Done - theoretically. I never got it to pass the GH node workflow check 100% here in the PR (and with my latest push it's waiting on approval to run again).

Since the comp'd assets get stashed in the commit it appears the GH node.yml workflow builds them again and diffs them. They always come up different than fail. Is that how it's supposed to work? Am I doing something wrong? Do they get rebuilt again during the final merge on your side?

@szaimen szaimen enabled auto-merge April 14, 2023 13:30
@szaimen szaimen merged commit d4341a2 into nextcloud:master Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UX: Forgot password form should trim user input
6 participants