Normalize username before interacting with Synapse #163

H-Shay · 2022-04-25T23:26:55Z

Currently the LDAP module does not check if a username is compatible with the matrix id grammar before attempting to register or login with it in Synapse, resulting in failure. This PR runs all usernames through a normalization function after successfully authenticating against an LDAP server so that they are compatible with Synapse. I added a rather anemic test that only tests one of two login scenarios as the other path (registering with synapse) seems pretty difficult to meaningfully test. Fixes #72.

H-Shay · 2022-04-26T00:02:05Z

CI failure on check_codestyle is due to an import error in black and should be fixed by #164.

squahtx

Thank you for taking this fix on!

The changes and new test look good functionality-wise.

squahtx · 2022-04-26T18:34:04Z

ldap_auth_provider.py

@@ -23,7 +23,7 @@
 import synapse
 from pkg_resources import parse_version
 from synapse.module_api import ModuleApi
-from synapse.types import JsonDict
+from synapse.types import JsonDict, map_username_to_mxid_localpart


It's a bit naughty to be importing things from synapse.types, since it's not part of the official module API, and hence subject to change.

The previous code should really have imported JsonDict from synapse.module_api.
Unfortunately we can't do the same for map_username_to_mxid_localpart, since it's not part of the module API yet. The right thing to do might be to add it to the module API first, or roll our own version in this module. Or go for a simple str.lower().

As far as rolling a version for this module, any reason I can't just port (copy) the function from synapse? Or is it cleaner to add it to the module API? I could see it possibly being useful in the module API so perhaps that's the way to go?

Copying the function from synapse is a little distasteful because of code duplication. Given enough time, copies of things tend to get out of sync.

I think it's reasonable to add it to the module API, since I agree that other auth modules might find it useful. I'd check in #synapse-devs in case anyone objects.

Per the conversation here I just decided to lower the string as there seems to be no consensus on whether it's okay to add the function to the module api, add it to matrix-python-common, or copy it.

ldap_auth_provider.py

H-Shay · 2022-05-04T16:59:56Z

The mypy failure is unrelated to this change I believe.

squahtx

Looks good to me!
It's a shame that map_username_to_mxid_localpart is not a fit for the module API.

squahtx · 2022-05-05T12:39:14Z

ldap_auth_provider.py

@@ -183,7 +183,7 @@ async def check_auth(
                return None

            # Get full user id from localpart
-            user_id = self.account_handler.get_qualified_user_id(localpart)
+            user_id = self.account_handler.get_qualified_user_id(localpart.lower())


nit: I still think it'd be cleaner to do
localpart = localpart.lower(), above once

Totally fair, I've changed it to reflect this.

Synapse 1.62.0rc3 (2022-07-04) ============================== Bugfixes -------- - Update the version of the [ldap3 plugin](https://github.com/matrix-org/matrix-synapse-ldap3/) included in the `matrixdotorg/synapse` DockerHub images and the Debian packages hosted on `packages.matrix.org` to 0.2.1. This fixes [a bug](matrix-org/matrix-synapse-ldap3#163) with usernames containing uppercase characters. ([\#13156](#13156)) - Fix a bug introduced in Synapse 1.62.0rc1 affecting unread counts for users on small servers. ([\#13168](#13168))

@Thumbscrew

Synapse 1.62.0 (2022-07-05) =========================== No significant changes since 1.62.0rc3. Authors of spam-checker plugins should consult the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.62/docs/upgrade.md#upgrading-to-v1620) to learn about the enriched signatures for spam checker callbacks, which are supported with this release of Synapse. Synapse 1.62.0rc3 (2022-07-04) ============================== Bugfixes -------- - Update the version of the [ldap3 plugin](https://github.com/matrix-org/matrix-synapse-ldap3/) included in the `matrixdotorg/synapse` DockerHub images and the Debian packages hosted on `packages.matrix.org` to 0.2.1. This fixes [a bug](matrix-org/matrix-synapse-ldap3#163) with usernames containing uppercase characters. ([\matrix-org#13156](matrix-org#13156)) - Fix a bug introduced in Synapse 1.62.0rc1 affecting unread counts for users on small servers. ([\matrix-org#13168](matrix-org#13168)) Synapse 1.62.0rc2 (2022-07-01) ============================== Bugfixes -------- - Fix unread counts for users on large servers. Introduced in v1.62.0rc1. ([\matrix-org#13140](matrix-org#13140)) - Fix DB performance when deleting old push notifications. Introduced in v1.62.0rc1. ([\matrix-org#13141](matrix-org#13141)) Synapse 1.62.0rc1 (2022-06-28) ============================== Features -------- - Port the spam-checker API callbacks to a new, richer API. This is part of an ongoing change to let spam-checker modules inform users of the reason their event or operation is rejected. ([\matrix-org#12857](matrix-org#12857), [\matrix-org#13047](matrix-org#13047)) - Allow server admins to customise the response of the `/.well-known/matrix/client` endpoint. ([\matrix-org#13035](matrix-org#13035)) - Add metrics measuring the CPU and DB time spent in state resolution. ([\matrix-org#13036](matrix-org#13036)) - Speed up fetching of device list changes in `/sync` and `/keys/changes`. ([\matrix-org#13045](matrix-org#13045), [\matrix-org#13098](matrix-org#13098)) - Improve URL previews for sites which only provide Twitter Card metadata, e.g. LWN.net. ([\matrix-org#13056](matrix-org#13056)) Bugfixes -------- - Update [MSC3786](matrix-org/matrix-spec-proposals#3786) implementation to check `state_key`. ([\matrix-org#12939](matrix-org#12939)) - Fix a bug introduced in Synapse 1.58 where Synapse would not report full version information when installed from a git checkout. This is a best-effort affair and not guaranteed to be stable. ([\matrix-org#12973](matrix-org#12973)) - Fix a bug introduced in Synapse 1.60 where Synapse would fail to start if the `sqlite3` module was not available. ([\matrix-org#12979](matrix-org#12979)) - Fix a bug where non-standard information was required when requesting the `/hierarchy` API over federation. Introduced in Synapse v1.41.0. ([\matrix-org#12991](matrix-org#12991)) - Fix a long-standing bug which meant that rate limiting was not restrictive enough in some cases. ([\matrix-org#13018](matrix-org#13018)) - Fix a bug introduced in Synapse 1.58 where profile requests for a malformed user ID would ccause an internal error. Synapse now returns 400 Bad Request in this situation. ([\matrix-org#13041](matrix-org#13041)) - Fix some inconsistencies in the event authentication code. ([\matrix-org#13087](matrix-org#13087), [\matrix-org#13088](matrix-org#13088)) - Fix a long-standing bug where room directory requests would cause an internal server error if given a malformed room alias. ([\matrix-org#13106](matrix-org#13106)) Improved Documentation ---------------------- - Add documentation for how to configure Synapse with Workers using Docker Compose. Includes example worker config and docker-compose.yaml. Contributed by @Thumbscrew. ([\matrix-org#12737](matrix-org#12737)) - Ensure the [Poetry cheat sheet](https://matrix-org.github.io/synapse/develop/development/dependencies.html) is available in the online documentation. ([\matrix-org#13022](matrix-org#13022)) - Mention removed community/group worker endpoints in upgrade.md. Contributed by @olmari. ([\matrix-org#13023](matrix-org#13023)) - Add instructions for running Complement with `gotestfmt`-formatted output locally. ([\matrix-org#13073](matrix-org#13073)) - Update OpenTracing docs to reference the configuration manual rather than the configuration file. ([\matrix-org#13076](matrix-org#13076)) - Update information on downstream Debian packages. ([\matrix-org#13095](matrix-org#13095)) - Remove documentation for the Delete Group Admin API which no longer exists. ([\matrix-org#13112](matrix-org#13112)) Deprecations and Removals ------------------------- - Remove the unspecced `DELETE /directory/list/room/{roomId}` endpoint, which hid rooms from the [public room directory](https://spec.matrix.org/v1.3/client-server-api/#listing-rooms). Instead, `PUT` to the same URL with a visibility of `"private"`. ([\matrix-org#13123](matrix-org#13123)) Internal Changes ---------------- - Add tests for cancellation of `GET /rooms/$room_id/members` and `GET /rooms/$room_id/state` requests. ([\matrix-org#12674](matrix-org#12674)) - Report login failures due to unknown third party identifiers in the same way as failures due to invalid passwords. This prevents an attacker from using the error response to determine if the identifier exists. Contributed by Daniel Aloni. ([\matrix-org#12738](matrix-org#12738)) - Merge the Complement testing Docker images into a single, multi-purpose image. ([\matrix-org#12881](matrix-org#12881), [\matrix-org#13075](matrix-org#13075)) - Simplify the database schema for `event_edges`. ([\matrix-org#12893](matrix-org#12893)) - Clean up the test code for client disconnection. ([\matrix-org#12929](matrix-org#12929)) - Remove code generating comments in configuration. ([\matrix-org#12941](matrix-org#12941)) - Add `Cross-Origin-Resource-Policy: cross-origin` header to content repository's thumbnail and download endpoints. ([\matrix-org#12944](matrix-org#12944)) - Replace noop background updates with `DELETE` delta. ([\matrix-org#12954](matrix-org#12954), [\matrix-org#13050](matrix-org#13050)) - Use lower isolation level when inserting read receipts to avoid serialization errors. Contributed by Nick @ Beeper. ([\matrix-org#12957](matrix-org#12957)) - Reduce the amount of state we pull from the DB. ([\matrix-org#12963](matrix-org#12963)) - Enable testing against PostgreSQL databases in Complement CI. ([\matrix-org#12965](matrix-org#12965), [\matrix-org#13034](matrix-org#13034)) - Fix an inaccurate comment. ([\matrix-org#12969](matrix-org#12969)) - Remove the `delete_device` method and always call `delete_devices`. ([\matrix-org#12970](matrix-org#12970)) - Use a GitHub form for issues rather than a hard-to-read, easy-to-ignore template. ([\matrix-org#12982](matrix-org#12982)) - Move [MSC3715](matrix-org/matrix-spec-proposals#3715) behind an experimental config flag. ([\matrix-org#12984](matrix-org#12984)) - Add type hints to tests. ([\matrix-org#12985](matrix-org#12985), [\matrix-org#13099](matrix-org#13099)) - Refactor macaroon tokens generation and move the unsubscribe link in notification emails to `/_synapse/client/unsubscribe`. ([\matrix-org#12986](matrix-org#12986)) - Fix documentation for running complement tests. ([\matrix-org#12990](matrix-org#12990)) - Faster joins: add issue links to the TODO comments in the code. ([\matrix-org#13004](matrix-org#13004)) - Reduce DB usage of `/sync` when a large number of unread messages have recently been sent in a room. ([\matrix-org#13005](matrix-org#13005), [\matrix-org#13096](matrix-org#13096), [\matrix-org#13118](matrix-org#13118)) - Replaced usage of PyJWT with methods from Authlib in `org.matrix.login.jwt`. Contributed by Hannes Lerchl. ([\matrix-org#13011](matrix-org#13011)) - Modernize the `contrib/graph/` scripts. ([\matrix-org#13013](matrix-org#13013)) - Remove redundant `room_version` parameters from event auth functions. ([\matrix-org#13017](matrix-org#13017)) - Decouple `synapse.api.auth_blocking.AuthBlocking` from `synapse.api.auth.Auth`. ([\matrix-org#13021](matrix-org#13021)) - Add type annotations to `synapse.storage.databases.main.devices`. ([\matrix-org#13025](matrix-org#13025)) - Set default `sync_response_cache_duration` to two minutes. ([\matrix-org#13042](matrix-org#13042)) - Rename CI test runs. ([\matrix-org#13046](matrix-org#13046)) - Increase timeout of complement CI test runs. ([\matrix-org#13048](matrix-org#13048)) - Refactor entry points so that they all have a `main` function. ([\matrix-org#13052](matrix-org#13052)) - Refactor the Dockerfile-workers configuration script to use Jinja2 templates in Synapse workers' Supervisord blocks. ([\matrix-org#13054](matrix-org#13054)) - Add headers to individual options in config documentation to allow for linking. ([\matrix-org#13055](matrix-org#13055)) - Make Complement CI logs easier to read. ([\matrix-org#13057](matrix-org#13057), [\matrix-org#13058](matrix-org#13058), [\matrix-org#13069](matrix-org#13069)) - Don't instantiate modules with keyword arguments. ([\matrix-org#13060](matrix-org#13060)) - Fix type checking errors against Twisted trunk. ([\matrix-org#13061](matrix-org#13061)) - Allow MSC3030 `timestamp_to_event` calls from anyone on world-readable rooms. ([\matrix-org#13062](matrix-org#13062)) - Add a CI job to check that schema deltas are in the correct folder. ([\matrix-org#13063](matrix-org#13063)) - Avoid rechecking event auth rules which are independent of room state. ([\matrix-org#13065](matrix-org#13065)) - Reduce the duplication of code that invokes the rate limiter. ([\matrix-org#13070](matrix-org#13070)) - Add a Subject Alternative Name to the certificate generated for Complement tests. ([\matrix-org#13071](matrix-org#13071)) - Add more tests for room upgrades. ([\matrix-org#13074](matrix-org#13074)) - Pin dependencies maintained by matrix.org to [semantic version](https://semver.org/) bounds. ([\matrix-org#13082](matrix-org#13082)) - Correctly report prometheus DB stats for `get_earliest_token_for_stats`. ([\matrix-org#13085](matrix-org#13085)) - Fix a long-standing bug where a finished logging context would be re-started when Synapse failed to persist an event from federation. ([\matrix-org#13089](matrix-org#13089)) - Simplify the alias deletion logic as an application service. ([\matrix-org#13093](matrix-org#13093)) - Add type annotations to `tests.test_server`. ([\matrix-org#13124](matrix-org#13124))

H-Shay added 2 commits April 25, 2022 16:12

normalize localpart before interacting with synapse

49b9fc8

use better variables

1c16263

H-Shay requested a review from a team as a code owner April 25, 2022 23:26

properly use import

b21c007

squahtx reviewed Apr 26, 2022

View reviewed changes

H-Shay added 2 commits May 4, 2022 09:51

lower the localpart

6407156

lint

f2cd96c

H-Shay requested a review from squahtx May 4, 2022 17:00

squahtx approved these changes May 5, 2022

View reviewed changes

only call .lower() once

bc87e5a

H-Shay merged commit 4ae8c9a into main May 6, 2022

H-Shay deleted the shay/normalize_username branch May 6, 2022 21:25

richvdh mentioned this pull request Jul 21, 2022

regression: login breaks for accounts with uppercase username #167

Closed

H-Shay mentioned this pull request Jul 26, 2022

Fix a regression wherein users with historical usernames containing capital letters can't log in #168

Merged

erikjohnston mentioned this pull request Nov 29, 2023

devices table has case-sensitive user_id which leads to not removing devices from user matrix-org/synapse#16474

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize username before interacting with Synapse #163

Normalize username before interacting with Synapse #163

H-Shay commented Apr 25, 2022 •

edited

Loading

H-Shay commented Apr 26, 2022

squahtx left a comment

squahtx Apr 26, 2022

H-Shay Apr 26, 2022

squahtx Apr 27, 2022 •

edited

Loading

H-Shay May 4, 2022

H-Shay commented May 4, 2022

squahtx left a comment

squahtx May 5, 2022 •

edited

Loading

H-Shay May 6, 2022

Normalize username before interacting with Synapse #163

Normalize username before interacting with Synapse #163

Conversation

H-Shay commented Apr 25, 2022 • edited Loading

H-Shay commented Apr 26, 2022

squahtx left a comment

Choose a reason for hiding this comment

squahtx Apr 26, 2022

Choose a reason for hiding this comment

H-Shay Apr 26, 2022

Choose a reason for hiding this comment

squahtx Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

H-Shay May 4, 2022

Choose a reason for hiding this comment

H-Shay commented May 4, 2022

squahtx left a comment

Choose a reason for hiding this comment

squahtx May 5, 2022 • edited Loading

Choose a reason for hiding this comment

H-Shay May 6, 2022

Choose a reason for hiding this comment

H-Shay commented Apr 25, 2022 •

edited

Loading

squahtx Apr 27, 2022 •

edited

Loading

squahtx May 5, 2022 •

edited

Loading