Merge branch 'develop' into anoa/edit_redacting

* develop:
  Remove access-token support from RegistrationStore.register (#5642)
  Don't bundle aggregations when retrieving the original event (#5654)
  Add a linting script (#5627)
  Correct pep517 flag in readme (#5651)
  remove unused and unnecessary check for FederationDeniedError (#5645)
  Changelog
  Lint
  Use application/json when querying the IS's /store-invite endpoint

README.rst

@@ -272,7 +272,7 @@ to install using pip and a virtualenv::
 
     virtualenv -p python3 env
     source env/bin/activate
-    python -m pip install --no-pep-517 -e .[all]
+    python -m pip install --no-use-pep517 -e .[all]
 
 This will run a process of downloading and installing all the needed
 dependencies into a virtual env.

changelog.d/5627.misc

@@ -0,0 +1 @@
+Add `lint.sh` to the scripts-dev folder which will run all linting steps required by CI.

changelog.d/5638.bugfix

@@ -0,0 +1 @@
+Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format.

changelog.d/5642.misc

@@ -0,0 +1 @@
+Remove access-token support from `RegistrationStore.register`, and rename it.

changelog.d/5645.misc

@@ -0,0 +1 @@
+Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure.

changelog.d/5651.doc

@@ -0,0 +1 @@
+--no-pep517 should be --no-use-pep517 in the documentation to setup the development environment.

changelog.d/5654.bugfix

@@ -0,0 +1 @@
+Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`.

scripts-dev/lint.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+#
+# Runs linting scripts over the local Synapse checkout
+# isort - sorts import statements
+# flake8 - lints and finds mistakes
+# black - opinionated code formatter
+
+set -e
+
+isort -y -rc synapse tests scripts-dev scripts
+flake8 synapse tests
+python3 -m black synapse tests scripts-dev scripts

synapse/handlers/e2e_keys.py

@@ -22,7 +22,7 @@
 
 from twisted.internet import defer
 
-from synapse.api.errors import CodeMessageException, FederationDeniedError, SynapseError
+from synapse.api.errors import CodeMessageException, SynapseError
 from synapse.logging.context import make_deferred_yieldable, run_in_background
 from synapse.types import UserID, get_domain_from_id
 from synapse.util.retryutils import NotRetryingDestination
@@ -350,9 +350,6 @@ def _exception_to_failure(e):
     if isinstance(e, NotRetryingDestination):
         return {"status": 503, "message": "Not ready for retry"}
 
-    if isinstance(e, FederationDeniedError):
-        return {"status": 403, "message": "Federation Denied"}
-
     # include ConnectionRefused and other errors
     #
     # Note that some Exceptions (notably twisted's ResponseFailed etc) don't

synapse/handlers/register.py

@@ -584,7 +584,7 @@ def register_with_store(
                 address=address,
             )
         else:
-            return self.store.register(
+            return self.store.register_user(
                 user_id=user_id,
                 password_hash=password_hash,
                 was_guest=was_guest,

synapse/handlers/room_member.py

@@ -29,7 +29,7 @@
 import synapse.server
 import synapse.types
 from synapse.api.constants import EventTypes, Membership
-from synapse.api.errors import AuthError, Codes, SynapseError
+from synapse.api.errors import AuthError, Codes, HttpResponseException, SynapseError
 from synapse.types import RoomID, UserID
 from synapse.util.async_helpers import Linearizer
 from synapse.util.distributor import user_joined_room, user_left_room
@@ -872,9 +872,23 @@ def _ask_id_server_for_third_party_invite(
             "sender_avatar_url": inviter_avatar_url,
         }
 
-        data = yield self.simple_http_client.post_urlencoded_get_json(
-            is_url, invite_config
-        )
+        try:
+            data = yield self.simple_http_client.post_json_get_json(
+                is_url, invite_config
+            )
+        except HttpResponseException as e:
+            # Some identity servers may only support application/x-www-form-urlencoded
+            # types. This is especially true with old instances of Sydent, see
+            # https://github.com/matrix-org/sydent/pull/170
+            logger.info(
+                "Failed to POST %s with JSON, falling back to urlencoded form: %s",
+                is_url,
+                e,
+            )
+            data = yield self.simple_http_client.post_urlencoded_get_json(
+                is_url, invite_config
+            )
+
         # TODO: Check for success
         token = data["token"]
         public_keys = data.get("public_keys", [])

synapse/rest/client/v2_alpha/relations.py

@@ -179,8 +179,18 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non
         )
 
         now = self.clock.time_msec()
-        original_event = yield self._event_serializer.serialize_event(event, now)
-        events = yield self._event_serializer.serialize_events(events, now)
+        # We set bundle_aggregations to False when retrieving the original
+        # event because we want the content before relations were applied to
+        # it.
+        original_event = yield self._event_serializer.serialize_event(
+            event, now, bundle_aggregations=False
+        )
+        # Similarly, we don't allow relations to be applied to relations, so we
+        # return the original relations without any aggregations on top of them
+        # here.
+        events = yield self._event_serializer.serialize_events(
+            events, now, bundle_aggregations=False
+        )
 
         return_value = result.to_dict()
         return_value["chunk"] = events

synapse/storage/registration.py

@@ -698,10 +698,9 @@ def add_access_token_to_user(self, user_id, token, device_id=None):
             desc="add_access_token_to_user",
         )
 
-    def register(
+    def register_user(
         self,
         user_id,
-        token=None,
         password_hash=None,
         was_guest=False,
         make_guest=False,
@@ -714,9 +713,6 @@ def register(
 
         Args:
             user_id (str): The desired user ID to register.
-            token (str): The desired access token to use for this user. If this
-                is not None, the given access token is associated with the user
-                id.
             password_hash (str): Optional. The password hash for this user.
             was_guest (bool): Optional. Whether this is a guest account being
                 upgraded to a non-guest account.
@@ -733,10 +729,9 @@ def register(
             StoreError if the user_id could not be registered.
         """
         return self.runInteraction(
-            "register",
-            self._register,
+            "register_user",
+            self._register_user,
             user_id,
-            token,
             password_hash,
             was_guest,
             make_guest,
@@ -746,11 +741,10 @@ def register(
             user_type,
         )
 
-    def _register(
+    def _register_user(
         self,
         txn,
         user_id,
-        token,
         password_hash,
         was_guest,
         make_guest,
@@ -763,8 +757,6 @@ def _register(
 
         now = int(self.clock.time())
 
-        next_id = self._access_tokens_id_gen.get_next()
-
         try:
             if was_guest:
                 # Ensure that the guest user actually exists
@@ -812,14 +804,6 @@ def _register(
         if self._account_validity.enabled:
             self.set_expiration_date_for_user_txn(txn, user_id)
 
-        if token:
-            # it's possible for this to get a conflict, but only for a single user
-            # since tokens are namespaced based on their user ID
-            txn.execute(
-                "INSERT INTO access_tokens(id, user_id, token)" " VALUES (?,?,?)",
-                (next_id, user_id, token),
-            )
-
         if create_profile_with_displayname:
             # set a default displayname serverside to avoid ugly race
             # between auto-joins and clients trying to set displaynames

tests/api/test_auth.py

@@ -325,7 +325,7 @@ def test_reserved_threepid(self):
         unknown_threepid = {"medium": "email", "address": "unreserved@server.com"}
         self.hs.config.mau_limits_reserved_threepids = [threepid]
 
-        yield self.store.register(user_id="user1", token="123", password_hash=None)
+        yield self.store.register_user(user_id="user1", password_hash=None)
         with self.assertRaises(ResourceLimitError):
             yield self.auth.check_auth_blocking()
 

tests/handlers/test_register.py

@@ -77,11 +77,7 @@ def test_if_user_exists(self):
         store = self.hs.get_datastore()
         frank = UserID.from_string("@frank:test")
         self.get_success(
-            store.register(
-                user_id=frank.to_string(),
-                token="jkv;g498752-43gj['eamb!-5",
-                password_hash=None,
-            )
+            store.register_user(user_id=frank.to_string(), password_hash=None)
         )
         local_part = frank.localpart
         user_id = frank.to_string()

tests/handlers/test_user_directory.py

@@ -47,11 +47,8 @@ def prepare(self, reactor, clock, hs):
     def test_handle_local_profile_change_with_support_user(self):
         support_user_id = "@support:test"
         self.get_success(
-            self.store.register(
-                user_id=support_user_id,
-                token="123",
-                password_hash=None,
-                user_type=UserTypes.SUPPORT,
+            self.store.register_user(
+                user_id=support_user_id, password_hash=None, user_type=UserTypes.SUPPORT
             )
         )
 
@@ -73,11 +70,8 @@ def test_handle_local_profile_change_with_support_user(self):
     def test_handle_user_deactivated_support_user(self):
         s_user_id = "@support:test"
         self.get_success(
-            self.store.register(
-                user_id=s_user_id,
-                token="123",
-                password_hash=None,
-                user_type=UserTypes.SUPPORT,
+            self.store.register_user(
+                user_id=s_user_id, password_hash=None, user_type=UserTypes.SUPPORT
             )
         )
 
@@ -90,7 +84,7 @@ def test_handle_user_deactivated_support_user(self):
     def test_handle_user_deactivated_regular_user(self):
         r_user_id = "@regular:test"
         self.get_success(
-            self.store.register(user_id=r_user_id, token="123", password_hash=None)
+            self.store.register_user(user_id=r_user_id, password_hash=None)
         )
         self.store.remove_from_user_dir = Mock()
         self.get_success(self.handler.handle_user_deactivated(r_user_id))

tests/storage/test_client_ips.py

@@ -185,9 +185,7 @@ def test_updating_monthly_active_user_when_space(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
         user_id = "@user:server"
-        self.get_success(
-            self.store.register(user_id=user_id, token="123", password_hash=None)
-        )
+        self.get_success(self.store.register_user(user_id=user_id, password_hash=None))
 
         active = self.get_success(self.store.user_last_seen_monthly_active(user_id))
         self.assertFalse(active)

tests/storage/test_monthly_active_users.py

@@ -53,10 +53,10 @@ def test_initialise_reserved_users(self):
         # -1 because user3 is a support user and does not count
         user_num = len(threepids) - 1
 
-        self.store.register(user_id=user1, token="123", password_hash=None)
-        self.store.register(user_id=user2, token="456", password_hash=None)
-        self.store.register(
-            user_id=user3, token="789", password_hash=None, user_type=UserTypes.SUPPORT
+        self.store.register_user(user_id=user1, password_hash=None)
+        self.store.register_user(user_id=user2, password_hash=None)
+        self.store.register_user(
+            user_id=user3, password_hash=None, user_type=UserTypes.SUPPORT
         )
         self.pump()
 
@@ -161,9 +161,7 @@ def test_reap_monthly_active_users(self):
     def test_populate_monthly_users_is_guest(self):
         # Test that guest users are not added to mau list
         user_id = "@user_id:host"
-        self.store.register(
-            user_id=user_id, token="123", password_hash=None, make_guest=True
-        )
+        self.store.register_user(user_id=user_id, password_hash=None, make_guest=True)
         self.store.upsert_monthly_active_user = Mock()
         self.store.populate_monthly_active_users(user_id)
         self.pump()
@@ -216,8 +214,8 @@ def test_get_reserved_real_user_account(self):
         self.assertEquals(self.get_success(count), 0)
 
         # Test reserved registed users
-        self.store.register(user_id=user1, token="123", password_hash=None)
-        self.store.register(user_id=user2, token="456", password_hash=None)
+        self.store.register_user(user_id=user1, password_hash=None)
+        self.store.register_user(user_id=user2, password_hash=None)
         self.pump()
 
         now = int(self.hs.get_clock().time_msec())
@@ -232,11 +230,8 @@ def test_support_user_not_add_to_mau_limits(self):
         self.pump()
         self.assertEqual(self.get_success(count), 0)
 
-        self.store.register(
-            user_id=support_user_id,
-            token="123",
-            password_hash=None,
-            user_type=UserTypes.SUPPORT,
+        self.store.register_user(
+            user_id=support_user_id, password_hash=None, user_type=UserTypes.SUPPORT
         )
 
         self.store.upsert_monthly_active_user(support_user_id)

tests/storage/test_registration.py

@@ -37,7 +37,7 @@ def setUp(self):
 
     @defer.inlineCallbacks
     def test_register(self):
-        yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
+        yield self.store.register_user(self.user_id, self.pwhash)
 
         self.assertEquals(
             {
@@ -53,15 +53,9 @@ def test_register(self):
             (yield self.store.get_user_by_id(self.user_id)),
         )
 
-        result = yield self.store.get_user_by_access_token(self.tokens[0])
-
-        self.assertDictContainsSubset({"name": self.user_id}, result)
-
-        self.assertTrue("token_id" in result)
-
     @defer.inlineCallbacks
     def test_add_tokens(self):
-        yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
+        yield self.store.register_user(self.user_id, self.pwhash)
         yield self.store.add_access_token_to_user(
             self.user_id, self.tokens[1], self.device_id
         )
@@ -77,7 +71,8 @@ def test_add_tokens(self):
     @defer.inlineCallbacks
     def test_user_delete_access_tokens(self):
         # add some tokens
-        yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
+        yield self.store.register_user(self.user_id, self.pwhash)
+        yield self.store.add_access_token_to_user(self.user_id, self.tokens[0])
         yield self.store.add_access_token_to_user(
             self.user_id, self.tokens[1], self.device_id
         )
@@ -108,24 +103,12 @@ def test_is_support_user(self):
 
         res = yield self.store.is_support_user(None)
         self.assertFalse(res)
-        yield self.store.register(user_id=TEST_USER, token="123", password_hash=None)
+        yield self.store.register_user(user_id=TEST_USER, password_hash=None)
         res = yield self.store.is_support_user(TEST_USER)
         self.assertFalse(res)
 
-        yield self.store.register(
-            user_id=SUPPORT_USER,
-            token="456",
-            password_hash=None,
-            user_type=UserTypes.SUPPORT,
+        yield self.store.register_user(
+            user_id=SUPPORT_USER, password_hash=None, user_type=UserTypes.SUPPORT
         )
         res = yield self.store.is_support_user(SUPPORT_USER)
         self.assertTrue(res)
-
-
-class TokenGenerator:
-    def __init__(self):
-        self._last_issued_token = 0
-
-    def generate(self, user_id):
-        self._last_issued_token += 1
-        return "%s-%d" % (user_id, self._last_issued_token)