From 74c46d81fa7c3e4f1cfc3688d9ce3f46d35ee5a5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 Mar 2019 16:50:23 +0000 Subject: Only require consent for events with an associated request There are a number of instances where a server or admin may puppet a user to join/leave rooms, which we don't want to fail if the user has not consented to the privacy policy. We fix this by adding a check to test if the requester has an associated access_token, which is used as a proxy to answer the question of whether the action is being done on behalf of a real request from the user. --- synapse/handlers/message.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'synapse/handlers/message.py') diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 55787563c0..ac9d9c1a83 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -316,8 +316,12 @@ class EventCreationHandler(object): target, e ) + # Check if the user has accepted the privacy policy. We only do this if + # the requester has an associated access_token_id, which indicates that + # this action came from a user request rather than an automatice server + # or admin action. is_exempt = yield self._is_exempt_from_privacy_policy(builder, requester) - if not is_exempt: + if requester.access_token_id and not is_exempt: yield self.assert_accepted_privacy_policy(requester) if token_id is not None: -- cgit 1.5.1 From 7d47cc1305e1504af78ff13c49b43b81b1ac5791 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 Mar 2019 17:08:36 +0000 Subject: Move requester check into assert_accepted_privacy_policy --- synapse/handlers/message.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'synapse/handlers/message.py') diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index ac9d9c1a83..345a3e0ecd 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -316,12 +316,8 @@ class EventCreationHandler(object): target, e ) - # Check if the user has accepted the privacy policy. We only do this if - # the requester has an associated access_token_id, which indicates that - # this action came from a user request rather than an automatice server - # or admin action. is_exempt = yield self._is_exempt_from_privacy_policy(builder, requester) - if requester.access_token_id and not is_exempt: + if not is_exempt: yield self.assert_accepted_privacy_policy(requester) if token_id is not None: @@ -396,6 +392,13 @@ class EventCreationHandler(object): if requester.app_service is not None: return + # Check if the user has accepted the privacy policy. We only do this if + # the requester has an associated access_token_id, which indicates that + # this action came from a user request rather than an automatice server + # or admin action. + if requester.access_token_id is None: + return + user_id = requester.user.to_string() # exempt the system notices user -- cgit 1.5.1 From aa959a6c0705067cd01d1fd0ba42f51f320ed51b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 Mar 2019 17:39:29 +0000 Subject: Use flags --- synapse/handlers/_base.py | 1 + synapse/handlers/deactivate_account.py | 1 + synapse/handlers/message.py | 18 +++++------------- synapse/handlers/room_member.py | 6 ++++++ synapse/rest/client/v1/admin.py | 6 ++++-- 5 files changed, 17 insertions(+), 15 deletions(-) (limited to 'synapse/handlers/message.py') diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index d8d86d6ff3..ac09d03ba9 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -165,6 +165,7 @@ class BaseHandler(object): member_event.room_id, "leave", ratelimit=False, + require_consent=False, ) except Exception as e: logger.exception("Error kicking guest user: %s" % (e,)) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 75fe50c42c..97d3f31d98 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -164,6 +164,7 @@ class DeactivateAccountHandler(BaseHandler): room_id, "leave", ratelimit=False, + require_consent=False, ) except Exception: logger.exception( diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 345a3e0ecd..587fbfbe86 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -255,7 +255,7 @@ class EventCreationHandler(object): @defer.inlineCallbacks def create_event(self, requester, event_dict, token_id=None, txn_id=None, - prev_events_and_hashes=None): + prev_events_and_hashes=None, require_consent=True): """ Given a dict from a client, create a new event. @@ -276,6 +276,9 @@ class EventCreationHandler(object): where *hashes* is a map from algorithm to hash. If None, they will be requested from the database. + + require_consent (bool): Whether to check if the requester has + consented to privacy policy. Raises: ResourceLimitError if server is blocked to some resource being exceeded @@ -317,7 +320,7 @@ class EventCreationHandler(object): ) is_exempt = yield self._is_exempt_from_privacy_policy(builder, requester) - if not is_exempt: + if require_consent and not is_exempt: yield self.assert_accepted_privacy_policy(requester) if token_id is not None: @@ -388,17 +391,6 @@ class EventCreationHandler(object): if self._block_events_without_consent_error is None: return - # exempt AS users from needing consent - if requester.app_service is not None: - return - - # Check if the user has accepted the privacy policy. We only do this if - # the requester has an associated access_token_id, which indicates that - # this action came from a user request rather than an automatice server - # or admin action. - if requester.access_token_id is None: - return - user_id = requester.user.to_string() # exempt the system notices user diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index aead9e4608..71ce5b54e5 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -160,6 +160,7 @@ class RoomMemberHandler(object): txn_id=None, ratelimit=True, content=None, + require_consent=True, ): user_id = target.to_string() @@ -185,6 +186,7 @@ class RoomMemberHandler(object): token_id=requester.access_token_id, txn_id=txn_id, prev_events_and_hashes=prev_events_and_hashes, + require_consent=require_consent, ) # Check if this event matches the previous membership event for the user. @@ -305,6 +307,7 @@ class RoomMemberHandler(object): third_party_signed=None, ratelimit=True, content=None, + require_consent=True, ): key = (room_id,) @@ -319,6 +322,7 @@ class RoomMemberHandler(object): third_party_signed=third_party_signed, ratelimit=ratelimit, content=content, + require_consent=require_consent, ) defer.returnValue(result) @@ -335,6 +339,7 @@ class RoomMemberHandler(object): third_party_signed=None, ratelimit=True, content=None, + require_consent=True, ): content_specified = bool(content) if content is None: @@ -516,6 +521,7 @@ class RoomMemberHandler(object): ratelimit=ratelimit, prev_events_and_hashes=prev_events_and_hashes, content=content, + require_consent=require_consent, ) defer.returnValue(res) diff --git a/synapse/rest/client/v1/admin.py b/synapse/rest/client/v1/admin.py index 56ad65515a..e788769639 100644 --- a/synapse/rest/client/v1/admin.py +++ b/synapse/rest/client/v1/admin.py @@ -516,7 +516,8 @@ class ShutdownRoomRestServlet(ClientV1RestServlet): room_id=room_id, action=Membership.LEAVE, content={}, - ratelimit=False + ratelimit=False, + require_consent=False, ) yield self.room_member_handler.forget(target_requester.user, room_id) @@ -527,7 +528,8 @@ class ShutdownRoomRestServlet(ClientV1RestServlet): room_id=new_room_id, action=Membership.JOIN, content={}, - ratelimit=False + ratelimit=False, + require_consent=False, ) kicked_users.append(user_id) -- cgit 1.5.1 From cd62981a6a083945547100c8d0f30380ea17c6e3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 20 Mar 2019 17:51:27 +0000 Subject: Revert spurious delete --- synapse/handlers/message.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'synapse/handlers/message.py') diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 587fbfbe86..9b41c7b205 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -391,6 +391,10 @@ class EventCreationHandler(object): if self._block_events_without_consent_error is None: return + # exempt AS users from needing consent + if requester.app_service is not None: + return + user_id = requester.user.to_string() # exempt the system notices user -- cgit 1.5.1