-
Notifications
You must be signed in to change notification settings - Fork 386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MSC2181: Add an Error Code for Signaling a Deactivated User #2181
Merged
Merged
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
bf747b6
Add proposal regarding user deactivated err codes
anoadragon453 7a08bc9
Remove template bit
anoadragon453 4f54b8b
Update with review comments
anoadragon453 028ea89
/available
anoadragon453 7f1e822
Update proposals/2181-user-deactivated-errcode.md
anoadragon453 c1dd804
Specify /login should have M_USER_DEACTIVATED
anoadragon453 063261b
Merge branch 'anoa/user_deactivated_msc' of github.com:matrix-org/mat…
anoadragon453 f3ca033
kill the hashes
anoadragon453 a126fbb
Implementation details
anoadragon453 8c993ae
respond to review comments
anoadragon453 f3358e4
Fix typo
anoadragon453 502f5d5
Be less wishy-washy
anoadragon453 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# Add an Error Code for Signaling a Deactivated User | ||
|
||
Currently, when a user attempts to log in, they will receive an `M_FORBIDDEN` | ||
errcode if their password is incorrect. However, if the user's account is | ||
deactivated, they will also receive an `M_FORBIDDEN`, leaving clients in a | ||
state where they are unable to inform the user that the reason they cannot | ||
log in is that their account has been deactivated. This leads to confusion | ||
and password resetting with ultimately results in unnecessary support | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
requests. | ||
|
||
## Proposal | ||
|
||
This proposal asks to create a new errcode, `M_USER_DEACTIVATED`, that can be | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
returned whenever an action is attempted that requires an activited user, but | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
the authenticating user is deactivated. The HTTP code to return alongside is | ||
`403`. | ||
Half-Shot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Tradeoffs | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The alternative is to continue returning an `M_FORBIDDEN`, but send back a | ||
different errmsg. This is undesirable as clients are supposed to treat the | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
message as an opaque string, and should not be performing any | ||
pattern-matching on it. | ||
|
||
## Potential issues | ||
|
||
None | ||
|
||
## Security considerations | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
While the existence of a user was already public knowledge (one can check if | ||
the User ID is available through `/register`, this proposal would allow any | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
user to be able to detect if a registered account has been deactivated. | ||
|
||
## Conclusion | ||
|
||
Adding `M_USER_DEACTIVATED` would better inform clients about the state of a | ||
user's account, and lead to less confusion when they cannot log in. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should still return
M_USER_DEACTIVATED
. Password hashes are wiped (at least in Synapse) upon user deactivation.Privacy implications are here whether password hashes are retained or not, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... 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