Skip to content

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Jan 7, 2025

Fixes #3032

This can be reviewed commit by commit:

  • Render an earlier loading screen fallback shows a spinner earlier
  • Fix the cross signing reset cancel and error screens as there were some layout issues
  • Typo in the consent screen
  • Make the rate limiter available to the GraphQL API handlers so that we can rate limit the email recovery resend
  • Polish the password recovery page, which includes:
    • showing an error message if the recovery link is expired, with a button to resend the email
    • showing an error message if the recovery link has already been used
    • including an invisible username field in the form, so that password managers can save the new password
  • Better collapsible sections, in sync with the latest Figma designs
  • Adjust layout spacings, to make them more in line with the designs

Copy link

cloudflare-workers-and-pages bot commented Jan 7, 2025

Deploying matrix-authentication-service-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7c57d09
Status: ✅  Deploy successful!
Preview URL: https://546fe133.matrix-authentication-service-docs.pages.dev
Branch Preview URL: https://quenting-design-review-jan-2.matrix-authentication-service-docs.pages.dev

View logs

@sandhose sandhose force-pushed the quenting/design-review-jan-25 branch 2 times, most recently from cac9d67 to 20aa7ce Compare January 8, 2025 14:31
@sandhose sandhose requested a review from reivilibre January 8, 2025 14:41
@sandhose sandhose marked this pull request as ready for review January 8, 2025 14:41
Copy link
Contributor

@reivilibre reivilibre left a 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

Comment on lines +331 to +336
/// The input for the `resendRecoveryEmail` mutation.
#[derive(InputObject)]
pub struct ResendRecoveryEmailInput {
/// The recovery ticket to use.
ticket: String,
}
Copy link
Contributor

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?

Copy link
Member Author

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

@sandhose sandhose requested a review from reivilibre January 9, 2025 08:48
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
@sandhose sandhose force-pushed the quenting/design-review-jan-25 branch from e708212 to 7c57d09 Compare January 13, 2025 15:49
@sandhose sandhose enabled auto-merge (rebase) January 13, 2025 15:50
@sandhose sandhose merged commit 2605d3a into main Jan 13, 2025
20 checks passed
@sandhose sandhose deleted the quenting/design-review-jan-25 branch January 13, 2025 15:58
@sandhose sandhose added A-Local-Password Related to the local password database T-Enhancement New feature of request labels Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Local-Password Related to the local password database T-Enhancement New feature of request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Include an invisible username field in password change forms

2 participants