From f331e7f62df72f2adf9623feebeea2c720457393 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 27 Aug 2020 12:35:52 -0400
Subject: [PATCH] Ensure that the remote ID is a string.

---
 changelog.d/8190.bugfix          |  1 +
 synapse/handlers/oidc_handler.py |  3 +++
 tests/handlers/test_oidc.py      | 41 ++++++++++++++++++++++++++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/8190.bugfix

diff --git a/changelog.d/8190.bugfix b/changelog.d/8190.bugfix
new file mode 100644
index 000000000000..bf6717ab28d9
--- /dev/null
+++ b/changelog.d/8190.bugfix
@@ -0,0 +1 @@
+Fix logging in via OpenID Connect with a provider that uses integer user IDs.
diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py
index c5bd2fea68ff..b18d75dd6b4c 100644
--- a/synapse/handlers/oidc_handler.py
+++ b/synapse/handlers/oidc_handler.py
@@ -869,6 +869,9 @@ async def _map_userinfo_to_user(
             raise MappingException(
                 "Failed to extract subject from OIDC response: %s" % (e,)
             )
+        # Some OIDC providers use integer IDs, but Synapse expects them to be
+        # strings. Really make sure it is a string.
+        remote_user_id = str(remote_user_id)
 
         logger.info(
             "Looking for existing mapping for user %s:%s",
diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py
index f92f3b8c1527..89ec5fcb31bb 100644
--- a/tests/handlers/test_oidc.py
+++ b/tests/handlers/test_oidc.py
@@ -75,7 +75,17 @@ def deliverBody(self, protocol):
 COOKIE_NAME = b"oidc_session"
 COOKIE_PATH = "/_synapse/oidc"
 
-MockedMappingProvider = Mock(OidcMappingProvider)
+
+class TestMappingProvider(OidcMappingProvider):
+    @staticmethod
+    def parse_config(config):
+        return
+
+    def get_remote_user_id(self, userinfo):
+        return userinfo["sub"]
+
+    async def map_user_attributes(self, userinfo, token):
+        return {"localpart": userinfo["username"], "display_name": None}
 
 
 def simple_async_mock(return_value=None, raises=None):
@@ -123,7 +133,7 @@ def make_homeserver(self, reactor, clock):
         oidc_config["issuer"] = ISSUER
         oidc_config["scopes"] = SCOPES
         oidc_config["user_mapping_provider"] = {
-            "module": __name__ + ".MockedMappingProvider"
+            "module": __name__ + ".TestMappingProvider",
         }
         config["oidc_config"] = oidc_config
 
@@ -580,3 +590,30 @@ def test_exchange_code(self):
         with self.assertRaises(OidcError) as exc:
             yield defer.ensureDeferred(self.handler._exchange_code(code))
         self.assertEqual(exc.exception.error, "some_error")
+
+    def test_map_userinfo_to_user(self):
+        """Ensure that mapping the userinfo returned from a provider to an MXID works properly."""
+        userinfo = {
+            "sub": "test_user",
+            "username": "test_user",
+        }
+        # The token doesn't matter with the default user mapping provider.
+        token = {}
+        mxid = self.get_success(
+            self.handler._map_userinfo_to_user(
+                userinfo, token, "user-agent", "10.10.10.10"
+            )
+        )
+        self.assertEqual(mxid, "@test_user:test")
+
+        # Some providers return an integer ID.
+        userinfo = {
+            "sub": 1234,
+            "username": "test_user_2",
+        }
+        mxid = self.get_success(
+            self.handler._map_userinfo_to_user(
+                userinfo, token, "user-agent", "10.10.10.10"
+            )
+        )
+        self.assertEqual(mxid, "@test_user_2:test")