MSC2181: Add an Error Code for Signaling a Deactivated User

[anoadragon453]

Rendered

PRs: #2234

[dbkr]

otherwise this lgtm

[anoadragon453]

otherwise this lgtm

Did you mean to leave a feedback as well?

Edit: Found his comment in my email. It was:

Probably better would be if the server only returned this error if the username and password was correct, which synapse would be able to do if it didn't implement account deactivation by nulling out the password.

Which has already been discussed below :)

[turt2live]

lgtm

[Half-Shot]

Looks like a sane thing to have, but there are a few bits that don't sit right with me.

[Half-Shot]

Question: What do homeservers do if the user is deactivated and somebody tries to log in with an incorrect password? Is the homeserver expected to retain the password forever? If the password is not retained, should all attempts to login as a deactivated user return the deactivated error (which may have some privacy implications?).

[anoadragon453]

It should still return M_USER_DEACTIVATED. Password hashes are wiped (at least in Synapse) upon user deactivation.

which may have some privacy implications?

Privacy implications are here whether password hashes are retained or not, no?

[anoadragon453]

The problem with shifting it so that you need to login to see if you're deactivated, is that we already have tons of deactivated users whose password hashes have been cleared.

Also worth noting reddit's APIs allow you to tell if any user has been shadowbanned, something that ideally even the user wouldn't know, and that doesn't seem to have caused their service any harm. https://nullprogram.com/am-i-shadowbanned/

[turt2live]

... also by nature of being deactivated you shouldn't be allowed back in. Why would we let people get that far into the process without telling them to go away? I think the proposed approach is fine

[anoadragon453]

I think this is trending towards the proposed solution, so

@mscbot fcp merge

[mscbot]

Team member @anoadragon453 has proposed to merge this. The next step is review by the rest of the tagged people:

• @KitsuneRal
• @anoadragon453
• @ara4n
• @dbkr
• @erikjohnston
• @richvdh
• @turt2live
• @uhoreg

Concerns:

• Users who log in with 3PIDs won't be told they're deactivated. resolved by MSC2181: Add an Error Code for Signaling a Deactivated User #2181 (comment)

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[anoadragon453]

@mscbot concern Users who log in with 3PIDs won't be told they're deactivated.

3PIDs are deleted upon deactivation (https://github.com/matrix-org/synapse/blob/32e7c9e7f20b57dd081023ac42d6931a8da9b3a3/synapse/handlers/deactivate_account.py#L92) so how do we tell those users they were deactivated?

[anoadragon453]

@mscbot resolve Users who log in with 3PIDs won't be told they're deactivated.

Implementations can hash user's 3PIDs instead of removing them, thus allowing them to continue to check if a user is deactivated without keeping personal information about.

[KitsuneRal]

Assuming that it just defines a new error code, I'm fine with the proposal. I'm not sure why we don't at least recommend servers to use this error code, though.

[mscbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[mscbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[turt2live]

This has an implementation so bumping it to spec-pr-missing: matrix-org/synapse#5686

[anoadragon453]

Spec PR has been created: #2234

[turt2live]

Implementations were:

• Let user know their account has been deactivated upon trying to login matrix-react-sdk#3280
• Change user deactivated errcode to USER_DEACTIVATED and use it synapse#5686