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

Add some clarifying comments and refactor a portion of the Keyring class for readability #14804

Merged
merged 6 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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/14804.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add some clarifying comments and refactor a portion of the `Keyring` class for readability.
30 changes: 17 additions & 13 deletions synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,11 @@ def __init__(

if key_fetchers is None:
key_fetchers = (
# Fetch keys from the database.
StoreKeyFetcher(hs),
# Fetch keys from a configured Perspectives server.
PerspectivesKeyFetcher(hs),
# Fetch keys from the origin server directly.
ServerKeyFetcher(hs),
)
self._key_fetchers = key_fetchers
Expand Down Expand Up @@ -279,6 +282,11 @@ async def process_request(self, verify_request: VerifyJsonRequest) -> None:

key_ids_to_find = set(verify_request.key_ids) - found_keys.keys()
if key_ids_to_find:
# We're still missing some keys. Consult each of our `KeyFetcher` instances
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
# (stored in `self._key_fetchers`) to try and find them.
# Key fetch attempts are queued via `self._server_queue` below, and carried
# out in `self._inner_fetch_key_requests`.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

# Add the keys we need to verify to the queue for retrieval. We queue
# up requests for the same server so we don't end up with many in flight
# requests for the same keys.
Expand Down Expand Up @@ -420,26 +428,22 @@ async def _inner_fetch_key_request(
if not key:
continue

# If we already have a result for the given key ID we keep the
# If we already have a result for the given key ID, we keep the
# one with the highest `valid_until_ts`.
existing_key = found_keys.get(key_id)
if existing_key:
if key.valid_until_ts <= existing_key.valid_until_ts:
continue
if existing_key and existing_key.valid_until_ts >= key.valid_until_ts:
continue
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

# We always store the returned key even if it doesn't the
# Check if this key's expiry timestamp is valid for the verify request.
if key.valid_until_ts >= verify_request.minimum_valid_until_ts:
# Stop looking for this key from subsequent fetchers.
missing_key_ids.discard(key_id)

# We always store the returned key even if it doesn't meet the
# `minimum_valid_until_ts` requirement, as some verification
# requests may still be able to be satisfied by it.
#
# We still keep looking for the key from other fetchers in that
# case though.
found_keys[key_id] = key

if key.valid_until_ts < verify_request.minimum_valid_until_ts:
continue

missing_key_ids.discard(key_id)

return found_keys


Expand Down