From a3bad89d57645b2ea304d2900adab71a786b0172 Mon Sep 17 00:00:00 2001 From: Warren Bailey Date: Thu, 30 Mar 2023 12:09:41 +0100 Subject: [PATCH] Add the ability to enable/disable registrations when in the OIDC flow (#14978) Signed-off-by: Warren Bailey --- changelog.d/14978.feature | 1 + .../usage/configuration/config_documentation.md | 6 ++++++ synapse/config/oidc.py | 5 +++++ synapse/handlers/oidc.py | 1 + synapse/handlers/sso.py | 17 +++++++++++++++-- tests/handlers/test_oidc.py | 17 ++++++++++++++++- 6 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 changelog.d/14978.feature diff --git a/changelog.d/14978.feature b/changelog.d/14978.feature new file mode 100644 index 0000000000..14f6fee658 --- /dev/null +++ b/changelog.d/14978.feature @@ -0,0 +1 @@ +Add the ability to enable/disable registrations when in the OIDC flow. \ No newline at end of file diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 060d0d5e69..c5c2c2b615 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3100,6 +3100,11 @@ Options for each entry include: match a pre-existing account instead of failing. This could be used if switching from password logins to OIDC. Defaults to false. +* `enable_registration`: set to 'false' to disable automatic registration of new + users. This allows the OIDC SSO flow to be limited to sign in only, rather than + automatically registering users that have a valid SSO login but do not have + a pre-registered account. Defaults to true. + * `user_mapping_provider`: Configuration for how attributes returned from a OIDC provider are mapped onto a matrix user. This setting has the following sub-properties: @@ -3216,6 +3221,7 @@ oidc_providers: userinfo_endpoint: "https://accounts.example.com/userinfo" jwks_uri: "https://accounts.example.com/.well-known/jwks.json" skip_verification: true + enable_registration: true user_mapping_provider: config: subject_claim: "id" diff --git a/synapse/config/oidc.py b/synapse/config/oidc.py index df8c422043..77c1d1dc8e 100644 --- a/synapse/config/oidc.py +++ b/synapse/config/oidc.py @@ -136,6 +136,7 @@ OIDC_PROVIDER_CONFIG_SCHEMA = { "type": "array", "items": SsoAttributeRequirement.JSON_SCHEMA, }, + "enable_registration": {"type": "boolean"}, }, } @@ -306,6 +307,7 @@ def _parse_oidc_config_dict( user_mapping_provider_class=user_mapping_provider_class, user_mapping_provider_config=user_mapping_provider_config, attribute_requirements=attribute_requirements, + enable_registration=oidc_config.get("enable_registration", True), ) @@ -405,3 +407,6 @@ class OidcProviderConfig: # required attributes to require in userinfo to allow login/registration attribute_requirements: List[SsoAttributeRequirement] + + # Whether automatic registrations are enabled in the ODIC flow. Defaults to True + enable_registration: bool diff --git a/synapse/handlers/oidc.py b/synapse/handlers/oidc.py index 0fc829acf7..e7e0b5e049 100644 --- a/synapse/handlers/oidc.py +++ b/synapse/handlers/oidc.py @@ -1239,6 +1239,7 @@ class OidcProvider: grandfather_existing_users, extra_attributes, auth_provider_session_id=sid, + registration_enabled=self._config.enable_registration, ) def _remote_id_from_userinfo(self, userinfo: UserInfo) -> str: diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index 4a27c0f051..c28325323c 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -383,6 +383,7 @@ class SsoHandler: grandfather_existing_users: Callable[[], Awaitable[Optional[str]]], extra_login_attributes: Optional[JsonDict] = None, auth_provider_session_id: Optional[str] = None, + registration_enabled: bool = True, ) -> None: """ Given an SSO ID, retrieve the user ID for it and possibly register the user. @@ -435,6 +436,10 @@ class SsoHandler: auth_provider_session_id: An optional session ID from the IdP. + registration_enabled: An optional boolean to enable/disable automatic + registrations of new users. If false and the user does not exist then the + flow is aborted. Defaults to true. + Raises: MappingException if there was a problem mapping the response to a user. RedirectException: if the mapping provider needs to redirect the user @@ -462,8 +467,16 @@ class SsoHandler: auth_provider_id, remote_user_id, user_id ) - # Otherwise, generate a new user. - if not user_id: + if not user_id and not registration_enabled: + logger.info( + "User does not exist and registration are disabled for IdP '%s' and remote_user_id '%s'", + auth_provider_id, + remote_user_id, + ) + raise MappingException( + "User does not exist and registrations are disabled" + ) + elif not user_id: # Otherwise, generate a new user. attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper) next_step_url = self._get_url_for_next_new_user_step( diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index 951caaa6b3..0a8bae54fb 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -922,7 +922,7 @@ class OidcHandlerTestCase(HomeserverTestCase): auth_provider_session_id=None, ) - @override_config({"oidc_config": DEFAULT_CONFIG}) + @override_config({"oidc_config": {**DEFAULT_CONFIG, "enable_registration": True}}) def test_map_userinfo_to_user(self) -> None: """Ensure that mapping the userinfo returned from a provider to an MXID works properly.""" userinfo: dict = { @@ -975,6 +975,21 @@ class OidcHandlerTestCase(HomeserverTestCase): "Mapping provider does not support de-duplicating Matrix IDs", ) + @override_config({"oidc_config": {**DEFAULT_CONFIG, "enable_registration": False}}) + def test_map_userinfo_to_user_does_not_register_new_user(self) -> None: + """Ensures new users are not registered if the enabled registration flag is disabled.""" + userinfo: dict = { + "sub": "test_user", + "username": "test_user", + } + request, _ = self.start_authorization(userinfo) + self.get_success(self.handler.handle_oidc_callback(request)) + self.complete_sso_login.assert_not_called() + self.assertRenderedError( + "mapping_error", + "User does not exist and registrations are disabled", + ) + @override_config({"oidc_config": {**DEFAULT_CONFIG, "allow_existing_users": True}}) def test_map_userinfo_to_existing_user(self) -> None: """Existing users can log in with OpenID Connect when allow_existing_users is True."""