diff --git a/changelog.d/20448.misc b/changelog.d/20448.misc new file mode 100644 index 0000000000..abb6775083 --- /dev/null +++ b/changelog.d/20448.misc @@ -0,0 +1 @@ +Let users who are not members of a server announcement room tag it, and deliver those tags in legacy `/sync`. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 43eb29989e..ac82baad94 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1246,6 +1246,8 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): ) -> None: """ Check if the user has any membership in the room and raise error if not. + This includes the synthetic membership users have in server announcement + rooms. Args: user_id: The user to check. @@ -1254,6 +1256,9 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): Raises: AuthError if the user doesn't have any membership in the room. """ + if room_id in await self.store.get_server_announcement_room_ids(): + return + result = await self.store.get_local_current_membership_for_user_in_room( user_id=user_id, room_id=room_id ) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 9a48c3b28e..74b617cde1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -62,6 +62,7 @@ from synapse.logging.opentracing import ( trace, ) from synapse.metrics import SERVER_NAME_LABEL +from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG from synapse.storage.databases.main.event_push_actions import RoomNotifCounts from synapse.storage.databases.main.receipts import ReceiptInRoom from synapse.storage.databases.main.roommember import extract_heroes_from_room_summary @@ -3000,15 +3001,15 @@ class SyncHandler: # 3. Work out which rooms need reporting in the sync response. ignored_users = await self.store.ignored_users(user_id) if since_token: + tags_by_room = await self.store.get_updated_tags( + user_id, since_token.account_data_key + ) room_changes = await self._get_room_changes_for_incremental_sync( sync_result_builder, ignored_users, - account_data_room_ids=account_data_by_room.keys(), + account_data_room_ids=account_data_by_room.keys() | tags_by_room.keys(), ephemeral_room_ids=frozenset(ephemeral_by_room), ) - tags_by_room = await self.store.get_updated_tags( - user_id, since_token.account_data_key - ) else: room_changes = await self._get_room_changes_for_initial_sync( sync_result_builder, ignored_users @@ -3406,9 +3407,10 @@ class SyncHandler: room_ids: The announcement rooms to consider. left_room_ids: Rooms among `room_ids` that the user left or was removed from since the `since_token` of `sync_result_builder`. - account_data_room_ids: Rooms whose room account data for the user - changed since the `since_token` of `sync_result_builder`. - They may include rooms that are not in `room_ids`. + account_data_room_ids: Rooms whose room account data for the user, + including tags, changed since the `since_token` of + `sync_result_builder`. They may include rooms that are not in + `room_ids`. ephemeral_room_ids: Rooms with new ephemeral events for the user since the `since_token` of `sync_result_builder`. """ @@ -3718,6 +3720,9 @@ class SyncHandler: if not tags: tags = None + if room_id in sync_result_builder.synthetic_join_announcement_room_ids: + tags = {SERVER_NOTICE_ROOM_TAG: {}, **(tags or {})} + account_data_events = [] if tags is not None: account_data_events.append( diff --git a/synapse/rest/client/tags.py b/synapse/rest/client/tags.py index f7272754e6..04b2a75f50 100644 --- a/synapse/rest/client/tags.py +++ b/synapse/rest/client/tags.py @@ -27,6 +27,7 @@ from synapse.api.errors import AuthError, Codes, SynapseError from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.http.site import SynapseRequest +from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG from synapse.types import JsonDict from ._base import client_patterns @@ -82,6 +83,7 @@ class TagServlet(RestServlet): self.auth = hs.get_auth() self.handler = hs.get_account_data_handler() self.room_member_handler = hs.get_room_member_handler() + self.store = hs.get_datastores().main async def on_PUT( self, request: SynapseRequest, user_id: str, room_id: str, tag: str @@ -119,6 +121,15 @@ class TagServlet(RestServlet): if user_id != requester.user.to_string(): raise AuthError(403, "Cannot add tags for other users.") + if ( + tag == SERVER_NOTICE_ROOM_TAG + and room_id in await self.store.get_server_announcement_room_ids() + ): + raise AuthError( + 403, + f"Cannot remove the {SERVER_NOTICE_ROOM_TAG} tag from a server announcement room.", + ) + await self.handler.remove_tag_from_room(user_id, room_id, tag) return 200, {} diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py index 418c2810dd..a73db1d1e6 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -47,6 +47,7 @@ from synapse.rest.client import ( receipts, room, sync, + tags, ) from synapse.server import HomeServer from synapse.types import JsonDict, RoomStreamToken, StreamKeyType, StreamToken @@ -2212,6 +2213,7 @@ class ServerAnnouncementsSyncTestCase(unittest.HomeserverTestCase): room.register_servlets, sync.register_servlets, account_data.register_servlets, + tags.register_servlets, ] def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: @@ -2584,7 +2586,10 @@ class ServerAnnouncementsSyncTestCase(unittest.HomeserverTestCase): self.assertEqual(self._timeline_ids(room), []) self.assertEqual( room["account_data"]["events"], - [{"type": "m.marked_unread", "content": {"unread": True}}], + [ + {"type": "m.tag", "content": {"tags": {"m.server_notice": {}}}}, + {"type": "m.marked_unread", "content": {"unread": True}}, + ], ) def test_marked_unread_with_new_announcement(self) -> None: @@ -2608,9 +2613,166 @@ class ServerAnnouncementsSyncTestCase(unittest.HomeserverTestCase): self.assertEqual(self._timeline_ids(room), [event_id]) self.assertEqual( room["account_data"]["events"], - [{"type": "m.marked_unread", "content": {"unread": True}}], + [ + {"type": "m.tag", "content": {"tags": {"m.server_notice": {}}}}, + {"type": "m.marked_unread", "content": {"unread": True}}, + ], ) + def test_tag_without_new_events(self) -> None: + """A non-member who tags an announcement room gets `m.tag` on their next + incremental sync, even when nothing else happened in the room. + """ + self._announce("hello") + since = self._sync(self.bob_tok)["next_batch"] + + channel = self.make_request( + "PUT", + f"/user/{self.bob}/rooms/{self.room_id}/tags/m.favourite", + {"order": 0.5}, + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + + room = self._assert_delivered(self._sync(self.bob_tok, since), self.bob) + self.assertEqual(self._timeline_ids(room), []) + self.assertEqual( + room["account_data"]["events"], + [ + { + "type": "m.tag", + "content": { + "tags": {"m.favourite": {"order": 0.5}, "m.server_notice": {}} + }, + } + ], + ) + + def test_tag_removed_without_new_events(self) -> None: + """A non-member who removes their last tag from an announcement room gets + `m.tag` with only `m.server_notice` on their next incremental sync, even + when nothing else happened in the room. + """ + self._announce("hello") + channel = self.make_request( + "PUT", + f"/user/{self.bob}/rooms/{self.room_id}/tags/m.favourite", + {"order": 0.5}, + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + since = self._sync(self.bob_tok)["next_batch"] + + channel = self.make_request( + "DELETE", + f"/user/{self.bob}/rooms/{self.room_id}/tags/m.favourite", + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + + room = self._assert_delivered(self._sync(self.bob_tok, since), self.bob) + self.assertEqual(self._timeline_ids(room), []) + self.assertEqual( + room["account_data"]["events"], + [{"type": "m.tag", "content": {"tags": {"m.server_notice": {}}}}], + ) + + def test_server_notice_tag_on_initial_sync(self) -> None: + """A non-member gets the `m.server_notice` tag for an announcement room on + an initial sync, without it being stored for them. + """ + self._announce("hello") + + room = self._assert_delivered(self._sync(self.bob_tok), self.bob) + self.assertEqual( + room["account_data"]["events"], + [{"type": "m.tag", "content": {"tags": {"m.server_notice": {}}}}], + ) + self.assertEqual( + self.get_success(self.store.get_tags_for_room(self.bob, self.room_id)), {} + ) + + def test_server_notice_tag_with_new_announcement(self) -> None: + """A non-member gets the `m.server_notice` tag alongside a new announcement + on an incremental sync. + """ + self._announce("hello") + since = self._sync(self.bob_tok)["next_batch"] + event_id = self._announce("again") + + room = self._assert_delivered(self._sync(self.bob_tok, since), self.bob) + self.assertEqual(self._timeline_ids(room), [event_id]) + self.assertEqual( + room["account_data"]["events"], + [{"type": "m.tag", "content": {"tags": {"m.server_notice": {}}}}], + ) + + def test_server_notice_tag_respects_filter(self) -> None: + """A non-member whose filter hides `m.tag` doesn't get the `m.server_notice` + tag, but still gets the announcement. + """ + event_id = self._announce("hello") + + room = self._assert_delivered( + self._sync( + self.bob_tok, + filter={"room": {"account_data": {"not_types": ["m.tag"]}}}, + ), + self.bob, + ) + self.assertIn(event_id, self._timeline_ids(room)) + self.assertEqual(room["account_data"]["events"], []) + + def test_server_notice_tag_not_sent_to_room_members(self) -> None: + """A sender who is really joined to an announcement room doesn't get the + `m.server_notice` tag. + """ + self._announce("hello") + + response = self._sync(self.announcer_tok) + room = response["rooms"]["join"][self.room_id] + self.assertEqual(room["account_data"]["events"], []) + + def test_server_notice_tag_cannot_be_removed(self) -> None: + """A user can't remove the `m.server_notice` tag from an announcement room, + and still gets it on their next sync. + """ + self._announce("hello") + + channel = self.make_request( + "DELETE", + f"/user/{self.bob}/rooms/{self.room_id}/tags/m.server_notice", + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, 403, channel.json_body) + self.assertEqual(channel.json_body["errcode"], "M_FORBIDDEN") + + room = self._assert_delivered(self._sync(self.bob_tok), self.bob) + self.assertEqual( + room["account_data"]["events"], + [{"type": "m.tag", "content": {"tags": {"m.server_notice": {}}}}], + ) + + def test_server_notice_tag_removable_from_other_rooms(self) -> None: + """A user can still remove the `m.server_notice` tag from a room that isn't + an announcement room. + """ + other_room_id = self.helper.create_room_as(self.bob, tok=self.bob_tok) + channel = self.make_request( + "PUT", + f"/user/{self.bob}/rooms/{other_room_id}/tags/m.server_notice", + {}, + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + + channel = self.make_request( + "DELETE", + f"/user/{self.bob}/rooms/{other_room_id}/tags/m.server_notice", + access_token=self.bob_tok, + ) + self.assertEqual(channel.code, 200, channel.json_body) + def test_state_after(self) -> None: """With `use_state_after`, a non-member gets the synthetic join in `state_after`, including a former member whose real leave is in the