Merge pull request #3557 from matrix-org/dbkr/delete_profiles

Remove deactivated users from profile search

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)

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,10 +100,10 @@ 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,)
@@ -245,6 +246,27 @@ 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! 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()
+            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):

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")
 
@@ -375,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")
 

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": int(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.
 

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 SMALLINT DEFAULT 1 NOT NULL;