From 585972b51a033d7082b3fba4013ad2ca544c846b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 27 Oct 2017 09:44:34 +0100 Subject: Don't generate group attestations for local users --- synapse/groups/groups_server.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'synapse/groups') diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py index 23beb3187e..96f112b580 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -609,6 +609,8 @@ class GroupsServerHandler(object): raise SynapseError(403, "User not invited to 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( @@ -617,10 +619,9 @@ class GroupsServerHandler(object): 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( -- cgit 1.4.1 From d8dde19f04799270186723f7f35dc32217dda33e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 27 Oct 2017 09:55:01 +0100 Subject: Log if we try to do attestations for our own user and group --- synapse/groups/attestations.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'synapse/groups') diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py index b751cf5e43..2e252b66a7 100644 --- a/synapse/groups/attestations.py +++ b/synapse/groups/attestations.py @@ -130,10 +130,16 @@ class GroupAttestionRenewer(object): 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) + else 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, + ) + return yield self.transport_client.renew_group_attestation( destination, group_id, user_id, -- cgit 1.4.1 From 195abfe7a5ec3b0d52812a3d7a04264f97376771 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 27 Oct 2017 09:58:13 +0100 Subject: Remove incorrect attestations --- synapse/groups/attestations.py | 1 + synapse/storage/group_server.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) (limited to 'synapse/groups') diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py index 2e252b66a7..0bd73b6a61 100644 --- a/synapse/groups/attestations.py +++ b/synapse/groups/attestations.py @@ -139,6 +139,7 @@ class GroupAttestionRenewer(object): "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 yield self.transport_client.renew_group_attestation( diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index 9e63db5c6c..ed2ee61ad2 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -1086,6 +1086,24 @@ class GroupServerStore(SQLBaseStore): desc="update_remote_attestion", ) + def remove_attestation_renewal(self, group_id, user_id): + """Remove an attestation that we thought we should renew, but actually + shouldn't. Ideally this would never get called as we would never + incorrectly try and do attestations for local users on local groups. + + Args: + group_id (str) + user_id (str) + """ + return self._simple_update_one( + table="_simple_delete", + keyvalues={ + "group_id": group_id, + "user_id": user_id, + }, + desc="remove_attestation_renewal", + ) + @defer.inlineCallbacks def get_remote_attestation(self, group_id, user_id): """Get the attestation that proves the remote agrees that the user is -- cgit 1.4.1 From 82d8c1bacb085588b59021d21cd4df56b0d8411a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 27 Oct 2017 10:30:21 +0100 Subject: Fixup --- synapse/groups/attestations.py | 6 +++--- synapse/storage/group_server.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'synapse/groups') diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py index 0bd73b6a61..4656e854f0 100644 --- a/synapse/groups/attestations.py +++ b/synapse/groups/attestations.py @@ -128,11 +128,9 @@ class GroupAttestionRenewer(object): @defer.inlineCallbacks def _renew_attestation(group_id, user_id): - attestation = self.attestations.create_attestation(group_id, user_id) - if not self.is_mine_id(group_id): destination = get_domain_from_id(group_id) - else not self.is_mine_id(user_id): + elif not self.is_mine_id(user_id): destination = get_domain_from_id(user_id) else: logger.warn( @@ -142,6 +140,8 @@ class GroupAttestionRenewer(object): 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, content={"attestation": attestation}, diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index ed2ee61ad2..ba3f5617fa 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -1095,8 +1095,8 @@ class GroupServerStore(SQLBaseStore): group_id (str) user_id (str) """ - return self._simple_update_one( - table="_simple_delete", + return self._simple_delete( + table="group_attestations_renewals", keyvalues={ "group_id": group_id, "user_id": user_id, -- cgit 1.4.1 From e27b76d11728ba0fa2cbbd99ac50d33dee95da63 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 27 Oct 2017 10:54:02 +0100 Subject: Import logger --- synapse/groups/attestations.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'synapse/groups') diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py index 4656e854f0..c060cff5dd 100644 --- a/synapse/groups/attestations.py +++ b/synapse/groups/attestations.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging + from twisted.internet import defer from synapse.api.errors import SynapseError @@ -22,6 +24,9 @@ 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 -- cgit 1.4.1