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

Commit b055dc9

Browse files
authored
Ensure that the OpenID Connect remote ID is a string. (#8190)
1 parent 5c03134 commit b055dc9

File tree

3 files changed

+43
-2
lines changed

3 files changed

+43
-2
lines changed

changelog.d/8190.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix logging in via OpenID Connect with a provider that uses integer user IDs.

synapse/handlers/oidc_handler.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,6 +869,9 @@ async def _map_userinfo_to_user(
869869
raise MappingException(
870870
"Failed to extract subject from OIDC response: %s" % (e,)
871871
)
872+
# Some OIDC providers use integer IDs, but Synapse expects external IDs
873+
# to be strings.
874+
remote_user_id = str(remote_user_id)
872875

873876
logger.info(
874877
"Looking for existing mapping for user %s:%s",

tests/handlers/test_oidc.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,17 @@ def deliverBody(self, protocol):
7575
COOKIE_NAME = b"oidc_session"
7676
COOKIE_PATH = "/_synapse/oidc"
7777

78-
MockedMappingProvider = Mock(OidcMappingProvider)
78+
79+
class TestMappingProvider(OidcMappingProvider):
80+
@staticmethod
81+
def parse_config(config):
82+
return
83+
84+
def get_remote_user_id(self, userinfo):
85+
return userinfo["sub"]
86+
87+
async def map_user_attributes(self, userinfo, token):
88+
return {"localpart": userinfo["username"], "display_name": None}
7989

8090

8191
def simple_async_mock(return_value=None, raises=None):
@@ -123,7 +133,7 @@ def make_homeserver(self, reactor, clock):
123133
oidc_config["issuer"] = ISSUER
124134
oidc_config["scopes"] = SCOPES
125135
oidc_config["user_mapping_provider"] = {
126-
"module": __name__ + ".MockedMappingProvider"
136+
"module": __name__ + ".TestMappingProvider",
127137
}
128138
config["oidc_config"] = oidc_config
129139

@@ -580,3 +590,30 @@ def test_exchange_code(self):
580590
with self.assertRaises(OidcError) as exc:
581591
yield defer.ensureDeferred(self.handler._exchange_code(code))
582592
self.assertEqual(exc.exception.error, "some_error")
593+
594+
def test_map_userinfo_to_user(self):
595+
"""Ensure that mapping the userinfo returned from a provider to an MXID works properly."""
596+
userinfo = {
597+
"sub": "test_user",
598+
"username": "test_user",
599+
}
600+
# The token doesn't matter with the default user mapping provider.
601+
token = {}
602+
mxid = self.get_success(
603+
self.handler._map_userinfo_to_user(
604+
userinfo, token, "user-agent", "10.10.10.10"
605+
)
606+
)
607+
self.assertEqual(mxid, "@test_user:test")
608+
609+
# Some providers return an integer ID.
610+
userinfo = {
611+
"sub": 1234,
612+
"username": "test_user_2",
613+
}
614+
mxid = self.get_success(
615+
self.handler._map_userinfo_to_user(
616+
userinfo, token, "user-agent", "10.10.10.10"
617+
)
618+
)
619+
self.assertEqual(mxid, "@test_user_2:test")

0 commit comments

Comments
 (0)