From bf98fa0864fe62dbf1303acc778912d69fd4a738 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 8 May 2018 15:58:35 +0100 Subject: [PATCH 1/8] Part user from rooms on account deactivate This implements this very crudely: this probably isn't viable because parting a user from all their rooms could take a long time, and if the HS gets restarted in that time the process will be aborted. --- synapse/handlers/deactivate_account.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index b1d3814909..387620c9c6 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Copyright 2017 New Vector Ltd +# Copyright 2017, 2018 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -15,6 +15,7 @@ from twisted.internet import defer from ._base import BaseHandler +from synapse.types import UserID, create_requester import logging @@ -27,6 +28,7 @@ class DeactivateAccountHandler(BaseHandler): super(DeactivateAccountHandler, self).__init__(hs) self._auth_handler = hs.get_auth_handler() self._device_handler = hs.get_device_handler() + self._room_member_handler = hs.get_room_member_handler() @defer.inlineCallbacks def deactivate_account(self, user_id): @@ -50,3 +52,15 @@ class DeactivateAccountHandler(BaseHandler): yield self.store.user_delete_threepids(user_id) yield self.store.user_set_password_hash(user_id, None) + + user = UserID.from_string(user_id) + + rooms_for_user = yield self.store.get_rooms_for_user(user_id) + for room_id in rooms_for_user: + yield self._room_member_handler.update_membership( + create_requester(user), + user, + room_id, + "leave", + ratelimit=False, + ) From 7e8726b8fb297f255ed1fd78f8e3be1d7f21dcc7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 9 May 2018 14:54:28 +0100 Subject: [PATCH 2/8] Part deactivated users in the background One room at a time so we don't take out the whole server with leave events, and restart at server restart. --- synapse/handlers/deactivate_account.py | 35 +++++++++++++++++++++++++- synapse/storage/registration.py | 27 ++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 387620c9c6..5bb6c11e8f 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -12,10 +12,11 @@ # 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. -from twisted.internet import defer +from twisted.internet import defer, reactor from ._base import BaseHandler from synapse.types import UserID, create_requester +from synapse.util.logcontext import run_in_background import logging @@ -30,6 +31,10 @@ class DeactivateAccountHandler(BaseHandler): self._device_handler = hs.get_device_handler() self._room_member_handler = hs.get_room_member_handler() + self._user_parter_running = False + + reactor.callWhenRunning(self.start_user_parting) + @defer.inlineCallbacks def deactivate_account(self, user_id): """Deactivate a user's account @@ -53,10 +58,38 @@ class DeactivateAccountHandler(BaseHandler): yield self.store.user_delete_threepids(user_id) yield self.store.user_set_password_hash(user_id, None) + yield self.store.add_user_pending_deactivation(user_id) + + self.start_user_parting() + + def start_user_parting(self): + if not self._user_parter_running: + run_in_background(self.user_parter_loop()) + + @defer.inlineCallbacks + def user_parter_loop(self): + self._user_parter_running = True + logger.info("Starting user parter") + try: + while True: + user_id = yield self.store.get_user_pending_deactivation() + if user_id is None: + break + logger.info("User parter parting %r", user_id) + yield self.part_user(user_id) + yield self.store.del_user_pending_deactivation(user_id) + logger.info("User parter finished parting %r", user_id) + logger.info("User parter finished: stopping") + finally: + self._user_parter_running = False + + @defer.inlineCallbacks + def part_user(self, user_id): user = UserID.from_string(user_id) rooms_for_user = yield self.store.get_rooms_for_user(user_id) for room_id in rooms_for_user: + logger.info("User parter parting %r from %r", user_id, room_id) yield self._room_member_handler.update_membership( create_requester(user), user, diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index a50717db2d..de068a55d2 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -526,3 +526,30 @@ class RegistrationStore(RegistrationWorkerStore, except self.database_engine.module.IntegrityError: ret = yield self.get_3pid_guest_access_token(medium, address) defer.returnValue(ret) + + def add_user_pending_deactivation(self, user_id): + return self._simple_insert( + "users_pending_deactivation", + values={ + "user_id": user_id, + }, + desc="add_user_pending_deactivation", + ) + + def del_user_pending_deactivation(self, user_id): + return self._simple_delete_one( + "users_pending_deactivation", + keyvalues={ + "user_id": user_id, + }, + desc="del_user_pending_deactivation", + ) + + def get_user_pending_deactivation(self): + return self._simple_select_one_onecol( + "users_pending_deactivation", + keyvalues={}, + retcol="user_id", + allow_none=True, + desc="get_users_pending_deactivation", + ) From 52281e4c548233087f03a014fdb21783a471b2a7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 9 May 2018 15:06:16 +0100 Subject: [PATCH 3/8] Indent fail --- synapse/handlers/deactivate_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 5bb6c11e8f..f99935b4ee 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -63,7 +63,7 @@ class DeactivateAccountHandler(BaseHandler): self.start_user_parting() def start_user_parting(self): - if not self._user_parter_running: + if not self._user_parter_running: run_in_background(self.user_parter_loop()) @defer.inlineCallbacks From 46df23f581bc52313cb1c9366e967ebcddef07cf Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 9 May 2018 15:07:54 +0100 Subject: [PATCH 4/8] Add the schema file --- .../schema/delta/48/deactivated_users.sql | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 synapse/storage/schema/delta/48/deactivated_users.sql diff --git a/synapse/storage/schema/delta/48/deactivated_users.sql b/synapse/storage/schema/delta/48/deactivated_users.sql new file mode 100644 index 0000000000..e9013a6969 --- /dev/null +++ b/synapse/storage/schema/delta/48/deactivated_users.sql @@ -0,0 +1,25 @@ +/* Copyright 2018 New Vector Ltd + * + * 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. + */ + +/* + * Store any accounts that have been requested to be deactivated. + * We part the account from all the rooms its in when its + * deactivated. This can take some time and synapse may be restarted + * before it completes, so store the user IDs here until the process + * is complete. + */ +CREATE TABLE users_pending_deactivation ( + user_id TEXT NOT NULL +); From 294e9a0c9ba0c4ad69605eb781d9ae56b8982a96 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 9 May 2018 15:10:37 +0100 Subject: [PATCH 5/8] Prefix internal functions --- synapse/handlers/deactivate_account.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index f99935b4ee..81e0a75e78 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -33,7 +33,7 @@ class DeactivateAccountHandler(BaseHandler): self._user_parter_running = False - reactor.callWhenRunning(self.start_user_parting) + reactor.callWhenRunning(self._start_user_parting) @defer.inlineCallbacks def deactivate_account(self, user_id): @@ -60,14 +60,14 @@ class DeactivateAccountHandler(BaseHandler): yield self.store.add_user_pending_deactivation(user_id) - self.start_user_parting() + self._start_user_parting() - def start_user_parting(self): + def _start_user_parting(self): if not self._user_parter_running: - run_in_background(self.user_parter_loop()) + run_in_background(self._user_parter_loop()) @defer.inlineCallbacks - def user_parter_loop(self): + def _user_parter_loop(self): self._user_parter_running = True logger.info("Starting user parter") try: @@ -76,7 +76,7 @@ class DeactivateAccountHandler(BaseHandler): if user_id is None: break logger.info("User parter parting %r", user_id) - yield self.part_user(user_id) + yield self._part_user(user_id) yield self.store.del_user_pending_deactivation(user_id) logger.info("User parter finished parting %r", user_id) logger.info("User parter finished: stopping") @@ -84,7 +84,7 @@ class DeactivateAccountHandler(BaseHandler): self._user_parter_running = False @defer.inlineCallbacks - def part_user(self, user_id): + def _part_user(self, user_id): user = UserID.from_string(user_id) rooms_for_user = yield self.store.get_rooms_for_user(user_id) From 4d298506ddb65f642d899dffa236e4499f0c427b Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 10 May 2018 11:57:13 +0100 Subject: [PATCH 6/8] Oops, don't call function passed to run_in_background --- synapse/handlers/deactivate_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 81e0a75e78..78558dd309 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -64,7 +64,7 @@ class DeactivateAccountHandler(BaseHandler): def _start_user_parting(self): if not self._user_parter_running: - run_in_background(self._user_parter_loop()) + run_in_background(self._user_parter_loop) @defer.inlineCallbacks def _user_parter_loop(self): From 217bc53c981c03ce880d1fa6afad2970be3f1d78 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 10 May 2018 12:20:40 +0100 Subject: [PATCH 7/8] Many docstrings --- synapse/handlers/deactivate_account.py | 24 ++++++++++++++++++++++++ synapse/storage/registration.py | 12 ++++++++++++ 2 files changed, 36 insertions(+) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 78558dd309..c00ec18627 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -31,8 +31,11 @@ class DeactivateAccountHandler(BaseHandler): self._device_handler = hs.get_device_handler() self._room_member_handler = hs.get_room_member_handler() + # Flag that indicates whether the process to part users from rooms is running self._user_parter_running = False + # Start the user parter loop so it can resume parting users from rooms where + # it left off (if it has work left to do). reactor.callWhenRunning(self._start_user_parting) @defer.inlineCallbacks @@ -58,16 +61,32 @@ class DeactivateAccountHandler(BaseHandler): yield self.store.user_delete_threepids(user_id) yield self.store.user_set_password_hash(user_id, None) + # Add the user to a table of users penpding deactivation (ie. + # removal from all the rooms they're a member of) yield self.store.add_user_pending_deactivation(user_id) + # Now start the process that goes through that list and + # parts users from rooms (if it isn't already running) self._start_user_parting() def _start_user_parting(self): + """ + Start the process that goes through the table of users + pending deactivation, if it isn't already running. + + Returns: + None + """ if not self._user_parter_running: run_in_background(self._user_parter_loop) @defer.inlineCallbacks def _user_parter_loop(self): + """Loop that parts deactivated users from rooms + + Returns: + None + """ self._user_parter_running = True logger.info("Starting user parter") try: @@ -85,6 +104,11 @@ class DeactivateAccountHandler(BaseHandler): @defer.inlineCallbacks def _part_user(self, user_id): + """Causes the given user_id to leave all the rooms they're joined to + + Returns: + None + """ user = UserID.from_string(user_id) rooms_for_user = yield self.store.get_rooms_for_user(user_id) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index de068a55d2..c05ce4612f 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -528,6 +528,10 @@ class RegistrationStore(RegistrationWorkerStore, defer.returnValue(ret) def add_user_pending_deactivation(self, user_id): + """ + Adds a user to the table of users who need to be parted from all the rooms they're + in + """ return self._simple_insert( "users_pending_deactivation", values={ @@ -537,6 +541,10 @@ class RegistrationStore(RegistrationWorkerStore, ) def del_user_pending_deactivation(self, user_id): + """ + Removes the given user to the table of users who need to be parted from all the + rooms they're in, effectively marking that user as fully deactivated. + """ return self._simple_delete_one( "users_pending_deactivation", keyvalues={ @@ -546,6 +554,10 @@ class RegistrationStore(RegistrationWorkerStore, ) def get_user_pending_deactivation(self): + """ + Gets one user from the table of users waiting to be parted from all the rooms + they're in. + """ return self._simple_select_one_onecol( "users_pending_deactivation", keyvalues={}, From 6b49628e3bf18f6cc1a1347fef8e4180e854d245 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 10 May 2018 12:23:53 +0100 Subject: [PATCH 8/8] Catch failure to part user from room --- synapse/handlers/deactivate_account.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index c00ec18627..4eb18775e8 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -114,10 +114,16 @@ class DeactivateAccountHandler(BaseHandler): rooms_for_user = yield self.store.get_rooms_for_user(user_id) for room_id in rooms_for_user: logger.info("User parter parting %r from %r", user_id, room_id) - yield self._room_member_handler.update_membership( - create_requester(user), - user, - room_id, - "leave", - ratelimit=False, - ) + try: + yield self._room_member_handler.update_membership( + create_requester(user), + user, + room_id, + "leave", + ratelimit=False, + ) + except Exception: + logger.exception( + "Failed to part user %r from room %r: ignoring and continuing", + user_id, room_id, + )