From 0476852fc6c517bb80fa9008f667d90171fe74e7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 18 Jul 2018 18:05:29 +0100 Subject: Remove deactivated users from profile search --- synapse/handlers/deactivate_account.py | 4 ++++ synapse/handlers/profile.py | 27 +++++++++++++++++++--- synapse/storage/profile.py | 18 ++++++++++++++- .../schema/delta/50/profiles_deactivated_users.sql | 23 ++++++++++++++++++ 4 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 synapse/storage/schema/delta/50/profiles_deactivated_users.sql (limited to 'synapse') diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index a84b7b8b80..88aa394427 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -32,6 +32,7 @@ class DeactivateAccountHandler(BaseHandler): self._device_handler = hs.get_device_handler() self._room_member_handler = hs.get_room_member_handler() self._identity_handler = hs.get_handlers().identity_handler + self._profile_handler = hs.get_profile_handler() self.user_directory_handler = hs.get_user_directory_handler() # Flag that indicates whether the process to part users from rooms is running @@ -86,6 +87,9 @@ class DeactivateAccountHandler(BaseHandler): yield self.store.user_set_password_hash(user_id, None) + user = UserID.from_string(user_id) + yield self._profile_handler.set_active(user, False) + # Add the user to a table of users pending deactivation (ie. # removal from all the rooms they're a member of) yield self.store.add_user_pending_deactivation(user_id) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index a9c76f323e..02240fd720 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd +# 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. @@ -99,13 +100,13 @@ class ProfileHandler(BaseHandler): logger.info("Replicating profile batch %d to %s", batchnum, host) batch_rows = yield self.store.get_profile_batch(batchnum) batch = { - UserID(r["user_id"], self.hs.hostname).to_string(): { + UserID(r["user_id"], self.hs.hostname).to_string(): ({ "display_name": r["displayname"], "avatar_url": r["avatar_url"], - } for r in batch_rows + } if r["active"] else None) for r in batch_rows } - url = "https://%s/_matrix/identity/api/v1/replicate_profiles" % (host,) + url = "http://%s/_matrix/identity/api/v1/replicate_profiles" % (host,) body = { "batchnum": batchnum, "batch": batch, @@ -245,6 +246,26 @@ class ProfileHandler(BaseHandler): # start a profile replication push run_in_background(self._replicate_profiles) + @defer.inlineCallbacks + def set_active(self, target_user, active): + """ + Sets the 'active' flag on a user profile. If set to false, the user account is + considered deactivated. + Note that unlike set_displayname and set_avatar_url, this does *not* perform + authorization checks! + """ + if len(self.hs.config.replicate_user_profiles_to) > 0: + cur_batchnum = yield self.store.get_latest_profile_replication_batch_number() + new_batchnum = 0 if cur_batchnum is None else cur_batchnum + 1 + else: + new_batchnum = None + yield self.store.set_profile_active( + target_user.localpart, active, new_batchnum + ) + + # start a profile replication push + run_in_background(self._replicate_profiles) + @defer.inlineCallbacks def get_avatar_url(self, target_user): if self.hs.is_mine(target_user): diff --git a/synapse/storage/profile.py b/synapse/storage/profile.py index da1db0ebae..d547a90624 100644 --- a/synapse/storage/profile.py +++ b/synapse/storage/profile.py @@ -80,7 +80,7 @@ class ProfileWorkerStore(SQLBaseStore): keyvalues={ "batch": batchnum, }, - retcols=("user_id", "displayname", "avatar_url"), + retcols=("user_id", "displayname", "avatar_url", "active"), desc="get_profile_batch", ) @@ -149,6 +149,22 @@ class ProfileStore(ProfileWorkerStore): lock=False # we can do this because user_id has a unique index ) + def set_profile_active(self, user_localpart, active, batchnum): + values = { + "active": active, + "batch": batchnum, + } + if not active: + values["avatar_url"] = None + values["displayname"] = None + return self._simple_upsert( + table="profiles", + keyvalues={"user_id": user_localpart}, + values=values, + desc="set_profile_active", + lock=False # we can do this because user_id has a unique index + ) + def add_remote_profile_cache(self, user_id, displayname, avatar_url): """Ensure we are caching the remote user's profiles. diff --git a/synapse/storage/schema/delta/50/profiles_deactivated_users.sql b/synapse/storage/schema/delta/50/profiles_deactivated_users.sql new file mode 100644 index 0000000000..6c7c8ab734 --- /dev/null +++ b/synapse/storage/schema/delta/50/profiles_deactivated_users.sql @@ -0,0 +1,23 @@ +/* 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. + */ + +/* + * A flag saying whether the user owning the profile has been deactivated + * This really belongs on the users table, not here, but the users table + * stores users by their full user_id and profiles stores them by localpart, + * so we can't easily join between the two tables. Plus, the batch number + * realy ought to represent data in this table that has changed. + */ +ALTER TABLE profiles ADD COLUMN active BOOLEAN DEFAULT 1 NOT NULL; -- cgit 1.5.1 From dbd0821c43dc85c9ba4d808a348efb70ad2c57f1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 18 Jul 2018 20:50:20 +0100 Subject: Oops, didn't mean to commit that --- synapse/handlers/profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 02240fd720..ec8197f92b 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -106,7 +106,7 @@ class ProfileHandler(BaseHandler): } if r["active"] else None) for r in batch_rows } - url = "http://%s/_matrix/identity/api/v1/replicate_profiles" % (host,) + url = "https://%s/_matrix/identity/api/v1/replicate_profiles" % (host,) body = { "batchnum": batchnum, "batch": batch, -- cgit 1.5.1 From 45d06c754a38b1dc8ed4daab7596c1600641b092 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 18 Jul 2018 20:52:21 +0100 Subject: Add hopefully enlightening comment --- synapse/handlers/profile.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index ec8197f92b..91418fb83e 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -252,7 +252,8 @@ class ProfileHandler(BaseHandler): Sets the 'active' flag on a user profile. If set to false, the user account is considered deactivated. Note that unlike set_displayname and set_avatar_url, this does *not* perform - authorization checks! + authorization checks! This is because the only place it's used currently is + in account deactivation where we've already done these checks anyway. """ if len(self.hs.config.replicate_user_profiles_to) > 0: cur_batchnum = yield self.store.get_latest_profile_replication_batch_number() -- cgit 1.5.1 From 022469d8193596d9b93b049606db8fc40d939f19 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jul 2018 10:28:26 +0100 Subject: Change column def so it works on pgsql & sqlite Now I remember discovering previously there was no way to make boolean columns work --- synapse/storage/schema/delta/50/profiles_deactivated_users.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/storage/schema/delta/50/profiles_deactivated_users.sql b/synapse/storage/schema/delta/50/profiles_deactivated_users.sql index 6c7c8ab734..c8893ecbe8 100644 --- a/synapse/storage/schema/delta/50/profiles_deactivated_users.sql +++ b/synapse/storage/schema/delta/50/profiles_deactivated_users.sql @@ -20,4 +20,4 @@ * so we can't easily join between the two tables. Plus, the batch number * realy ought to represent data in this table that has changed. */ -ALTER TABLE profiles ADD COLUMN active BOOLEAN DEFAULT 1 NOT NULL; +ALTER TABLE profiles ADD COLUMN active SMALLINT DEFAULT 1 NOT NULL; -- cgit 1.5.1 From aa2a4b4b421f92a77eb65ed4798d4a995e98052c Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jul 2018 14:48:24 +0100 Subject: run_on_reactor is dead --- synapse/rest/client/v2_alpha/account.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'synapse') diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index f6ef4ccfec..5844ad4f79 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -318,8 +318,6 @@ class ThreepidRestServlet(RestServlet): @defer.inlineCallbacks def on_POST(self, request): - yield run_on_reactor() - if self.hs.config.disable_3pid_changes: raise SynapseError(400, "3PID changes disabled on this server") -- cgit 1.5.1 From 650761666dd5452f2e3bb760997f1331a86da72e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jul 2018 14:52:35 +0100 Subject: More run_on_reactor --- synapse/rest/client/v2_alpha/account.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'synapse') diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 5844ad4f79..f3d86a2e14 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -373,8 +373,6 @@ class ThreepidDeleteRestServlet(RestServlet): @defer.inlineCallbacks def on_POST(self, request): - yield run_on_reactor() - if self.hs.config.disable_3pid_changes: raise SynapseError(400, "3PID changes disabled on this server") -- cgit 1.5.1 From 0cb5d34756bae925e5bbee0aa5971c5c83b45ee3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jul 2018 15:12:48 +0100 Subject: Hopefully fix postgres --- synapse/storage/profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/storage/profile.py b/synapse/storage/profile.py index d547a90624..485b855ec6 100644 --- a/synapse/storage/profile.py +++ b/synapse/storage/profile.py @@ -151,7 +151,7 @@ class ProfileStore(ProfileWorkerStore): def set_profile_active(self, user_localpart, active, batchnum): values = { - "active": active, + "active": int(active), "batch": batchnum, } if not active: -- cgit 1.5.1