Skip to content

feat: Change password from account settings#2751

Open
Sollace wants to merge 9 commits intocinnyapp:devfrom
Open-Fiction:feat/change_password_box
Open

feat: Change password from account settings#2751
Sollace wants to merge 9 commits intocinnyapp:devfrom
Open-Fiction:feat/change_password_box

Conversation

@Sollace
Copy link
Copy Markdown

@Sollace Sollace commented Mar 9, 2026

Description

I am reviving the PR from #2419. I'd like to thank @thmasq for their contribution. Using that as the base I have:

  1. Updated (rebased) onto the current /dev branch
  2. Applied some of the feedback posted to the original PR
  3. Changed wording/phrasing in some places and adjusted the UI elements to fit in better with the rest of cinny

Mind I haven't yet fully tested this as I don't want to go fussing around with my actual account. I'm going to test it properly on a throwaway before I mark this as ready to review.

Fixes #2417

image image

Some nice to haves that could be added onto this:

  • Handle cases where an account doesn't have a password and the user would like to add one
  • Support for other auth workflows (Multi-Factor - Add/Remove device) depending what the homeserver advertises support for in this same section

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@Sollace
Copy link
Copy Markdown
Author

Sollace commented Mar 9, 2026

I understand the code a little better now and I... don't like how it's handling UIA. Basically it tries to do the password change, fails, then uses the authData it got back to then show a proper auth dialog to the user.

This is kind of hacky and I can't seem to find any way to do it properly. (i.e. ask for the auth flows, authenticate, and then let them change password)

@thmasq
Copy link
Copy Markdown

thmasq commented Mar 9, 2026

Hello there. Although that feels wrong, I believe that's the proper way to handle password changes according to the matrix spec.

Looking at ChangePassword.tsx from Element Web, I believe their implementation is actually incorrect according to the spec. Their client assumes the server wants m.login.password, prompts for the old password and new password at the same time, builds the auth dictionary, and sends it.
While this standard flow looks correct, like a regular web form, if the server actually requires Single Sign-On (SSO), email verification, or a ReCAPTCHA, the request fails with a 401. Thus the old password the user typed is thrown away, and the client has to fall back to popping up a generic UIA modal anyway.

I guess for most deployments this behavior is fine, but I avoided it in my original implementation.

Really this wouldn't be a problem if the endpoint for changing passwords had a preflight endpoint like login does. Now I do understand there's a security reason why there isn't one, which I won't go in detail. That's just the way it had to be implemented on the server side.

@Sollace
Copy link
Copy Markdown
Author

Sollace commented Mar 9, 2026

Hello there. Although that feels wrong, I believe that's the proper way to handle password changes according to the matrix spec.

Looking at ChangePassword.tsx from Element Web, I believe their implementation is actually incorrect according to the spec. Their client assumes the server wants m.login.password, prompts for the old password and new password at the same time, builds the auth dictionary, and sends it. While this standard flow looks correct, like a regular web form, if the server actually requires Single Sign-On (SSO), email verification, or a ReCAPTCHA, the request fails with a 401. Thus the old password the user typed is thrown away, and the client has to fall back to popping up a generic UIA modal anyway.

I guess for most deployments this behavior is fine, but I avoided it in my original implementation.

Really this wouldn't be a problem if the endpoint for changing passwords had a preflight endpoint like login does. Now I do understand there's a security reason why there isn't one, which I won't go in detail. That's just the way it had to be implemented on the server side.

Alright thanks for the extra details!

I do still think it would be better if there was a way to get the methods first so we could include the password field in the main form (and then show other auth methods after. i.e. submit current pass+new pass+confirm pass, then do OTP or OAuth after) but like you said, if the only way to do that is with a preflight endpoint which we don't have then I suppose we don't have any choice.

As it is then I think it's okay to mark for review and we can find any other issues from there.

@Sollace Sollace marked this pull request as ready for review March 9, 2026 23:51
@kfiven kfiven changed the title [Feat] Change password from account settings feat: Change password from account settings Mar 17, 2026
@github-actions
Copy link
Copy Markdown

Preview: https://2751--pr-cinny.netlify.app
⚠️ Exercise caution. Use test accounts. ⚠️

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.

Change password from account settings

2 participants