Skip to content
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

Return a different error from Invalid Password when a user is deactivated #5674

merged 4 commits into from Jul 15, 2019


Copy link

commented Jul 12, 2019

Return This account has been deactivated instead of Invalid password when a user is deactivated.

Riot web still displays Incorrect username and/or password. as I believe it does that for any 403 error returned by the server.

Unfortunately, Riot-web will need to match on the error field to show an error properly. Let me know if there's a better way this can be done (HTTP code, new errcode [requires MSC]).

Also note: this allows anyone to figure out that an account was deactivated. There's no way to gatekeep this behind requiring the correct password, as we remove user's password hashes upon deactivation.

@anoadragon453 anoadragon453 requested a review from matrix-org/synapse-core Jul 12, 2019

@anoadragon453 anoadragon453 added this to In progress in Homeserver Task Board via automation Jul 12, 2019

anoadragon453 added some commits Jul 12, 2019


This comment has been minimized.

Copy link

commented Jul 12, 2019

Codecov Report

Merging #5674 into develop will increase coverage by 0.01%.
The diff coverage is 28.57%.

@@             Coverage Diff             @@
##           develop    #5674      +/-   ##
+ Coverage    63.22%   63.23%   +0.01%     
  Files          331      328       -3     
  Lines        36037    35858     -179     
  Branches      5931     5912      -19     
- Hits         22784    22675     -109     
+ Misses       11623    11557      -66     
+ Partials      1630     1626       -4
Copy link

left a comment

lgtm otherwise

Show resolved Hide resolved synapse/api/ Outdated

@anoadragon453 anoadragon453 merged commit 18c5166 into develop Jul 15, 2019

19 checks passed

buildkite/synapse Build #2810 passed (20 minutes, 48 seconds)
buildkite/synapse/check-sample-config Passed (1 minute, 10 seconds)
buildkite/synapse/check-style Passed (1 minute, 12 seconds)
buildkite/synapse/isort Passed (19 seconds)
buildkite/synapse/newspaper-newsfile Passed (17 seconds)
buildkite/synapse/packaging Passed (22 seconds)
buildkite/synapse/pipeline Passed (8 seconds)
buildkite/synapse/python-3-dot-5-slash-postgres-9-dot-5 Passed (17 minutes, 20 seconds)
buildkite/synapse/python-3-dot-5-slash-sqlite Passed (4 minutes, 3 seconds)
buildkite/synapse/python-3-dot-5-slash-sqlite-slash-old-deps Passed (5 minutes, 39 seconds)
buildkite/synapse/python-3-dot-6-slash-sqlite Passed (4 minutes, 28 seconds)
buildkite/synapse/python-3-dot-7-slash-postgres-11 Passed (17 minutes, 21 seconds)
buildkite/synapse/python-3-dot-7-slash-postgres-9-dot-5 Passed (17 minutes, 4 seconds)
buildkite/synapse/python-3-dot-7-slash-sqlite Passed (4 minutes, 35 seconds)
buildkite/synapse/sytest-python-3-dot-5-slash-postgres-9-dot-6-slash-monolith Passed (5 minutes, 53 seconds)
buildkite/synapse/sytest-python-3-dot-5-slash-postgres-9-dot-6-slash-workers Soft failed (exit status 1)
buildkite/synapse/sytest-python-3-dot-5-slash-sqlite-slash-monolith Passed (4 minutes, 46 seconds)
codecov/patch Coverage not affected when comparing ba5a909...4111739
codecov/project 63.23% (target 0%)

Homeserver Task Board automation moved this from In progress to Done Jul 15, 2019

@anoadragon453 anoadragon453 deleted the anoa/deactivated_user_error_msg branch Jul 15, 2019

anoadragon453 added a commit that referenced this pull request Jul 22, 2019

Merge tag 'v1.2.0rc1' into develop


- Add support for opentracing. ([\#5544](#5544), [\#5712](#5712))
- Add ability to pull all locally stored events out of synapse that a particular user can see. ([\#5589](#5589))
- Add a basic admin command app to allow server operators to run Synapse admin commands separately from the main production instance. ([\#5597](#5597))
- Add `sender` and `origin_server_ts` fields to `m.replace`. ([\#5613](#5613))
- Add default push rule to ignore reactions. ([\#5623](#5623))
- Include the original event when asking for its relations. ([\#5626](#5626))
- Implement `session_lifetime` configuration option, after which access tokens will expire. ([\#5660](#5660))
- Return "This account has been deactivated" when a deactivated user tries to login. ([\#5674](#5674))
- Enable aggregations support by default ([\#5714](#5714))


- Fix 'utime went backwards' errors on daemonization. ([\#5609](#5609))
- Various minor fixes to the federation request rate limiter. ([\#5621](#5621))
- Forbid viewing relations on an event once it has been redacted. ([\#5629](#5629))
- Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format. ([\#5638](#5638))
- Fix newly-registered users not being able to lookup their own profile without joining a room. ([\#5644](#5644))
- Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`. ([\#5654](#5654))
- Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated. ([\#5658](#5658))
- Fix some problems with authenticating redactions in recent room versions. ([\#5699](#5699), [\#5700](#5700), [\#5707](#5707))
- Ignore redactions of events. ([\#5701](#5701))

Updates to the Docker image

- Base Docker image on a newer Alpine Linux version (3.8 -> 3.10). ([\#5619](#5619))
- Add missing space in default logging file format generated by the Docker image. ([\#5620](#5620))

Improved Documentation

- Add information about nginx normalisation to reverse_proxy.rst. Contributed by @skalarproduktraum - thanks! ([\#5397](#5397))
- --no-pep517 should be --no-use-pep517 in the documentation to setup the development environment. ([\#5651](#5651))
- Improvements to Postgres setup instructions. Contributed by @Lrizika - thanks! ([\#5661](#5661))
- Minor tweaks to postgres documentation. ([\#5675](#5675))

Deprecations and Removals

- Remove support for the `invite_3pid_guest` configuration setting. ([\#5625](#5625))

Internal Changes

- Move logging code out of `synapse.util` and into `synapse.logging`. ([\#5606](#5606), [\#5617](#5617))
- Add a blacklist file to the repo to blacklist certain sytests from failing CI. ([\#5611](#5611))
- Make runtime errors surrounding password reset emails much clearer. ([\#5616](#5616))
- Remove dead code for persiting outgoing federation transactions. ([\#5622](#5622))
- Add `` to the scripts-dev folder which will run all linting steps required by CI. ([\#5627](#5627))
- Move RegistrationHandler.get_or_create_user to test code. ([\#5628](#5628))
- Add some more common python virtual-environment paths to the black exclusion list. ([\#5630](#5630))
- Some counter metrics exposed over Prometheus have been renamed, with the old names preserved for backwards compatibility and deprecated. See `docs/metrics-howto.rst` for details. ([\#5636](#5636))
- Unblacklist some user_directory sytests. ([\#5637](#5637))
- Factor out some redundant code in the login implementation. ([\#5639](#5639))
- Update ModuleApi to avoid register(generate_token=True). ([\#5640](#5640))
- Remove access-token support from `RegistrationHandler.register`, and rename it. ([\#5641](#5641))
- Remove access-token support from `RegistrationStore.register`, and rename it. ([\#5642](#5642))
- Improve logging for auto-join when a new user is created. ([\#5643](#5643))
- Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure. ([\#5645](#5645))
- Fix a small typo in a code comment. ([\#5655](#5655))
- Clean up exception handling around client access tokens. ([\#5656](#5656))
- Add a mechanism for per-test homeserver configuration in the unit tests. ([\#5657](#5657))
- Inline issue_access_token. ([\#5659](#5659))
- Update the sytest BuildKite configuration to checkout Synapse in `/src`. ([\#5664](#5664))
- Add a `docker` type to the towncrier configuration. ([\#5673](#5673))
- Convert `synapse.federation.transport.server` to `async`. Might improve some stack traces. ([\#5689](#5689))
- Documentation for opentracing. ([\#5703](#5703))

anoadragon453 added a commit that referenced this pull request Jul 31, 2019

Change user deactivated errcode to USER_DEACTIVATED and use it (#5686)
This is intended as an amendment to #5674 as using M_UNKNOWN as the errcode makes it hard for clients to differentiate between an invalid password and a deactivated user (the problem we were trying to solve in the first place).

M_UNKNOWN was originally chosen as it was presumed than an MSC would have to be carried out to add a new code, but as Synapse often is the testing bed for new MSC implementations, it makes sense to try it out first in the wild and then add it into the spec if it is successful. Thus this PR return a new M_USER_DEACTIVATED code when a deactivated user attempts to login.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.