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..addc70ce94 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)
@@ -425,7 +462,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 +496,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 +507,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 +517,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)
@@ -494,30 +529,33 @@ class GroupsServerHandler(object):
})
@defer.inlineCallbacks
- def add_room_to_group(self, group_id, requester_user_id, room_id, content):
- """Add room to group
+ def update_room_group_association(self, group_id, requester_user_id, room_id,
+ content):
+ """Add or update an association between room and group
"""
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)
- yield self.store.add_room_to_group(group_id, room_id, is_public=is_public)
+ yield self.store.update_room_group_association(
+ group_id, room_id, is_public=is_public
+ )
defer.returnValue({})
@defer.inlineCallbacks
- def remove_room_from_group(self, group_id, requester_user_id, room_id):
+ def delete_room_group_association(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)
+ yield self.store.delete_room_group_association(group_id, room_id)
defer.returnValue({})
@@ -527,7 +565,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 +634,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 +680,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 +735,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 +746,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 +770,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"),
)
|
