-
Notifications
You must be signed in to change notification settings - Fork 14
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
Polish the password recovery flow and other small design tweaks #3780
base: main
Are you sure you want to change the base?
Conversation
Deploying matrix-authentication-service-docs with Cloudflare Pages
|
This includes: - show an error message if the recovery link is expired, with a button to resend the email - show an error message if the recovery link has already been used - include an invisible username field in the form, so that password managers can save the new password
ab7be10
to
0c88b88
Compare
cac9d67
to
20aa7ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one (probably small) thing
/// The input for the `resendRecoveryEmail` mutation. | ||
#[derive(InputObject)] | ||
pub struct ResendRecoveryEmailInput { | ||
/// The recovery ticket to use. | ||
ticket: String, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the intended flow for this.
I assumed the recovery ticket is the code that arrives in the e-mail (embedded in a URL, probably) and that lets you recover the account.
So why would you use a recovery ticket to resend the recovery e-mail?
If the ticket is more public than that, I don't think I am happy with the idea that you can use a ticket to request the username of the user...
edit: from the client code I think I gleam that it's for expired tickets. If so, can you state that more clearly in the documentation for the resend operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, I've added a comment in 8a69c9a
Fixes #3032
This can be reviewed commit by commit: