Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Calling the deactivation API without rate limiting causes room inconsistencies #16290

Open
ThoreKr opened this issue Sep 10, 2023 · 3 comments
Open
Labels
A-Admin-API O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@ThoreKr
Copy link

ThoreKr commented Sep 10, 2023

Description

When bulk removing users in the IdM (keycloak), at the same time multiple requests will be sent to the /_synapse/admin/v1/deactivate/<user_id> endpoint to deactivate (erase=true) the concerning accounts in synapse.

When not rate limiting these delete requests, the erasure can have the following side effect for a subset of the deactivated users:

  • user unset their display name
  • user left the room
  • user joined the room

These hulls of deactivated accounts then sit in the room as regular members. Manually calling the deactivate endpoint again will remove them from the room again, but they shouldn't be rejoining in the first place.

Steps to reproduce

  • Have a room with about 1k users (authenticated via OIDC) all auto-joining into a shared public, non-encrypted room
  • In a for loop without wait times call the deactivate endpoint for 20-30 users
  • Watch them unsetting their display name and leaving
  • Some will shortly after rejoin the room

Homeserver

selfhosted

Synapse Version

1.89.0

Installation Method

Docker (matrixdotorg/synapse)

Database

postgresql, single instance

Workers

Single process

Platform

x86 (64 bit) VM

Configuration

  • Presence is enabled
  • Message retention is enabled
  purge_jobs:
    - longest_max_lifetime: 2d
      interval: 1h
    - shortest_max_lifetime: 2d
      interval: 1d

Relevant log output

Only the api call to the deactivate endpoint is logged.

Anything else that would be useful to know?

No response

@reivilibre
Copy link
Contributor

possibly related: if we deduplicated deactivations, it might sort out the issues seen here — #16055

@erikjohnston
Copy link
Member

erikjohnston commented Sep 26, 2023

I think we can also probably just skip setting peoples display names in rooms when doing the deactivation? That would fix the race and reduce the cost of deactivation.

I think that would be a case of only calling the following on deactivation?

await self._update_join_states(requester, target_user)

@erikjohnston erikjohnston added A-Admin-API S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 26, 2023
@clokep
Copy link
Member

clokep commented Sep 26, 2023

I think we can also probably just skip setting peoples display names in rooms when doing the deactivation?

Or could we do it as a background task?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Admin-API O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

4 participants