From 6b05530bb98b4572fe2f180d87d8e596bf5ebaea Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 28 Jun 2021 15:30:27 -0400 Subject: [PATCH 1/9] Move compute_auth_events to EventAuthHandler. --- synapse/api/auth.py | 41 +---------------------- synapse/events/builder.py | 12 ++++--- synapse/handlers/event_auth.py | 42 +++++++++++++++++++++++- synapse/handlers/federation.py | 2 +- synapse/handlers/message.py | 5 +-- synapse/push/bulk_push_rule_evaluator.py | 4 +-- tests/handlers/test_presence.py | 4 +-- 7 files changed, 57 insertions(+), 53 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index f8b068e56385..6d696c991569 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple import pymacaroons from netaddr import IPAddress @@ -31,7 +31,6 @@ from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.appservice import ApplicationService from synapse.events import EventBase -from synapse.events.builder import EventBuilder from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest from synapse.logging import opentracing as opentracing @@ -489,44 +488,6 @@ async def is_server_admin(self, user: UserID) -> bool: """ return await self.store.is_server_admin(user) - def compute_auth_events( - self, - event: Union[EventBase, EventBuilder], - current_state_ids: StateMap[str], - for_verification: bool = False, - ) -> List[str]: - """Given an event and current state return the list of event IDs used - to auth an event. - - If `for_verification` is False then only return auth events that - should be added to the event's `auth_events`. - - Returns: - List of event IDs. - """ - - if event.type == EventTypes.Create: - return [] - - # Currently we ignore the `for_verification` flag even though there are - # some situations where we can drop particular auth events when adding - # to the event's `auth_events` (e.g. joins pointing to previous joins - # when room is publicly joinable). Dropping event IDs has the - # advantage that the auth chain for the room grows slower, but we use - # the auth chain in state resolution v2 to order events, which means - # care must be taken if dropping events to ensure that it doesn't - # introduce undesirable "state reset" behaviour. - # - # All of which sounds a bit tricky so we don't bother for now. - - auth_ids = [] - for etype, state_key in event_auth.auth_types_for_event(event): - auth_ev_id = current_state_ids.get((etype, state_key)) - if auth_ev_id: - auth_ids.append(auth_ev_id) - - return auth_ids - async def check_can_change_room_list(self, room_id: str, user: UserID) -> bool: """Determine whether the user is allowed to edit the room's entry in the published room list. diff --git a/synapse/events/builder.py b/synapse/events/builder.py index fb48ec8541e2..26e39508596e 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.py @@ -34,7 +34,7 @@ from synapse.util.stringutils import random_string if TYPE_CHECKING: - from synapse.api.auth import Auth + from synapse.handlers.event_auth import EventAuthHandler from synapse.server import HomeServer logger = logging.getLogger(__name__) @@ -66,7 +66,7 @@ class EventBuilder: """ _state: StateHandler - _auth: "Auth" + _event_auth_handler: "EventAuthHandler" _store: DataStore _clock: Clock _hostname: str @@ -125,7 +125,9 @@ async def build( state_ids = await self._state.get_current_state_ids( self.room_id, prev_event_ids ) - auth_event_ids = self._auth.compute_auth_events(self, state_ids) + auth_event_ids = self._event_auth_handler.compute_auth_events( + self, state_ids + ) format_version = self.room_version.event_format if format_version == EventFormatVersions.V1: @@ -193,7 +195,7 @@ def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.state = hs.get_state_handler() - self.auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() def new(self, room_version: str, key_values: dict) -> EventBuilder: """Generate an event builder appropriate for the given room version @@ -229,7 +231,7 @@ def for_room_version( return EventBuilder( store=self.store, state=self.state, - auth=self.auth, + event_auth_handler=self._event_auth_handler, clock=self.clock, hostname=self.hostname, signing_key=self.signing_key, diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 989996b628a1..d49b35065328 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -11,8 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Collection, Optional +from typing import TYPE_CHECKING, Collection, List, Optional, Union +from synapse import event_auth from synapse.api.constants import ( EventTypes, JoinRules, @@ -22,6 +23,7 @@ from synapse.api.errors import AuthError from synapse.api.room_versions import RoomVersion from synapse.events import EventBase +from synapse.events.builder import EventBuilder from synapse.types import StateMap if TYPE_CHECKING: @@ -36,6 +38,44 @@ class EventAuthHandler: def __init__(self, hs: "HomeServer"): self._store = hs.get_datastore() + def compute_auth_events( + self, + event: Union[EventBase, EventBuilder], + current_state_ids: StateMap[str], + for_verification: bool = False, + ) -> List[str]: + """Given an event and current state return the list of event IDs used + to auth an event. + + If `for_verification` is False then only return auth events that + should be added to the event's `auth_events`. + + Returns: + List of event IDs. + """ + + if event.type == EventTypes.Create: + return [] + + # Currently we ignore the `for_verification` flag even though there are + # some situations where we can drop particular auth events when adding + # to the event's `auth_events` (e.g. joins pointing to previous joins + # when room is publicly joinable). Dropping event IDs has the + # advantage that the auth chain for the room grows slower, but we use + # the auth chain in state resolution v2 to order events, which means + # care must be taken if dropping events to ensure that it doesn't + # introduce undesirable "state reset" behaviour. + # + # All of which sounds a bit tricky so we don't bother for now. + + auth_ids = [] + for etype, state_key in event_auth.auth_types_for_event(event): + auth_ev_id = current_state_ids.get((etype, state_key)) + if auth_ev_id: + auth_ids.append(auth_ev_id) + + return auth_ids + async def check_restricted_join_rules( self, state_ids: StateMap[str], diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index d929c65131d2..211e0bc69d7f 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -2562,7 +2562,7 @@ async def _check_event_auth( if not auth_events: prev_state_ids = await context.get_prev_state_ids() - auth_events_ids = self.auth.compute_auth_events( + auth_events_ids = self._event_auth_handler.compute_auth_events( event, prev_state_ids, for_verification=True ) auth_events_x = await self.store.get_events(auth_events_ids) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 364c5cd2d38d..e54c3d430e88 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -385,6 +385,7 @@ class EventCreationHandler: def __init__(self, hs: "HomeServer"): self.hs = hs self.auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() self.store = hs.get_datastore() self.storage = hs.get_storage() self.state = hs.get_state_handler() @@ -597,7 +598,7 @@ async def create_event( (e.type, e.state_key): e.event_id for e in auth_events } # Actually strip down and use the necessary auth events - auth_event_ids = self.auth.compute_auth_events( + auth_event_ids = self._event_auth_handler.compute_auth_events( event=temp_event, current_state_ids=auth_event_state_map, for_verification=False, @@ -1381,7 +1382,7 @@ async def persist_and_notify_client_event( raise AuthError(403, "Redacting server ACL events is not permitted") prev_state_ids = await context.get_prev_state_ids() - auth_events_ids = self.auth.compute_auth_events( + auth_events_ids = self._event_auth_handler.compute_auth_events( event, prev_state_ids, for_verification=True ) auth_events_map = await self.store.get_events(auth_events_ids) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 350646f45888..669ea462e29e 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -104,7 +104,7 @@ class BulkPushRuleEvaluator: def __init__(self, hs: "HomeServer"): self.hs = hs self.store = hs.get_datastore() - self.auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() # Used by `RulesForRoom` to ensure only one thing mutates the cache at a # time. Keyed off room_id. @@ -172,7 +172,7 @@ async def _get_power_levels_and_sender_level( # not having a power level event is an extreme edge case auth_events = {POWER_KEY: await self.store.get_event(pl_event_id)} else: - auth_events_ids = self.auth.compute_auth_events( + auth_events_ids = self._event_auth_handler.compute_auth_events( event, prev_state_ids, for_verification=False ) auth_events_dict = await self.store.get_events(auth_events_ids) diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index dfb9b3a0fa4b..18e92e90d7f4 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -734,7 +734,7 @@ def prepare(self, reactor, clock, hs): self.store = hs.get_datastore() self.state = hs.get_state_handler() - self.auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() # We don't actually check signatures in tests, so lets just create a # random key to use. @@ -846,7 +846,7 @@ def _add_new_user(self, room_id, user_id): builder = EventBuilder( state=self.state, - auth=self.auth, + event_auth_handler=self._event_auth_handler, store=self.store, clock=self.clock, hostname=hostname, From 7aa680df05e962a602e59be4c198043ce2545021 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 07:29:31 -0400 Subject: [PATCH 2/9] Move check_from_context to EventAuthHandler. --- synapse/api/auth.py | 13 ------------- synapse/handlers/event_auth.py | 14 +++++++++++++- synapse/handlers/federation.py | 14 +++++++++----- synapse/handlers/message.py | 4 +++- synapse/handlers/room.py | 3 ++- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 6d696c991569..0d3fc536311f 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -28,7 +28,6 @@ InvalidClientTokenError, MissingClientTokenError, ) -from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.appservice import ApplicationService from synapse.events import EventBase from synapse.http import get_request_user_agent @@ -89,18 +88,6 @@ def __init__(self, hs: "HomeServer"): self._macaroon_secret_key = hs.config.macaroon_secret_key self._force_tracing_for_users = hs.config.tracing.force_tracing_for_users - async def check_from_context( - self, room_version: str, event, context, do_sig_check=True - ) -> None: - auth_event_ids = event.auth_event_ids() - auth_events_by_id = await self.store.get_events(auth_event_ids) - auth_events = {(e.type, e.state_key): e for e in auth_events_by_id.values()} - - room_version_obj = KNOWN_ROOM_VERSIONS[room_version] - event_auth.check( - room_version_obj, event, auth_events=auth_events, do_sig_check=do_sig_check - ) - async def check_user_in_room( self, room_id: str, diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index d49b35065328..5d74a76c38eb 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -21,7 +21,7 @@ RestrictedJoinRuleTypes, ) from synapse.api.errors import AuthError -from synapse.api.room_versions import RoomVersion +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.events import EventBase from synapse.events.builder import EventBuilder from synapse.types import StateMap @@ -38,6 +38,18 @@ class EventAuthHandler: def __init__(self, hs: "HomeServer"): self._store = hs.get_datastore() + async def check_from_context( + self, room_version: str, event, context, do_sig_check=True + ) -> None: + auth_event_ids = event.auth_event_ids() + auth_events_by_id = await self._store.get_events(auth_event_ids) + auth_events = {(e.type, e.state_key): e for e in auth_events_by_id.values()} + + room_version_obj = KNOWN_ROOM_VERSIONS[room_version] + event_auth.check( + room_version_obj, event, auth_events=auth_events, do_sig_check=do_sig_check + ) + def compute_auth_events( self, event: Union[EventBase, EventBuilder], diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 211e0bc69d7f..a0d86f260da4 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1705,7 +1705,7 @@ async def on_make_join_request( # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_join_request` - await self.auth.check_from_context( + await self._event_auth_handler.check_from_context( room_version, event, context, do_sig_check=False ) @@ -1877,7 +1877,7 @@ async def on_make_leave_request( try: # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_leave_request` - await self.auth.check_from_context( + await self._event_auth_handler.check_from_context( room_version, event, context, do_sig_check=False ) except AuthError as e: @@ -1939,7 +1939,7 @@ async def on_make_knock_request( try: # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_knock_request` - await self.auth.check_from_context( + await self._event_auth_handler.check_from_context( room_version, event, context, do_sig_check=False ) except AuthError as e: @@ -3011,7 +3011,9 @@ async def exchange_third_party_invite( event.internal_metadata.send_on_behalf_of = self.hs.hostname try: - await self.auth.check_from_context(room_version, event, context) + await self._event_auth_handler.check_from_context( + room_version, event, context + ) except AuthError as e: logger.warning("Denying new third party invite %r because %s", event, e) raise e @@ -3054,7 +3056,9 @@ async def on_exchange_third_party_invite_request( ) try: - await self.auth.check_from_context(room_version, event, context) + await self._event_auth_handler.check_from_context( + room_version, event, context + ) except AuthError as e: logger.warning("Denying third party invite %r because %s", event, e) raise e diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index e54c3d430e88..66e40a915d04 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1057,7 +1057,9 @@ async def handle_new_client_event( assert event.content["membership"] == Membership.LEAVE else: try: - await self.auth.check_from_context(room_version, event, context) + await self._event_auth_handler.check_from_context( + room_version, event, context + ) except AuthError as err: logger.warning("Denying new event %r because %s", event, err) raise err diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 835d874ceedb..579b1b93c5fa 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -83,6 +83,7 @@ def __init__(self, hs: "HomeServer"): self.spam_checker = hs.get_spam_checker() self.event_creation_handler = hs.get_event_creation_handler() self.room_member_handler = hs.get_room_member_handler() + self._event_auth_handler = hs.get_event_auth_handler() self.config = hs.config # Room state based off defined presets @@ -226,7 +227,7 @@ async def _upgrade_room( }, ) old_room_version = await self.store.get_room_version_id(old_room_id) - await self.auth.check_from_context( + await self._event_auth_handler.check_from_context( old_room_version, tombstone_event, tombstone_context ) From 58fc2715488910422ebe9b589b51519628b15bd1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 07:44:11 -0400 Subject: [PATCH 3/9] Remove get_public_keys wrapper. --- synapse/api/auth.py | 5 +---- synapse/handlers/federation.py | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 0d3fc536311f..081c7eab5fef 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple +from typing import TYPE_CHECKING, Optional, Tuple import pymacaroons from netaddr import IPAddress @@ -142,9 +142,6 @@ async def check_host_in_room(self, room_id: str, host: str) -> bool: with Measure(self.clock, "check_host_in_room"): return await self.store.is_host_joined(room_id, host) - def get_public_keys(self, invite_event: EventBase) -> List[Dict[str, Any]]: - return event_auth.get_public_keys(invite_event) - async def get_user_by_req( self, request: SynapseRequest, diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index a0d86f260da4..8d16e80c9875 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -3146,7 +3146,7 @@ async def _check_signature(self, event: EventBase, context: EventContext) -> Non last_exception = None # type: Optional[Exception] # for each public key in the 3pid invite event - for public_key_object in self.hs.get_auth().get_public_keys(invite_event): + for public_key_object in event_auth.get_public_keys(invite_event): try: # for each sig on the third_party_invite block of the actual invite for server, signature_block in signed["signatures"].items(): From 12ae3d85b07085d73ef4c78b7d0d89194c9c00d3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 07:57:36 -0400 Subject: [PATCH 4/9] Move check_host_in_room to EventAuthHandler. --- synapse/api/auth.py | 5 ----- synapse/federation/federation_server.py | 6 +++--- synapse/handlers/event_auth.py | 6 ++++++ synapse/handlers/federation.py | 18 ++++++++++++------ synapse/handlers/space_summary.py | 6 ++++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 081c7eab5fef..a150ceb49398 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -37,7 +37,6 @@ from synapse.types import Requester, StateMap, UserID, create_requester from synapse.util.caches.lrucache import LruCache from synapse.util.macaroons import get_value_from_macaroon, satisfy_expiry -from synapse.util.metrics import Measure if TYPE_CHECKING: from synapse.server import HomeServer @@ -138,10 +137,6 @@ async def check_user_in_room( raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) - async def check_host_in_room(self, room_id: str, host: str) -> bool: - with Measure(self.clock, "check_host_in_room"): - return await self.store.is_host_joined(room_id, host) - async def get_user_by_req( self, request: SynapseRequest, diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index e93b7577fe95..b312d0b80921 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -108,9 +108,9 @@ class FederationServer(FederationBase): def __init__(self, hs: "HomeServer"): super().__init__(hs) - self.auth = hs.get_auth() self.handler = hs.get_federation_handler() self.state = hs.get_state_handler() + self._event_auth_handler = hs.get_event_auth_handler() self.device_handler = hs.get_device_handler() @@ -420,7 +420,7 @@ async def on_room_state_request( origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, room_id) - in_room = await self.auth.check_host_in_room(room_id, origin) + in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) if not in_room: raise AuthError(403, "Host not in room.") @@ -453,7 +453,7 @@ async def on_state_ids_request( origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, room_id) - in_room = await self.auth.check_host_in_room(room_id, origin) + in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) if not in_room: raise AuthError(403, "Host not in room.") diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 5d74a76c38eb..41dbdfd0a1b6 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -25,6 +25,7 @@ from synapse.events import EventBase from synapse.events.builder import EventBuilder from synapse.types import StateMap +from synapse.util.metrics import Measure if TYPE_CHECKING: from synapse.server import HomeServer @@ -36,6 +37,7 @@ class EventAuthHandler: """ def __init__(self, hs: "HomeServer"): + self._clock = hs.get_clock() self._store = hs.get_datastore() async def check_from_context( @@ -88,6 +90,10 @@ def compute_auth_events( return auth_ids + async def check_host_in_room(self, room_id: str, host: str) -> bool: + with Measure(self._clock, "check_host_in_room"): + return await self._store.is_host_joined(room_id, host) + async def check_restricted_join_rules( self, state_ids: StateMap[str], diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 8d16e80c9875..991ec9919a95 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -250,7 +250,9 @@ async def on_receive_pdu( # # Note that if we were never in the room then we would have already # dropped the event, since we wouldn't know the room version. - is_in_room = await self.auth.check_host_in_room(room_id, self.server_name) + is_in_room = await self._event_auth_handler.check_host_in_room( + room_id, self.server_name + ) if not is_in_room: logger.info( "Ignoring PDU from %s as we're not in the room", @@ -1674,7 +1676,9 @@ async def on_make_join_request( room_version = await self.store.get_room_version_id(room_id) # now check that we are *still* in the room - is_in_room = await self.auth.check_host_in_room(room_id, self.server_name) + is_in_room = await self._event_auth_handler.check_host_in_room( + room_id, self.server_name + ) if not is_in_room: logger.info( "Got /make_join request for room %s we are no longer in", @@ -2111,7 +2115,7 @@ async def get_state_ids_for_pdu(self, room_id: str, event_id: str) -> List[str]: async def on_backfill_request( self, origin: str, room_id: str, pdu_list: List[str], limit: int ) -> List[EventBase]: - in_room = await self.auth.check_host_in_room(room_id, origin) + in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) if not in_room: raise AuthError(403, "Host not in room.") @@ -2146,7 +2150,9 @@ async def get_persisted_pdu( ) if event: - in_room = await self.auth.check_host_in_room(event.room_id, origin) + in_room = await self._event_auth_handler.check_host_in_room( + event.room_id, origin + ) if not in_room: raise AuthError(403, "Host not in room.") @@ -2499,7 +2505,7 @@ async def on_get_missing_events( latest_events: List[str], limit: int, ) -> List[EventBase]: - in_room = await self.auth.check_host_in_room(room_id, origin) + in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) if not in_room: raise AuthError(403, "Host not in room.") @@ -2991,7 +2997,7 @@ async def exchange_third_party_invite( "state_key": target_user_id, } - if await self.auth.check_host_in_room(room_id, self.hs.hostname): + if await self._event_auth_handler.check_host_in_room(room_id, self.hs.hostname): room_version = await self.store.get_room_version_id(room_id) builder = self.event_builder_factory.new(room_version, event_dict) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 266f36988331..b585057ec3a5 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -472,7 +472,7 @@ async def _is_room_accessible( # If this is a request over federation, check if the host is in the room or # is in one of the spaces specified via the join rules. elif origin: - if await self._auth.check_host_in_room(room_id, origin): + if await self._event_auth_handler.check_host_in_room(room_id, origin): return True # Alternately, if the host has a user in any of the spaces specified @@ -485,7 +485,9 @@ async def _is_room_accessible( await self._event_auth_handler.get_rooms_that_allow_join(state_ids) ) for space_id in allowed_rooms: - if await self._auth.check_host_in_room(space_id, origin): + if await self._event_auth_handler.check_host_in_room( + space_id, origin + ): return True # otherwise, check if the room is peekable From 3b07593f2e793501da7b9053616182db4da31a4c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 08:17:07 -0400 Subject: [PATCH 5/9] Move check_user_in_room, check_user_in_room_or_world_readable, and check_can_change_room_list to EventAuthHandler. --- synapse/api/auth.py | 134 +--------------------- synapse/event_auth.py | 10 +- synapse/handlers/directory.py | 11 +- synapse/handlers/event_auth.py | 134 +++++++++++++++++++++- synapse/handlers/initial_sync.py | 3 +- synapse/handlers/message.py | 10 +- synapse/handlers/pagination.py | 4 +- synapse/handlers/space_summary.py | 4 +- synapse/handlers/typing.py | 6 +- synapse/rest/client/v2_alpha/relations.py | 9 +- tests/handlers/test_typing.py | 2 +- 11 files changed, 172 insertions(+), 155 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index a150ceb49398..9d1e0cf8b88b 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -19,9 +19,8 @@ from twisted.web.server import Request -from synapse import event_auth from synapse.api.auth_blocking import AuthBlocking -from synapse.api.constants import EventTypes, HistoryVisibility, Membership +from synapse.api.constants import EventTypes from synapse.api.errors import ( AuthError, Codes, @@ -29,12 +28,11 @@ MissingClientTokenError, ) from synapse.appservice import ApplicationService -from synapse.events import EventBase from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest from synapse.logging import opentracing as opentracing from synapse.storage.databases.main.registration import TokenLookupResult -from synapse.types import Requester, StateMap, UserID, create_requester +from synapse.types import Requester, UserID, create_requester from synapse.util.caches.lrucache import LruCache from synapse.util.macaroons import get_value_from_macaroon, satisfy_expiry @@ -87,56 +85,6 @@ def __init__(self, hs: "HomeServer"): self._macaroon_secret_key = hs.config.macaroon_secret_key self._force_tracing_for_users = hs.config.tracing.force_tracing_for_users - async def check_user_in_room( - self, - room_id: str, - user_id: str, - current_state: Optional[StateMap[EventBase]] = None, - allow_departed_users: bool = False, - ) -> EventBase: - """Check if the user is in the room, or was at some point. - Args: - room_id: The room to check. - - user_id: The user to check. - - current_state: Optional map of the current state of the room. - If provided then that map is used to check whether they are a - member of the room. Otherwise the current membership is - loaded from the database. - - allow_departed_users: if True, accept users that were previously - members but have now departed. - - Raises: - AuthError if the user is/was not in the room. - Returns: - Membership event for the user if the user was in the - room. This will be the join event if they are currently joined to - the room. This will be the leave event if they have left the room. - """ - if current_state: - member = current_state.get((EventTypes.Member, user_id), None) - else: - member = await self.state.get_current_state( - room_id=room_id, event_type=EventTypes.Member, state_key=user_id - ) - - if member: - membership = member.membership - - if membership == Membership.JOIN: - return member - - # XXX this looks totally bogus. Why do we not allow users who have been banned, - # or those who were members previously and have been re-invited? - if allow_departed_users and membership == Membership.LEAVE: - forgot = await self.store.did_forget(user_id, room_id) - if not forgot: - return member - - raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) - async def get_user_by_req( self, request: SynapseRequest, @@ -467,40 +415,6 @@ async def is_server_admin(self, user: UserID) -> bool: """ return await self.store.is_server_admin(user) - async def check_can_change_room_list(self, room_id: str, user: UserID) -> bool: - """Determine whether the user is allowed to edit the room's entry in the - published room list. - - Args: - room_id - user - """ - - is_admin = await self.is_server_admin(user) - if is_admin: - return True - - user_id = user.to_string() - await self.check_user_in_room(room_id, user_id) - - # We currently require the user is a "moderator" in the room. We do this - # by checking if they would (theoretically) be able to change the - # m.room.canonical_alias events - power_level_event = await self.state.get_current_state( - room_id, EventTypes.PowerLevels, "" - ) - - auth_events = {} - if power_level_event: - auth_events[(EventTypes.PowerLevels, "")] = power_level_event - - send_level = event_auth.get_send_level( - EventTypes.CanonicalAlias, "", power_level_event - ) - user_level = event_auth.get_user_power_level(user_id, auth_events) - - return user_level >= send_level - @staticmethod def has_access_token(request: Request) -> bool: """Checks if the request has an access_token. @@ -553,49 +467,5 @@ def get_access_token_from_request(request: Request) -> str: return query_params[0].decode("ascii") - async def check_user_in_room_or_world_readable( - self, room_id: str, user_id: str, allow_departed_users: bool = False - ) -> Tuple[str, Optional[str]]: - """Checks that the user is or was in the room or the room is world - readable. If it isn't then an exception is raised. - - Args: - room_id: room to check - user_id: user to check - allow_departed_users: if True, accept users that were previously - members but have now departed - - Returns: - Resolves to the current membership of the user in the room and the - membership event ID of the user. If the user is not in the room and - never has been, then `(Membership.JOIN, None)` is returned. - """ - - try: - # check_user_in_room will return the most recent membership - # event for the user if: - # * The user is a non-guest user, and was ever in the room - # * The user is a guest user, and has joined the room - # else it will throw. - member_event = await self.check_user_in_room( - room_id, user_id, allow_departed_users=allow_departed_users - ) - return member_event.membership, member_event.event_id - except AuthError: - visibility = await self.state.get_current_state( - room_id, EventTypes.RoomHistoryVisibility, "" - ) - if ( - visibility - and visibility.content.get("history_visibility") - == HistoryVisibility.WORLD_READABLE - ): - return Membership.JOIN, None - raise AuthError( - 403, - "User %s not in room %s, and room previews are disabled" - % (user_id, room_id), - ) - async def check_auth_blocking(self, *args, **kwargs) -> None: await self._auth_blocking.check_auth_blocking(*args, **kwargs) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 89bcf8151589..4ce4f4814f37 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -14,7 +14,7 @@ # limitations under the License. import logging -from typing import Any, Dict, List, Optional, Set, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple, Union from canonicaljson import encode_canonical_json from signedjson.key import decode_verify_key_bytes @@ -29,9 +29,11 @@ RoomVersion, ) from synapse.events import EventBase -from synapse.events.builder import EventBuilder from synapse.types import StateMap, UserID, get_domain_from_id +if TYPE_CHECKING: + from synapse.events.builder import EventBuilder + logger = logging.getLogger(__name__) @@ -725,7 +727,9 @@ def get_public_keys(invite_event: EventBase) -> List[Dict[str, Any]]: return public_keys -def auth_types_for_event(event: Union[EventBase, EventBuilder]) -> Set[Tuple[str, str]]: +def auth_types_for_event( + event: Union[EventBase, "EventBuilder"] +) -> Set[Tuple[str, str]]: """Given an event, return a list of (EventType, StateKey) that may be needed to auth the event. The returned list may be a superset of what would actually be required depending on the full state of the room. diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index 4064a2b85913..dc4ad7c37717 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -45,6 +45,7 @@ def __init__(self, hs: "HomeServer"): self.state = hs.get_state_handler() self.appservice_handler = hs.get_application_service_handler() self.event_creation_handler = hs.get_event_creation_handler() + self._event_auth_handler = hs.get_event_auth_handler() self.store = hs.get_datastore() self.config = hs.config self.enable_room_list_search = hs.config.enable_room_list_search @@ -403,7 +404,7 @@ async def _user_can_delete_alias(self, alias: RoomAlias, user_id: str) -> bool: if not room_id: return False - return await self.auth.check_can_change_room_list( + return await self._event_auth_handler.check_can_change_room_list( room_id, UserID.from_string(user_id) ) @@ -439,8 +440,10 @@ async def edit_published_room_list( if room is None: raise SynapseError(400, "Unknown room") - can_change_room_list = await self.auth.check_can_change_room_list( - room_id, requester.user + can_change_room_list = ( + await self._event_auth_handler.check_can_change_room_list( + room_id, requester.user + ) ) if not can_change_room_list: raise AuthError( @@ -503,7 +506,7 @@ async def get_aliases_for_room( # allow access to server admins and current members of the room is_admin = await self.auth.is_server_admin(requester.user) if not is_admin: - await self.auth.check_user_in_room_or_world_readable( + await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, requester.user.to_string() ) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 41dbdfd0a1b6..9235419806cb 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -11,11 +11,12 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Collection, List, Optional, Union +from typing import TYPE_CHECKING, Collection, List, Optional, Tuple, Union from synapse import event_auth from synapse.api.constants import ( EventTypes, + HistoryVisibility, JoinRules, Membership, RestrictedJoinRuleTypes, @@ -24,7 +25,7 @@ from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.events import EventBase from synapse.events.builder import EventBuilder -from synapse.types import StateMap +from synapse.types import StateMap, UserID from synapse.util.metrics import Measure if TYPE_CHECKING: @@ -39,6 +40,7 @@ class EventAuthHandler: def __init__(self, hs: "HomeServer"): self._clock = hs.get_clock() self._store = hs.get_datastore() + self._state_handler = hs.get_state_handler() async def check_from_context( self, room_version: str, event, context, do_sig_check=True @@ -90,10 +92,138 @@ def compute_auth_events( return auth_ids + async def check_user_in_room( + self, + room_id: str, + user_id: str, + current_state: Optional[StateMap[EventBase]] = None, + allow_departed_users: bool = False, + ) -> EventBase: + """Check if the user is in the room, or was at some point. + Args: + room_id: The room to check. + + user_id: The user to check. + + current_state: Optional map of the current state of the room. + If provided then that map is used to check whether they are a + member of the room. Otherwise the current membership is + loaded from the database. + + allow_departed_users: if True, accept users that were previously + members but have now departed. + + Raises: + AuthError if the user is/was not in the room. + Returns: + Membership event for the user if the user was in the + room. This will be the join event if they are currently joined to + the room. This will be the leave event if they have left the room. + """ + if current_state: + member = current_state.get((EventTypes.Member, user_id), None) + else: + member = await self._state_handler.get_current_state( + room_id=room_id, event_type=EventTypes.Member, state_key=user_id + ) + + if member: + membership = member.membership + + if membership == Membership.JOIN: + return member + + # XXX this looks totally bogus. Why do we not allow users who have been banned, + # or those who were members previously and have been re-invited? + if allow_departed_users and membership == Membership.LEAVE: + forgot = await self._store.did_forget(user_id, room_id) + if not forgot: + return member + + raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) + + async def check_user_in_room_or_world_readable( + self, room_id: str, user_id: str, allow_departed_users: bool = False + ) -> Tuple[str, Optional[str]]: + """Checks that the user is or was in the room or the room is world + readable. If it isn't then an exception is raised. + + Args: + room_id: room to check + user_id: user to check + allow_departed_users: if True, accept users that were previously + members but have now departed + + Returns: + Resolves to the current membership of the user in the room and the + membership event ID of the user. If the user is not in the room and + never has been, then `(Membership.JOIN, None)` is returned. + """ + + try: + # check_user_in_room will return the most recent membership + # event for the user if: + # * The user is a non-guest user, and was ever in the room + # * The user is a guest user, and has joined the room + # else it will throw. + member_event = await self.check_user_in_room( + room_id, user_id, allow_departed_users=allow_departed_users + ) + return member_event.membership, member_event.event_id + except AuthError: + visibility = await self._state_handler.get_current_state( + room_id, EventTypes.RoomHistoryVisibility, "" + ) + if ( + visibility + and visibility.content.get("history_visibility") + == HistoryVisibility.WORLD_READABLE + ): + return Membership.JOIN, None + raise AuthError( + 403, + "User %s not in room %s, and room previews are disabled" + % (user_id, room_id), + ) + async def check_host_in_room(self, room_id: str, host: str) -> bool: with Measure(self._clock, "check_host_in_room"): return await self._store.is_host_joined(room_id, host) + async def check_can_change_room_list(self, room_id: str, user: UserID) -> bool: + """Determine whether the user is allowed to edit the room's entry in the + published room list. + + Args: + room_id + user + """ + + is_admin = await self._store.is_server_admin(user) + if is_admin: + return True + + user_id = user.to_string() + await self.check_user_in_room(room_id, user_id) + + # We currently require the user is a "moderator" in the room. We do this + # by checking if they would (theoretically) be able to change the + # m.room.canonical_alias events + power_level_event = await self._state_handler.get_current_state( + room_id, EventTypes.PowerLevels, "" + ) + + auth_events = {} + if power_level_event: + auth_events[(EventTypes.PowerLevels, "")] = power_level_event + + send_level = event_auth.get_send_level( + EventTypes.CanonicalAlias, "", power_level_event + ) + user_level = event_auth.get_user_power_level(user_id, auth_events) + + return user_level >= send_level + async def check_restricted_join_rules( self, state_ids: StateMap[str], diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 76242865ae28..cac9733ce291 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -44,6 +44,7 @@ def __init__(self, hs: "HomeServer"): super().__init__(hs) self.hs = hs self.state = hs.get_state_handler() + self._event_auth_handler = hs.get_event_auth_handler() self.clock = hs.get_clock() self.validator = EventValidator() self.snapshot_cache = ResponseCache( @@ -286,7 +287,7 @@ async def room_initial_sync( ( membership, member_event_id, - ) = await self.auth.check_user_in_room_or_world_readable( + ) = await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True, diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 66e40a915d04..9a31f5aba3b7 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -73,6 +73,7 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() self.clock = hs.get_clock() self.state = hs.get_state_handler() + self._event_auth_handler = hs.get_event_auth_handler() self.store = hs.get_datastore() self.storage = hs.get_storage() self.state_store = self.storage.state @@ -110,7 +111,7 @@ async def get_room_data( ( membership, membership_event_id, - ) = await self.auth.check_user_in_room_or_world_readable( + ) = await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) @@ -209,7 +210,7 @@ async def get_state_events( ( membership, membership_event_id, - ) = await self.auth.check_user_in_room_or_world_readable( + ) = await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) @@ -253,7 +254,10 @@ async def get_joined_members(self, requester: Requester, room_id: str) -> dict: if not requester.app_service: # We check AS auth after fetching the room membership, as it # requires us to pull out all joined members anyway. - membership, _ = await self.auth.check_user_in_room_or_world_readable( + ( + membership, + _, + ) = await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) if membership != Membership.JOIN: diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 1e1186c29e7d..9a8dd26bcd12 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -73,7 +73,7 @@ class PaginationHandler: def __init__(self, hs: "HomeServer"): self.hs = hs - self.auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() self.store = hs.get_datastore() self.storage = hs.get_storage() self.state_store = self.storage.state @@ -360,7 +360,7 @@ async def get_messages( ( membership, member_event_id, - ) = await self.auth.check_user_in_room_or_world_readable( + ) = await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index b585057ec3a5..eae74e9ed871 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -83,7 +83,9 @@ async def get_space_summary( """ # first of all, check that the user is in the room in question (or it's # world-readable) - await self._auth.check_user_in_room_or_world_readable(room_id, requester) + await self._event_auth_handler.check_user_in_room_or_world_readable( + room_id, requester + ) # the queue of rooms to process room_queue = deque((_RoomQueueEntry(room_id, ()),)) diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index e22393adc48d..e91640c46231 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -206,7 +206,7 @@ def __init__(self, hs: "HomeServer"): assert hs.config.worker.writers.typing == hs.get_instance_name() - self.auth = hs.get_auth() + self._event_auth_handler = hs.get_event_auth_handler() self.notifier = hs.get_notifier() self.hs = hs @@ -253,7 +253,7 @@ async def started_typing( await self.clock.sleep(random.randint(1, 10)) raise ShadowBanError() - await self.auth.check_user_in_room(room_id, target_user_id) + await self._event_auth_handler.check_user_in_room(room_id, target_user_id) logger.debug("%s has started typing in %s", target_user_id, room_id) @@ -289,7 +289,7 @@ async def stopped_typing( await self.clock.sleep(random.randint(1, 10)) raise ShadowBanError() - await self.auth.check_user_in_room(room_id, target_user_id) + await self._event_auth_handler.check_user_in_room(room_id, target_user_id) logger.debug("%s has stopped typing in %s", target_user_id, room_id) diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index c7da6759dbf8..29b3cad6bdc1 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -143,13 +143,14 @@ def __init__(self, hs): self.clock = hs.get_clock() self._event_serializer = hs.get_event_client_serializer() self.event_handler = hs.get_event_handler() + self._event_auth_handler = hs.get_event_auth_handler() async def on_GET( self, request, room_id, parent_id, relation_type=None, event_type=None ): requester = await self.auth.get_user_by_req(request, allow_guest=True) - await self.auth.check_user_in_room_or_world_readable( + await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, requester.user.to_string(), allow_departed_users=True ) @@ -236,13 +237,14 @@ def __init__(self, hs): self.auth = hs.get_auth() self.store = hs.get_datastore() self.event_handler = hs.get_event_handler() + self._event_auth_handler = hs.get_event_auth_handler() async def on_GET( self, request, room_id, parent_id, relation_type=None, event_type=None ): requester = await self.auth.get_user_by_req(request, allow_guest=True) - await self.auth.check_user_in_room_or_world_readable( + await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, requester.user.to_string(), allow_departed_users=True, @@ -318,11 +320,12 @@ def __init__(self, hs): self.clock = hs.get_clock() self._event_serializer = hs.get_event_client_serializer() self.event_handler = hs.get_event_handler() + self._event_auth_handler = hs.get_event_auth_handler() async def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): requester = await self.auth.get_user_by_req(request, allow_guest=True) - await self.auth.check_user_in_room_or_world_readable( + await self._event_auth_handler.check_user_in_room_or_world_readable( room_id, requester.user.to_string(), allow_departed_users=True, diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index f58afbc244a8..e3f7eae204aa 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -113,7 +113,7 @@ async def check_user_in_room(room_id, user_id): raise AuthError(401, "User is not in the room") return None - hs.get_auth().check_user_in_room = check_user_in_room + hs.get_event_auth_handler().check_user_in_room = check_user_in_room def get_joined_hosts_for_room(room_id): return {member.domain for member in self.room_members} From af81dba2805be44c8eb4a0785e019617de54f2c6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 08:17:30 -0400 Subject: [PATCH 6/9] Remove unused constant. --- synapse/api/auth.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 9d1e0cf8b88b..a3d4d4302e08 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -20,7 +20,6 @@ from twisted.web.server import Request from synapse.api.auth_blocking import AuthBlocking -from synapse.api.constants import EventTypes from synapse.api.errors import ( AuthError, Codes, @@ -42,15 +41,6 @@ logger = logging.getLogger(__name__) -AuthEventTypes = ( - EventTypes.Create, - EventTypes.Member, - EventTypes.PowerLevels, - EventTypes.JoinRules, - EventTypes.RoomHistoryVisibility, - EventTypes.ThirdPartyInvite, -) - # guests always get this device id. GUEST_DEVICE_ID = "guest_device" From 83db39306a37cf021983d7411e070e46cefcbe8d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 08:18:03 -0400 Subject: [PATCH 7/9] Remove FIXME comment. --- synapse/api/auth.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index a3d4d4302e08..8bb8655c4056 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -51,9 +51,7 @@ class _InvalidMacaroonException(Exception): class Auth: """ - FIXME: This class contains a mix of functions for authenticating users - of our client-server API and authenticating events added to room graphs. - The latter should be moved to synapse.handlers.event_auth.EventAuthHandler. + This class contains functions for authenticating users of our client-server API. """ def __init__(self, hs: "HomeServer"): From bb48a9b8384f8fdc61a48efbc3287ffb3ac0f3d4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 29 Jun 2021 10:43:26 -0400 Subject: [PATCH 8/9] Newsfragment --- changelog.d/10268.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10268.misc diff --git a/changelog.d/10268.misc b/changelog.d/10268.misc new file mode 100644 index 000000000000..9e3f60c72fd2 --- /dev/null +++ b/changelog.d/10268.misc @@ -0,0 +1 @@ +Move event authentication methods from `Auth` to `EventAuthHandler`. From 19424a98c0a1228c2f069b3efb00577680b60121 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 1 Jul 2021 07:46:21 -0400 Subject: [PATCH 9/9] Revert "Move check_user_in_room, check_user_in_room_or_world_readable, and check_can_change_room_list to EventAuthHandler." This reverts commit 3b07593f2e793501da7b9053616182db4da31a4c. --- synapse/api/auth.py | 133 ++++++++++++++++++++- synapse/event_auth.py | 10 +- synapse/handlers/directory.py | 11 +- synapse/handlers/event_auth.py | 134 +--------------------- synapse/handlers/initial_sync.py | 3 +- synapse/handlers/message.py | 10 +- synapse/handlers/pagination.py | 4 +- synapse/handlers/space_summary.py | 4 +- synapse/handlers/typing.py | 6 +- synapse/rest/client/v2_alpha/relations.py | 9 +- tests/handlers/test_typing.py | 2 +- 11 files changed, 155 insertions(+), 171 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 8bb8655c4056..307f5f9a9463 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -19,7 +19,9 @@ from twisted.web.server import Request +from synapse import event_auth from synapse.api.auth_blocking import AuthBlocking +from synapse.api.constants import EventTypes, HistoryVisibility, Membership from synapse.api.errors import ( AuthError, Codes, @@ -27,11 +29,12 @@ MissingClientTokenError, ) from synapse.appservice import ApplicationService +from synapse.events import EventBase from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest from synapse.logging import opentracing as opentracing from synapse.storage.databases.main.registration import TokenLookupResult -from synapse.types import Requester, UserID, create_requester +from synapse.types import Requester, StateMap, UserID, create_requester from synapse.util.caches.lrucache import LruCache from synapse.util.macaroons import get_value_from_macaroon, satisfy_expiry @@ -73,6 +76,56 @@ def __init__(self, hs: "HomeServer"): self._macaroon_secret_key = hs.config.macaroon_secret_key self._force_tracing_for_users = hs.config.tracing.force_tracing_for_users + async def check_user_in_room( + self, + room_id: str, + user_id: str, + current_state: Optional[StateMap[EventBase]] = None, + allow_departed_users: bool = False, + ) -> EventBase: + """Check if the user is in the room, or was at some point. + Args: + room_id: The room to check. + + user_id: The user to check. + + current_state: Optional map of the current state of the room. + If provided then that map is used to check whether they are a + member of the room. Otherwise the current membership is + loaded from the database. + + allow_departed_users: if True, accept users that were previously + members but have now departed. + + Raises: + AuthError if the user is/was not in the room. + Returns: + Membership event for the user if the user was in the + room. This will be the join event if they are currently joined to + the room. This will be the leave event if they have left the room. + """ + if current_state: + member = current_state.get((EventTypes.Member, user_id), None) + else: + member = await self.state.get_current_state( + room_id=room_id, event_type=EventTypes.Member, state_key=user_id + ) + + if member: + membership = member.membership + + if membership == Membership.JOIN: + return member + + # XXX this looks totally bogus. Why do we not allow users who have been banned, + # or those who were members previously and have been re-invited? + if allow_departed_users and membership == Membership.LEAVE: + forgot = await self.store.did_forget(user_id, room_id) + if not forgot: + return member + + raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) + async def get_user_by_req( self, request: SynapseRequest, @@ -403,6 +456,40 @@ async def is_server_admin(self, user: UserID) -> bool: """ return await self.store.is_server_admin(user) + async def check_can_change_room_list(self, room_id: str, user: UserID) -> bool: + """Determine whether the user is allowed to edit the room's entry in the + published room list. + + Args: + room_id + user + """ + + is_admin = await self.is_server_admin(user) + if is_admin: + return True + + user_id = user.to_string() + await self.check_user_in_room(room_id, user_id) + + # We currently require the user is a "moderator" in the room. We do this + # by checking if they would (theoretically) be able to change the + # m.room.canonical_alias events + power_level_event = await self.state.get_current_state( + room_id, EventTypes.PowerLevels, "" + ) + + auth_events = {} + if power_level_event: + auth_events[(EventTypes.PowerLevels, "")] = power_level_event + + send_level = event_auth.get_send_level( + EventTypes.CanonicalAlias, "", power_level_event + ) + user_level = event_auth.get_user_power_level(user_id, auth_events) + + return user_level >= send_level + @staticmethod def has_access_token(request: Request) -> bool: """Checks if the request has an access_token. @@ -455,5 +542,49 @@ def get_access_token_from_request(request: Request) -> str: return query_params[0].decode("ascii") + async def check_user_in_room_or_world_readable( + self, room_id: str, user_id: str, allow_departed_users: bool = False + ) -> Tuple[str, Optional[str]]: + """Checks that the user is or was in the room or the room is world + readable. If it isn't then an exception is raised. + + Args: + room_id: room to check + user_id: user to check + allow_departed_users: if True, accept users that were previously + members but have now departed + + Returns: + Resolves to the current membership of the user in the room and the + membership event ID of the user. If the user is not in the room and + never has been, then `(Membership.JOIN, None)` is returned. + """ + + try: + # check_user_in_room will return the most recent membership + # event for the user if: + # * The user is a non-guest user, and was ever in the room + # * The user is a guest user, and has joined the room + # else it will throw. + member_event = await self.check_user_in_room( + room_id, user_id, allow_departed_users=allow_departed_users + ) + return member_event.membership, member_event.event_id + except AuthError: + visibility = await self.state.get_current_state( + room_id, EventTypes.RoomHistoryVisibility, "" + ) + if ( + visibility + and visibility.content.get("history_visibility") + == HistoryVisibility.WORLD_READABLE + ): + return Membership.JOIN, None + raise AuthError( + 403, + "User %s not in room %s, and room previews are disabled" + % (user_id, room_id), + ) + async def check_auth_blocking(self, *args, **kwargs) -> None: await self._auth_blocking.check_auth_blocking(*args, **kwargs) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 4ce4f4814f37..89bcf8151589 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -14,7 +14,7 @@ # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple, Union +from typing import Any, Dict, List, Optional, Set, Tuple, Union from canonicaljson import encode_canonical_json from signedjson.key import decode_verify_key_bytes @@ -29,11 +29,9 @@ RoomVersion, ) from synapse.events import EventBase +from synapse.events.builder import EventBuilder from synapse.types import StateMap, UserID, get_domain_from_id -if TYPE_CHECKING: - from synapse.events.builder import EventBuilder - logger = logging.getLogger(__name__) @@ -727,9 +725,7 @@ def get_public_keys(invite_event: EventBase) -> List[Dict[str, Any]]: return public_keys -def auth_types_for_event( - event: Union[EventBase, "EventBuilder"] -) -> Set[Tuple[str, str]]: +def auth_types_for_event(event: Union[EventBase, EventBuilder]) -> Set[Tuple[str, str]]: """Given an event, return a list of (EventType, StateKey) that may be needed to auth the event. The returned list may be a superset of what would actually be required depending on the full state of the room. diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index dc4ad7c37717..4064a2b85913 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -45,7 +45,6 @@ def __init__(self, hs: "HomeServer"): self.state = hs.get_state_handler() self.appservice_handler = hs.get_application_service_handler() self.event_creation_handler = hs.get_event_creation_handler() - self._event_auth_handler = hs.get_event_auth_handler() self.store = hs.get_datastore() self.config = hs.config self.enable_room_list_search = hs.config.enable_room_list_search @@ -404,7 +403,7 @@ async def _user_can_delete_alias(self, alias: RoomAlias, user_id: str) -> bool: if not room_id: return False - return await self._event_auth_handler.check_can_change_room_list( + return await self.auth.check_can_change_room_list( room_id, UserID.from_string(user_id) ) @@ -440,10 +439,8 @@ async def edit_published_room_list( if room is None: raise SynapseError(400, "Unknown room") - can_change_room_list = ( - await self._event_auth_handler.check_can_change_room_list( - room_id, requester.user - ) + can_change_room_list = await self.auth.check_can_change_room_list( + room_id, requester.user ) if not can_change_room_list: raise AuthError( @@ -506,7 +503,7 @@ async def get_aliases_for_room( # allow access to server admins and current members of the room is_admin = await self.auth.is_server_admin(requester.user) if not is_admin: - await self._event_auth_handler.check_user_in_room_or_world_readable( + await self.auth.check_user_in_room_or_world_readable( room_id, requester.user.to_string() ) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 9235419806cb..41dbdfd0a1b6 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -11,12 +11,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Collection, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Collection, List, Optional, Union from synapse import event_auth from synapse.api.constants import ( EventTypes, - HistoryVisibility, JoinRules, Membership, RestrictedJoinRuleTypes, @@ -25,7 +24,7 @@ from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.events import EventBase from synapse.events.builder import EventBuilder -from synapse.types import StateMap, UserID +from synapse.types import StateMap from synapse.util.metrics import Measure if TYPE_CHECKING: @@ -40,7 +39,6 @@ class EventAuthHandler: def __init__(self, hs: "HomeServer"): self._clock = hs.get_clock() self._store = hs.get_datastore() - self._state_handler = hs.get_state_handler() async def check_from_context( self, room_version: str, event, context, do_sig_check=True @@ -92,138 +90,10 @@ def compute_auth_events( return auth_ids - async def check_user_in_room( - self, - room_id: str, - user_id: str, - current_state: Optional[StateMap[EventBase]] = None, - allow_departed_users: bool = False, - ) -> EventBase: - """Check if the user is in the room, or was at some point. - Args: - room_id: The room to check. - - user_id: The user to check. - - current_state: Optional map of the current state of the room. - If provided then that map is used to check whether they are a - member of the room. Otherwise the current membership is - loaded from the database. - - allow_departed_users: if True, accept users that were previously - members but have now departed. - - Raises: - AuthError if the user is/was not in the room. - Returns: - Membership event for the user if the user was in the - room. This will be the join event if they are currently joined to - the room. This will be the leave event if they have left the room. - """ - if current_state: - member = current_state.get((EventTypes.Member, user_id), None) - else: - member = await self._state_handler.get_current_state( - room_id=room_id, event_type=EventTypes.Member, state_key=user_id - ) - - if member: - membership = member.membership - - if membership == Membership.JOIN: - return member - - # XXX this looks totally bogus. Why do we not allow users who have been banned, - # or those who were members previously and have been re-invited? - if allow_departed_users and membership == Membership.LEAVE: - forgot = await self._store.did_forget(user_id, room_id) - if not forgot: - return member - - raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) - - async def check_user_in_room_or_world_readable( - self, room_id: str, user_id: str, allow_departed_users: bool = False - ) -> Tuple[str, Optional[str]]: - """Checks that the user is or was in the room or the room is world - readable. If it isn't then an exception is raised. - - Args: - room_id: room to check - user_id: user to check - allow_departed_users: if True, accept users that were previously - members but have now departed - - Returns: - Resolves to the current membership of the user in the room and the - membership event ID of the user. If the user is not in the room and - never has been, then `(Membership.JOIN, None)` is returned. - """ - - try: - # check_user_in_room will return the most recent membership - # event for the user if: - # * The user is a non-guest user, and was ever in the room - # * The user is a guest user, and has joined the room - # else it will throw. - member_event = await self.check_user_in_room( - room_id, user_id, allow_departed_users=allow_departed_users - ) - return member_event.membership, member_event.event_id - except AuthError: - visibility = await self._state_handler.get_current_state( - room_id, EventTypes.RoomHistoryVisibility, "" - ) - if ( - visibility - and visibility.content.get("history_visibility") - == HistoryVisibility.WORLD_READABLE - ): - return Membership.JOIN, None - raise AuthError( - 403, - "User %s not in room %s, and room previews are disabled" - % (user_id, room_id), - ) - async def check_host_in_room(self, room_id: str, host: str) -> bool: with Measure(self._clock, "check_host_in_room"): return await self._store.is_host_joined(room_id, host) - async def check_can_change_room_list(self, room_id: str, user: UserID) -> bool: - """Determine whether the user is allowed to edit the room's entry in the - published room list. - - Args: - room_id - user - """ - - is_admin = await self._store.is_server_admin(user) - if is_admin: - return True - - user_id = user.to_string() - await self.check_user_in_room(room_id, user_id) - - # We currently require the user is a "moderator" in the room. We do this - # by checking if they would (theoretically) be able to change the - # m.room.canonical_alias events - power_level_event = await self._state_handler.get_current_state( - room_id, EventTypes.PowerLevels, "" - ) - - auth_events = {} - if power_level_event: - auth_events[(EventTypes.PowerLevels, "")] = power_level_event - - send_level = event_auth.get_send_level( - EventTypes.CanonicalAlias, "", power_level_event - ) - user_level = event_auth.get_user_power_level(user_id, auth_events) - - return user_level >= send_level - async def check_restricted_join_rules( self, state_ids: StateMap[str], diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index cac9733ce291..76242865ae28 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -44,7 +44,6 @@ def __init__(self, hs: "HomeServer"): super().__init__(hs) self.hs = hs self.state = hs.get_state_handler() - self._event_auth_handler = hs.get_event_auth_handler() self.clock = hs.get_clock() self.validator = EventValidator() self.snapshot_cache = ResponseCache( @@ -287,7 +286,7 @@ async def room_initial_sync( ( membership, member_event_id, - ) = await self._event_auth_handler.check_user_in_room_or_world_readable( + ) = await self.auth.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True, diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 9a31f5aba3b7..66e40a915d04 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -73,7 +73,6 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() self.clock = hs.get_clock() self.state = hs.get_state_handler() - self._event_auth_handler = hs.get_event_auth_handler() self.store = hs.get_datastore() self.storage = hs.get_storage() self.state_store = self.storage.state @@ -111,7 +110,7 @@ async def get_room_data( ( membership, membership_event_id, - ) = await self._event_auth_handler.check_user_in_room_or_world_readable( + ) = await self.auth.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) @@ -210,7 +209,7 @@ async def get_state_events( ( membership, membership_event_id, - ) = await self._event_auth_handler.check_user_in_room_or_world_readable( + ) = await self.auth.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) @@ -254,10 +253,7 @@ async def get_joined_members(self, requester: Requester, room_id: str) -> dict: if not requester.app_service: # We check AS auth after fetching the room membership, as it # requires us to pull out all joined members anyway. - ( - membership, - _, - ) = await self._event_auth_handler.check_user_in_room_or_world_readable( + membership, _ = await self.auth.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) if membership != Membership.JOIN: diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 9a8dd26bcd12..1e1186c29e7d 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -73,7 +73,7 @@ class PaginationHandler: def __init__(self, hs: "HomeServer"): self.hs = hs - self._event_auth_handler = hs.get_event_auth_handler() + self.auth = hs.get_auth() self.store = hs.get_datastore() self.storage = hs.get_storage() self.state_store = self.storage.state @@ -360,7 +360,7 @@ async def get_messages( ( membership, member_event_id, - ) = await self._event_auth_handler.check_user_in_room_or_world_readable( + ) = await self.auth.check_user_in_room_or_world_readable( room_id, user_id, allow_departed_users=True ) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index eae74e9ed871..b585057ec3a5 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -83,9 +83,7 @@ async def get_space_summary( """ # first of all, check that the user is in the room in question (or it's # world-readable) - await self._event_auth_handler.check_user_in_room_or_world_readable( - room_id, requester - ) + await self._auth.check_user_in_room_or_world_readable(room_id, requester) # the queue of rooms to process room_queue = deque((_RoomQueueEntry(room_id, ()),)) diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index e91640c46231..e22393adc48d 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -206,7 +206,7 @@ def __init__(self, hs: "HomeServer"): assert hs.config.worker.writers.typing == hs.get_instance_name() - self._event_auth_handler = hs.get_event_auth_handler() + self.auth = hs.get_auth() self.notifier = hs.get_notifier() self.hs = hs @@ -253,7 +253,7 @@ async def started_typing( await self.clock.sleep(random.randint(1, 10)) raise ShadowBanError() - await self._event_auth_handler.check_user_in_room(room_id, target_user_id) + await self.auth.check_user_in_room(room_id, target_user_id) logger.debug("%s has started typing in %s", target_user_id, room_id) @@ -289,7 +289,7 @@ async def stopped_typing( await self.clock.sleep(random.randint(1, 10)) raise ShadowBanError() - await self._event_auth_handler.check_user_in_room(room_id, target_user_id) + await self.auth.check_user_in_room(room_id, target_user_id) logger.debug("%s has stopped typing in %s", target_user_id, room_id) diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index 29b3cad6bdc1..c7da6759dbf8 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -143,14 +143,13 @@ def __init__(self, hs): self.clock = hs.get_clock() self._event_serializer = hs.get_event_client_serializer() self.event_handler = hs.get_event_handler() - self._event_auth_handler = hs.get_event_auth_handler() async def on_GET( self, request, room_id, parent_id, relation_type=None, event_type=None ): requester = await self.auth.get_user_by_req(request, allow_guest=True) - await self._event_auth_handler.check_user_in_room_or_world_readable( + await self.auth.check_user_in_room_or_world_readable( room_id, requester.user.to_string(), allow_departed_users=True ) @@ -237,14 +236,13 @@ def __init__(self, hs): self.auth = hs.get_auth() self.store = hs.get_datastore() self.event_handler = hs.get_event_handler() - self._event_auth_handler = hs.get_event_auth_handler() async def on_GET( self, request, room_id, parent_id, relation_type=None, event_type=None ): requester = await self.auth.get_user_by_req(request, allow_guest=True) - await self._event_auth_handler.check_user_in_room_or_world_readable( + await self.auth.check_user_in_room_or_world_readable( room_id, requester.user.to_string(), allow_departed_users=True, @@ -320,12 +318,11 @@ def __init__(self, hs): self.clock = hs.get_clock() self._event_serializer = hs.get_event_client_serializer() self.event_handler = hs.get_event_handler() - self._event_auth_handler = hs.get_event_auth_handler() async def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): requester = await self.auth.get_user_by_req(request, allow_guest=True) - await self._event_auth_handler.check_user_in_room_or_world_readable( + await self.auth.check_user_in_room_or_world_readable( room_id, requester.user.to_string(), allow_departed_users=True, diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index e3f7eae204aa..f58afbc244a8 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -113,7 +113,7 @@ async def check_user_in_room(room_id, user_id): raise AuthError(401, "User is not in the room") return None - hs.get_event_auth_handler().check_user_in_room = check_user_in_room + hs.get_auth().check_user_in_room = check_user_in_room def get_joined_hosts_for_room(room_id): return {member.domain for member in self.room_members}