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

Correct type hints for computing auth events. #10253

Merged
merged 4 commits into from
Jun 30, 2021
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/10253.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix type hints for computing auth events.
5 changes: 3 additions & 2 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, Any, Dict, List, Optional, Tuple, Union

import pymacaroons
from netaddr import IPAddress
Expand All @@ -31,6 +31,7 @@
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
Expand Down Expand Up @@ -490,7 +491,7 @@ async def is_server_admin(self, user: UserID) -> bool:

def compute_auth_events(
self,
event,
event: Union[EventBase, EventBuilder],
current_state_ids: StateMap[str],
for_verification: bool = False,
) -> List[str]:
Expand Down
5 changes: 3 additions & 2 deletions synapse/event_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.

import logging
from typing import Any, Dict, List, Optional, Set, Tuple
from typing import Any, Dict, List, Optional, Set, Tuple, Union

from canonicaljson import encode_canonical_json
from signedjson.key import decode_verify_key_bytes
Expand All @@ -29,6 +29,7 @@
RoomVersion,
)
from synapse.events import EventBase
from synapse.events.builder import EventBuilder
from synapse.types import StateMap, UserID, get_domain_from_id

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -724,7 +725,7 @@ def get_public_keys(invite_event: EventBase) -> List[Dict[str, Any]]:
return public_keys


def auth_types_for_event(event: EventBase) -> 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.
Expand Down
2 changes: 1 addition & 1 deletion synapse/events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def __init__(self, internal_metadata_dict: JsonDict):
proactively_send = DictProperty("proactively_send") # type: bool
redacted = DictProperty("redacted") # type: bool
txn_id = DictProperty("txn_id") # type: str
token_id = DictProperty("token_id") # type: str
token_id = DictProperty("token_id") # type: int
historical = DictProperty("historical") # type: bool

# XXX: These are set by StreamWorkerStore._set_before_and_after.
Expand Down
69 changes: 36 additions & 33 deletions synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import Any, Dict, List, Optional, Tuple, Union
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union

import attr
from nacl.signing import SigningKey

from synapse.api.auth import Auth
from synapse.api.constants import MAX_DEPTH
from synapse.api.errors import UnsupportedRoomVersionError
from synapse.api.room_versions import (
Expand All @@ -34,10 +33,14 @@
from synapse.util import Clock
from synapse.util.stringutils import random_string

if TYPE_CHECKING:
from synapse.api.auth import Auth
from synapse.server import HomeServer

logger = logging.getLogger(__name__)


@attr.s(slots=True, cmp=False, frozen=True)
@attr.s(slots=True, cmp=False, frozen=True, auto_attribs=True)
class EventBuilder:
"""A format independent event builder used to build up the event content
before signing the event.
Expand All @@ -62,31 +65,30 @@ class EventBuilder:
_signing_key: The signing key to use to sign the event as the server
"""

_state = attr.ib(type=StateHandler)
_auth = attr.ib(type=Auth)
_store = attr.ib(type=DataStore)
_clock = attr.ib(type=Clock)
_hostname = attr.ib(type=str)
_signing_key = attr.ib(type=SigningKey)
_state: StateHandler
_auth: "Auth"
_store: DataStore
_clock: Clock
_hostname: str
_signing_key: SigningKey

room_version = attr.ib(type=RoomVersion)
room_version: RoomVersion

room_id = attr.ib(type=str)
type = attr.ib(type=str)
sender = attr.ib(type=str)
room_id: str
type: str
sender: str

content = attr.ib(default=attr.Factory(dict), type=JsonDict)
unsigned = attr.ib(default=attr.Factory(dict), type=JsonDict)
content: JsonDict = attr.Factory(dict)
unsigned: JsonDict = attr.Factory(dict)

# These only exist on a subset of events, so they raise AttributeError if
# someone tries to get them when they don't exist.
_state_key = attr.ib(default=None, type=Optional[str])
_redacts = attr.ib(default=None, type=Optional[str])
_origin_server_ts = attr.ib(default=None, type=Optional[int])
_state_key: Optional[str] = None
_redacts: Optional[str] = None
_origin_server_ts: Optional[int] = None

internal_metadata = attr.ib(
default=attr.Factory(lambda: _EventInternalMetadata({})),
type=_EventInternalMetadata,
internal_metadata: _EventInternalMetadata = attr.Factory(
lambda: _EventInternalMetadata({})
)

@property
Expand Down Expand Up @@ -184,7 +186,7 @@ async def build(


class EventBuilderFactory:
def __init__(self, hs):
def __init__(self, hs: "HomeServer"):
self.clock = hs.get_clock()
self.hostname = hs.hostname
self.signing_key = hs.signing_key
Expand All @@ -193,15 +195,14 @@ def __init__(self, hs):
self.state = hs.get_state_handler()
self.auth = hs.get_auth()

def new(self, room_version, key_values):
def new(self, room_version: str, key_values: dict) -> EventBuilder:
"""Generate an event builder appropriate for the given room version

Deprecated: use for_room_version with a RoomVersion object instead

Args:
room_version (str): Version of the room that we're creating an event builder
for
key_values (dict): Fields used as the basis of the new event
room_version: Version of the room that we're creating an event builder for
key_values: Fields used as the basis of the new event

Returns:
EventBuilder
Expand All @@ -212,13 +213,15 @@ def new(self, room_version, key_values):
raise UnsupportedRoomVersionError()
return self.for_room_version(v, key_values)

def for_room_version(self, room_version, key_values):
def for_room_version(
self, room_version: RoomVersion, key_values: dict
) -> EventBuilder:
"""Generate an event builder appropriate for the given room version

Args:
room_version (synapse.api.room_versions.RoomVersion):
room_version:
Version of the room that we're creating an event builder for
key_values (dict): Fields used as the basis of the new event
key_values: Fields used as the basis of the new event

Returns:
EventBuilder
Expand Down Expand Up @@ -286,15 +289,15 @@ def create_local_event_from_event_dict(
_event_id_counter = 0


def _create_event_id(clock, hostname):
def _create_event_id(clock: Clock, hostname: str) -> str:
"""Create a new event ID

Args:
clock (Clock)
hostname (str): The server name for the event ID
clock
hostname: The server name for the event ID

Returns:
str
The new event ID
"""

global _event_id_counter
Expand Down
7 changes: 7 additions & 0 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ async def create_event(
Should normally be left as None, which will cause them to be calculated
based on the room state at the prev_events.

If non-None, prev_event_ids must also be provided.

require_consent: Whether to check if the requester has
consented to the privacy policy.

Expand Down Expand Up @@ -581,6 +583,9 @@ async def create_event(
# Strip down the auth_event_ids to only what we need to auth the event.
# For example, we don't need extra m.room.member that don't match event.sender
if auth_event_ids is not None:
# If auth events are provided, prev events must be also.
assert prev_event_ids is not None
Comment on lines +586 to +587
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 discussed this with @MadLittleMods, although the docstring says that prev_event_ids will be computed if not given this happens in the call to create_new_client_event ~20 lines below this.

Copy link
Contributor

Choose a reason for hiding this comment

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


temp_event = await builder.build(
prev_event_ids=prev_event_ids,
auth_event_ids=auth_event_ids,
Expand Down Expand Up @@ -784,6 +789,8 @@ async def create_and_send_nonmember_event(
The event ids to use as the auth_events for the new event.
Should normally be left as None, which will cause them to be calculated
based on the room state at the prev_events.

If non-None, prev_event_ids must also be provided.
ratelimit: Whether to rate limit this send.
txn_id: The transaction ID.
ignore_shadow_ban: True if shadow-banned users should be allowed to
Expand Down