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

Ensure pushers are deleted for deactivated accounts #9285

Merged
merged 5 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9285.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug where users' pushers were not all deleted when they deactivated their account.
5 changes: 5 additions & 0 deletions synapse/handlers/deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ async def deactivate_account(

await self.store.user_set_password_hash(user_id, None)

# Most of the pushers will have been deleted when we logged out the
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
# associated devices above, but we still need to delete pushers not
# associated with devices, e.g. email pushers.
await self.store.delete_all_pushers_for_user(user_id)

# Add the user to a table of users pending deactivation (ie.
# removal from all the rooms they're a member of)
await self.store.add_user_pending_deactivation(user_id)
Expand Down
41 changes: 41 additions & 0 deletions synapse/storage/databases/main/pusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,3 +373,44 @@ def delete_pusher_txn(txn, stream_id):
await self.db_pool.runInteraction(
"delete_pusher", delete_pusher_txn, stream_id
)

async def delete_all_pushers_for_user(self, user_id: str) -> None:
"""Delete all pushers associated with an account."""

# We want to generate a row in `deleted_pushers` for each pusher we're
# deleting, so we fetch the list now so we can generate the appropriate
# number of stream IDs.
#
# Note: technically there could be a race here between adding/deleting
# pushers, but a) the worst case if we don't stop a pusher until the
# next restart and b) this is only called when we're deactivating an
# account.
pushers = list(await self.get_pushers_by_user_id(user_id))

def delete_pushers_txn(txn, stream_ids):
self._invalidate_cache_and_stream( # type: ignore
txn, self.get_if_user_has_pusher, (user_id,)
)

self.db_pool.simple_delete_txn(
txn,
table="pushers",
keyvalues={"user_name": user_id},
)

for stream_id, pusher in zip(stream_ids, pushers):
self.db_pool.simple_insert_txn(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this could be simple_insert_many_txn, but not sure if that's an optimization or not. (Is there any reason not to do that?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, simple_insert_many is quicker

txn,
table="deleted_pushers",
values={
"stream_id": stream_id,
"app_id": pusher.app_id,
"pushkey": pusher.pushkey,
"user_id": user_id,
},
)

async with self._pushers_id_gen.get_next_mult(len(pushers)) as stream_ids:
await self.db_pool.runInteraction(
"delete_all_pushers_for_user", delete_pushers_txn, stream_ids
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* Copyright 2021 The Matrix.org Foundation C.I.C
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* 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.
*/


-- We may not have deleted all pushers for deactivated accounts. Do so now.
--
-- Note: We don't bother updating the `deleted_pushers` table as it's just use
-- to stop pushers on workers, and that will happen when they get next restarted.
DELETE FROM pushers WHERE user_name IN (SELECT name FROM users WHERE deactivated = 1);