Test `get_extra_attributes` fallback

despite the warnings saying "don't implement get_extra_attributes", we had
implemented it, so the tests weren't doing what we thought they were.
pull/8951/head
Richard van der Hoff 2020-12-15 12:39:56 +00:00
parent 01333681bc
commit 2dd2e90e2b
2 changed files with 22 additions and 12 deletions

View File

@ -19,7 +19,7 @@ from mock import ANY, Mock, patch
import pymacaroons import pymacaroons
from synapse.handlers.oidc_handler import OidcError, OidcMappingProvider from synapse.handlers.oidc_handler import OidcError
from synapse.handlers.sso import MappingException from synapse.handlers.sso import MappingException
from synapse.types import UserID from synapse.types import UserID
@ -55,11 +55,14 @@ COOKIE_NAME = b"oidc_session"
COOKIE_PATH = "/_synapse/oidc" COOKIE_PATH = "/_synapse/oidc"
class TestMappingProvider(OidcMappingProvider): class TestMappingProvider:
@staticmethod @staticmethod
def parse_config(config): def parse_config(config):
return return
def __init__(self, config):
pass
def get_remote_user_id(self, userinfo): def get_remote_user_id(self, userinfo):
return userinfo["sub"] return userinfo["sub"]
@ -360,6 +363,13 @@ class OidcHandlerTestCase(HomeserverTestCase):
- when the userinfo fetching fails - when the userinfo fetching fails
- when the code exchange fails - when the code exchange fails
""" """
# ensure that we are correctly testing the fallback when "get_extra_attributes"
# is not implemented.
mapping_provider = self.handler._user_mapping_provider
with self.assertRaises(AttributeError):
_ = mapping_provider.get_extra_attributes
token = { token = {
"type": "bearer", "type": "bearer",
"id_token": "id_token", "id_token": "id_token",
@ -396,7 +406,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
self.get_success(self.handler.handle_oidc_callback(request)) self.get_success(self.handler.handle_oidc_callback(request))
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
expected_user_id, request, client_redirect_url, {}, expected_user_id, request, client_redirect_url, None,
) )
self.handler._exchange_code.assert_called_once_with(code) self.handler._exchange_code.assert_called_once_with(code)
self.handler._parse_id_token.assert_called_once_with(token, nonce=nonce) self.handler._parse_id_token.assert_called_once_with(token, nonce=nonce)
@ -427,7 +437,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
self.get_success(self.handler.handle_oidc_callback(request)) self.get_success(self.handler.handle_oidc_callback(request))
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
expected_user_id, request, client_redirect_url, {}, expected_user_id, request, client_redirect_url, None,
) )
self.handler._exchange_code.assert_called_once_with(code) self.handler._exchange_code.assert_called_once_with(code)
self.handler._parse_id_token.assert_not_called() self.handler._parse_id_token.assert_not_called()
@ -616,7 +626,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
} }
self._make_callback_with_userinfo(userinfo) self._make_callback_with_userinfo(userinfo)
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
"@test_user:test", ANY, ANY, {} "@test_user:test", ANY, ANY, None,
) )
auth_handler.complete_sso_login.reset_mock() auth_handler.complete_sso_login.reset_mock()
@ -627,7 +637,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
} }
self._make_callback_with_userinfo(userinfo) self._make_callback_with_userinfo(userinfo)
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
"@test_user_2:test", ANY, ANY, {} "@test_user_2:test", ANY, ANY, None,
) )
auth_handler.complete_sso_login.reset_mock() auth_handler.complete_sso_login.reset_mock()
@ -664,14 +674,14 @@ class OidcHandlerTestCase(HomeserverTestCase):
} }
self._make_callback_with_userinfo(userinfo) self._make_callback_with_userinfo(userinfo)
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
user.to_string(), ANY, ANY, {}, user.to_string(), ANY, ANY, None,
) )
auth_handler.complete_sso_login.reset_mock() auth_handler.complete_sso_login.reset_mock()
# Subsequent calls should map to the same mxid. # Subsequent calls should map to the same mxid.
self._make_callback_with_userinfo(userinfo) self._make_callback_with_userinfo(userinfo)
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
user.to_string(), ANY, ANY, {}, user.to_string(), ANY, ANY, None,
) )
auth_handler.complete_sso_login.reset_mock() auth_handler.complete_sso_login.reset_mock()
@ -686,7 +696,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
} }
self._make_callback_with_userinfo(userinfo) self._make_callback_with_userinfo(userinfo)
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
user.to_string(), ANY, ANY, {}, user.to_string(), ANY, ANY, None,
) )
auth_handler.complete_sso_login.reset_mock() auth_handler.complete_sso_login.reset_mock()
@ -722,7 +732,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
self._make_callback_with_userinfo(userinfo) self._make_callback_with_userinfo(userinfo)
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
"@TEST_USER_2:test", ANY, ANY, {}, "@TEST_USER_2:test", ANY, ANY, None,
) )
def test_map_userinfo_to_invalid_localpart(self): def test_map_userinfo_to_invalid_localpart(self):
@ -756,7 +766,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
# test_user is already taken, so test_user1 gets registered instead. # test_user is already taken, so test_user1 gets registered instead.
auth_handler.complete_sso_login.assert_called_once_with( auth_handler.complete_sso_login.assert_called_once_with(
"@test_user1:test", ANY, ANY, {}, "@test_user1:test", ANY, ANY, None,
) )
auth_handler.complete_sso_login.reset_mock() auth_handler.complete_sso_login.reset_mock()

View File

@ -445,7 +445,7 @@ class RestHelper:
return channel.json_body return channel.json_body
# an 'oidc_config' suitable for login_with_oidc. # an 'oidc_config' suitable for login_via_oidc.
TEST_OIDC_CONFIG = { TEST_OIDC_CONFIG = {
"enabled": True, "enabled": True,
"discover": False, "discover": False,