Merge pull request #6806 from matrix-org/rav/redact_changes/3

Pass room_version into add_hashes_and_signatures

6 files changed, 49 insertions, 41 deletions

diff --git a/changelog.d/6806.misc b/changelog.d/6806.misc
new file mode 100644
index 0000000000..08aa80bcd9
--- /dev/null
+++ b/changelog.d/6806.misc
@@ -0,0 +1 @@
+Refactoring work in preparation for changing the event redaction algorithm.
diff --git a/synapse/crypto/event_signing.py b/synapse/crypto/event_signing.py
index e65bd61d97..1f2bccf700 100644
--- a/synapse/crypto/event_signing.py
+++ b/synapse/crypto/event_signing.py
@@ -20,10 +20,13 @@ import logging
 
 from canonicaljson import encode_canonical_json
 from signedjson.sign import sign_json
+from signedjson.types import SigningKey
 from unpaddedbase64 import decode_base64, encode_base64
 
 from synapse.api.errors import Codes, SynapseError
+from synapse.api.room_versions import RoomVersion
 from synapse.events.utils import prune_event, prune_event_dict
+from synapse.types import JsonDict
 
 logger = logging.getLogger(__name__)
 
@@ -137,20 +140,23 @@ def compute_event_signature(event_dict, signature_name, signing_key):
 
 
 def add_hashes_and_signatures(
-    event_dict, signature_name, signing_key, hash_algorithm=hashlib.sha256
+    room_version: RoomVersion,
+    event_dict: JsonDict,
+    signature_name: str,
+    signing_key: SigningKey,
 ):
     """Add content hash and sign the event
 
     Args:
-        event_dict (dict): The event to add hashes to and sign
-        signature_name (str): The name of the entity signing the event
+        room_version: the version of the room this event is in
+
+        event_dict: The event to add hashes to and sign
+        signature_name: The name of the entity signing the event
             (typically the server's hostname).
-        signing_key (syutil.crypto.SigningKey): The key to sign with
-        hash_algorithm: A hasher from `hashlib`, e.g. hashlib.sha256, to use
-            to hash the event
+        signing_key: The key to sign with
     """
 
-    name, digest = compute_content_hash(event_dict, hash_algorithm=hash_algorithm)
+    name, digest = compute_content_hash(event_dict, hash_algorithm=hashlib.sha256)
 
     event_dict.setdefault("hashes", {})[name] = encode_base64(digest)
 
diff --git a/synapse/events/builder.py b/synapse/events/builder.py
index 3997751337..8d63ad6dc3 100644
--- a/synapse/events/builder.py
+++ b/synapse/events/builder.py
@@ -12,8 +12,10 @@
 # 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 Optional
 
 import attr
+from nacl.signing import SigningKey
 
 from twisted.internet import defer
 
@@ -23,13 +25,18 @@ from synapse.api.room_versions import (
     KNOWN_EVENT_FORMAT_VERSIONS,
     KNOWN_ROOM_VERSIONS,
     EventFormatVersions,
+    RoomVersion,
 )
 from synapse.crypto.event_signing import add_hashes_and_signatures
-from synapse.types import EventID
+from synapse.events import (
+    EventBase,
+    _EventInternalMetadata,
+    event_type_from_format_version,
+)
+from synapse.types import EventID, JsonDict
+from synapse.util import Clock
 from synapse.util.stringutils import random_string
 
-from . import _EventInternalMetadata, event_type_from_format_version
-
 
 @attr.s(slots=True, cmp=False, frozen=True)
 class EventBuilder(object):
@@ -40,7 +47,7 @@ class EventBuilder(object):
     content/unsigned/internal_metadata fields are still mutable)
 
     Attributes:
-        format_version (int): Event format version
+        room_version: Version of the target room
         room_id (str)
         type (str)
         sender (str)
@@ -63,7 +70,7 @@ class EventBuilder(object):
     _hostname = attr.ib()
     _signing_key = attr.ib()
 
-    format_version = attr.ib()
+    room_version = attr.ib(type=RoomVersion)
 
     room_id = attr.ib()
     type = attr.ib()
@@ -108,7 +115,8 @@ class EventBuilder(object):
         )
         auth_ids = yield self._auth.compute_auth_events(self, state_ids)
 
-        if self.format_version == EventFormatVersions.V1:
+        format_version = self.room_version.event_format
+        if format_version == EventFormatVersions.V1:
             auth_events = yield self._store.add_event_hashes(auth_ids)
             prev_events = yield self._store.add_event_hashes(prev_event_ids)
         else:
@@ -148,7 +156,7 @@ class EventBuilder(object):
             clock=self._clock,
             hostname=self._hostname,
             signing_key=self._signing_key,
-            format_version=self.format_version,
+            room_version=self.room_version,
             event_dict=event_dict,
             internal_metadata_dict=self.internal_metadata.get_dict(),
         )
@@ -201,7 +209,7 @@ class EventBuilderFactory(object):
             clock=self.clock,
             hostname=self.hostname,
             signing_key=self.signing_key,
-            format_version=room_version.event_format,
+            room_version=room_version,
             type=key_values["type"],
             state_key=key_values.get("state_key"),
             room_id=key_values["room_id"],
@@ -214,29 +222,19 @@ class EventBuilderFactory(object):
 
 
 def create_local_event_from_event_dict(
-    clock,
-    hostname,
-    signing_key,
-    format_version,
-    event_dict,
-    internal_metadata_dict=None,
-):
+    clock: Clock,
+    hostname: str,
+    signing_key: SigningKey,
+    room_version: RoomVersion,
+    event_dict: JsonDict,
+    internal_metadata_dict: Optional[JsonDict] = None,
+) -> EventBase:
     """Takes a fully formed event dict, ensuring that fields like `origin`
     and `origin_server_ts` have correct values for a locally produced event,
     then signs and hashes it.
-
-    Args:
-        clock (Clock)
-        hostname (str)
-        signing_key
-        format_version (int)
-        event_dict (dict)
-        internal_metadata_dict (dict|None)
-
-    Returns:
-        FrozenEvent
     """
 
+    format_version = room_version.event_format
     if format_version not in KNOWN_EVENT_FORMAT_VERSIONS:
         raise Exception("No event format defined for version %r" % (format_version,))
 
@@ -257,7 +255,7 @@ def create_local_event_from_event_dict(
 
     event_dict.setdefault("signatures", {})
 
-    add_hashes_and_signatures(event_dict, hostname, signing_key)
+    add_hashes_and_signatures(room_version, event_dict, hostname, signing_key)
     return event_type_from_format_version(format_version)(
         event_dict, internal_metadata_dict=internal_metadata_dict
     )
diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py
index 4ac3d81cba..f99d17a7de 100644
--- a/synapse/federation/federation_client.py
+++ b/synapse/federation/federation_client.py
@@ -470,8 +470,6 @@ class FederationClient(FederationBase):
             if not room_version:
                 raise UnsupportedRoomVersionError()
 
-            event_format = room_version_to_event_format(room_version_id)
-
             pdu_dict = ret.get("event", None)
             if not isinstance(pdu_dict, dict):
                 raise InvalidResponseError("Bad 'event' field in response")
@@ -490,7 +488,7 @@ class FederationClient(FederationBase):
                 self._clock,
                 self.hostname,
                 self.signing_key,
-                format_version=event_format,
+                room_version=room_version,
                 event_dict=pdu_dict,
             )
 
diff --git a/tests/crypto/test_event_signing.py b/tests/crypto/test_event_signing.py
index 126e176004..6143a50ab2 100644
--- a/tests/crypto/test_event_signing.py
+++ b/tests/crypto/test_event_signing.py
@@ -17,6 +17,7 @@
 import nacl.signing
 from unpaddedbase64 import decode_base64
 
+from synapse.api.room_versions import RoomVersions
 from synapse.crypto.event_signing import add_hashes_and_signatures
 from synapse.events import FrozenEvent
 
@@ -49,7 +50,9 @@ class EventSigningTestCase(unittest.TestCase):
             "unsigned": {"age_ts": 1000000},
         }
 
-        add_hashes_and_signatures(event_dict, HOSTNAME, self.signing_key)
+        add_hashes_and_signatures(
+            RoomVersions.V1, event_dict, HOSTNAME, self.signing_key
+        )
 
         event = FrozenEvent(event_dict)
 
@@ -81,7 +84,9 @@ class EventSigningTestCase(unittest.TestCase):
             "unsigned": {"age_ts": 1000000},
         }
 
-        add_hashes_and_signatures(event_dict, HOSTNAME, self.signing_key)
+        add_hashes_and_signatures(
+            RoomVersions.V1, event_dict, HOSTNAME, self.signing_key
+        )
 
         event = FrozenEvent(event_dict)
 
diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py
index e92e090c3c..c171038df8 100644
--- a/tests/handlers/test_presence.py
+++ b/tests/handlers/test_presence.py
@@ -19,7 +19,7 @@ from mock import Mock, call
 from signedjson.key import generate_signing_key
 
 from synapse.api.constants import EventTypes, Membership, PresenceState
-from synapse.events import room_version_to_event_format
+from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
 from synapse.events.builder import EventBuilder
 from synapse.handlers.presence import (
     EXTERNAL_PROCESS_EXPIRY,
@@ -597,7 +597,7 @@ class PresenceJoinTestCase(unittest.HomeserverTestCase):
             clock=self.clock,
             hostname=hostname,
             signing_key=self.random_signing_key,
-            format_version=room_version_to_event_format(room_version),
+            room_version=KNOWN_ROOM_VERSIONS[room_version],
             room_id=room_id,
             type=EventTypes.Member,
             sender=user_id,