From adf0830928a0b1a9fc6868d64c5f5fe58b5b0ef2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 14:07:18 -0400 Subject: [PATCH 1/4] Rename handler modules to drop _handler. --- docs/sample_config.yaml | 2 +- synapse/config/oidc_config.py | 5 ++++- synapse/config/saml2_config.py | 5 ++++- synapse/handlers/{cas_handler.py => cas.py} | 0 synapse/handlers/{oidc_handler.py => oidc.py} | 0 synapse/handlers/{saml_handler.py => saml.py} | 0 synapse/server.py | 10 +++++----- tests/handlers/test_cas.py | 2 +- tests/handlers/test_oidc.py | 8 ++++---- 9 files changed, 19 insertions(+), 13 deletions(-) rename synapse/handlers/{cas_handler.py => cas.py} (100%) rename synapse/handlers/{oidc_handler.py => oidc.py} (100%) rename synapse/handlers/{saml_handler.py => saml.py} (100%) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 9182dcd98716..e92149ae0883 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1878,7 +1878,7 @@ saml2_config: # sub-properties: # # module: The class name of a custom mapping module. Default is -# 'synapse.handlers.oidc_handler.JinjaOidcMappingProvider'. +# 'synapse.handlers.oidc.JinjaOidcMappingProvider'. # See https://github.com/matrix-org/synapse/blob/master/docs/sso_mapping_providers.md#openid-mapping-providers # for information on implementing a custom mapping provider. # diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index 5fb94376fdb0..e6817210a5c5 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -27,7 +27,8 @@ from ._base import Config, ConfigError, read_file -DEFAULT_USER_MAPPING_PROVIDER = "synapse.handlers.oidc_handler.JinjaOidcMappingProvider" +DEFAULT_USER_MAPPING_PROVIDER = "synapse.handlers.oidc.JinjaOidcMappingProvider" +LEGACY_USER_MAPPING_PROVIDER = "synapse.handlers.oidc_handler.JinjaOidcMappingProvider" class OIDCConfig(Config): @@ -403,6 +404,8 @@ def _parse_oidc_config_dict( """ ump_config = oidc_config.get("user_mapping_provider", {}) ump_config.setdefault("module", DEFAULT_USER_MAPPING_PROVIDER) + if ump_config.get("module") == LEGACY_USER_MAPPING_PROVIDER: + ump_config["module"] = DEFAULT_USER_MAPPING_PROVIDER ump_config.setdefault("config", {}) ( diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 55a7838b1004..0cc517ab99e4 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -25,7 +25,8 @@ logger = logging.getLogger(__name__) -DEFAULT_USER_MAPPING_PROVIDER = ( +DEFAULT_USER_MAPPING_PROVIDER = "synapse.handlers.saml.DefaultSamlMappingProvider" +LEGACY_USER_MAPPING_PROVIDER = ( "synapse.handlers.saml_handler.DefaultSamlMappingProvider" ) @@ -97,6 +98,8 @@ def read_config(self, config, **kwargs): # Use the default user mapping provider if not set ump_dict.setdefault("module", DEFAULT_USER_MAPPING_PROVIDER) + if ump_dict.get("module") == LEGACY_USER_MAPPING_PROVIDER: + ump_dict["module"] = DEFAULT_USER_MAPPING_PROVIDER # Ensure a config is present ump_dict["config"] = ump_dict.get("config") or {} diff --git a/synapse/handlers/cas_handler.py b/synapse/handlers/cas.py similarity index 100% rename from synapse/handlers/cas_handler.py rename to synapse/handlers/cas.py diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc.py similarity index 100% rename from synapse/handlers/oidc_handler.py rename to synapse/handlers/oidc.py diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml.py similarity index 100% rename from synapse/handlers/saml_handler.py rename to synapse/handlers/saml.py diff --git a/synapse/server.py b/synapse/server.py index 95a2cd2e5d08..6b0c7fd6b061 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -70,7 +70,7 @@ from synapse.handlers.admin import AdminHandler from synapse.handlers.appservice import ApplicationServicesHandler from synapse.handlers.auth import AuthHandler, MacaroonGenerator -from synapse.handlers.cas_handler import CasHandler +from synapse.handlers.cas import CasHandler from synapse.handlers.deactivate_account import DeactivateAccountHandler from synapse.handlers.device import DeviceHandler, DeviceWorkerHandler from synapse.handlers.devicemessage import DeviceMessageHandler @@ -145,8 +145,8 @@ if TYPE_CHECKING: from txredisapi import RedisProtocol - from synapse.handlers.oidc_handler import OidcHandler - from synapse.handlers.saml_handler import SamlHandler + from synapse.handlers.oidc import OidcHandler + from synapse.handlers.saml import SamlHandler T = TypeVar("T", bound=Callable[..., Any]) @@ -699,13 +699,13 @@ def get_cas_handler(self) -> CasHandler: @cache_in_self def get_saml_handler(self) -> "SamlHandler": - from synapse.handlers.saml_handler import SamlHandler + from synapse.handlers.saml import SamlHandler return SamlHandler(self) @cache_in_self def get_oidc_handler(self) -> "OidcHandler": - from synapse.handlers.oidc_handler import OidcHandler + from synapse.handlers.oidc import OidcHandler return OidcHandler(self) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 0444b2679889..b625995d1253 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -13,7 +13,7 @@ # limitations under the License. from unittest.mock import Mock -from synapse.handlers.cas_handler import CasResponse +from synapse.handlers.cas import CasResponse from tests.test_utils import simple_async_mock from tests.unittest import HomeserverTestCase, override_config diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index 34d2fc1dfb11..a25c89bd5bd3 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -499,7 +499,7 @@ def test_callback(self): self.assertRenderedError("fetch_error") # Handle code exchange failure - from synapse.handlers.oidc_handler import OidcError + from synapse.handlers.oidc import OidcError self.provider._exchange_code = simple_async_mock( raises=OidcError("invalid_request") @@ -583,7 +583,7 @@ def test_exchange_code(self): body=b'{"error": "foo", "error_description": "bar"}', ) ) - from synapse.handlers.oidc_handler import OidcError + from synapse.handlers.oidc import OidcError exc = self.get_failure(self.provider._exchange_code(code), OidcError) self.assertEqual(exc.value.error, "foo") @@ -1126,7 +1126,7 @@ def _generate_oidc_session_token( client_redirect_url: str, ui_auth_session_id: str = "", ) -> str: - from synapse.handlers.oidc_handler import OidcSessionData + from synapse.handlers.oidc import OidcSessionData return self.handler._token_generator.generate_oidc_session_token( state=state, @@ -1152,7 +1152,7 @@ async def _make_callback_with_userinfo( userinfo: the OIDC userinfo dict client_redirect_url: the URL to redirect to on success. """ - from synapse.handlers.oidc_handler import OidcSessionData + from synapse.handlers.oidc import OidcSessionData handler = hs.get_oidc_handler() provider = handler._providers["oidc"] From 783ce5524a4c5d1bd12561b693190103ee425110 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 14:10:42 -0400 Subject: [PATCH 2/4] Rename config modules to drop _config. --- changelog.d/9816.misc | 1 + synapse/config/_base.pyi | 20 +++++++++---------- .../config/{consent_config.py => consent.py} | 0 synapse/config/homeserver.py | 10 +++++----- synapse/config/{jwt_config.py => jwt.py} | 0 synapse/config/{oidc_config.py => oidc.py} | 0 synapse/config/{saml2_config.py => saml2.py} | 0 ...er_notices_config.py => server_notices.py} | 0 synapse/handlers/oidc.py | 5 +---- synapse/rest/client/v2_alpha/register.py | 2 +- 10 files changed, 18 insertions(+), 20 deletions(-) create mode 100644 changelog.d/9816.misc rename synapse/config/{consent_config.py => consent.py} (100%) rename synapse/config/{jwt_config.py => jwt.py} (100%) rename synapse/config/{oidc_config.py => oidc.py} (100%) rename synapse/config/{saml2_config.py => saml2.py} (100%) rename synapse/config/{server_notices_config.py => server_notices.py} (100%) diff --git a/changelog.d/9816.misc b/changelog.d/9816.misc new file mode 100644 index 000000000000..d0981225006e --- /dev/null +++ b/changelog.d/9816.misc @@ -0,0 +1 @@ +Rename some handlers and config modules to not duplicate the top-level module. diff --git a/synapse/config/_base.pyi b/synapse/config/_base.pyi index e896fd34e23c..6b82ace458ff 100644 --- a/synapse/config/_base.pyi +++ b/synapse/config/_base.pyi @@ -6,16 +6,16 @@ from synapse.config import ( auth, captcha, cas, - consent_config, + consent, database, emailconfig, experimental, groups, - jwt_config, + jwt, key, logger, metrics, - oidc_config, + oidc, password_auth_providers, push, ratelimiting, @@ -23,9 +23,9 @@ from synapse.config import ( registration, repository, room_directory, - saml2_config, + saml2, server, - server_notices_config, + server_notices, spam_checker, sso, stats, @@ -63,11 +63,11 @@ class RootConfig: api: api.ApiConfig appservice: appservice.AppServiceConfig key: key.KeyConfig - saml2: saml2_config.SAML2Config + saml2: saml2.SAML2Config cas: cas.CasConfig sso: sso.SSOConfig - oidc: oidc_config.OIDCConfig - jwt: jwt_config.JWTConfig + oidc: oidc.OIDCConfig + jwt: jwt.JWTConfig auth: auth.AuthConfig email: emailconfig.EmailConfig worker: workers.WorkerConfig @@ -76,9 +76,9 @@ class RootConfig: spamchecker: spam_checker.SpamCheckerConfig groups: groups.GroupsConfig userdirectory: user_directory.UserDirectoryConfig - consent: consent_config.ConsentConfig + consent: consent.ConsentConfig stats: stats.StatsConfig - servernotices: server_notices_config.ServerNoticesConfig + servernotices: server_notices.ServerNoticesConfig roomdirectory: room_directory.RoomDirectoryConfig thirdpartyrules: third_party_event_rules.ThirdPartyRulesConfig tracer: tracer.TracerConfig diff --git a/synapse/config/consent_config.py b/synapse/config/consent.py similarity index 100% rename from synapse/config/consent_config.py rename to synapse/config/consent.py diff --git a/synapse/config/homeserver.py b/synapse/config/homeserver.py index 130953506825..0e12b4071cea 100644 --- a/synapse/config/homeserver.py +++ b/synapse/config/homeserver.py @@ -20,17 +20,17 @@ from .cache import CacheConfig from .captcha import CaptchaConfig from .cas import CasConfig -from .consent_config import ConsentConfig +from .consent import ConsentConfig from .database import DatabaseConfig from .emailconfig import EmailConfig from .experimental import ExperimentalConfig from .federation import FederationConfig from .groups import GroupsConfig -from .jwt_config import JWTConfig +from .jwt import JWTConfig from .key import KeyConfig from .logger import LoggingConfig from .metrics import MetricsConfig -from .oidc_config import OIDCConfig +from .oidc import OIDCConfig from .password_auth_providers import PasswordAuthProviderConfig from .push import PushConfig from .ratelimiting import RatelimitConfig @@ -39,9 +39,9 @@ from .repository import ContentRepositoryConfig from .room import RoomConfig from .room_directory import RoomDirectoryConfig -from .saml2_config import SAML2Config +from .saml2 import SAML2Config from .server import ServerConfig -from .server_notices_config import ServerNoticesConfig +from .server_notices import ServerNoticesConfig from .spam_checker import SpamCheckerConfig from .sso import SSOConfig from .stats import StatsConfig diff --git a/synapse/config/jwt_config.py b/synapse/config/jwt.py similarity index 100% rename from synapse/config/jwt_config.py rename to synapse/config/jwt.py diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc.py similarity index 100% rename from synapse/config/oidc_config.py rename to synapse/config/oidc.py diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2.py similarity index 100% rename from synapse/config/saml2_config.py rename to synapse/config/saml2.py diff --git a/synapse/config/server_notices_config.py b/synapse/config/server_notices.py similarity index 100% rename from synapse/config/server_notices_config.py rename to synapse/config/server_notices.py diff --git a/synapse/handlers/oidc.py b/synapse/handlers/oidc.py index b156196a70b7..45514be50fef 100644 --- a/synapse/handlers/oidc.py +++ b/synapse/handlers/oidc.py @@ -37,10 +37,7 @@ from twisted.web.http_headers import Headers from synapse.config import ConfigError -from synapse.config.oidc_config import ( - OidcProviderClientSecretJwtKey, - OidcProviderConfig, -) +from synapse.config.oidc import OidcProviderClientSecretJwtKey, OidcProviderConfig from synapse.handlers.sso import MappingException, UserAttributes from synapse.http.site import SynapseRequest from synapse.logging.context import make_deferred_yieldable diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index b26aad7b3419..c5a6800b8a42 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -30,7 +30,7 @@ ) from synapse.config import ConfigError from synapse.config.captcha import CaptchaConfig -from synapse.config.consent_config import ConsentConfig +from synapse.config.consent import ConsentConfig from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.ratelimiting import FederationRateLimitConfig from synapse.config.registration import RegistrationConfig From 567f4afb42c2429776d31ee4db68bca39e733b3c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 14 Apr 2021 15:12:04 -0400 Subject: [PATCH 3/4] Update docs. --- docs/sso_mapping_providers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/sso_mapping_providers.md b/docs/sso_mapping_providers.md index e1d6ede7bac3..50020d1a4ad1 100644 --- a/docs/sso_mapping_providers.md +++ b/docs/sso_mapping_providers.md @@ -106,7 +106,7 @@ A custom mapping provider must specify the following methods: Synapse has a built-in OpenID mapping provider if a custom provider isn't specified in the config. It is located at -[`synapse.handlers.oidc_handler.JinjaOidcMappingProvider`](../synapse/handlers/oidc_handler.py). +[`synapse.handlers.oidc.JinjaOidcMappingProvider`](../synapse/handlers/oidc.py). ## SAML Mapping Providers @@ -190,4 +190,4 @@ A custom mapping provider must specify the following methods: Synapse has a built-in SAML mapping provider if a custom provider isn't specified in the config. It is located at -[`synapse.handlers.saml_handler.DefaultSamlMappingProvider`](../synapse/handlers/saml_handler.py). +[`synapse.handlers.saml.DefaultSamlMappingProvider`](../synapse/handlers/saml.py). From eee01eef204813347e31433d0b244c785c3622c6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 15 Apr 2021 10:35:06 -0400 Subject: [PATCH 4/4] Add comments. --- synapse/config/oidc.py | 2 ++ synapse/config/saml2.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/synapse/config/oidc.py b/synapse/config/oidc.py index e6817210a5c5..72402eb81d98 100644 --- a/synapse/config/oidc.py +++ b/synapse/config/oidc.py @@ -28,6 +28,8 @@ from ._base import Config, ConfigError, read_file DEFAULT_USER_MAPPING_PROVIDER = "synapse.handlers.oidc.JinjaOidcMappingProvider" +# The module that JinjaOidcMappingProvider is in was renamed, we want to +# transparently handle both the same. LEGACY_USER_MAPPING_PROVIDER = "synapse.handlers.oidc_handler.JinjaOidcMappingProvider" diff --git a/synapse/config/saml2.py b/synapse/config/saml2.py index 0cc517ab99e4..3d1218c8d14b 100644 --- a/synapse/config/saml2.py +++ b/synapse/config/saml2.py @@ -26,6 +26,8 @@ logger = logging.getLogger(__name__) DEFAULT_USER_MAPPING_PROVIDER = "synapse.handlers.saml.DefaultSamlMappingProvider" +# The module that DefaultSamlMappingProvider is in was renamed, we want to +# transparently handle both the same. LEGACY_USER_MAPPING_PROVIDER = ( "synapse.handlers.saml_handler.DefaultSamlMappingProvider" )