diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py index c52e020989..28bbff9bbb 100644 --- a/synapse/groups/attestations.py +++ b/synapse/groups/attestations.py @@ -35,6 +35,8 @@ An attestsation is a signed blob of json that looks like: } """ +import logging + from twisted.internet import defer from synapse.api.errors import SynapseError @@ -44,6 +46,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 @@ -150,12 +155,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 4f9e459136..7406f67d07 100644 --- a/synapse/groups/groups_server.py +++ b/synapse/groups/groups_server.py @@ -646,6 +646,7 @@ class GroupsServerHandler(object): 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, user_id) remote_attestation = content["attestation"] yield self.attestations.verify_attestation( @@ -654,13 +655,9 @@ class GroupsServerHandler(object): group_id=group_id, ) else: + local_attestation = None remote_attestation = None - local_attestation = self.attestations.create_attestation( - group_id, - requester_user_id, - ) - is_public = _parse_visibility_from_contents(content) yield self.store.add_user_to_group( diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py index 095a3dd382..8c4ad0a9a9 100644 --- a/synapse/storage/group_server.py +++ b/synapse/storage/group_server.py @@ -1089,6 +1089,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_delete( + table="group_attestations_renewals", + 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