Merge branch 'release-v0.25.0' of github.com:matrix-org/synapse v0.25.0

2 files changed, 196 insertions, 75 deletions

diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py
index b751cf5e43..1fb709e6c3 100644
--- a/synapse/groups/attestations.py
+++ b/synapse/groups/attestations.py
@@ -13,6 +13,31 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""Attestations ensure that users and groups can't lie about their memberships.
+
+When a user joins a group the HS and GS swap attestations, which allow them
+both to independently prove to third parties their membership.These
+attestations have a validity period so need to be periodically renewed.
+
+If a user leaves (or gets kicked out of) a group, either side can still use
+their attestation to "prove" their membership, until the attestation expires.
+Therefore attestations shouldn't be relied on to prove membership in important
+cases, but can for less important situtations, e.g. showing a users membership
+of groups on their profile, showing flairs, etc.abs
+
+An attestsation is a signed blob of json that looks like:
+
+    {
+        "user_id": "@foo:a.example.com",
+        "group_id": "+bar:b.example.com",
+        "valid_until_ms": 1507994728530,
+        "signatures":{"matrix.org":{"ed25519:auto":"..."}}
+    }
+"""
+
+import logging
+import random
+
 from twisted.internet import defer
 
 from synapse.api.errors import SynapseError
@@ -22,9 +47,17 @@ from synapse.util.logcontext import preserve_fn
 from signedjson.sign import sign_json
 
 
+logger = logging.getLogger(__name__)
+
+
 # Default validity duration for new attestations we create
 DEFAULT_ATTESTATION_LENGTH_MS = 3 * 24 * 60 * 60 * 1000
 
+# We add some jitter to the validity duration of attestations so that if we
+# add lots of users at once we don't need to renew them all at once.
+# The jitter is a multiplier picked randomly between the first and second number
+DEFAULT_ATTESTATION_JITTER = (0.9, 1.3)
+
 # Start trying to update our attestations when they come this close to expiring
 UPDATE_ATTESTATION_TIME_MS = 1 * 24 * 60 * 60 * 1000
 
@@ -73,10 +106,14 @@ class GroupAttestationSigning(object):
         """Create an attestation for the group_id and user_id with default
         validity length.
         """
+        validity_period = DEFAULT_ATTESTATION_LENGTH_MS
+        validity_period *= random.uniform(*DEFAULT_ATTESTATION_JITTER)
+        valid_until_ms = int(self.clock.time_msec() + validity_period)
+
         return sign_json({
             "group_id": group_id,
             "user_id": user_id,
-            "valid_until_ms": self.clock.time_msec() + DEFAULT_ATTESTATION_LENGTH_MS,
+            "valid_until_ms": valid_until_ms,
         }, self.server_name, self.signing_key)
 
 
@@ -128,12 +165,19 @@ class GroupAttestionRenewer(object):
 
         @defer.inlineCallbacks
         def _renew_attestation(group_id, user_id):
-            attestation = self.attestations.create_attestation(group_id, user_id)
-
-            if self.is_mine_id(group_id):
+            if not self.is_mine_id(group_id):
+                destination = get_domain_from_id(group_id)
+            elif not self.is_mine_id(user_id):
                 destination = get_domain_from_id(user_id)
             else:
-                destination = get_domain_from_id(group_id)
+                logger.warn(
+                    "Incorrectly trying to do attestations for user: %r in %r",
+                    user_id, group_id,
+                )
+                yield self.store.remove_attestation_renewal(group_id, user_id)
+                return
+
+            attestation = self.attestations.create_attestation(group_id, user_id)
 
             yield self.transport_client.renew_group_attestation(
                 destination, group_id, user_id,
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 23beb3187e..0b995aed70 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -49,7 +49,8 @@ class GroupsServerHandler(object):
         hs.get_groups_attestation_renewer()
 
     @defer.inlineCallbacks
-    def check_group_is_ours(self, group_id, and_exists=False, and_is_admin=None):
+    def check_group_is_ours(self, group_id, requester_user_id,
+                            and_exists=False, and_is_admin=None):
         """Check that the group is ours, and optionally if it exists.
 
         If group does exist then return group.
@@ -67,6 +68,10 @@ class GroupsServerHandler(object):
         if and_exists and not group:
             raise SynapseError(404, "Unknown group")
 
+        is_user_in_group = yield self.store.is_user_in_group(requester_user_id, group_id)
+        if group and not is_user_in_group and not group["is_public"]:
+            raise SynapseError(404, "Unknown group")
+
         if and_is_admin:
             is_admin = yield self.store.is_user_admin_in_group(group_id, and_is_admin)
             if not is_admin:
@@ -84,7 +89,7 @@ class GroupsServerHandler(object):
 
         A user/room may appear in multiple roles/categories.
         """
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         is_user_in_group = yield self.store.is_user_in_group(requester_user_id, group_id)
 
@@ -153,10 +158,16 @@ class GroupsServerHandler(object):
         })
 
     @defer.inlineCallbacks
-    def update_group_summary_room(self, group_id, user_id, room_id, category_id, content):
+    def update_group_summary_room(self, group_id, requester_user_id,
+                                  room_id, category_id, content):
         """Add/update a room to the group summary
         """
-        yield self.check_group_is_ours(group_id, and_exists=True, and_is_admin=user_id)
+        yield self.check_group_is_ours(
+            group_id,
+            requester_user_id,
+            and_exists=True,
+            and_is_admin=requester_user_id,
+        )
 
         RoomID.from_string(room_id)  # Ensure valid room id
 
@@ -175,10 +186,16 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def delete_group_summary_room(self, group_id, user_id, room_id, category_id):
+    def delete_group_summary_room(self, group_id, requester_user_id,
+                                  room_id, category_id):
         """Remove a room from the summary
         """
-        yield self.check_group_is_ours(group_id, and_exists=True, and_is_admin=user_id)
+        yield self.check_group_is_ours(
+            group_id,
+            requester_user_id,
+            and_exists=True,
+            and_is_admin=requester_user_id,
+        )
 
         yield self.store.remove_room_from_summary(
             group_id=group_id,
@@ -189,10 +206,10 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def get_group_categories(self, group_id, user_id):
+    def get_group_categories(self, group_id, requester_user_id):
         """Get all categories in a group (as seen by user)
         """
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         categories = yield self.store.get_group_categories(
             group_id=group_id,
@@ -200,10 +217,10 @@ class GroupsServerHandler(object):
         defer.returnValue({"categories": categories})
 
     @defer.inlineCallbacks
-    def get_group_category(self, group_id, user_id, category_id):
+    def get_group_category(self, group_id, requester_user_id, category_id):
         """Get a specific category in a group (as seen by user)
         """
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         res = yield self.store.get_group_category(
             group_id=group_id,
@@ -213,10 +230,15 @@ class GroupsServerHandler(object):
         defer.returnValue(res)
 
     @defer.inlineCallbacks
-    def update_group_category(self, group_id, user_id, category_id, content):
+    def update_group_category(self, group_id, requester_user_id, category_id, content):
         """Add/Update a group category
         """
-        yield self.check_group_is_ours(group_id, and_exists=True, and_is_admin=user_id)
+        yield self.check_group_is_ours(
+            group_id,
+            requester_user_id,
+            and_exists=True,
+            and_is_admin=requester_user_id,
+        )
 
         is_public = _parse_visibility_from_contents(content)
         profile = content.get("profile")
@@ -231,10 +253,15 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def delete_group_category(self, group_id, user_id, category_id):
+    def delete_group_category(self, group_id, requester_user_id, category_id):
         """Delete a group category
         """
-        yield self.check_group_is_ours(group_id, and_exists=True, and_is_admin=user_id)
+        yield self.check_group_is_ours(
+            group_id,
+            requester_user_id,
+            and_exists=True,
+            and_is_admin=requester_user_id
+        )
 
         yield self.store.remove_group_category(
             group_id=group_id,
@@ -244,10 +271,10 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def get_group_roles(self, group_id, user_id):
+    def get_group_roles(self, group_id, requester_user_id):
         """Get all roles in a group (as seen by user)
         """
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         roles = yield self.store.get_group_roles(
             group_id=group_id,
@@ -255,10 +282,10 @@ class GroupsServerHandler(object):
         defer.returnValue({"roles": roles})
 
     @defer.inlineCallbacks
-    def get_group_role(self, group_id, user_id, role_id):
+    def get_group_role(self, group_id, requester_user_id, role_id):
         """Get a specific role in a group (as seen by user)
         """
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         res = yield self.store.get_group_role(
             group_id=group_id,
@@ -267,10 +294,15 @@ class GroupsServerHandler(object):
         defer.returnValue(res)
 
     @defer.inlineCallbacks
-    def update_group_role(self, group_id, user_id, role_id, content):
+    def update_group_role(self, group_id, requester_user_id, role_id, content):
         """Add/update a role in a group
         """
-        yield self.check_group_is_ours(group_id, and_exists=True, and_is_admin=user_id)
+        yield self.check_group_is_ours(
+            group_id,
+            requester_user_id,
+            and_exists=True,
+            and_is_admin=requester_user_id,
+        )
 
         is_public = _parse_visibility_from_contents(content)
 
@@ -286,10 +318,15 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def delete_group_role(self, group_id, user_id, role_id):
+    def delete_group_role(self, group_id, requester_user_id, role_id):
         """Remove role from group
         """
-        yield self.check_group_is_ours(group_id, and_exists=True, and_is_admin=user_id)
+        yield self.check_group_is_ours(
+            group_id,
+            requester_user_id,
+            and_exists=True,
+            and_is_admin=requester_user_id,
+        )
 
         yield self.store.remove_group_role(
             group_id=group_id,
@@ -304,7 +341,7 @@ class GroupsServerHandler(object):
         """Add/update a users entry in the group summary
         """
         yield self.check_group_is_ours(
-            group_id, and_exists=True, and_is_admin=requester_user_id,
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id,
         )
 
         order = content.get("order", None)
@@ -326,7 +363,7 @@ class GroupsServerHandler(object):
         """Remove a user from the group summary
         """
         yield self.check_group_is_ours(
-            group_id, and_exists=True, and_is_admin=requester_user_id,
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id,
         )
 
         yield self.store.remove_user_from_summary(
@@ -342,7 +379,7 @@ class GroupsServerHandler(object):
         """Get the group profile as seen by requester_user_id
         """
 
-        yield self.check_group_is_ours(group_id)
+        yield self.check_group_is_ours(group_id, requester_user_id)
 
         group_description = yield self.store.get_group(group_id)
 
@@ -356,7 +393,7 @@ class GroupsServerHandler(object):
         """Update the group profile
         """
         yield self.check_group_is_ours(
-            group_id, and_exists=True, and_is_admin=requester_user_id,
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id,
         )
 
         profile = {}
@@ -377,7 +414,7 @@ class GroupsServerHandler(object):
         The ordering is arbitrary at the moment
         """
 
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         is_user_in_group = yield self.store.is_user_in_group(requester_user_id, group_id)
 
@@ -389,14 +426,15 @@ class GroupsServerHandler(object):
         for user_result in user_results:
             g_user_id = user_result["user_id"]
             is_public = user_result["is_public"]
+            is_privileged = user_result["is_admin"]
 
             entry = {"user_id": g_user_id}
 
             profile = yield self.profile_handler.get_profile_from_cache(g_user_id)
             entry.update(profile)
 
-            if not is_public:
-                entry["is_public"] = False
+            entry["is_public"] = bool(is_public)
+            entry["is_privileged"] = bool(is_privileged)
 
             if not self.is_mine_id(g_user_id):
                 attestation = yield self.store.get_remote_attestation(group_id, g_user_id)
@@ -425,7 +463,7 @@ class GroupsServerHandler(object):
         The ordering is arbitrary at the moment
         """
 
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         is_user_in_group = yield self.store.is_user_in_group(requester_user_id, group_id)
 
@@ -459,7 +497,7 @@ class GroupsServerHandler(object):
         This returns rooms in order of decreasing number of joined users
         """
 
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         is_user_in_group = yield self.store.is_user_in_group(requester_user_id, group_id)
 
@@ -470,7 +508,6 @@ class GroupsServerHandler(object):
         chunk = []
         for room_result in room_results:
             room_id = room_result["room_id"]
-            is_public = room_result["is_public"]
 
             joined_users = yield self.store.get_users_in_room(room_id)
             entry = yield self.room_list_handler.generate_room_entry(
@@ -481,8 +518,7 @@ class GroupsServerHandler(object):
             if not entry:
                 continue
 
-            if not is_public:
-                entry["is_public"] = False
+            entry["is_public"] = bool(room_result["is_public"])
 
             chunk.append(entry)
 
@@ -500,7 +536,7 @@ class GroupsServerHandler(object):
         RoomID.from_string(room_id)  # Ensure valid room id
 
         yield self.check_group_is_ours(
-            group_id, and_exists=True, and_is_admin=requester_user_id
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
         )
 
         is_public = _parse_visibility_from_contents(content)
@@ -510,11 +546,34 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
+    def update_room_in_group(self, group_id, requester_user_id, room_id, config_key,
+                             content):
+        """Update room in group
+        """
+        RoomID.from_string(room_id)  # Ensure valid room id
+
+        yield self.check_group_is_ours(
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
+        )
+
+        if config_key == "m.visibility":
+            is_public = _parse_visibility_dict(content)
+
+            yield self.store.update_room_in_group_visibility(
+                group_id, room_id,
+                is_public=is_public,
+            )
+        else:
+            raise SynapseError(400, "Uknown config option")
+
+        defer.returnValue({})
+
+    @defer.inlineCallbacks
     def remove_room_from_group(self, group_id, requester_user_id, room_id):
         """Remove room from group
         """
         yield self.check_group_is_ours(
-            group_id, and_exists=True, and_is_admin=requester_user_id
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
         )
 
         yield self.store.remove_room_from_group(group_id, room_id)
@@ -527,7 +586,7 @@ class GroupsServerHandler(object):
         """
 
         group = yield self.check_group_is_ours(
-            group_id, and_exists=True, and_is_admin=requester_user_id
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
         )
 
         # TODO: Check if user knocked
@@ -596,35 +655,40 @@ class GroupsServerHandler(object):
             raise SynapseError(502, "Unknown state returned by HS")
 
     @defer.inlineCallbacks
-    def accept_invite(self, group_id, user_id, content):
+    def accept_invite(self, group_id, requester_user_id, content):
         """User tries to accept an invite to the group.
 
         This is different from them asking to join, and so should error if no
         invite exists (and they're not a member of the group)
         """
 
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
-        if not self.store.is_user_invited_to_local_group(group_id, user_id):
+        is_invited = yield self.store.is_user_invited_to_local_group(
+            group_id, requester_user_id,
+        )
+        if not is_invited:
             raise SynapseError(403, "User not invited to group")
 
-        if not self.hs.is_mine_id(user_id):
+        if not self.hs.is_mine_id(requester_user_id):
+            local_attestation = self.attestations.create_attestation(
+                group_id, requester_user_id,
+            )
             remote_attestation = content["attestation"]
 
             yield self.attestations.verify_attestation(
                 remote_attestation,
-                user_id=user_id,
+                user_id=requester_user_id,
                 group_id=group_id,
             )
         else:
+            local_attestation = None
             remote_attestation = None
 
-        local_attestation = self.attestations.create_attestation(group_id, user_id)
-
         is_public = _parse_visibility_from_contents(content)
 
         yield self.store.add_user_to_group(
-            group_id, user_id,
+            group_id, requester_user_id,
             is_admin=False,
             is_public=is_public,
             local_attestation=local_attestation,
@@ -637,31 +701,31 @@ class GroupsServerHandler(object):
         })
 
     @defer.inlineCallbacks
-    def knock(self, group_id, user_id, content):
+    def knock(self, group_id, requester_user_id, content):
         """A user requests becoming a member of the group
         """
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         raise NotImplementedError()
 
     @defer.inlineCallbacks
-    def accept_knock(self, group_id, user_id, content):
+    def accept_knock(self, group_id, requester_user_id, content):
         """Accept a users knock to the room.
 
         Errors if the user hasn't knocked, rather than inviting them.
         """
 
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         raise NotImplementedError()
 
     @defer.inlineCallbacks
     def remove_user_from_group(self, group_id, user_id, requester_user_id, content):
-        """Remove a user from the group; either a user is leaving or and admin
-        kicked htem.
+        """Remove a user from the group; either a user is leaving or an admin
+        kicked them.
         """
 
-        yield self.check_group_is_ours(group_id, and_exists=True)
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
 
         is_kick = False
         if requester_user_id != user_id:
@@ -692,8 +756,8 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def create_group(self, group_id, user_id, content):
-        group = yield self.check_group_is_ours(group_id)
+    def create_group(self, group_id, requester_user_id, content):
+        group = yield self.check_group_is_ours(group_id, requester_user_id)
 
         logger.info("Attempting to create group with ID: %r", group_id)
 
@@ -703,11 +767,11 @@ class GroupsServerHandler(object):
         if group:
             raise SynapseError(400, "Group already exists")
 
-        is_admin = yield self.auth.is_server_admin(UserID.from_string(user_id))
+        is_admin = yield self.auth.is_server_admin(UserID.from_string(requester_user_id))
         if not is_admin:
             if not self.hs.config.enable_group_creation:
                 raise SynapseError(
-                    403, "Only server admin can create group on this server",
+                    403, "Only a server admin can create groups on this server",
                 )
             localpart = group_id_obj.localpart
             if not localpart.startswith(self.hs.config.group_creation_prefix):
@@ -727,38 +791,41 @@ class GroupsServerHandler(object):
 
         yield self.store.create_group(
             group_id,
-            user_id,
+            requester_user_id,
             name=name,
             avatar_url=avatar_url,
             short_description=short_description,
             long_description=long_description,
         )
 
-        if not self.hs.is_mine_id(user_id):
+        if not self.hs.is_mine_id(requester_user_id):
             remote_attestation = content["attestation"]
 
             yield self.attestations.verify_attestation(
                 remote_attestation,
-                user_id=user_id,
+                user_id=requester_user_id,
                 group_id=group_id,
             )
 
-            local_attestation = self.attestations.create_attestation(group_id, user_id)
+            local_attestation = self.attestations.create_attestation(
+                group_id,
+                requester_user_id,
+            )
         else:
             local_attestation = None
             remote_attestation = None
 
         yield self.store.add_user_to_group(
-            group_id, user_id,
+            group_id, requester_user_id,
             is_admin=True,
             is_public=True,  # TODO
             local_attestation=local_attestation,
             remote_attestation=remote_attestation,
         )
 
-        if not self.hs.is_mine_id(user_id):
+        if not self.hs.is_mine_id(requester_user_id):
             yield self.store.add_remote_profile_cache(
-                user_id,
+                requester_user_id,
                 displayname=user_profile.get("displayname"),
                 avatar_url=user_profile.get("avatar_url"),
             )
@@ -773,15 +840,25 @@ def _parse_visibility_from_contents(content):
     public or not
     """
 
-    visibility = content.get("visibility")
+    visibility = content.get("m.visibility")
     if visibility:
-        vis_type = visibility["type"]
-        if vis_type not in ("public", "private"):
-            raise SynapseError(
-                400, "Synapse only supports 'public'/'private' visibility"
-            )
-        is_public = vis_type == "public"
+        return _parse_visibility_dict(visibility)
     else:
         is_public = True
 
     return is_public
+
+
+def _parse_visibility_dict(visibility):
+    """Given a dict for the "m.visibility" config return if the entity should
+    be public or not
+    """
+    vis_type = visibility.get("type")
+    if not vis_type:
+        return True
+
+    if vis_type not in ("public", "private"):
+        raise SynapseError(
+            400, "Synapse only supports 'public'/'private' visibility"
+        )
+    return vis_type == "public"