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

Commit df54c84

Browse files
reivilibreclokep
andauthored
Remove account data (including client config, push rules and ignored users) upon user deactivation. (#11621)
Co-authored-by: Patrick Cloke <[email protected]>
1 parent 9006ee3 commit df54c84

File tree

5 files changed

+299
-3
lines changed

5 files changed

+299
-3
lines changed

changelog.d/11621.feature

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Remove account data (including client config, push rules and ignored users) upon user deactivation.

docs/admin_api/user_admin_api.md

+5-1
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,11 @@ The following actions are performed when deactivating an user:
353353
- Remove the user from the user directory
354354
- Reject all pending invites
355355
- Remove all account validity information related to the user
356+
- Remove the arbitrary data store known as *account data*. For example, this includes:
357+
- list of ignored users;
358+
- push rules;
359+
- secret storage keys; and
360+
- cross-signing keys.
356361

357362
The following additional actions are performed during deactivation if `erase`
358363
is set to `true`:
@@ -366,7 +371,6 @@ The following actions are **NOT** performed. The list may be incomplete.
366371
- Remove mappings of SSO IDs
367372
- [Delete media uploaded](#delete-media-uploaded-by-a-user) by user (included avatar images)
368373
- Delete sent and received messages
369-
- Delete E2E cross-signing keys
370374
- Remove the user's creation (registration) timestamp
371375
- [Remove rate limit overrides](#override-ratelimiting-for-users)
372376
- Remove from monthly active users

synapse/handlers/deactivate_account.py

+3
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ async def deactivate_account(
157157
# Mark the user as deactivated.
158158
await self.store.set_user_deactivated_status(user_id, True)
159159

160+
# Remove account data (including ignored users and push rules).
161+
await self.store.purge_account_data_for_user(user_id)
162+
160163
return identity_server_supports_unbinding
161164

162165
async def _reject_pending_invites_for_user(self, user_id: str) -> None:

synapse/storage/databases/main/account_data.py

+71-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
LoggingTransaction,
2727
)
2828
from synapse.storage.databases.main.cache import CacheInvalidationWorkerStore
29+
from synapse.storage.databases.main.push_rule import PushRulesWorkerStore
2930
from synapse.storage.engines import PostgresEngine
3031
from synapse.storage.util.id_generators import (
3132
AbstractStreamIdGenerator,
@@ -44,7 +45,7 @@
4445
logger = logging.getLogger(__name__)
4546

4647

47-
class AccountDataWorkerStore(CacheInvalidationWorkerStore):
48+
class AccountDataWorkerStore(PushRulesWorkerStore, CacheInvalidationWorkerStore):
4849
def __init__(
4950
self,
5051
database: DatabasePool,
@@ -179,7 +180,7 @@ async def get_global_account_data_by_type_for_user(
179180
else:
180181
return None
181182

182-
@cached(num_args=2)
183+
@cached(num_args=2, tree=True)
183184
async def get_account_data_for_room(
184185
self, user_id: str, room_id: str
185186
) -> Dict[str, JsonDict]:
@@ -546,6 +547,74 @@ def _add_account_data_for_user(
546547
for ignored_user_id in previously_ignored_users ^ currently_ignored_users:
547548
self._invalidate_cache_and_stream(txn, self.ignored_by, (ignored_user_id,))
548549

550+
async def purge_account_data_for_user(self, user_id: str) -> None:
551+
"""
552+
Removes the account data for a user.
553+
554+
This is intended to be used upon user deactivation and also removes any
555+
derived information from account data (e.g. push rules and ignored users).
556+
557+
Args:
558+
user_id: The user ID to remove data for.
559+
"""
560+
561+
def purge_account_data_for_user_txn(txn: LoggingTransaction) -> None:
562+
# Purge from the primary account_data tables.
563+
self.db_pool.simple_delete_txn(
564+
txn, table="account_data", keyvalues={"user_id": user_id}
565+
)
566+
567+
self.db_pool.simple_delete_txn(
568+
txn, table="room_account_data", keyvalues={"user_id": user_id}
569+
)
570+
571+
# Purge from ignored_users where this user is the ignorer.
572+
# N.B. We don't purge where this user is the ignoree, because that
573+
# interferes with other users' account data.
574+
# It's also not this user's data to delete!
575+
self.db_pool.simple_delete_txn(
576+
txn, table="ignored_users", keyvalues={"ignorer_user_id": user_id}
577+
)
578+
579+
# Remove the push rules
580+
self.db_pool.simple_delete_txn(
581+
txn, table="push_rules", keyvalues={"user_name": user_id}
582+
)
583+
self.db_pool.simple_delete_txn(
584+
txn, table="push_rules_enable", keyvalues={"user_name": user_id}
585+
)
586+
self.db_pool.simple_delete_txn(
587+
txn, table="push_rules_stream", keyvalues={"user_id": user_id}
588+
)
589+
590+
# Invalidate caches as appropriate
591+
self._invalidate_cache_and_stream(
592+
txn, self.get_account_data_for_room_and_type, (user_id,)
593+
)
594+
self._invalidate_cache_and_stream(
595+
txn, self.get_account_data_for_user, (user_id,)
596+
)
597+
self._invalidate_cache_and_stream(
598+
txn, self.get_global_account_data_by_type_for_user, (user_id,)
599+
)
600+
self._invalidate_cache_and_stream(
601+
txn, self.get_account_data_for_room, (user_id,)
602+
)
603+
self._invalidate_cache_and_stream(
604+
txn, self.get_push_rules_for_user, (user_id,)
605+
)
606+
self._invalidate_cache_and_stream(
607+
txn, self.get_push_rules_enabled_for_user, (user_id,)
608+
)
609+
# This user might be contained in the ignored_by cache for other users,
610+
# so we have to invalidate it all.
611+
self._invalidate_all_cache_and_stream(txn, self.ignored_by)
612+
613+
await self.db_pool.runInteraction(
614+
"purge_account_data_for_user_txn",
615+
purge_account_data_for_user_txn,
616+
)
617+
549618

550619
class AccountDataStore(AccountDataWorkerStore):
551620
pass
+219
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
# Copyright 2021 The Matrix.org Foundation C.I.C.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
from http import HTTPStatus
15+
from typing import Any, Dict
16+
17+
from twisted.test.proto_helpers import MemoryReactor
18+
19+
from synapse.api.constants import AccountDataTypes
20+
from synapse.push.rulekinds import PRIORITY_CLASS_MAP
21+
from synapse.rest import admin
22+
from synapse.rest.client import account, login
23+
from synapse.server import HomeServer
24+
from synapse.util import Clock
25+
26+
from tests.unittest import HomeserverTestCase
27+
28+
29+
class DeactivateAccountTestCase(HomeserverTestCase):
30+
servlets = [
31+
login.register_servlets,
32+
admin.register_servlets,
33+
account.register_servlets,
34+
]
35+
36+
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
37+
self._store = hs.get_datastore()
38+
39+
self.user = self.register_user("user", "pass")
40+
self.token = self.login("user", "pass")
41+
42+
def _deactivate_my_account(self):
43+
"""
44+
Deactivates the account `self.user` using `self.token` and asserts
45+
that it returns a 200 success code.
46+
"""
47+
req = self.get_success(
48+
self.make_request(
49+
"POST",
50+
"account/deactivate",
51+
{
52+
"auth": {
53+
"type": "m.login.password",
54+
"user": self.user,
55+
"password": "pass",
56+
},
57+
"erase": True,
58+
},
59+
access_token=self.token,
60+
)
61+
)
62+
self.assertEqual(req.code, HTTPStatus.OK, req)
63+
64+
def test_global_account_data_deleted_upon_deactivation(self) -> None:
65+
"""
66+
Tests that global account data is removed upon deactivation.
67+
"""
68+
# Add some account data
69+
self.get_success(
70+
self._store.add_account_data_for_user(
71+
self.user,
72+
AccountDataTypes.DIRECT,
73+
{"@someone:remote": ["!somewhere:remote"]},
74+
)
75+
)
76+
77+
# Check that we actually added some.
78+
self.assertIsNotNone(
79+
self.get_success(
80+
self._store.get_global_account_data_by_type_for_user(
81+
self.user, AccountDataTypes.DIRECT
82+
)
83+
),
84+
)
85+
86+
# Request the deactivation of our account
87+
self._deactivate_my_account()
88+
89+
# Check that the account data does not persist.
90+
self.assertIsNone(
91+
self.get_success(
92+
self._store.get_global_account_data_by_type_for_user(
93+
self.user, AccountDataTypes.DIRECT
94+
)
95+
),
96+
)
97+
98+
def test_room_account_data_deleted_upon_deactivation(self) -> None:
99+
"""
100+
Tests that room account data is removed upon deactivation.
101+
"""
102+
room_id = "!room:test"
103+
104+
# Add some room account data
105+
self.get_success(
106+
self._store.add_account_data_to_room(
107+
self.user,
108+
room_id,
109+
"m.fully_read",
110+
{"event_id": "$aaaa:test"},
111+
)
112+
)
113+
114+
# Check that we actually added some.
115+
self.assertIsNotNone(
116+
self.get_success(
117+
self._store.get_account_data_for_room_and_type(
118+
self.user, room_id, "m.fully_read"
119+
)
120+
),
121+
)
122+
123+
# Request the deactivation of our account
124+
self._deactivate_my_account()
125+
126+
# Check that the account data does not persist.
127+
self.assertIsNone(
128+
self.get_success(
129+
self._store.get_account_data_for_room_and_type(
130+
self.user, room_id, "m.fully_read"
131+
)
132+
),
133+
)
134+
135+
def _is_custom_rule(self, push_rule: Dict[str, Any]) -> bool:
136+
"""
137+
Default rules start with a dot: such as .m.rule and .im.vector.
138+
This function returns true iff a rule is custom (not default).
139+
"""
140+
return "/." not in push_rule["rule_id"]
141+
142+
def test_push_rules_deleted_upon_account_deactivation(self) -> None:
143+
"""
144+
Push rules are a special case of account data.
145+
They are stored separately but get sent to the client as account data in /sync.
146+
This tests that deactivating a user deletes push rules along with the rest
147+
of their account data.
148+
"""
149+
150+
# Add a push rule
151+
self.get_success(
152+
self._store.add_push_rule(
153+
self.user,
154+
"personal.override.rule1",
155+
PRIORITY_CLASS_MAP["override"],
156+
[],
157+
[],
158+
)
159+
)
160+
161+
# Test the rule exists
162+
push_rules = self.get_success(self._store.get_push_rules_for_user(self.user))
163+
# Filter out default rules; we don't care
164+
push_rules = list(filter(self._is_custom_rule, push_rules))
165+
# Check our rule made it
166+
self.assertEqual(
167+
push_rules,
168+
[
169+
{
170+
"user_name": "@user:test",
171+
"rule_id": "personal.override.rule1",
172+
"priority_class": 5,
173+
"priority": 0,
174+
"conditions": [],
175+
"actions": [],
176+
"default": False,
177+
}
178+
],
179+
push_rules,
180+
)
181+
182+
# Request the deactivation of our account
183+
self._deactivate_my_account()
184+
185+
push_rules = self.get_success(self._store.get_push_rules_for_user(self.user))
186+
# Filter out default rules; we don't care
187+
push_rules = list(filter(self._is_custom_rule, push_rules))
188+
# Check our rule no longer exists
189+
self.assertEqual(push_rules, [], push_rules)
190+
191+
def test_ignored_users_deleted_upon_deactivation(self) -> None:
192+
"""
193+
Ignored users are a special case of account data.
194+
They get denormalised into the `ignored_users` table upon being stored as
195+
account data.
196+
Test that a user's list of ignored users is deleted upon deactivation.
197+
"""
198+
199+
# Add an ignored user
200+
self.get_success(
201+
self._store.add_account_data_for_user(
202+
self.user,
203+
AccountDataTypes.IGNORED_USER_LIST,
204+
{"ignored_users": {"@sheltie:test": {}}},
205+
)
206+
)
207+
208+
# Test the user is ignored
209+
self.assertEqual(
210+
self.get_success(self._store.ignored_by("@sheltie:test")), {self.user}
211+
)
212+
213+
# Request the deactivation of our account
214+
self._deactivate_my_account()
215+
216+
# Test the user is no longer ignored by the user that was deactivated
217+
self.assertEqual(
218+
self.get_success(self._store.ignored_by("@sheltie:test")), set()
219+
)

0 commit comments

Comments
 (0)