Skip to content
This repository was archived by the owner on Sep 10, 2024. It is now read-only.

Support using recovery tickets in the GraphQL SetPassword mutation #2982

Closed
wants to merge 2 commits into from

Conversation

reivilibre
Copy link
Contributor

Part of #172 (in a roundabout way) — we want to push the account recovery form into the React frontend code so it can share the code for the preview of password complexity.

Copy link

cloudflare-workers-and-pages bot commented Jul 18, 2024

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

Latest commit: 92c3426
Status: ✅  Deploy successful!
Preview URL: https://265e9617.matrix-authentication-service-docs.pages.dev
Branch Preview URL: https://rei-be-pwrecovery.matrix-authentication-service-docs.pages.dev

View logs

@reivilibre reivilibre marked this pull request as ready for review July 18, 2024 18:49
@reivilibre reivilibre requested a review from sandhose July 19, 2024 10:56
@@ -542,7 +567,10 @@ impl UserMutations {
let user_id = NodeType::User.extract_ulid(&input.user_id)?;
Copy link
Member

@sandhose sandhose Jul 19, 2024

Choose a reason for hiding this comment

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

Isn't that fundamentally a problem with this mutation? When you have a recovery ticket sent by email, you don't know the user_id; it is figured out from the recovery ticket directly

This means that either we need to make the user_id optional, or we need to make a separate mutation which only takes a recovery ticket

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. We could include the user ID in the link that's in the e-mail? But I guess that's redundant, so making the user ID optional for the recovery ticket might make sense then. I am not sure if this will turn out better or worse than a separate mutation, I don't want to duplicate the checks at least.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants