Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Type checking for FederationHandler #7770

Merged
merged 1 commit into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7770.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix up `synapse.handlers.federation` to pass mypy.
47 changes: 30 additions & 17 deletions synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

import itertools
import logging
from collections import Container
from http import HTTPStatus
from typing import Dict, Iterable, List, Optional, Sequence, Tuple
from typing import Dict, Iterable, List, Optional, Sequence, Tuple, Union

import attr
from signedjson.key import decode_verify_key_bytes
Expand Down Expand Up @@ -742,6 +743,9 @@ async def _process_received_pdu(
# device and recognize the algorithm then we can work out the
# exact key to expect. Otherwise check it matches any key we
# have for that device.

current_keys = [] # type: Container[str]

if device:
keys = device.get("keys", {}).get("keys", {})

Expand All @@ -758,15 +762,15 @@ async def _process_received_pdu(
current_keys = keys.values()
elif device_id:
# We don't have any keys for the device ID.
current_keys = []
pass
else:
# The event didn't include a device ID, so we just look for
# keys across all devices.
current_keys = (
current_keys = [
key
for device in cached_devices
for key in device.get("keys", {}).get("keys", {}).values()
)
]

# We now check that the sender key matches (one of) the expected
# keys.
Expand Down Expand Up @@ -1011,7 +1015,7 @@ def get_domains_from_state(state):
if e_type == EventTypes.Member and event.membership == Membership.JOIN
]

joined_domains = {}
joined_domains = {} # type: Dict[str, int]
for u, d in joined_users:
try:
dom = get_domain_from_id(u)
Expand Down Expand Up @@ -1277,14 +1281,15 @@ async def do_invite_join(
try:
# Try the host we successfully got a response to /make_join/
# request first.
host_list = list(target_hosts)
try:
target_hosts.remove(origin)
target_hosts.insert(0, origin)
host_list.remove(origin)
host_list.insert(0, origin)
except ValueError:
pass

ret = await self.federation_client.send_join(
target_hosts, event, room_version_obj
host_list, event, room_version_obj
)

origin = ret["origin"]
Expand Down Expand Up @@ -1584,13 +1589,14 @@ async def do_remotely_reject_invite(

# Try the host that we succesfully called /make_leave/ on first for
# the /send_leave/ request.
host_list = list(target_hosts)
try:
target_hosts.remove(origin)
target_hosts.insert(0, origin)
host_list.remove(origin)
host_list.insert(0, origin)
except ValueError:
pass

await self.federation_client.send_leave(target_hosts, event)
await self.federation_client.send_leave(host_list, event)

context = await self.state_handler.compute_event_context(event)
stream_id = await self.persist_events_and_notify([(event, context)])
Expand All @@ -1604,7 +1610,7 @@ async def _make_and_verify_event(
user_id: str,
membership: str,
content: JsonDict = {},
params: Optional[Dict[str, str]] = None,
params: Optional[Dict[str, Union[str, Iterable[str]]]] = None,
) -> Tuple[str, EventBase, RoomVersion]:
(
origin,
Expand Down Expand Up @@ -2018,8 +2024,8 @@ async def _prep_event(
auth_events_ids = await self.auth.compute_auth_events(
event, prev_state_ids, for_verification=True
)
auth_events = await self.store.get_events(auth_events_ids)
auth_events = {(e.type, e.state_key): e for e in auth_events.values()}
auth_events_x = await self.store.get_events(auth_events_ids)
auth_events = {(e.type, e.state_key): e for e in auth_events_x.values()}

# This is a hack to fix some old rooms where the initial join event
# didn't reference the create event in its auth events.
Expand Down Expand Up @@ -2293,10 +2299,10 @@ async def _update_auth_events_and_context_for_auth(
remote_auth_chain = await self.federation_client.get_event_auth(
origin, event.room_id, event.event_id
)
except RequestSendFailed as e:
except RequestSendFailed as e1:
# The other side isn't around or doesn't implement the
# endpoint, so lets just bail out.
logger.info("Failed to get event auth from remote: %s", e)
logger.info("Failed to get event auth from remote: %s", e1)
return context

seen_remotes = await self.store.have_seen_events(
Expand Down Expand Up @@ -2774,7 +2780,8 @@ async def _check_signature(self, event, context):

logger.debug("Checking auth on event %r", event.content)

last_exception = None
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):
try:
Expand Down Expand Up @@ -2828,6 +2835,12 @@ async def _check_signature(self, event, context):
return
except Exception as e:
last_exception = e

if last_exception is None:
# we can only get here if get_public_keys() returned an empty list
# TODO: make this better
raise RuntimeError("no public key in invite event")
Comment on lines +2839 to +2842
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, but curious if we were actually running into it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, but I had to do something here to keep mypy happy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory I suspect we could run into it if the invite was malformed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! The result is similar either way, wasn't sure if you came across it in Sentry or something. 👍


raise last_exception

async def _check_key_revocation(self, public_key, url):
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ commands = mypy \
synapse/handlers/auth.py \
synapse/handlers/cas_handler.py \
synapse/handlers/directory.py \
synapse/handlers/federation.py \
synapse/handlers/oidc_handler.py \
synapse/handlers/presence.py \
synapse/handlers/room_member.py \
Expand Down