diff --git a/changelog.d/12083.misc b/changelog.d/12083.misc new file mode 100644 index 000000000000..88fd6b92ee4f --- /dev/null +++ b/changelog.d/12083.misc @@ -0,0 +1 @@ +Refactor `create_new_client_event` to use a new parameter, `state_event_ids`, which accurately describes the usage with [MSC2716](https://github.com/matrix-org/matrix-doc/pull/2716) instead of abusing `auth_event_ids`. diff --git a/changelog.d/12091.misc b/changelog.d/12091.misc new file mode 100644 index 000000000000..def44987b470 --- /dev/null +++ b/changelog.d/12091.misc @@ -0,0 +1 @@ +Refuse to start if registration is enabled without email, captcha, or token-based verification unless new config flag `enable_registration_without_verification` is set. diff --git a/changelog.d/12195.feature b/changelog.d/12195.feature new file mode 100644 index 000000000000..e8bcb950a1c6 --- /dev/null +++ b/changelog.d/12195.feature @@ -0,0 +1 @@ +Allow modules to store already existing 3PID associations. diff --git a/changelog.d/12261.bugfix b/changelog.d/12261.bugfix new file mode 100644 index 000000000000..1bfde4c38055 --- /dev/null +++ b/changelog.d/12261.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.52 where admins could not deactivate and GDPR-erase a user if Synapse was configured with limits on avatars. diff --git a/changelog.d/12272.misc b/changelog.d/12272.misc new file mode 100644 index 000000000000..95589f3361e0 --- /dev/null +++ b/changelog.d/12272.misc @@ -0,0 +1 @@ +Add a new cache `_get_membership_from_event_id` to speed up push rule calculations in large rooms. diff --git a/changelog.d/12275.doc b/changelog.d/12275.doc new file mode 100644 index 000000000000..2e26ad21eb49 --- /dev/null +++ b/changelog.d/12275.doc @@ -0,0 +1 @@ +Corrected Authentik OpenID typo, added helpful note for troubleshooting. Contributed by @IronTooch. diff --git a/changelog.d/12279.doc b/changelog.d/12279.doc new file mode 100644 index 000000000000..ca07104c905c --- /dev/null +++ b/changelog.d/12279.doc @@ -0,0 +1 @@ +HAProxy reverse proxy guide update to stop sending IPv4-mapped address to homeserver. Contributed by @villepeh. diff --git a/changelog.d/12288.misc b/changelog.d/12288.misc new file mode 100644 index 000000000000..ee8fbfd290d9 --- /dev/null +++ b/changelog.d/12288.misc @@ -0,0 +1 @@ +Refuse to start if DB has non-`C` locale, unless config flag `allow_unsafe_db_locale` is set to true. diff --git a/changelog.d/12301.misc b/changelog.d/12301.misc new file mode 100644 index 000000000000..a4cd94ee5e7b --- /dev/null +++ b/changelog.d/12301.misc @@ -0,0 +1 @@ +Enhance logging for inbound federation events. diff --git a/changelog.d/12311.misc b/changelog.d/12311.misc new file mode 100644 index 000000000000..df0e824a7ecc --- /dev/null +++ b/changelog.d/12311.misc @@ -0,0 +1 @@ +Improve type annotations for `execute_values`. \ No newline at end of file diff --git a/changelog.d/12313.misc b/changelog.d/12313.misc new file mode 100644 index 000000000000..f59f6cdc40da --- /dev/null +++ b/changelog.d/12313.misc @@ -0,0 +1 @@ +Fix compatibility with the recently-released Jinja 3.1. diff --git a/changelog.d/12316.misc b/changelog.d/12316.misc new file mode 100644 index 000000000000..9f333e718a86 --- /dev/null +++ b/changelog.d/12316.misc @@ -0,0 +1 @@ +Avoid trying to calculate the state at outlier events. diff --git a/demo/start.sh b/demo/start.sh index 55e69685e3c2..5a9972d24c2c 100755 --- a/demo/start.sh +++ b/demo/start.sh @@ -38,6 +38,7 @@ for port in 8080 8081 8082; do printf '\n\n# Customisation made by demo/start.sh\n\n' echo "public_baseurl: http://localhost:$port/" echo 'enable_registration: true' + echo 'enable_registration_without_verification: true' echo '' # Warning, this heredoc depends on the interaction of tabs and spaces. diff --git a/docs/openid.md b/docs/openid.md index 171ea3b7128b..19cacaafefe0 100644 --- a/docs/openid.md +++ b/docs/openid.md @@ -225,6 +225,8 @@ oidc_providers: 3. Create an application for synapse in Authentik and link it to the provider. 4. Note the slug of your application, Client ID and Client Secret. +Note: RSA keys must be used for signing for Authentik, ECC keys do not work. + Synapse config: ```yaml oidc_providers: @@ -240,7 +242,7 @@ oidc_providers: - "email" user_mapping_provider: config: - localpart_template: "{{ user.preferred_username }}}" + localpart_template: "{{ user.preferred_username }}" display_name_template: "{{ user.preferred_username|capitalize }}" # TO BE FILLED: If your users have names in Authentik and you want those in Synapse, this should be replaced with user.name|capitalize. ``` diff --git a/docs/reverse_proxy.md b/docs/reverse_proxy.md index 1a89da50fd97..5a0c847951a6 100644 --- a/docs/reverse_proxy.md +++ b/docs/reverse_proxy.md @@ -182,7 +182,7 @@ matrix.example.com { ``` frontend https - bind :::443 v4v6 ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1 + bind *:443,[::]:443 ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1 http-request set-header X-Forwarded-Proto https if { ssl_fc } http-request set-header X-Forwarded-Proto http if !{ ssl_fc } http-request set-header X-Forwarded-For %[src] @@ -195,7 +195,7 @@ frontend https use_backend matrix if matrix-host matrix-path frontend matrix-federation - bind :::8448 v4v6 ssl crt /etc/ssl/haproxy/synapse.pem alpn h2,http/1.1 + bind *:8448,[::]:8448 ssl crt /etc/ssl/haproxy/synapse.pem alpn h2,http/1.1 http-request set-header X-Forwarded-Proto https if { ssl_fc } http-request set-header X-Forwarded-Proto http if !{ ssl_fc } http-request set-header X-Forwarded-For %[src] diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 9c2359ed8e90..a21b48ab2e63 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1218,10 +1218,18 @@ oembed: # Registration can be rate-limited using the parameters in the "Ratelimiting" # section of this file. -# Enable registration for new users. +# Enable registration for new users. Defaults to 'false'. It is highly recommended that if you enable registration, +# you use either captcha, email, or token-based verification to verify that new users are not bots. In order to enable registration +# without any verification, you must also set `enable_registration_without_verification`, found below. # #enable_registration: false +# Enable registration without email or captcha verification. Note: this option is *not* recommended, +# as registration without verification is a known vector for spam and abuse. Defaults to false. Has no effect +# unless `enable_registration` is also enabled. +# +#enable_registration_without_verification: true + # Time that a user's session remains valid for, after they log in. # # Note that this is not currently compatible with guest logins. diff --git a/docs/upgrade.md b/docs/upgrade.md index f9ac605e7b29..062e823333c7 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -99,8 +99,21 @@ experimental_features: groups_enabled: false ``` +## Change in behaviour for PostgreSQL databases with unsafe locale + +Synapse now refuses to start when using PostgreSQL with non-`C` values for `COLLATE` and +`CTYPE` unless the config flag `allow_unsafe_locale`, found in the database section of +the configuration file, is set to `true`. See the [PostgreSQL documentation](https://matrix-org.github.io/synapse/latest/postgres.html#fixing-incorrect-collate-or-ctype) +for more information and instructions on how to fix a database with incorrect values. + # Upgrading to v1.55.0 +## Open registration without verification is now disabled by default + +Synapse will refuse to start if registration is enabled without email, captcha, or token-based verification unless the new config +flag `enable_registration_without_verification` is set to "true". + + ## `synctl` script has been moved The `synctl` script diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index ad2b7c9515d8..0f75e7b9d491 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -351,6 +351,23 @@ def setup(config_options: List[str]) -> SynapseHomeServer: if config.server.gc_seconds: synapse.metrics.MIN_TIME_BETWEEN_GCS = config.server.gc_seconds + if ( + config.registration.enable_registration + and not config.registration.enable_registration_without_verification + ): + if ( + not config.captcha.enable_registration_captcha + and not config.registration.registrations_require_3pid + and not config.registration.registration_requires_token + ): + + raise ConfigError( + "You have enabled open registration without any verification. This is a known vector for " + "spam and abuse. If you would like to allow public registration, please consider adding email, " + "captcha, or token-based verification. Otherwise this check can be removed by setting the " + "`enable_registration_without_verification` config option to `true`." + ) + hs = SynapseHomeServer( config.server.server_name, config=config, diff --git a/synapse/config/registration.py b/synapse/config/registration.py index ea9b50fe97e4..40fb329a7fac 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -33,6 +33,10 @@ def read_config(self, config, **kwargs): str(config["disable_registration"]) ) + self.enable_registration_without_verification = strtobool( + str(config.get("enable_registration_without_verification", False)) + ) + self.registrations_require_3pid = config.get("registrations_require_3pid", []) self.allowed_local_3pids = config.get("allowed_local_3pids", []) self.enable_3pid_lookup = config.get("enable_3pid_lookup", True) @@ -207,10 +211,18 @@ def generate_config_section(self, generate_secrets=False, **kwargs): # Registration can be rate-limited using the parameters in the "Ratelimiting" # section of this file. - # Enable registration for new users. + # Enable registration for new users. Defaults to 'false'. It is highly recommended that if you enable registration, + # you use either captcha, email, or token-based verification to verify that new users are not bots. In order to enable registration + # without any verification, you must also set `enable_registration_without_verification`, found below. # #enable_registration: false + # Enable registration without email or captcha verification. Note: this option is *not* recommended, + # as registration without verification is a known vector for spam and abuse. Defaults to false. Has no effect + # unless `enable_registration` is also enabled. + # + #enable_registration_without_verification: true + # Time that a user's session remains valid for, after they log in. # # Note that this is not currently compatible with guest logins. diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index af2d0f7d7932..c7400c737bd7 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -1092,7 +1092,7 @@ async def _process_incoming_pdus_in_room_inner( # has started processing). while True: async with lock: - logger.info("handling received PDU: %s", event) + logger.info("handling received PDU in room %s: %s", room_id, event) try: with nested_logging_context(event.event_id): await self._federation_event_handler.on_receive_pdu( diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index f9544fe7fb83..1c4fb4360af9 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -493,6 +493,7 @@ async def create_event( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, require_consent: bool = True, outlier: bool = False, historical: bool = False, @@ -527,6 +528,15 @@ async def create_event( If non-None, prev_event_ids must also be provided. + state_event_ids: + The full state at a given event. This is used particularly by the MSC2716 + /batch_send endpoint. One use case is with insertion events which float at + the beginning of a historical batch and don't have any `prev_events` to + derive from; we add all of these state events as the explicit state so the + rest of the historical batch can inherit the same state and state_group. + This should normally be left as None, which will cause the auth_event_ids + to be calculated based on the room state at the prev_events. + require_consent: Whether to check if the requester has consented to the privacy policy. @@ -612,6 +622,7 @@ async def create_event( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + state_event_ids=state_event_ids, depth=depth, ) @@ -772,6 +783,7 @@ async def create_and_send_nonmember_event( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, ratelimit: bool = True, txn_id: Optional[str] = None, ignore_shadow_ban: bool = False, @@ -801,6 +813,14 @@ async def create_and_send_nonmember_event( based on the room state at the prev_events. If non-None, prev_event_ids must also be provided. + state_event_ids: + The full state at a given event. This is used particularly by the MSC2716 + /batch_send endpoint. One use case is with insertion events which float at + the beginning of a historical batch and don't have any `prev_events` to + derive from; we add all of these state events as the explicit state so the + rest of the historical batch can inherit the same state and state_group. + This should normally be left as None, which will cause the auth_event_ids + to be calculated based on the room state at the prev_events. ratelimit: Whether to rate limit this send. txn_id: The transaction ID. ignore_shadow_ban: True if shadow-banned users should be allowed to @@ -856,8 +876,10 @@ async def create_and_send_nonmember_event( requester, event_dict, txn_id=txn_id, + allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + state_event_ids=state_event_ids, outlier=outlier, historical=historical, depth=depth, @@ -893,6 +915,7 @@ async def create_new_client_event( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, depth: Optional[int] = None, ) -> Tuple[EventBase, EventContext]: """Create a new event for a local client @@ -915,6 +938,15 @@ async def create_new_client_event( Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + state_event_ids: + The full state at a given event. This is used particularly by the MSC2716 + /batch_send endpoint. One use case is with insertion events which float at + the beginning of a historical batch and don't have any `prev_events` to + derive from; we add all of these state events as the explicit state so the + rest of the historical batch can inherit the same state and state_group. + This should normally be left as None, which will cause the auth_event_ids + to be calculated based on the room state at the prev_events. + depth: Override the depth used to order the event in the DAG. Should normally be set to None, which will cause the depth to be calculated based on the prev_events. @@ -922,31 +954,26 @@ async def create_new_client_event( Returns: Tuple of created event, context """ - # Strip down the auth_event_ids to only what we need to auth the event. + # Strip down the state_event_ids to only what we need to auth the event. # For example, we don't need extra m.room.member that don't match event.sender - full_state_ids_at_event = None - if auth_event_ids is not None: - # If auth events are provided, prev events must be also. + if state_event_ids is not None: + # Do a quick check to make sure that prev_event_ids is present to + # make the type-checking around `builder.build` happy. # prev_event_ids could be an empty array though. assert prev_event_ids is not None - # Copy the full auth state before it stripped down - full_state_ids_at_event = auth_event_ids.copy() - temp_event = await builder.build( prev_event_ids=prev_event_ids, - auth_event_ids=auth_event_ids, + auth_event_ids=state_event_ids, depth=depth, ) - auth_events = await self.store.get_events_as_list(auth_event_ids) + state_events = await self.store.get_events_as_list(state_event_ids) # Create a StateMap[str] - auth_event_state_map = { - (e.type, e.state_key): e.event_id for e in auth_events - } - # Actually strip down and use the necessary auth events + state_map = {(e.type, e.state_key): e.event_id for e in state_events} + # Actually strip down and only use the necessary auth events auth_event_ids = self._event_auth_handler.compute_auth_events( event=temp_event, - current_state_ids=auth_event_state_map, + current_state_ids=state_map, for_verification=False, ) @@ -989,12 +1016,16 @@ async def create_new_client_event( context = EventContext.for_outlier() elif ( event.type == EventTypes.MSC2716_INSERTION - and full_state_ids_at_event + and state_event_ids and builder.internal_metadata.is_historical() ): + # Add explicit state to the insertion event so it has state to derive + # from even though it's floating with no `prev_events`. The rest of + # the batch can derive from this state and state_group. + # # TODO(faster_joins): figure out how this works, and make sure that the # old state is complete. - old_state = await self.store.get_events_as_list(full_state_ids_at_event) + old_state = await self.store.get_events_as_list(state_event_ids) context = await self.state.compute_event_context(event, old_state=old_state) else: context = await self.state.compute_event_context(event) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 6554c0d3c209..239b0aa7447a 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -336,12 +336,18 @@ async def check_avatar_size_and_mime_type(self, mxc: str) -> bool: """Check that the size and content type of the avatar at the given MXC URI are within the configured limits. + If the given `mxc` is empty, no checks are performed. (Users are always able to + unset their avatar.) + Args: mxc: The MXC URI at which the avatar can be found. Returns: A boolean indicating whether the file can be allowed to be set as an avatar. """ + if mxc == "": + return True + if not self.max_avatar_size and not self.allowed_avatar_mimetypes: return True diff --git a/synapse/handlers/room_batch.py b/synapse/handlers/room_batch.py index abbf7b7b27a8..a0255bd1439c 100644 --- a/synapse/handlers/room_batch.py +++ b/synapse/handlers/room_batch.py @@ -121,12 +121,11 @@ async def create_requester_for_user_id_from_app_service( return create_requester(user_id, app_service=app_service) - async def get_most_recent_auth_event_ids_from_event_id_list( + async def get_most_recent_full_state_ids_from_event_id_list( self, event_ids: List[str] ) -> List[str]: - """Find the most recent auth event ids (derived from state events) that - allowed that message to be sent. We will use this as a base - to auth our historical messages against. + """Find the most recent event_id and grab the full state at that event. + We will use this as a base to auth our historical messages against. Args: event_ids: List of event ID's to look at @@ -136,38 +135,37 @@ async def get_most_recent_auth_event_ids_from_event_id_list( """ ( - most_recent_prev_event_id, + most_recent_event_id, _, ) = await self.store.get_max_depth_of(event_ids) # mapping from (type, state_key) -> state_event_id prev_state_map = await self.state_store.get_state_ids_for_event( - most_recent_prev_event_id + most_recent_event_id ) # List of state event ID's - prev_state_ids = list(prev_state_map.values()) - auth_event_ids = prev_state_ids + full_state_ids = list(prev_state_map.values()) - return auth_event_ids + return full_state_ids async def persist_state_events_at_start( self, state_events_at_start: List[JsonDict], room_id: str, - initial_auth_event_ids: List[str], + initial_state_event_ids: List[str], app_service_requester: Requester, ) -> List[str]: """Takes all `state_events_at_start` event dictionaries and creates/persists - them as floating state events which don't resolve into the current room state. - They are floating because they reference a fake prev_event which doesn't connect - to the normal DAG at all. + them in a floating state event chain which don't resolve into the current room + state. They are floating because they reference no prev_events and are marked + as outliers which disconnects them from the normal DAG. Args: state_events_at_start: room_id: Room where you want the events persisted in. - initial_auth_event_ids: These will be the auth_events for the first - state event created. Each event created afterwards will be - added to the list of auth events for the next state event - created. + initial_state_event_ids: + The base set of state for the historical batch which the floating + state chain will derive from. This should probably be the state + from the `prev_event` defined by `/batch_send?prev_event_id=$abc`. app_service_requester: The requester of an application service. Returns: @@ -176,7 +174,7 @@ async def persist_state_events_at_start( assert app_service_requester.app_service state_event_ids_at_start = [] - auth_event_ids = initial_auth_event_ids.copy() + state_event_ids = initial_state_event_ids.copy() # Make the state events float off on their own by specifying no # prev_events for the first one in the chain so we don't have a bunch of @@ -189,9 +187,7 @@ async def persist_state_events_at_start( ) logger.debug( - "RoomBatchSendEventRestServlet inserting state_event=%s, auth_event_ids=%s", - state_event, - auth_event_ids, + "RoomBatchSendEventRestServlet inserting state_event=%s", state_event ) event_dict = { @@ -217,16 +213,26 @@ async def persist_state_events_at_start( room_id=room_id, action=membership, content=event_dict["content"], + # Mark as an outlier to disconnect it from the normal DAG + # and not show up between batches of history. outlier=True, historical=True, - # Only the first event in the chain should be floating. + # Only the first event in the state chain should be floating. # The rest should hang off each other in a chain. allow_no_prev_events=index == 0, prev_event_ids=prev_event_ids_for_state_chain, + # Since each state event is marked as an outlier, the + # `EventContext.for_outlier()` won't have any `state_ids` + # set and therefore can't derive any state even though the + # prev_events are set. Also since the first event in the + # state chain is floating with no `prev_events`, it can't + # derive state from anywhere automatically. So we need to + # set some state explicitly. + # # Make sure to use a copy of this list because we modify it # later in the loop here. Otherwise it will be the same # reference and also update in the event when we append later. - auth_event_ids=auth_event_ids.copy(), + state_event_ids=state_event_ids.copy(), ) else: # TODO: Add some complement tests that adds state that is not member joins @@ -240,21 +246,31 @@ async def persist_state_events_at_start( state_event["sender"], app_service_requester.app_service ), event_dict, + # Mark as an outlier to disconnect it from the normal DAG + # and not show up between batches of history. outlier=True, historical=True, - # Only the first event in the chain should be floating. + # Only the first event in the state chain should be floating. # The rest should hang off each other in a chain. allow_no_prev_events=index == 0, prev_event_ids=prev_event_ids_for_state_chain, + # Since each state event is marked as an outlier, the + # `EventContext.for_outlier()` won't have any `state_ids` + # set and therefore can't derive any state even though the + # prev_events are set. Also since the first event in the + # state chain is floating with no `prev_events`, it can't + # derive state from anywhere automatically. So we need to + # set some state explicitly. + # # Make sure to use a copy of this list because we modify it # later in the loop here. Otherwise it will be the same # reference and also update in the event when we append later. - auth_event_ids=auth_event_ids.copy(), + state_event_ids=state_event_ids.copy(), ) event_id = event.event_id state_event_ids_at_start.append(event_id) - auth_event_ids.append(event_id) + state_event_ids.append(event_id) # Connect all the state in a floating chain prev_event_ids_for_state_chain = [event_id] @@ -265,7 +281,7 @@ async def persist_historical_events( events_to_create: List[JsonDict], room_id: str, inherited_depth: int, - auth_event_ids: List[str], + initial_state_event_ids: List[str], app_service_requester: Requester, ) -> List[str]: """Create and persists all events provided sequentially. Handles the @@ -281,8 +297,10 @@ async def persist_historical_events( room_id: Room where you want the events persisted in. inherited_depth: The depth to create the events at (you will probably by calling inherit_depth_from_prev_ids(...)). - auth_event_ids: Define which events allow you to create the given - event in the room. + initial_state_event_ids: + This is used to set explicit state for the insertion event at + the start of the historical batch since it's floating with no + prev_events to derive state from automatically. app_service_requester: The requester of an application service. Returns: @@ -290,6 +308,11 @@ async def persist_historical_events( """ assert app_service_requester.app_service + # We expect the first event in a historical batch to be an insertion event + assert events_to_create[0]["type"] == EventTypes.MSC2716_INSERTION + # We expect the last event in a historical batch to be an batch event + assert events_to_create[-1]["type"] == EventTypes.MSC2716_BATCH + # Make the historical event chain float off on its own by specifying no # prev_events for the first event in the chain which causes the HS to # ask for the state at the start of the batch later. @@ -321,11 +344,16 @@ async def persist_historical_events( ev["sender"], app_service_requester.app_service ), event_dict, - # Only the first event in the chain should be floating. - # The rest should hang off each other in a chain. + # Only the first event (which is the insertion event) in the + # chain should be floating. The rest should hang off each other + # in a chain. allow_no_prev_events=index == 0, prev_event_ids=event_dict.get("prev_events"), - auth_event_ids=auth_event_ids, + # Since the first event (which is the insertion event) in the + # chain is floating with no `prev_events`, it can't derive state + # from anywhere automatically. So we need to set some state + # explicitly. + state_event_ids=initial_state_event_ids if index == 0 else None, historical=True, depth=inherited_depth, ) @@ -343,10 +371,9 @@ async def persist_historical_events( ) logger.debug( - "RoomBatchSendEventRestServlet inserting event=%s, prev_event_ids=%s, auth_event_ids=%s", + "RoomBatchSendEventRestServlet inserting event=%s, prev_event_ids=%s", event, prev_event_ids, - auth_event_ids, ) events_to_persist.append((event, context)) @@ -376,12 +403,12 @@ async def handle_batch_of_events( room_id: str, batch_id_to_connect_to: str, inherited_depth: int, - auth_event_ids: List[str], + initial_state_event_ids: List[str], app_service_requester: Requester, ) -> Tuple[List[str], str]: """ - Handles creating and persisting all of the historical events as well - as insertion and batch meta events to make the batch navigable in the DAG. + Handles creating and persisting all of the historical events as well as + insertion and batch meta events to make the batch navigable in the DAG. Args: events_to_create: List of historical events to create in JSON @@ -391,8 +418,13 @@ async def handle_batch_of_events( want this batch to connect to. inherited_depth: The depth to create the events at (you will probably by calling inherit_depth_from_prev_ids(...)). - auth_event_ids: Define which events allow you to create the given - event in the room. + initial_state_event_ids: + This is used to set explicit state for the insertion event at + the start of the historical batch since it's floating with no + prev_events to derive state from automatically. This should + probably be the state from the `prev_event` defined by + `/batch_send?prev_event_id=$abc` plus the outcome of + `persist_state_events_at_start` app_service_requester: The requester of an application service. Returns: @@ -438,7 +470,7 @@ async def handle_batch_of_events( events_to_create=events_to_create, room_id=room_id, inherited_depth=inherited_depth, - auth_event_ids=auth_event_ids, + initial_state_event_ids=initial_state_event_ids, app_service_requester=app_service_requester, ) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 7cbc484b0654..a33fa34aa8df 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -272,6 +272,7 @@ async def _local_membership_update( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, txn_id: Optional[str] = None, ratelimit: bool = True, content: Optional[dict] = None, @@ -298,6 +299,14 @@ async def _local_membership_update( The event ids to use as the auth_events for the new event. Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + state_event_ids: + The full state at a given event. This is used particularly by the MSC2716 + /batch_send endpoint. One use case is the historical `state_events_at_start`; + since each is marked as an `outlier`, the `EventContext.for_outlier()` won't + have any `state_ids` set and therefore can't derive any state even though the + prev_events are set so we need to set them ourself via this argument. + This should normally be left as None, which will cause the auth_event_ids + to be calculated based on the room state at the prev_events. txn_id: ratelimit: @@ -353,6 +362,7 @@ async def _local_membership_update( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + state_event_ids=state_event_ids, require_consent=require_consent, outlier=outlier, historical=historical, @@ -456,6 +466,7 @@ async def update_membership( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, ) -> Tuple[str, int]: """Update a user's membership in a room. @@ -487,6 +498,14 @@ async def update_membership( The event ids to use as the auth_events for the new event. Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + state_event_ids: + The full state at a given event. This is used particularly by the MSC2716 + /batch_send endpoint. One use case is the historical `state_events_at_start`; + since each is marked as an `outlier`, the `EventContext.for_outlier()` won't + have any `state_ids` set and therefore can't derive any state even though the + prev_events are set so we need to set them ourself via this argument. + This should normally be left as None, which will cause the auth_event_ids + to be calculated based on the room state at the prev_events. Returns: A tuple of the new event ID and stream ID. @@ -526,6 +545,7 @@ async def update_membership( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + state_event_ids=state_event_ids, ) return result @@ -548,6 +568,7 @@ async def update_membership_locked( allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + state_event_ids: Optional[List[str]] = None, ) -> Tuple[str, int]: """Helper for update_membership. @@ -581,6 +602,14 @@ async def update_membership_locked( The event ids to use as the auth_events for the new event. Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + state_event_ids: + The full state at a given event. This is used particularly by the MSC2716 + /batch_send endpoint. One use case is the historical `state_events_at_start`; + since each is marked as an `outlier`, the `EventContext.for_outlier()` won't + have any `state_ids` set and therefore can't derive any state even though the + prev_events are set so we need to set them ourself via this argument. + This should normally be left as None, which will cause the auth_event_ids + to be calculated based on the room state at the prev_events. Returns: A tuple of the new event ID and stream ID. @@ -708,6 +737,7 @@ async def update_membership_locked( allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + state_event_ids=state_event_ids, content=content, require_consent=require_consent, outlier=outlier, @@ -932,6 +962,7 @@ async def update_membership_locked( ratelimit=ratelimit, prev_event_ids=latest_event_ids, auth_event_ids=auth_event_ids, + state_event_ids=state_event_ids, content=content, require_consent=require_consent, outlier=outlier, diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index a628faaf6520..ba9755f08b8c 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -671,7 +671,8 @@ def register_device( def record_user_external_id( self, auth_provider_id: str, remote_user_id: str, registered_user_id: str ) -> defer.Deferred: - """Record a mapping from an external user id to a mxid + """Record a mapping between an external user id from a single sign-on provider + and a mxid. Added in Synapse v1.9.0. @@ -1286,6 +1287,30 @@ async def check_username(self, username: str) -> None: """ await self._registration_handler.check_username(username) + async def store_remote_3pid_association( + self, user_id: str, medium: str, address: str, id_server: str + ) -> None: + """Stores an existing association between a user ID and a third-party identifier. + + The association must already exist on the remote identity server. + + Added in Synapse v1.56.0. + + Args: + user_id: The user ID that's been associated with the 3PID. + medium: The medium of the 3PID (current supported values are "msisdn" and + "email"). + address: The address of the 3PID. + id_server: The identity server the 3PID association has been registered on. + This should only be the domain (or IP address, optionally with the port + number) for the identity server. This will be used to reach out to the + identity server using HTTPS (unless specified otherwise by Synapse's + configuration) when attempting to unbind the third-party identifier. + + + """ + await self._store.add_user_bound_threepid(user_id, medium, address, id_server) + class PublicRoomListManager: """Contains methods for adding to, removing from and querying whether a room diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 030898e4d0cc..a402a3e40374 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -24,6 +24,7 @@ from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.state import POWER_KEY +from synapse.storage.databases.main.roommember import EventIdMembership from synapse.util.async_helpers import Linearizer from synapse.util.caches import CacheMetric, register_cache from synapse.util.caches.descriptors import lru_cache @@ -292,7 +293,7 @@ def _condition_checker( return True -MemberMap = Dict[str, Tuple[str, str]] +MemberMap = Dict[str, Optional[EventIdMembership]] Rule = Dict[str, dict] RulesByUser = Dict[str, List[Rule]] StateGroup = Union[object, int] @@ -306,7 +307,7 @@ class RulesForRoomData: *only* include data, and not references to e.g. the data stores. """ - # event_id -> (user_id, state) + # event_id -> EventIdMembership member_map: MemberMap = attr.Factory(dict) # user_id -> rules rules_by_user: RulesByUser = attr.Factory(dict) @@ -447,11 +448,10 @@ async def get_rules( res = self.data.member_map.get(event_id, None) if res: - user_id, state = res - if state == Membership.JOIN: - rules = self.data.rules_by_user.get(user_id, None) + if res.membership == Membership.JOIN: + rules = self.data.rules_by_user.get(res.user_id, None) if rules: - ret_rules_by_user[user_id] = rules + ret_rules_by_user[res.user_id] = rules continue # If a user has left a room we remove their push rule. If they @@ -502,24 +502,26 @@ async def _update_rules_with_member_event_ids( """ sequence = self.data.sequence - rows = await self.store.get_membership_from_event_ids(member_event_ids.values()) - - members = {row["event_id"]: (row["user_id"], row["membership"]) for row in rows} + members = await self.store.get_membership_from_event_ids( + member_event_ids.values() + ) - # If the event is a join event then it will be in current state evnts + # If the event is a join event then it will be in current state events # map but not in the DB, so we have to explicitly insert it. if event.type == EventTypes.Member: for event_id in member_event_ids.values(): if event_id == event.event_id: - members[event_id] = (event.state_key, event.membership) + members[event_id] = EventIdMembership( + user_id=event.state_key, membership=event.membership + ) if logger.isEnabledFor(logging.DEBUG): logger.debug("Found members %r: %r", self.room_id, members.values()) joined_user_ids = { - user_id - for user_id, membership in members.values() - if membership == Membership.JOIN + entry.user_id + for entry in members.values() + if entry and entry.membership == Membership.JOIN } logger.debug("Joined: %r", joined_user_ids) diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 649a4f49d024..5ccdd88364d7 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -18,6 +18,7 @@ import bleach import jinja2 +from markupsafe import Markup from synapse.api.constants import EventTypes, Membership, RoomTypes from synapse.api.errors import StoreError @@ -867,7 +868,7 @@ def _make_unsubscribe_link( ) -def safe_markup(raw_html: str) -> jinja2.Markup: +def safe_markup(raw_html: str) -> Markup: """ Sanitise a raw HTML string to a set of allowed tags and attributes, and linkify any bare URLs. @@ -877,7 +878,7 @@ def safe_markup(raw_html: str) -> jinja2.Markup: Returns: A Markup object ready to safely use in a Jinja template. """ - return jinja2.Markup( + return Markup( bleach.linkify( bleach.clean( raw_html, @@ -891,7 +892,7 @@ def safe_markup(raw_html: str) -> jinja2.Markup: ) -def safe_text(raw_text: str) -> jinja2.Markup: +def safe_text(raw_text: str) -> Markup: """ Sanitise text (escape any HTML tags), and then linkify any bare URLs. @@ -901,7 +902,7 @@ def safe_text(raw_text: str) -> jinja2.Markup: Returns: A Markup object ready to safely use in a Jinja template. """ - return jinja2.Markup( + return Markup( bleach.linkify(bleach.clean(raw_text, tags=[], attributes=[], strip=False)) ) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 79ae06ce5d07..8419ab3aca95 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -74,8 +74,10 @@ # Note: 21.1.0 broke `/sync`, see #9936 "attrs>=19.2.0,!=21.1.0", "netaddr>=0.7.18", - # Jinja2 3.1.0 removes the deprecated jinja2.Markup class, which we rely on. - "Jinja2<3.1.0", + # Jinja 2.x is incompatible with MarkupSafe>=2.1. To ensure that admins do not + # end up with a broken installation, with recent MarkupSafe but old Jinja, we + # add a lower bound to the Jinja2 dependency. + "Jinja2>=3.0", "bleach>=1.4.3", # We use `ParamSpec`, which was added in `typing-extensions` 3.10.0.0. "typing-extensions>=3.10.0", diff --git a/synapse/rest/client/room_batch.py b/synapse/rest/client/room_batch.py index 0048973e5961..dd91dabedd66 100644 --- a/synapse/rest/client/room_batch.py +++ b/synapse/rest/client/room_batch.py @@ -123,21 +123,26 @@ async def on_POST( errcode=Codes.INVALID_PARAM, ) - # For the event we are inserting next to (`prev_event_ids_from_query`), - # find the most recent auth events (derived from state events) that - # allowed that message to be sent. We will use that as a base - # to auth our historical messages against. - auth_event_ids = await self.room_batch_handler.get_most_recent_auth_event_ids_from_event_id_list( + # Make sure that the prev_event_ids exist and aren't outliers - ie, they are + # regular parts of the room DAG where we know the state. + non_outlier_prev_events = await self.store.have_events_in_timeline( prev_event_ids_from_query ) + for prev_event_id in prev_event_ids_from_query: + if prev_event_id not in non_outlier_prev_events: + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "prev_event %s does not exist, or is an outlier" % (prev_event_id,), + errcode=Codes.INVALID_PARAM, + ) - if not auth_event_ids: - raise SynapseError( - HTTPStatus.BAD_REQUEST, - "No auth events found for given prev_event query parameter. The prev_event=%s probably does not exist." - % prev_event_ids_from_query, - errcode=Codes.INVALID_PARAM, - ) + # For the event we are inserting next to (`prev_event_ids_from_query`), + # find the most recent state events that allowed that message to be + # sent. We will use that as a base to auth our historical messages + # against. + state_event_ids = await self.room_batch_handler.get_most_recent_full_state_ids_from_event_id_list( + prev_event_ids_from_query + ) state_event_ids_at_start = [] # Create and persist all of the state events that float off on their own @@ -148,13 +153,13 @@ async def on_POST( await self.room_batch_handler.persist_state_events_at_start( state_events_at_start=body["state_events_at_start"], room_id=room_id, - initial_auth_event_ids=auth_event_ids, + initial_state_event_ids=state_event_ids, app_service_requester=requester, ) ) # Update our ongoing auth event ID list with all of the new state we # just created - auth_event_ids.extend(state_event_ids_at_start) + state_event_ids.extend(state_event_ids_at_start) inherited_depth = await self.room_batch_handler.inherit_depth_from_prev_ids( prev_event_ids_from_query @@ -196,7 +201,12 @@ async def on_POST( ), base_insertion_event_dict, prev_event_ids=base_insertion_event_dict.get("prev_events"), - auth_event_ids=auth_event_ids, + # Also set the explicit state here because we want to resolve + # any `state_events_at_start` here too. It's not strictly + # necessary to accomplish anything but if someone asks for the + # state at this point, we probably want to show them the + # historical state that was part of this batch. + state_event_ids=state_event_ids, historical=True, depth=inherited_depth, ) @@ -212,7 +222,7 @@ async def on_POST( room_id=room_id, batch_id_to_connect_to=batch_id_to_connect_to, inherited_depth=inherited_depth, - auth_event_ids=auth_event_ids, + initial_state_event_ids=state_event_ids, app_service_requester=requester, ) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 367709a1a710..72fef1533faa 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -290,11 +290,15 @@ def execute_batch(self, sql: str, args: Iterable[Iterable[Any]]) -> None: if isinstance(self.database_engine, PostgresEngine): from psycopg2.extras import execute_batch - self._do_execute(lambda *x: execute_batch(self.txn, *x), sql, args) + self._do_execute( + lambda the_sql: execute_batch(self.txn, the_sql, args), sql + ) else: self.executemany(sql, args) - def execute_values(self, sql: str, *args: Any, fetch: bool = True) -> List[Tuple]: + def execute_values( + self, sql: str, values: Iterable[Iterable[Any]], fetch: bool = True + ) -> List[Tuple]: """Corresponds to psycopg2.extras.execute_values. Only available when using postgres. @@ -305,15 +309,8 @@ def execute_values(self, sql: str, *args: Any, fetch: bool = True) -> List[Tuple from psycopg2.extras import execute_values return self._do_execute( - # Type ignore: mypy is unhappy because if `x` is a 5-tuple, then there will - # be two values for `fetch`: one given positionally, and another given - # as a keyword argument. We might be able to fix this by - # - propagating the signature of psycopg2.extras.execute_values to this - # function, or - # - changing `*args: Any` to `values: T` for some appropriate T. - lambda *x: execute_values(self.txn, *x, fetch=fetch), # type: ignore[misc] + lambda the_sql: execute_values(self.txn, the_sql, values, fetch=fetch), sql, - *args, ) def execute(self, sql: str, *args: Any) -> None: diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index 2d7511d61391..dd4e83a2ad19 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -192,6 +192,10 @@ def _invalidate_caches_for_event( self.get_unread_event_push_actions_by_room_for_user.invalidate((room_id,)) + # The `_get_membership_from_event_id` is immutable, except for the + # case where we look up an event *before* persisting it. + self._get_membership_from_event_id.invalidate((event_id,)) + if not backfilled: self._events_stream_cache.entity_has_changed(room_id, stream_ordering) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 1f60aef180d0..d253243125fb 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1745,6 +1745,13 @@ def non_null_str_or_none(val: Any) -> Optional[str]: (event.state_key,), ) + # The `_get_membership_from_event_id` is immutable, except for the + # case where we look up an event *before* persisting it. + txn.call_after( + self.store._get_membership_from_event_id.invalidate, + (event.event_id,), + ) + # We update the local_current_membership table only if the event is # "current", i.e., its something that has just happened. # diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index bef675b8453c..3248da5356a4 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -63,6 +63,14 @@ _CURRENT_STATE_MEMBERSHIP_UPDATE_NAME = "current_state_events_membership" +@attr.s(frozen=True, slots=True, auto_attribs=True) +class EventIdMembership: + """Returned by `get_membership_from_event_ids`""" + + user_id: str + membership: str + + class RoomMemberWorkerStore(EventsWorkerStore): def __init__( self, @@ -772,7 +780,7 @@ async def _get_joined_profiles_from_event_ids(self, event_ids: Iterable[str]): retcols=("user_id", "display_name", "avatar_url", "event_id"), keyvalues={"membership": Membership.JOIN}, batch_size=500, - desc="_get_membership_from_event_ids", + desc="_get_joined_profiles_from_event_ids", ) return { @@ -1000,12 +1008,26 @@ async def get_rooms_user_has_been_in(self, user_id: str) -> Set[str]: return set(room_ids) + @cached(max_entries=5000) + async def _get_membership_from_event_id( + self, member_event_id: str + ) -> Optional[EventIdMembership]: + raise NotImplementedError() + + @cachedList( + cached_method_name="_get_membership_from_event_id", list_name="member_event_ids" + ) async def get_membership_from_event_ids( self, member_event_ids: Iterable[str] - ) -> List[dict]: - """Get user_id and membership of a set of event IDs.""" + ) -> Dict[str, Optional[EventIdMembership]]: + """Get user_id and membership of a set of event IDs. + + Returns: + Mapping from event ID to `EventIdMembership` if the event is a + membership event, otherwise the value is None. + """ - return await self.db_pool.simple_select_many_batch( + rows = await self.db_pool.simple_select_many_batch( table="room_memberships", column="event_id", iterable=member_event_ids, @@ -1015,6 +1037,13 @@ async def get_membership_from_event_ids( desc="get_membership_from_event_ids", ) + return { + row["event_id"]: EventIdMembership( + membership=row["membership"], user_id=row["user_id"] + ) + for row in rows + } + async def is_local_host_in_room_ignoring_users( self, room_id: str, ignore_users: Collection[str] ) -> bool: diff --git a/synapse/storage/persist_events.py b/synapse/storage/persist_events.py index 7d543fdbe08a..b40292281767 100644 --- a/synapse/storage/persist_events.py +++ b/synapse/storage/persist_events.py @@ -1023,8 +1023,13 @@ async def _is_server_still_joined( # Check if any of the changes that we don't have events for are joins. if events_to_check: - rows = await self.main_store.get_membership_from_event_ids(events_to_check) - is_still_joined = any(row["membership"] == Membership.JOIN for row in rows) + members = await self.main_store.get_membership_from_event_ids( + events_to_check + ) + is_still_joined = any( + member and member.membership == Membership.JOIN + for member in members.values() + ) if is_still_joined: return True @@ -1060,9 +1065,11 @@ async def _is_server_still_joined( ), event_id in current_state.items() if typ == EventTypes.Member and not self.is_mine_id(state_key) ] - rows = await self.main_store.get_membership_from_event_ids(remote_event_ids) + members = await self.main_store.get_membership_from_event_ids(remote_event_ids) potentially_left_users.update( - row["user_id"] for row in rows if row["membership"] == Membership.JOIN + member.user_id + for member in members.values() + if member and member.membership == Membership.JOIN ) return False diff --git a/tests/config/test_registration_config.py b/tests/config/test_registration_config.py index 17a84d20d811..2acdb6ac6161 100644 --- a/tests/config/test_registration_config.py +++ b/tests/config/test_registration_config.py @@ -11,14 +11,16 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +import synapse.app.homeserver from synapse.config import ConfigError from synapse.config.homeserver import HomeServerConfig -from tests.unittest import TestCase +from tests.config.utils import ConfigFileTestCase from tests.utils import default_config -class RegistrationConfigTestCase(TestCase): +class RegistrationConfigTestCase(ConfigFileTestCase): def test_session_lifetime_must_not_be_exceeded_by_smaller_lifetimes(self): """ session_lifetime should logically be larger than, or at least as large as, @@ -76,3 +78,19 @@ def test_session_lifetime_must_not_be_exceeded_by_smaller_lifetimes(self): HomeServerConfig().parse_config_dict( {"session_lifetime": "31m", "refresh_token_lifetime": "31m", **config_dict} ) + + def test_refuse_to_start_if_open_registration_and_no_verification(self): + self.generate_config() + self.add_lines_to_config( + [ + " ", + "enable_registration: true", + "registrations_require_3pid: []", + "enable_registration_captcha: false", + "registration_requires_token: false", + ] + ) + + # Test that allowing open registration without verification raises an error + with self.assertRaises(ConfigError): + synapse.app.homeserver.setup(["-c", self.config_file]) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 08733a9f2d42..1ec105c37398 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -267,6 +267,12 @@ def test_avatar_constraints_no_config(self) -> None: ) self.assertTrue(res) + @unittest.override_config({"max_avatar_size": 50}) + def test_avatar_constraints_allow_empty_avatar_url(self) -> None: + """An empty avatar is always permitted.""" + res = self.get_success(self.handler.check_avatar_size_and_mime_type("")) + self.assertTrue(res) + @unittest.override_config({"max_avatar_size": 50}) def test_avatar_constraints_missing(self) -> None: """Tests that an avatar isn't allowed if the file at the given MXC URI couldn't diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index a60ea0a56305..bef911d5df41 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -1050,6 +1050,25 @@ def test_deactivate_user_erase_true(self) -> None: self._is_erased("@user:test", True) + @override_config({"max_avatar_size": 1234}) + def test_deactivate_user_erase_true_avatar_nonnull_but_empty(self) -> None: + """Check we can erase a user whose avatar is the empty string. + + Reproduces #12257. + """ + # Patch `self.other_user` to have an empty string as their avatar. + self.get_success(self.store.set_profile_avatar_url("user", "")) + + # Check we can still erase them. + channel = self.make_request( + "POST", + self.url, + access_token=self.admin_user_tok, + content={"erase": True}, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self._is_erased("@user:test", True) + def test_deactivate_user_erase_false(self) -> None: """ Test deactivating a user and set `erase` to `false`