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

Add a confirmation modal before account deactivation#6107

Closed
jaiwanth-v wants to merge 1 commit into
matrix-org:developfrom
jaiwanth-v:deactivate-dialog
Closed

Add a confirmation modal before account deactivation#6107
jaiwanth-v wants to merge 1 commit into
matrix-org:developfrom
jaiwanth-v:deactivate-dialog

Conversation

@jaiwanth-v
Copy link
Copy Markdown
Contributor

@jaiwanth-v jaiwanth-v commented May 27, 2021

Closes element-hq/element-web#17422

image

Signed-off-by: Jaiwanth jaiwanth2011@gmail.com

@t3chguy
Copy link
Copy Markdown
Member

t3chguy commented May 27, 2021

So the difficulty here is that there is already a modal, but we need to ask the server whether it accepts password or SSO for confirmation and the only way to do that is to hit the /deactivate endpoint, which sometimes the server will just do implicitly if you performed UIA/logged in recently.

image

So with this change, you may end up with two warnings. The one added here and then also the one in my screenshot, depending on the state of the server.

@jaiwanth-v
Copy link
Copy Markdown
Contributor Author

So the difficulty here is that there is already a modal

Yes, I knew about this. I tested this with one of my test accounts, the modal disappeared immediately and it was deactivated in no time with the settings panel still being active.

So with this change, you may end up with two warnings. The one added here and then also the one in my screenshot, depending on the state of the server.

Yeah, I think it would be better if we have two warnings as this is related to account deletion.

@t3chguy t3chguy requested a review from a team May 27, 2021 07:19
@aaronraimist
Copy link
Copy Markdown
Contributor

aaronraimist commented Jun 16, 2021

Regardless of whether there should be one warning or two warnings, it should be consistent. There should always be the same number of warnings. Some users should not get one warning while others get two.

(though as a temporarily workaround this is certainly better than nothing)

@jaiwanth-v
Copy link
Copy Markdown
Contributor Author

I agree, but as mentioned in the issue, this is just a temporary workaround to prevent accidental deletions. We would probably remove this warning in the future.

@MadLittleMods
Copy link
Copy Markdown
Contributor

Is this still relevant now that matrix-org/synapse#10184 is merged which will not skip UI auth for account deletion? The other modal will now show always AFAICT.

@jaiwanth-v
Copy link
Copy Markdown
Contributor Author

No, it isn't. Closing this.

@jaiwanth-v jaiwanth-v closed this Sep 15, 2021
@jaiwanth-v jaiwanth-v deleted the deactivate-dialog branch September 15, 2021 01:35
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.

Add temporary workaround to synapse UIA auto-advancing on deactivation

4 participants