Merge #20448: Server Announcement Room Tags

# Conflicts:
#	synapse/handlers/sync.py

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`.

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
         )

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(

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, {}

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