From b370fe61c0aeccac7745ad404dad925ec217ba6d Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 28 Mar 2018 17:18:02 +0100 Subject: [PATCH 01/14] 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(-) diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 3beab4783..370f7ba78 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 b98e30459..4c94d5a36 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[^/]*)/users/(?P[^/]*)/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 70781e185..77b6273e3 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 5f7b0ff30..977993e7d 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 db316a27e..16c11a056 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 From edb45aae38b7bdeae1cbf18b435be5f8610c2d48 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 28 Mar 2018 17:18:50 +0100 Subject: [PATCH 02/14] pep8 --- synapse/storage/group_server.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index 16c11a056..5fbe0ada4 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -56,7 +56,8 @@ class GroupServerStore(SQLBaseStore): "group_id": group_id, }, retcols=( - "name", "short_description", "long_description", "avatar_url", "is_public", "is_joinable", + "name", "short_description", "long_description", + "avatar_url", "is_public", "is_joinable", ), allow_none=True, desc="get_group", From 6eb3aa94b6befcc89f6fee426053b3feee316bb9 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 3 Apr 2018 11:48:17 +0100 Subject: [PATCH 03/14] Factor out add_user from accept_invite and join_group --- synapse/groups/groups_server.py | 84 ++++++++++++++------------------- 1 file changed, 36 insertions(+), 48 deletions(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 77b6273e3..2c02da472 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -677,6 +677,40 @@ class GroupsServerHandler(object): else: raise SynapseError(502, "Unknown state returned by HS") + @defer.inlineCallbacks + def add_user(self, group_id, user_id, content): + """Add a user to a group based on a content dict. + + See accept_invite, join_group. + """ + if not self.hs.is_mine_id(user_id): + local_attestation = self.attestations.create_attestation( + group_id, user_id, + ) + + remote_attestation = content["attestation"] + + yield self.attestations.verify_attestation( + remote_attestation, + user_id=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, user_id, + is_admin=False, + is_public=is_public, + local_attestation=local_attestation, + remote_attestation=remote_attestation, + ) + + defer.returnValue(local_attestation) + @defer.inlineCallbacks def accept_invite(self, group_id, requester_user_id, content): """User tries to accept an invite to the group. @@ -693,30 +727,7 @@ class GroupsServerHandler(object): if not is_invited: raise SynapseError(403, "User not invited to group") - 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, - ) + local_attestation = yield self.add_user(group_id, requester_user_id, content) defer.returnValue({ "state": "join", @@ -738,30 +749,7 @@ class GroupsServerHandler(object): 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, - ) + local_attestation = yield self.add_user(group_id, requester_user_id, content) defer.returnValue({ "state": "join", From f8d1917fce3b08b613f76131d6a82a7576379251 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 5 Apr 2018 16:31:24 +0100 Subject: [PATCH 04/14] Fix federation client `set_group_joinable` typo --- synapse/federation/transport/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 370f7ba78..67ff7da51 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 """ From ae85c7804e733aa1adaed06a9de51445a084858e Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 5 Apr 2018 16:31:57 +0100 Subject: [PATCH 05/14] is_joinable -> join_rule --- synapse/groups/groups_server.py | 2 +- synapse/storage/group_server.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 2c02da472..507ec232c 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -746,7 +746,7 @@ class GroupsServerHandler(object): group_info = yield self.store.get_group( group_id, ) - if not group_info['is_joinable']: + if group_info['join_policy'] != "open": raise SynapseError(403, "Group is not publicly joinable") local_attestation = yield self.add_user(group_id, requester_user_id, content) diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index 5fbe0ada4..d81609dd1 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -57,15 +57,12 @@ class GroupServerStore(SQLBaseStore): }, retcols=( "name", "short_description", "long_description", - "avatar_url", "is_public", "is_joinable", + "avatar_url", "is_public", "join_rule", ), allow_none=True, 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): From 87c864b6984f6876f224e0d4ac2e2b872d2723ce Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 5 Apr 2018 16:49:22 +0100 Subject: [PATCH 06/14] join_rule -> join_policy --- synapse/storage/group_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index d81609dd1..8bd8ca812 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -57,7 +57,7 @@ class GroupServerStore(SQLBaseStore): }, retcols=( "name", "short_description", "long_description", - "avatar_url", "is_public", "join_rule", + "avatar_url", "is_public", "join_policy", ), allow_none=True, desc="get_group", From cd087a265db8037ce2fe158ddc0e9b35a612d4bb Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 5 Apr 2018 17:14:27 +0100 Subject: [PATCH 07/14] Don't use redundant inlineCallbacks --- synapse/storage/group_server.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index 8bd8ca812..da05ccb02 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -48,9 +48,8 @@ class GroupServerStore(SQLBaseStore): desc="set_group_join_policy", ) - @defer.inlineCallbacks def get_group(self, group_id): - ret = yield self._simple_select_one( + return self._simple_select_one( table="groups", keyvalues={ "group_id": group_id, @@ -63,8 +62,6 @@ class GroupServerStore(SQLBaseStore): desc="get_group", ) - defer.returnValue(ret) - def get_users_in_group(self, group_id, include_private=False): # TODO: Pagination From 6850f8aea3482443dff1c330fb63f7ca8dde0025 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 5 Apr 2018 17:16:31 +0100 Subject: [PATCH 08/14] Get group_info from existing call to check_group_is_ours --- synapse/groups/groups_server.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 507ec232c..1b516011d 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -741,11 +741,7 @@ class GroupsServerHandler(object): 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, - ) + 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") From 112c2253e2846b48b75f4171d9dd94bfe6ecc8a1 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 5 Apr 2018 17:32:20 +0100 Subject: [PATCH 09/14] pep8 --- synapse/federation/transport/client.py | 2 +- synapse/groups/groups_server.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 67ff7da51..50a967a7e 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 1b516011d..15d31549d 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") From b4478e586fe52392a439c94f525c3d885454f0a8 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 6 Apr 2018 11:37:49 +0100 Subject: [PATCH 10/14] add_user -> _add_user --- synapse/groups/groups_server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 15d31549d..72dfed7db 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -678,7 +678,7 @@ class GroupsServerHandler(object): raise SynapseError(502, "Unknown state returned by HS") @defer.inlineCallbacks - def add_user(self, group_id, user_id, content): + def _add_user(self, group_id, user_id, content): """Add a user to a group based on a content dict. See accept_invite, join_group. @@ -727,7 +727,7 @@ class GroupsServerHandler(object): if not is_invited: raise SynapseError(403, "User not invited to group") - local_attestation = yield self.add_user(group_id, requester_user_id, content) + local_attestation = yield self._add_user(group_id, requester_user_id, content) defer.returnValue({ "state": "join", @@ -747,7 +747,7 @@ class GroupsServerHandler(object): if group_info['join_policy'] != "open": raise SynapseError(403, "Group is not publicly joinable") - local_attestation = yield self.add_user(group_id, requester_user_id, content) + local_attestation = yield self._add_user(group_id, requester_user_id, content) defer.returnValue({ "state": "join", From 6bd1b7053e41350fcc1b451e40da82257e2120b5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 6 Apr 2018 11:44:18 +0100 Subject: [PATCH 11/14] By default, join policy is "invite" --- synapse/groups/groups_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 72dfed7db..8310dea03 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -905,7 +905,7 @@ def _parse_join_policy_dict(join_policy_dict): """ join_policy_type = join_policy_dict.get("type") if not join_policy_type: - return True + return "invite" if join_policy_type not in ("invite", "open"): raise SynapseError( From 7945435587704c5abd075140bdd46c11db93c255 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 6 Apr 2018 14:06:32 +0100 Subject: [PATCH 12/14] When exposing group state, return is_openly_joinable as opposed to join_policy, which is really only pertinent to the synapse implementation of the group server. By doing this we keep the group server concept extensible by allowing arbitrarily complex rules for deciding whether a group is openly joinable. --- synapse/groups/groups_server.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 8310dea03..290eec712 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -407,6 +407,11 @@ class GroupsServerHandler(object): group_description = yield self.store.get_group(group_id) if group_description: + join_policy = group_description['join_policy'] + del group_description['join_policy'] + + group_description['is_openly_joinable'] = join_policy == "open" + defer.returnValue(group_description) else: raise SynapseError(404, "Unknown group") From db2fd801f722ae8341b36314fb8929a80fd53996 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 6 Apr 2018 15:51:15 +0100 Subject: [PATCH 13/14] Explicitly grab individual columns from group object --- synapse/groups/groups_server.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 290eec712..ad937c172 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -404,13 +404,15 @@ class GroupsServerHandler(object): yield self.check_group_is_ours(group_id, requester_user_id) - group_description = yield self.store.get_group(group_id) + group = yield self.store.get_group(group_id) - if group_description: - join_policy = group_description['join_policy'] - del group_description['join_policy'] - - group_description['is_openly_joinable'] = join_policy == "open" + if group: + cols = [ + "name", "short_description", "long_description", + "avatar_url", "is_public", + ] + group_description = { key: group[key] for key in cols } + group_description["is_openly_joinable"] = group['join_policy'] == "open" defer.returnValue(group_description) else: From 020a5013544b1f7046faa97b83eb2acc613334b1 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 6 Apr 2018 16:02:06 +0100 Subject: [PATCH 14/14] de-lint, quote consistency --- synapse/groups/groups_server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index ad937c172..2d95b04e0 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -411,8 +411,8 @@ class GroupsServerHandler(object): "name", "short_description", "long_description", "avatar_url", "is_public", ] - group_description = { key: group[key] for key in cols } - group_description["is_openly_joinable"] = group['join_policy'] == "open" + group_description = {key: group[key] for key in cols} + group_description["is_openly_joinable"] = group["join_policy"] == "open" defer.returnValue(group_description) else: