From 79452edeee94a09a826ee2b41a08811b823a3ad6 Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Wed, 28 Mar 2018 14:03:37 +0100
Subject: Add joinability for groups

Adds API to set the 'joinable' flag, and corresponding flag in the
table.
---
 synapse/federation/transport/client.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 5488e82985..46a797b4ba 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.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.
@@ -856,6 +857,22 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
+    @log_function
+    def set_group_joinable(self, destination, group_id, requester_user_id,
+                                  content):
+        """Sets whether a group is joinable without an invite or knock
+        """
+        path = PREFIX + "/groups/%s/joinable" % (group_id,)
+
+        return self.client.post_json(
+            destination=destination,
+            path=path,
+            args={"requester_user_id": requester_user_id},
+            data=content,
+            ignore_backoff=True,
+        )
+
+
     @log_function
     def delete_group_summary_user(self, destination, group_id, requester_user_id,
                                   user_id, role_id):
-- 
cgit 1.4.1


From 32260baa410e1ae8200f636861a57bf2039e2cf0 Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Wed, 28 Mar 2018 14:29:42 +0100
Subject: pep8

---
 synapse/federation/transport/client.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 46a797b4ba..5a6b63350b 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -859,7 +859,7 @@ class TransportLayerClient(object):
 
     @log_function
     def set_group_joinable(self, destination, group_id, requester_user_id,
-                                  content):
+                           content):
         """Sets whether a group is joinable without an invite or knock
         """
         path = PREFIX + "/groups/%s/joinable" % (group_id,)
@@ -872,7 +872,6 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
-
     @log_function
     def delete_group_summary_user(self, destination, group_id, requester_user_id,
                                   user_id, role_id):
-- 
cgit 1.4.1


From eb8d8d6f57c7f6017548aa95409bb8cc346a5ae0 Mon Sep 17 00:00:00 2001
From: Luke Barnard <luke@matrix.org>
Date: Tue, 3 Apr 2018 15:40:43 +0100
Subject: Use join_policy API instead of joinable

The API is now under
 /groups/$group_id/setting/m.join_policy

and expects a JSON blob of the shape

```json
{
  "m.join_policy": {
    "type": "invite"
  }
}
```

where "invite" could alternatively be "open".
---
 synapse/federation/transport/client.py             |  4 +--
 synapse/federation/transport/server.py             |  8 ++---
 synapse/groups/groups_server.py                    | 41 ++++++++++++++++++----
 synapse/handlers/groups_local.py                   |  2 +-
 synapse/rest/client/v2_alpha/groups.py             | 12 +++----
 synapse/storage/group_server.py                    |  6 ++--
 .../storage/schema/delta/48/groups_joinable.sql    |  8 ++++-
 7 files changed, 58 insertions(+), 23 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 5a6b63350b..0f7f656824 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -860,9 +860,9 @@ class TransportLayerClient(object):
     @log_function
     def set_group_joinable(self, destination, group_id, requester_user_id,
                            content):
-        """Sets whether a group is joinable without an invite or knock
+        """Sets the join policy for a group
         """
-        path = PREFIX + "/groups/%s/joinable" % (group_id,)
+        path = PREFIX + "/groups/%s/setting/m.join_policy" % (group_id,)
 
         return self.client.post_json(
             destination=destination,
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 107deb4e1e..a52d3948f4 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -1125,10 +1125,10 @@ class FederationGroupsBulkPublicisedServlet(BaseFederationServlet):
         defer.returnValue((200, resp))
 
 
-class FederationGroupsJoinableServlet(BaseFederationServlet):
+class FederationGroupsSettingJoinPolicyServlet(BaseFederationServlet):
     """Sets whether a group is joinable without an invite or knock
     """
-    PATH = "/groups/(?P<group_id>[^/]*)/joinable$"
+    PATH = "/groups/(?P<group_id>[^/]*)/setting/m.join_policy$"
 
     @defer.inlineCallbacks
     def on_POST(self, origin, content, query, group_id):
@@ -1136,7 +1136,7 @@ class FederationGroupsJoinableServlet(BaseFederationServlet):
         if get_domain_from_id(requester_user_id) != origin:
             raise SynapseError(403, "requester_user_id doesn't match origin")
 
-        new_content = yield self.handler.set_group_joinable(
+        new_content = yield self.handler.set_group_join_policy(
             group_id, requester_user_id, content
         )
 
@@ -1191,7 +1191,7 @@ GROUP_SERVER_SERVLET_CLASSES = (
     FederationGroupsSummaryUsersServlet,
     FederationGroupsAddRoomsServlet,
     FederationGroupsAddRoomsConfigServlet,
-    FederationGroupsJoinableServlet,
+    FederationGroupsSettingJoinPolicyServlet,
 )
 
 
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 25cbfb1691..70781e1854 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -207,20 +207,24 @@ class GroupsServerHandler(object):
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def set_group_joinable(self, group_id, requester_user_id, content):
-        """Sets whether a group is joinable without an invite or knock
+    def set_group_join_policy(self, group_id, requester_user_id, content):
+        """Sets the group join policy.
+
+        Currently supported policies are:
+         - "invite": an invite must be received and accepted in order to join.
+         - "open": anyone can join.
         """
         yield self.check_group_is_ours(
             group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
         )
 
-        is_joinable = content.get('joinable')
-        if is_joinable is None:
+        join_policy = _parse_join_policy_from_contents(content)
+        if join_policy is None:
             raise SynapseError(
-                400, "No value specified for 'joinable'"
+                400, "No value specified for 'm.join_policy'"
             )
 
-        yield self.store.set_group_joinable(group_id, is_joinable=is_joinable)
+        yield self.store.set_group_join_policy(group_id, join_policy=join_policy)
 
         defer.returnValue({})
 
@@ -854,6 +858,31 @@ class GroupsServerHandler(object):
         })
 
 
+def _parse_join_policy_from_contents(content):
+    """Given a content for a request, return the specified join policy or None
+    """
+
+    join_policy_dict = content.get("m.join_policy")
+    if join_policy_dict:
+        return _parse_join_policy_dict(join_policy_dict)
+    else:
+        return None
+
+
+def _parse_join_policy_dict(join_policy_dict):
+    """Given a dict for the "m.join_policy" config return the join policy specified
+    """
+    join_policy_type = join_policy_dict.get("type")
+    if not join_policy_type:
+        return True
+
+    if join_policy_type not in ("invite", "open"):
+        raise SynapseError(
+            400, "Synapse only supports 'invite'/'open' join rule"
+        )
+    return join_policy_type
+
+
 def _parse_visibility_from_contents(content):
     """Given a content for a request parse out whether the entity should be
     public or not
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index c9671b9046..5f7b0ff305 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -91,7 +91,7 @@ class GroupsLocalHandler(object):
     get_group_role = _create_rerouter("get_group_role")
     get_group_roles = _create_rerouter("get_group_roles")
 
-    set_group_joinable = _create_rerouter("set_group_joinable")
+    set_group_join_policy = _create_rerouter("set_group_join_policy")
 
     @defer.inlineCallbacks
     def get_group_summary(self, group_id, requester_user_id):
diff --git a/synapse/rest/client/v2_alpha/groups.py b/synapse/rest/client/v2_alpha/groups.py
index aa94130e57..8faaa1d6a0 100644
--- a/synapse/rest/client/v2_alpha/groups.py
+++ b/synapse/rest/client/v2_alpha/groups.py
@@ -402,13 +402,13 @@ class GroupInvitedUsersServlet(RestServlet):
         defer.returnValue((200, result))
 
 
-class GroupJoinableServlet(RestServlet):
-    """Set whether a group is joinable without an invite
+class GroupSettingJoinPolicyServlet(RestServlet):
+    """Set group join policy
     """
-    PATTERNS = client_v2_patterns("/groups/(?P<group_id>[^/]*)/joinable$")
+    PATTERNS = client_v2_patterns("/groups/(?P<group_id>[^/]*)/setting/m.join_policy$")
 
     def __init__(self, hs):
-        super(GroupJoinableServlet, self).__init__()
+        super(GroupSettingJoinPolicyServlet, self).__init__()
         self.auth = hs.get_auth()
         self.groups_handler = hs.get_groups_local_handler()
 
@@ -419,7 +419,7 @@ class GroupJoinableServlet(RestServlet):
 
         content = parse_json_object_from_request(request)
 
-        result = yield self.groups_handler.set_group_joinable(
+        result = yield self.groups_handler.set_group_join_policy(
             group_id,
             requester_user_id,
             content,
@@ -765,7 +765,7 @@ def register_servlets(hs, http_server):
     GroupInvitedUsersServlet(hs).register(http_server)
     GroupUsersServlet(hs).register(http_server)
     GroupRoomServlet(hs).register(http_server)
-    GroupJoinableServlet(hs).register(http_server)
+    GroupSettingJoinPolicyServlet(hs).register(http_server)
     GroupCreateServlet(hs).register(http_server)
     GroupAdminRoomsServlet(hs).register(http_server)
     GroupAdminRoomsConfigServlet(hs).register(http_server)
diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py
index 96553d4fb1..db66ea1eb0 100644
--- a/synapse/storage/group_server.py
+++ b/synapse/storage/group_server.py
@@ -30,16 +30,16 @@ _DEFAULT_ROLE_ID = ""
 
 
 class GroupServerStore(SQLBaseStore):
-    def set_group_joinable(self, group_id, is_joinable):
+    def set_group_join_policy(self, group_id, join_policy):
         return self._simple_update_one(
             table="groups",
             keyvalues={
                 "group_id": group_id,
             },
             updatevalues={
-                "is_joinable": is_joinable,
+                "join_policy": join_policy,
             },
-            desc="set_group_joinable",
+            desc="set_group_join_policy",
         )
 
     def get_group(self, group_id):
diff --git a/synapse/storage/schema/delta/48/groups_joinable.sql b/synapse/storage/schema/delta/48/groups_joinable.sql
index ace7d0a723..ab3b00286d 100644
--- a/synapse/storage/schema/delta/48/groups_joinable.sql
+++ b/synapse/storage/schema/delta/48/groups_joinable.sql
@@ -13,4 +13,10 @@
  * limitations under the License.
  */
 
-ALTER TABLE groups ADD COLUMN is_joinable SMALLINT DEFAULT 0 NOT NULL;
+/* 
+ * This isn't a real ENUM because sqlite doesn't support it
+ * and we use a default of NULL for inserted rows and interpret
+ * NULL at the python store level as necessary so that existing
+ * rows are given the correct default policy.
+ */
+ALTER TABLE groups ADD COLUMN join_policy TEXT DEFAULT NULL;
-- 
cgit 1.4.1


From 104c0bc1d5d1f2a487c50d63b22caa477b091976 Mon Sep 17 00:00:00 2001
From: Luke Barnard <luke@matrix.org>
Date: Thu, 5 Apr 2018 14:07:16 +0100
Subject: Use "/settings/" (plural)

---
 synapse/federation/transport/client.py | 2 +-
 synapse/federation/transport/server.py | 2 +-
 synapse/rest/client/v2_alpha/groups.py | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 0f7f656824..1fe162d55b 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -862,7 +862,7 @@ class TransportLayerClient(object):
                            content):
         """Sets the join policy for a group
         """
-        path = PREFIX + "/groups/%s/setting/m.join_policy" % (group_id,)
+        path = PREFIX + "/groups/%s/settings/m.join_policy" % (group_id,)
 
         return self.client.post_json(
             destination=destination,
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index a52d3948f4..3658ca75f3 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -1128,7 +1128,7 @@ class FederationGroupsBulkPublicisedServlet(BaseFederationServlet):
 class FederationGroupsSettingJoinPolicyServlet(BaseFederationServlet):
     """Sets whether a group is joinable without an invite or knock
     """
-    PATH = "/groups/(?P<group_id>[^/]*)/setting/m.join_policy$"
+    PATH = "/groups/(?P<group_id>[^/]*)/settings/m.join_policy$"
 
     @defer.inlineCallbacks
     def on_POST(self, origin, content, query, group_id):
diff --git a/synapse/rest/client/v2_alpha/groups.py b/synapse/rest/client/v2_alpha/groups.py
index 8faaa1d6a0..3bb1ec2af6 100644
--- a/synapse/rest/client/v2_alpha/groups.py
+++ b/synapse/rest/client/v2_alpha/groups.py
@@ -405,7 +405,7 @@ class GroupInvitedUsersServlet(RestServlet):
 class GroupSettingJoinPolicyServlet(RestServlet):
     """Set group join policy
     """
-    PATTERNS = client_v2_patterns("/groups/(?P<group_id>[^/]*)/setting/m.join_policy$")
+    PATTERNS = client_v2_patterns("/groups/(?P<group_id>[^/]*)/settings/m.join_policy$")
 
     def __init__(self, hs):
         super(GroupSettingJoinPolicyServlet, self).__init__()
-- 
cgit 1.4.1


From 1d71f484d4ec00fd41e3ef195622d0d5dba6d372 Mon Sep 17 00:00:00 2001
From: Krombel <krombel@krombel.de>
Date: Fri, 6 Apr 2018 12:54:09 +0200
Subject: use PUT instead of POST for federating groups/m.join_policy

---
 synapse/federation/transport/client.py | 2 +-
 synapse/federation/transport/server.py | 2 +-
 synapse/http/matrixfederationclient.py | 6 +++++-
 3 files changed, 7 insertions(+), 3 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 1fe162d55b..3beab47832 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -864,7 +864,7 @@ class TransportLayerClient(object):
         """
         path = PREFIX + "/groups/%s/settings/m.join_policy" % (group_id,)
 
-        return self.client.post_json(
+        return self.client.put_json(
             destination=destination,
             path=path,
             args={"requester_user_id": requester_user_id},
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 3658ca75f3..b98e30459c 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -1131,7 +1131,7 @@ class FederationGroupsSettingJoinPolicyServlet(BaseFederationServlet):
     PATH = "/groups/(?P<group_id>[^/]*)/settings/m.join_policy$"
 
     @defer.inlineCallbacks
-    def on_POST(self, origin, content, query, group_id):
+    def on_PUT(self, origin, content, query, group_id):
         requester_user_id = parse_string_from_args(query, "requester_user_id")
         if get_domain_from_id(requester_user_id) != origin:
             raise SynapseError(403, "requester_user_id doesn't match origin")
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 9145405cb0..60a29081e8 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -286,7 +286,8 @@ class MatrixFederationHttpClient(object):
         headers_dict[b"Authorization"] = auth_headers
 
     @defer.inlineCallbacks
-    def put_json(self, destination, path, data={}, json_data_callback=None,
+    def put_json(self, destination, path, args={}, data={},
+                 json_data_callback=None,
                  long_retries=False, timeout=None,
                  ignore_backoff=False,
                  backoff_on_404=False):
@@ -296,6 +297,7 @@ class MatrixFederationHttpClient(object):
             destination (str): The remote server to send the HTTP request
                 to.
             path (str): The HTTP path.
+            args (dict): query params
             data (dict): A dict containing the data that will be used as
                 the request body. This will be encoded as JSON.
             json_data_callback (callable): A callable returning the dict to
@@ -342,6 +344,7 @@ class MatrixFederationHttpClient(object):
             path,
             body_callback=body_callback,
             headers_dict={"Content-Type": ["application/json"]},
+            query_bytes=encode_query_args(args),
             long_retries=long_retries,
             timeout=timeout,
             ignore_backoff=ignore_backoff,
@@ -373,6 +376,7 @@ class MatrixFederationHttpClient(object):
                 giving up. None indicates no timeout.
             ignore_backoff (bool): true to ignore the historical backoff data and
                 try the request anyway.
+            args (dict): query params
         Returns:
             Deferred: Succeeds when we get a 2xx HTTP response. The result
             will be the decoded JSON body.
-- 
cgit 1.4.1


From b370fe61c0aeccac7745ad404dad925ec217ba6d Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Wed, 28 Mar 2018 17:18:02 +0100
Subject: Implement group join API

---
 synapse/federation/transport/client.py | 13 ++++++++++
 synapse/federation/transport/server.py | 18 ++++++++++++++
 synapse/groups/groups_server.py        | 45 ++++++++++++++++++++++++++++++++++
 synapse/handlers/groups_local.py       | 40 +++++++++++++++++++++++++++++-
 synapse/storage/group_server.py        | 12 ++++++---
 5 files changed, 124 insertions(+), 4 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 3beab47832..370f7ba78b 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -614,6 +614,19 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
+    @log_function
+    def join_group(self, destination, group_id, user_id, content):
+        """Attempts to join a group
+        """
+        path = PREFIX + "/groups/%s/users/%s/join" % (group_id, user_id)
+
+        return self.client.post_json(
+            destination=destination,
+            path=path,
+            data=content,
+            ignore_backoff=True,
+        )
+
     @log_function
     def invite_to_group(self, destination, group_id, user_id, requester_user_id, content):
         """Invite a user to a group
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index b98e30459c..4c94d5a36c 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -803,6 +803,23 @@ class FederationGroupsAcceptInviteServlet(BaseFederationServlet):
         defer.returnValue((200, new_content))
 
 
+class FederationGroupsJoinServlet(BaseFederationServlet):
+    """Attempt to join a group
+    """
+    PATH = "/groups/(?P<group_id>[^/]*)/users/(?P<user_id>[^/]*)/join$"
+
+    @defer.inlineCallbacks
+    def on_POST(self, origin, content, query, group_id, user_id):
+        if get_domain_from_id(user_id) != origin:
+            raise SynapseError(403, "user_id doesn't match origin")
+
+        new_content = yield self.handler.join_group(
+            group_id, user_id, content,
+        )
+
+        defer.returnValue((200, new_content))
+
+
 class FederationGroupsRemoveUserServlet(BaseFederationServlet):
     """Leave or kick a user from the group
     """
@@ -1182,6 +1199,7 @@ GROUP_SERVER_SERVLET_CLASSES = (
     FederationGroupsInvitedUsersServlet,
     FederationGroupsInviteServlet,
     FederationGroupsAcceptInviteServlet,
+    FederationGroupsJoinServlet,
     FederationGroupsRemoveUserServlet,
     FederationGroupsSummaryRoomsServlet,
     FederationGroupsCategoriesServlet,
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 70781e1854..77b6273e39 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -723,6 +723,51 @@ class GroupsServerHandler(object):
             "attestation": local_attestation,
         })
 
+    @defer.inlineCallbacks
+    def join_group(self, group_id, requester_user_id, content):
+        """User tries to join the group.
+
+        This will error if the group requires an invite/knock to join
+        """
+
+        yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
+
+        group_info = yield self.store.get_group(
+            group_id,
+        )
+        if not group_info['is_joinable']:
+            raise SynapseError(403, "Group is not publicly joinable")
+
+        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=requester_user_id,
+                group_id=group_id,
+            )
+        else:
+            local_attestation = None
+            remote_attestation = None
+
+        is_public = _parse_visibility_from_contents(content)
+
+        yield self.store.add_user_to_group(
+            group_id, requester_user_id,
+            is_admin=False,
+            is_public=is_public,
+            local_attestation=local_attestation,
+            remote_attestation=remote_attestation,
+        )
+
+        defer.returnValue({
+            "state": "join",
+            "attestation": local_attestation,
+        })
+
     @defer.inlineCallbacks
     def knock(self, group_id, requester_user_id, content):
         """A user requests becoming a member of the group
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index 5f7b0ff305..977993e7d4 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -229,7 +229,45 @@ class GroupsLocalHandler(object):
     def join_group(self, group_id, user_id, content):
         """Request to join a group
         """
-        raise NotImplementedError()  # TODO
+        if self.is_mine_id(group_id):
+            yield self.groups_server_handler.join_group(
+                group_id, user_id, content
+            )
+            local_attestation = None
+            remote_attestation = None
+        else:
+            local_attestation = self.attestations.create_attestation(group_id, user_id)
+            content["attestation"] = local_attestation
+
+            res = yield self.transport_client.join_group(
+                get_domain_from_id(group_id), group_id, user_id, content,
+            )
+
+            remote_attestation = res["attestation"]
+
+            yield self.attestations.verify_attestation(
+                remote_attestation,
+                group_id=group_id,
+                user_id=user_id,
+                server_name=get_domain_from_id(group_id),
+            )
+
+        # TODO: Check that the group is public and we're being added publically
+        is_publicised = content.get("publicise", False)
+
+        token = yield self.store.register_user_group_membership(
+            group_id, user_id,
+            membership="join",
+            is_admin=False,
+            local_attestation=local_attestation,
+            remote_attestation=remote_attestation,
+            is_publicised=is_publicised,
+        )
+        self.notifier.on_new_event(
+            "groups_key", token, users=[user_id],
+        )
+
+        defer.returnValue({})
 
     @defer.inlineCallbacks
     def accept_invite(self, group_id, user_id, content):
diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py
index db316a27ec..16c11a056f 100644
--- a/synapse/storage/group_server.py
+++ b/synapse/storage/group_server.py
@@ -48,19 +48,25 @@ class GroupServerStore(SQLBaseStore):
             desc="set_group_join_policy",
         )
 
+    @defer.inlineCallbacks
     def get_group(self, group_id):
-        return self._simple_select_one(
+        ret = yield self._simple_select_one(
             table="groups",
             keyvalues={
                 "group_id": group_id,
             },
             retcols=(
-                "name", "short_description", "long_description", "avatar_url", "is_public"
+                "name", "short_description", "long_description", "avatar_url", "is_public", "is_joinable",
             ),
             allow_none=True,
-            desc="is_user_in_group",
+            desc="get_group",
         )
 
+        if ret and 'is_joinable' in ret:
+            ret['is_joinable'] = bool(ret['is_joinable'])
+
+        defer.returnValue(ret)
+
     def get_users_in_group(self, group_id, include_private=False):
         # TODO: Pagination
 
-- 
cgit 1.4.1


From f8d1917fce3b08b613f76131d6a82a7576379251 Mon Sep 17 00:00:00 2001
From: Luke Barnard <luke@matrix.org>
Date: Thu, 5 Apr 2018 16:31:24 +0100
Subject: Fix federation client `set_group_joinable` typo

---
 synapse/federation/transport/client.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 370f7ba78b..67ff7da51b 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -871,7 +871,7 @@ class TransportLayerClient(object):
         )
 
     @log_function
-    def set_group_joinable(self, destination, group_id, requester_user_id,
+    def set_group_join_policy(self, destination, group_id, requester_user_id,
                            content):
         """Sets the join policy for a group
         """
-- 
cgit 1.4.1


From 112c2253e2846b48b75f4171d9dd94bfe6ecc8a1 Mon Sep 17 00:00:00 2001
From: Luke Barnard <luke@matrix.org>
Date: Thu, 5 Apr 2018 17:32:20 +0100
Subject: pep8

---
 synapse/federation/transport/client.py | 2 +-
 synapse/groups/groups_server.py        | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 67ff7da51b..50a967a7ec 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -872,7 +872,7 @@ class TransportLayerClient(object):
 
     @log_function
     def set_group_join_policy(self, destination, group_id, requester_user_id,
-                           content):
+                              content):
         """Sets the join policy for a group
         """
         path = PREFIX + "/groups/%s/settings/m.join_policy" % (group_id,)
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 1b516011d8..15d31549de 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -741,7 +741,9 @@ class GroupsServerHandler(object):
         This will error if the group requires an invite/knock to join
         """
 
-        group_info = yield self.check_group_is_ours(group_id, requester_user_id, and_exists=True)
+        group_info = yield self.check_group_is_ours(
+            group_id, requester_user_id, and_exists=True
+        )
         if group_info['join_policy'] != "open":
             raise SynapseError(403, "Group is not publicly joinable")
 
-- 
cgit 1.4.1


From dab87b84a330467abc037489ac2034b94ddadb63 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 10 Apr 2018 11:16:08 +0100
Subject: URL quote path segments over federation

---
 synapse/federation/transport/client.py | 128 ++++++++++++++++++++-------------
 1 file changed, 80 insertions(+), 48 deletions(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 50a967a7ec..1976e5d2fb 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -21,6 +21,7 @@ from synapse.api.urls import FEDERATION_PREFIX as PREFIX
 from synapse.util.logutils import log_function
 
 import logging
+import urllib
 
 
 logger = logging.getLogger(__name__)
@@ -50,7 +51,7 @@ class TransportLayerClient(object):
         logger.debug("get_room_state dest=%s, room=%s",
                      destination, room_id)
 
-        path = PREFIX + "/state/%s/" % room_id
+        path = _create_path(PREFIX, "/state/%s/", room_id)
         return self.client.get_json(
             destination, path=path, args={"event_id": event_id},
         )
@@ -72,7 +73,7 @@ class TransportLayerClient(object):
         logger.debug("get_room_state_ids dest=%s, room=%s",
                      destination, room_id)
 
-        path = PREFIX + "/state_ids/%s/" % room_id
+        path = _create_path(PREFIX, "/state_ids/%s/", room_id)
         return self.client.get_json(
             destination, path=path, args={"event_id": event_id},
         )
@@ -94,7 +95,7 @@ class TransportLayerClient(object):
         logger.debug("get_pdu dest=%s, event_id=%s",
                      destination, event_id)
 
-        path = PREFIX + "/event/%s/" % (event_id, )
+        path = _create_path(PREFIX, "/event/%s/", event_id)
         return self.client.get_json(destination, path=path, timeout=timeout)
 
     @log_function
@@ -120,7 +121,7 @@ class TransportLayerClient(object):
             # TODO: raise?
             return
 
-        path = PREFIX + "/backfill/%s/" % (room_id,)
+        path = _create_path(PREFIX, "/backfill/%s/", room_id)
 
         args = {
             "v": event_tuples,
@@ -158,9 +159,11 @@ class TransportLayerClient(object):
         # generated by the json_data_callback.
         json_data = transaction.get_dict()
 
+        path = _create_path(PREFIX, "/send/%s/", transaction.transaction_id)
+
         response = yield self.client.put_json(
             transaction.destination,
-            path=PREFIX + "/send/%s/" % transaction.transaction_id,
+            path=path,
             data=json_data,
             json_data_callback=json_data_callback,
             long_retries=True,
@@ -178,7 +181,7 @@ class TransportLayerClient(object):
     @log_function
     def make_query(self, destination, query_type, args, retry_on_dns_fail,
                    ignore_backoff=False):
-        path = PREFIX + "/query/%s" % query_type
+        path = _create_path(PREFIX, "/query/%s", query_type)
 
         content = yield self.client.get_json(
             destination=destination,
@@ -223,7 +226,7 @@ class TransportLayerClient(object):
                 "make_membership_event called with membership='%s', must be one of %s" %
                 (membership, ",".join(valid_memberships))
             )
-        path = PREFIX + "/make_%s/%s/%s" % (membership, room_id, user_id)
+        path = _create_path(PREFIX, "/make_%s/%s/%s", membership, room_id, user_id)
 
         ignore_backoff = False
         retry_on_dns_fail = False
@@ -249,7 +252,7 @@ class TransportLayerClient(object):
     @defer.inlineCallbacks
     @log_function
     def send_join(self, destination, room_id, event_id, content):
-        path = PREFIX + "/send_join/%s/%s" % (room_id, event_id)
+        path = _create_path(PREFIX, "/send_join/%s/%s", room_id, event_id)
 
         response = yield self.client.put_json(
             destination=destination,
@@ -262,7 +265,7 @@ class TransportLayerClient(object):
     @defer.inlineCallbacks
     @log_function
     def send_leave(self, destination, room_id, event_id, content):
-        path = PREFIX + "/send_leave/%s/%s" % (room_id, event_id)
+        path = _create_path(PREFIX, "/send_leave/%s/%s", room_id, event_id)
 
         response = yield self.client.put_json(
             destination=destination,
@@ -281,7 +284,7 @@ class TransportLayerClient(object):
     @defer.inlineCallbacks
     @log_function
     def send_invite(self, destination, room_id, event_id, content):
-        path = PREFIX + "/invite/%s/%s" % (room_id, event_id)
+        path = _create_path(PREFIX, "/invite/%s/%s", room_id, event_id)
 
         response = yield self.client.put_json(
             destination=destination,
@@ -323,7 +326,7 @@ class TransportLayerClient(object):
     @defer.inlineCallbacks
     @log_function
     def exchange_third_party_invite(self, destination, room_id, event_dict):
-        path = PREFIX + "/exchange_third_party_invite/%s" % (room_id,)
+        path = _create_path(PREFIX, "/exchange_third_party_invite/%s", room_id,)
 
         response = yield self.client.put_json(
             destination=destination,
@@ -336,7 +339,7 @@ class TransportLayerClient(object):
     @defer.inlineCallbacks
     @log_function
     def get_event_auth(self, destination, room_id, event_id):
-        path = PREFIX + "/event_auth/%s/%s" % (room_id, event_id)
+        path = _create_path(PREFIX, "/event_auth/%s/%s", room_id, event_id)
 
         content = yield self.client.get_json(
             destination=destination,
@@ -348,7 +351,7 @@ class TransportLayerClient(object):
     @defer.inlineCallbacks
     @log_function
     def send_query_auth(self, destination, room_id, event_id, content):
-        path = PREFIX + "/query_auth/%s/%s" % (room_id, event_id)
+        path = _create_path(PREFIX, "/query_auth/%s/%s", room_id, event_id)
 
         content = yield self.client.post_json(
             destination=destination,
@@ -410,7 +413,7 @@ class TransportLayerClient(object):
         Returns:
             A dict containg the device keys.
         """
-        path = PREFIX + "/user/devices/" + user_id
+        path = _create_path(PREFIX, "/user/devices/%s", user_id)
 
         content = yield self.client.get_json(
             destination=destination,
@@ -460,7 +463,7 @@ class TransportLayerClient(object):
     @log_function
     def get_missing_events(self, destination, room_id, earliest_events,
                            latest_events, limit, min_depth, timeout):
-        path = PREFIX + "/get_missing_events/%s" % (room_id,)
+        path = _create_path(PREFIX, "/get_missing_events/%s", room_id,)
 
         content = yield self.client.post_json(
             destination=destination,
@@ -480,7 +483,7 @@ class TransportLayerClient(object):
     def get_group_profile(self, destination, group_id, requester_user_id):
         """Get a group profile
         """
-        path = PREFIX + "/groups/%s/profile" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/profile", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -499,7 +502,7 @@ class TransportLayerClient(object):
             requester_user_id (str)
             content (dict): The new profile of the group
         """
-        path = PREFIX + "/groups/%s/profile" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/profile", group_id,)
 
         return self.client.post_json(
             destination=destination,
@@ -513,7 +516,7 @@ class TransportLayerClient(object):
     def get_group_summary(self, destination, group_id, requester_user_id):
         """Get a group summary
         """
-        path = PREFIX + "/groups/%s/summary" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/summary", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -526,7 +529,7 @@ class TransportLayerClient(object):
     def get_rooms_in_group(self, destination, group_id, requester_user_id):
         """Get all rooms in a group
         """
-        path = PREFIX + "/groups/%s/rooms" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/rooms", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -539,7 +542,7 @@ class TransportLayerClient(object):
                           content):
         """Add a room to a group
         """
-        path = PREFIX + "/groups/%s/room/%s" % (group_id, room_id,)
+        path = _create_path(PREFIX, "/groups/%s/room/%s", group_id, room_id,)
 
         return self.client.post_json(
             destination=destination,
@@ -553,7 +556,10 @@ class TransportLayerClient(object):
                              config_key, content):
         """Update room in group
         """
-        path = PREFIX + "/groups/%s/room/%s/config/%s" % (group_id, room_id, config_key,)
+        path = _create_path(
+            PREFIX, "/groups/%s/room/%s/config/%s",
+            group_id, room_id, config_key,
+        )
 
         return self.client.post_json(
             destination=destination,
@@ -566,7 +572,7 @@ class TransportLayerClient(object):
     def remove_room_from_group(self, destination, group_id, requester_user_id, room_id):
         """Remove a room from a group
         """
-        path = PREFIX + "/groups/%s/room/%s" % (group_id, room_id,)
+        path = _create_path(PREFIX, "/groups/%s/room/%s", group_id, room_id,)
 
         return self.client.delete_json(
             destination=destination,
@@ -579,7 +585,7 @@ class TransportLayerClient(object):
     def get_users_in_group(self, destination, group_id, requester_user_id):
         """Get users in a group
         """
-        path = PREFIX + "/groups/%s/users" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/users", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -592,7 +598,7 @@ class TransportLayerClient(object):
     def get_invited_users_in_group(self, destination, group_id, requester_user_id):
         """Get users that have been invited to a group
         """
-        path = PREFIX + "/groups/%s/invited_users" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/invited_users", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -605,7 +611,10 @@ class TransportLayerClient(object):
     def accept_group_invite(self, destination, group_id, user_id, content):
         """Accept a group invite
         """
-        path = PREFIX + "/groups/%s/users/%s/accept_invite" % (group_id, user_id)
+        path = _create_path(
+            PREFIX, "/groups/%s/users/%s/accept_invite",
+            group_id, user_id,
+        )
 
         return self.client.post_json(
             destination=destination,
@@ -618,7 +627,7 @@ class TransportLayerClient(object):
     def join_group(self, destination, group_id, user_id, content):
         """Attempts to join a group
         """
-        path = PREFIX + "/groups/%s/users/%s/join" % (group_id, user_id)
+        path = _create_path(PREFIX, "/groups/%s/users/%s/join", group_id, user_id)
 
         return self.client.post_json(
             destination=destination,
@@ -631,7 +640,7 @@ class TransportLayerClient(object):
     def invite_to_group(self, destination, group_id, user_id, requester_user_id, content):
         """Invite a user to a group
         """
-        path = PREFIX + "/groups/%s/users/%s/invite" % (group_id, user_id)
+        path = _create_path(PREFIX, "/groups/%s/users/%s/invite", group_id, user_id)
 
         return self.client.post_json(
             destination=destination,
@@ -647,7 +656,7 @@ class TransportLayerClient(object):
         invited.
         """
 
-        path = PREFIX + "/groups/local/%s/users/%s/invite" % (group_id, user_id)
+        path = _create_path(PREFIX, "/groups/local/%s/users/%s/invite", group_id, user_id)
 
         return self.client.post_json(
             destination=destination,
@@ -661,7 +670,7 @@ class TransportLayerClient(object):
                                user_id, content):
         """Remove a user fron a group
         """
-        path = PREFIX + "/groups/%s/users/%s/remove" % (group_id, user_id)
+        path = _create_path(PREFIX, "/groups/%s/users/%s/remove", group_id, user_id)
 
         return self.client.post_json(
             destination=destination,
@@ -678,7 +687,7 @@ class TransportLayerClient(object):
         kicked from the group.
         """
 
-        path = PREFIX + "/groups/local/%s/users/%s/remove" % (group_id, user_id)
+        path = _create_path(PREFIX, "/groups/local/%s/users/%s/remove", group_id, user_id)
 
         return self.client.post_json(
             destination=destination,
@@ -693,7 +702,7 @@ class TransportLayerClient(object):
         the attestations
         """
 
-        path = PREFIX + "/groups/%s/renew_attestation/%s" % (group_id, user_id)
+        path = _create_path(PREFIX, "/groups/%s/renew_attestation/%s", group_id, user_id)
 
         return self.client.post_json(
             destination=destination,
@@ -708,11 +717,12 @@ class TransportLayerClient(object):
         """Update a room entry in a group summary
         """
         if category_id:
-            path = PREFIX + "/groups/%s/summary/categories/%s/rooms/%s" % (
+            path = _create_path(
+                PREFIX, "/groups/%s/summary/categories/%s/rooms/%s",
                 group_id, category_id, room_id,
             )
         else:
-            path = PREFIX + "/groups/%s/summary/rooms/%s" % (group_id, room_id,)
+            path = _create_path(PREFIX, "/groups/%s/summary/rooms/%s", group_id, room_id,)
 
         return self.client.post_json(
             destination=destination,
@@ -728,11 +738,12 @@ class TransportLayerClient(object):
         """Delete a room entry in a group summary
         """
         if category_id:
-            path = PREFIX + "/groups/%s/summary/categories/%s/rooms/%s" % (
+            path = _create_path(
+                PREFIX + "/groups/%s/summary/categories/%s/rooms/%s",
                 group_id, category_id, room_id,
             )
         else:
-            path = PREFIX + "/groups/%s/summary/rooms/%s" % (group_id, room_id,)
+            path = _create_path(PREFIX, "/groups/%s/summary/rooms/%s", group_id, room_id,)
 
         return self.client.delete_json(
             destination=destination,
@@ -745,7 +756,7 @@ class TransportLayerClient(object):
     def get_group_categories(self, destination, group_id, requester_user_id):
         """Get all categories in a group
         """
-        path = PREFIX + "/groups/%s/categories" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/categories", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -758,7 +769,7 @@ class TransportLayerClient(object):
     def get_group_category(self, destination, group_id, requester_user_id, category_id):
         """Get category info in a group
         """
-        path = PREFIX + "/groups/%s/categories/%s" % (group_id, category_id,)
+        path = _create_path(PREFIX, "/groups/%s/categories/%s", group_id, category_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -772,7 +783,7 @@ class TransportLayerClient(object):
                               content):
         """Update a category in a group
         """
-        path = PREFIX + "/groups/%s/categories/%s" % (group_id, category_id,)
+        path = _create_path(PREFIX, "/groups/%s/categories/%s", group_id, category_id,)
 
         return self.client.post_json(
             destination=destination,
@@ -787,7 +798,7 @@ class TransportLayerClient(object):
                               category_id):
         """Delete a category in a group
         """
-        path = PREFIX + "/groups/%s/categories/%s" % (group_id, category_id,)
+        path = _create_path(PREFIX, "/groups/%s/categories/%s", group_id, category_id,)
 
         return self.client.delete_json(
             destination=destination,
@@ -800,7 +811,7 @@ class TransportLayerClient(object):
     def get_group_roles(self, destination, group_id, requester_user_id):
         """Get all roles in a group
         """
-        path = PREFIX + "/groups/%s/roles" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/roles", group_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -813,7 +824,7 @@ class TransportLayerClient(object):
     def get_group_role(self, destination, group_id, requester_user_id, role_id):
         """Get a roles info
         """
-        path = PREFIX + "/groups/%s/roles/%s" % (group_id, role_id,)
+        path = _create_path(PREFIX, "/groups/%s/roles/%s", group_id, role_id,)
 
         return self.client.get_json(
             destination=destination,
@@ -827,7 +838,7 @@ class TransportLayerClient(object):
                           content):
         """Update a role in a group
         """
-        path = PREFIX + "/groups/%s/roles/%s" % (group_id, role_id,)
+        path = _create_path(PREFIX, "/groups/%s/roles/%s", group_id, role_id,)
 
         return self.client.post_json(
             destination=destination,
@@ -841,7 +852,7 @@ class TransportLayerClient(object):
     def delete_group_role(self, destination, group_id, requester_user_id, role_id):
         """Delete a role in a group
         """
-        path = PREFIX + "/groups/%s/roles/%s" % (group_id, role_id,)
+        path = _create_path(PREFIX, "/groups/%s/roles/%s", group_id, role_id,)
 
         return self.client.delete_json(
             destination=destination,
@@ -856,11 +867,12 @@ class TransportLayerClient(object):
         """Update a users entry in a group
         """
         if role_id:
-            path = PREFIX + "/groups/%s/summary/roles/%s/users/%s" % (
+            path = _create_path(
+                PREFIX, "/groups/%s/summary/roles/%s/users/%s",
                 group_id, role_id, user_id,
             )
         else:
-            path = PREFIX + "/groups/%s/summary/users/%s" % (group_id, user_id,)
+            path = _create_path(PREFIX, "/groups/%s/summary/users/%s", group_id, user_id,)
 
         return self.client.post_json(
             destination=destination,
@@ -875,7 +887,7 @@ class TransportLayerClient(object):
                               content):
         """Sets the join policy for a group
         """
-        path = PREFIX + "/groups/%s/settings/m.join_policy" % (group_id,)
+        path = _create_path(PREFIX, "/groups/%s/settings/m.join_policy", group_id,)
 
         return self.client.put_json(
             destination=destination,
@@ -891,11 +903,12 @@ class TransportLayerClient(object):
         """Delete a users entry in a group
         """
         if role_id:
-            path = PREFIX + "/groups/%s/summary/roles/%s/users/%s" % (
+            path = _create_path(
+                PREFIX, "/groups/%s/summary/roles/%s/users/%s",
                 group_id, role_id, user_id,
             )
         else:
-            path = PREFIX + "/groups/%s/summary/users/%s" % (group_id, user_id,)
+            path = _create_path(PREFIX, "/groups/%s/summary/users/%s", group_id, user_id,)
 
         return self.client.delete_json(
             destination=destination,
@@ -918,3 +931,22 @@ class TransportLayerClient(object):
             data=content,
             ignore_backoff=True,
         )
+
+
+def _create_path(prefix, path, *args):
+    """Creates a path from the prefix, path template and args. Ensures that
+    all args are url encoded.
+
+    Example:
+
+        _create_path(PREFIX, "/event/%s/", event_id)
+
+    Args:
+        prefix (str)
+        path (str): String template for the path
+        args: ([str]): Args to insert into path. Each arg will be url encoded
+
+    Returns:
+        str
+    """
+    return prefix + path % tuple(urllib.quote(arg) for arg in args)
-- 
cgit 1.4.1


From 11d2609da70af797405241cdf7d9df19db5628f2 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 10 Apr 2018 11:24:40 +0100
Subject: Ensure slashes are escaped

---
 synapse/federation/transport/client.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'synapse/federation/transport/client.py')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 1976e5d2fb..6db8efa6dd 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -949,4 +949,4 @@ def _create_path(prefix, path, *args):
     Returns:
         str
     """
-    return prefix + path % tuple(urllib.quote(arg) for arg in args)
+    return prefix + path % tuple(urllib.quote(arg, "") for arg in args)
-- 
cgit 1.4.1