From dc3e58693816b897c612ea2b1d5a9f0656108d7d Mon Sep 17 00:00:00 2001 From: Alexander Trost Date: Sun, 2 Jun 2019 18:13:20 +0200 Subject: SAML2 Improvements and redirect stuff Signed-off-by: Alexander Trost --- synapse/handlers/auth.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'synapse/handlers') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index aa5d89a9ac..e6c8965a9d 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -727,6 +727,9 @@ class AuthHandler(BaseHandler): if canonical_user_id: defer.returnValue((canonical_user_id, None)) + if login_type == LoginType.SSO: + known_login_type = True + if not known_login_type: raise SynapseError(400, "Unknown login type %s" % login_type) -- cgit 1.5.1 From 426049247b271543a3a01e934851aefa727ba204 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 11 Jun 2019 00:03:57 +0100 Subject: Code cleanups and simplifications. Also: share the saml client between redirect and response handlers. --- synapse/api/constants.py | 1 - synapse/config/saml2_config.py | 7 ++- synapse/handlers/auth.py | 3 -- synapse/rest/client/v1/login.py | 83 ++++++++++++++++----------------- synapse/rest/saml2/response_resource.py | 4 +- synapse/server.py | 5 ++ 6 files changed, 53 insertions(+), 50 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 7444434048..ee129c8689 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -57,7 +57,6 @@ class LoginType(object): EMAIL_IDENTITY = u"m.login.email.identity" MSISDN = u"m.login.msisdn" RECAPTCHA = u"m.login.recaptcha" - SSO = u"m.login.sso" TERMS = u"m.login.terms" DUMMY = u"m.login.dummy" diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 60384d33ff..a6ff62df09 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from synapse.python_dependencies import DependencyException, check_requirements from ._base import Config, ConfigError @@ -25,6 +26,11 @@ class SAML2Config(Config): if not saml2_config or not saml2_config.get("enabled", True): return + try: + check_requirements('saml2') + except DependencyException as e: + raise ConfigError(e.message) + self.saml2_enabled = True import saml2.config @@ -75,7 +81,6 @@ class SAML2Config(Config): # override them. # #saml2_config: - # enabled: true # sp_config: # # point this to the IdP's metadata. You can use either a local file or # # (preferably) a URL. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index cb22869e33..7f8ddc99c6 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -767,9 +767,6 @@ class AuthHandler(BaseHandler): if canonical_user_id: defer.returnValue((canonical_user_id, None)) - if login_type == LoginType.SSO: - known_login_type = True - if not known_login_type: raise SynapseError(400, "Unknown login type %s" % login_type) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 4719712259..1a886cbbbf 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -34,10 +34,6 @@ from synapse.rest.well_known import WellKnownBuilder from synapse.types import UserID, map_username_to_mxid_localpart from synapse.util.msisdn import phone_number_to_msisdn -import saml2 -from saml2.client import Saml2Client - - logger = logging.getLogger(__name__) @@ -378,28 +374,49 @@ class LoginRestServlet(RestServlet): defer.returnValue(result) -class CasRedirectServlet(RestServlet): +class BaseSsoRedirectServlet(RestServlet): + """Common base class for /login/sso/redirect impls""" PATTERNS = client_patterns("/login/(cas|sso)/redirect", v1=True) + def on_GET(self, request): + args = request.args + if b"redirectUrl" not in args: + return 400, "Redirect URL not specified for SSO auth" + client_redirect_url = args[b"redirectUrl"][0] + sso_url = self.get_sso_url(client_redirect_url) + request.redirect(sso_url) + finish_request(request) + + def get_sso_url(self, client_redirect_url): + """Get the URL to redirect to, to perform SSO auth + + Args: + client_redirect_url (bytes): the URL that we should redirect the + client to when everything is done + + Returns: + bytes: URL to redirect to + """ + # to be implemented by subclasses + raise NotImplementedError() + + +class CasRedirectServlet(RestServlet): def __init__(self, hs): super(CasRedirectServlet, self).__init__() self.cas_server_url = hs.config.cas_server_url.encode('ascii') self.cas_service_url = hs.config.cas_service_url.encode('ascii') - def on_GET(self, request): - args = request.args - if b"redirectUrl" not in args: - return (400, "Redirect URL not specified for CAS auth") + def get_sso_url(self, client_redirect_url): client_redirect_url_param = urllib.parse.urlencode({ - b"redirectUrl": args[b"redirectUrl"][0] + b"redirectUrl": client_redirect_url }).encode('ascii') hs_redirect_url = (self.cas_service_url + b"/_matrix/client/r0/login/cas/ticket") service_param = urllib.parse.urlencode({ b"service": b"%s?%s" % (hs_redirect_url, client_redirect_url_param) }).encode('ascii') - request.redirect(b"%s/login?%s" % (self.cas_server_url, service_param)) - finish_request(request) + return b"%s/login?%s" % (self.cas_server_url, service_param) class CasTicketServlet(RestServlet): @@ -482,41 +499,23 @@ class CasTicketServlet(RestServlet): return user, attributes -class SSORedirectServlet(RestServlet): +class SAMLRedirectServlet(BaseSsoRedirectServlet): PATTERNS = client_patterns("/login/sso/redirect", v1=True) def __init__(self, hs): - super(SSORedirectServlet, self).__init__() - self.saml2_sp_config = hs.config.saml2_sp_config - - def on_GET(self, request): - args = request.args - - saml_client = Saml2Client(self.saml2_sp_config) - reqid, info = saml_client.prepare_for_authenticate() + self._saml_client = hs.get_saml_client() - redirect_url = None + def get_sso_url(self, client_redirect_url): + reqid, info = self._saml_client.prepare_for_authenticate( + relay_state=client_redirect_url, + ) - # Select the IdP URL to send the AuthN request to for key, value in info['headers']: - if key is 'Location': - redirect_url = value - - if redirect_url is None: - raise LoginError(401, "Unsuccessful SSO SAML2 redirect url response", - errcode=Codes.UNAUTHORIZED) - - relay_state = "/_matrix/client/r0/login" - if b"redirectUrl" in args: - relay_state = args[b"redirectUrl"][0] + if key == 'Location': + return value - url_parts = list(urllib.parse.urlparse(redirect_url)) - query = dict(urllib.parse.parse_qsl(url_parts[4])) - query.update({"RelayState": relay_state}) - url_parts[4] = urllib.parse.urlencode(query) - - request.redirect(urllib.parse.urlunparse(url_parts)) - finish_request(request) + # this shouldn't happen! + raise Exception("prepare_for_authenticate didn't return a Location header") class SSOAuthHandler(object): @@ -594,5 +593,5 @@ def register_servlets(hs, http_server): if hs.config.cas_enabled: CasRedirectServlet(hs).register(http_server) CasTicketServlet(hs).register(http_server) - if hs.config.saml2_enabled: - SSORedirectServlet(hs).register(http_server) + elif hs.config.saml2_enabled: + SAMLRedirectServlet(hs).register(http_server) diff --git a/synapse/rest/saml2/response_resource.py b/synapse/rest/saml2/response_resource.py index 69fb77b322..36ca1333a8 100644 --- a/synapse/rest/saml2/response_resource.py +++ b/synapse/rest/saml2/response_resource.py @@ -16,7 +16,6 @@ import logging import saml2 -from saml2.client import Saml2Client from twisted.web.resource import Resource from twisted.web.server import NOT_DONE_YET @@ -36,8 +35,7 @@ class SAML2ResponseResource(Resource): def __init__(self, hs): Resource.__init__(self) - - self._saml_client = Saml2Client(hs.config.saml2_sp_config) + self._saml_client = hs.get_saml_client() self._sso_auth_handler = SSOAuthHandler(hs) def render_POST(self, request): diff --git a/synapse/server.py b/synapse/server.py index 9229a68a8d..0eb8968674 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -189,6 +189,7 @@ class HomeServer(object): 'registration_handler', 'account_validity_handler', 'event_client_serializer', + 'saml_client', ] REQUIRED_ON_MASTER_STARTUP = [ @@ -522,6 +523,10 @@ class HomeServer(object): def build_event_client_serializer(self): return EventClientSerializer(self) + def build_saml_client(self): + from saml2.client import Saml2Client + return Saml2Client(self.config.saml2_sp_config) + def remove_pusher(self, app_id, push_key, user_id): return self.get_pusherpool().remove_pusher(app_id, push_key, user_id) -- cgit 1.5.1 From 8181e290a90bc7b7f950fb639b38d0212dca87da Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 21 Jun 2019 11:10:27 +0100 Subject: Fix sync tightloop bug. If, for some reason, presence updates take a while to persist then it can trigger clients to tightloop calling `/sync` due to the presence handler returning updates but not advancing the stream token. Fixes #5503. --- synapse/handlers/presence.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 5204073a38..3edd359985 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -1017,11 +1017,21 @@ class PresenceEventSource(object): if from_key is not None: from_key = int(from_key) + max_token = self.store.get_current_presence_token() + if from_key == max_token: + # This is necessary as due to the way stream ID generators work + # we may get updates that have a stream ID greater than the max + # token. This is usually fine, as it just means that we may send + # down some presence updates multiple times. However, we need to + # be careful that the sync stream actually does make some + # progress, otherwise clients will end up tight looping calling + # /sync due to it returning the same token repeatedly. Hence + # this guard. C.f. #5503. + defer.returnValue(([], max_token)) + presence = self.get_presence_handler() stream_change_cache = self.store.presence_stream_cache - max_token = self.store.get_current_presence_token() - users_interested_in = yield self._get_interested_in(user, explicit_room_id) user_ids_changed = set() -- cgit 1.5.1 From 370532210307822eb90dc656449b23df7c6c0dd8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jun 2019 22:52:02 +0100 Subject: Move all the saml stuff out to a centralised handler --- synapse/handlers/saml2_handler.py | 86 +++++++++++++++++++++++++++++++++ synapse/rest/client/v1/login.py | 13 +---- synapse/rest/saml2/response_resource.py | 35 +------------- synapse/server.py | 12 ++--- 4 files changed, 96 insertions(+), 50 deletions(-) create mode 100644 synapse/handlers/saml2_handler.py (limited to 'synapse/handlers') diff --git a/synapse/handlers/saml2_handler.py b/synapse/handlers/saml2_handler.py new file mode 100644 index 0000000000..880e6a625f --- /dev/null +++ b/synapse/handlers/saml2_handler.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import logging + +import saml2 +from saml2.client import Saml2Client + +from synapse.api.errors import CodeMessageException +from synapse.http.servlet import parse_string +from synapse.rest.client.v1.login import SSOAuthHandler + +logger = logging.getLogger(__name__) + + +class Saml2Handler: + def __init__(self, hs): + self._saml_client = Saml2Client(hs.config.saml2_sp_config) + self._sso_auth_handler = SSOAuthHandler(hs) + + def handle_redirect_request(self, client_redirect_url): + """Handle an incoming request to /login/sso/redirect + + Args: + client_redirect_url (bytes): the URL that we should redirect the + client to when everything is done + + Returns: + bytes: URL to redirect to + """ + reqid, info = self._saml_client.prepare_for_authenticate( + relay_state=client_redirect_url + ) + + for key, value in info["headers"]: + if key == "Location": + return value + + # this shouldn't happen! + raise Exception("prepare_for_authenticate didn't return a Location header") + + def handle_saml_response(self, request): + """Handle an incoming request to /_matrix/saml2/authn_response + + Args: + request (SynapseRequest): the incoming request from the browser. We'll + respond to it with a redirect. + + Returns: + Deferred[none]: Completes once we have handled the request. + """ + resp_bytes = parse_string(request, "SAMLResponse", required=True) + relay_state = parse_string(request, "RelayState", required=True) + + try: + saml2_auth = self._saml_client.parse_authn_request_response( + resp_bytes, saml2.BINDING_HTTP_POST + ) + except Exception as e: + logger.warning("Exception parsing SAML2 response", exc_info=1) + raise CodeMessageException(400, "Unable to parse SAML2 response: %s" % (e,)) + + if saml2_auth.not_signed: + raise CodeMessageException(400, "SAML2 response was not signed") + + if "uid" not in saml2_auth.ava: + raise CodeMessageException(400, "uid not in SAML2 response") + + username = saml2_auth.ava["uid"][0] + + displayName = saml2_auth.ava.get("displayName", [None])[0] + + return self._sso_auth_handler.on_successful_auth( + username, request, relay_state, user_display_name=displayName + ) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index a31d277935..b59aa3d5c9 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -487,19 +487,10 @@ class SAMLRedirectServlet(BaseSsoRedirectServlet): PATTERNS = client_patterns("/login/sso/redirect", v1=True) def __init__(self, hs): - self._saml_client = hs.get_saml_client() + self._saml_handler = hs.get_saml_handler() def get_sso_url(self, client_redirect_url): - reqid, info = self._saml_client.prepare_for_authenticate( - relay_state=client_redirect_url - ) - - for key, value in info["headers"]: - if key == "Location": - return value - - # this shouldn't happen! - raise Exception("prepare_for_authenticate didn't return a Location header") + return self._saml_handler.handle_redirect_request(client_redirect_url) class SSOAuthHandler(object): diff --git a/synapse/rest/saml2/response_resource.py b/synapse/rest/saml2/response_resource.py index 9ec56d6adb..8ee22473e9 100644 --- a/synapse/rest/saml2/response_resource.py +++ b/synapse/rest/saml2/response_resource.py @@ -13,19 +13,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import logging - -import saml2 from twisted.web.resource import Resource from twisted.web.server import NOT_DONE_YET -from synapse.api.errors import CodeMessageException from synapse.http.server import wrap_html_request_handler -from synapse.http.servlet import parse_string -from synapse.rest.client.v1.login import SSOAuthHandler - -logger = logging.getLogger(__name__) class SAML2ResponseResource(Resource): @@ -35,8 +27,7 @@ class SAML2ResponseResource(Resource): def __init__(self, hs): Resource.__init__(self) - self._saml_client = hs.get_saml_client() - self._sso_auth_handler = SSOAuthHandler(hs) + self._saml_handler = hs.get_saml_handler() def render_POST(self, request): self._async_render_POST(request) @@ -44,26 +35,4 @@ class SAML2ResponseResource(Resource): @wrap_html_request_handler def _async_render_POST(self, request): - resp_bytes = parse_string(request, "SAMLResponse", required=True) - relay_state = parse_string(request, "RelayState", required=True) - - try: - saml2_auth = self._saml_client.parse_authn_request_response( - resp_bytes, saml2.BINDING_HTTP_POST - ) - except Exception as e: - logger.warning("Exception parsing SAML2 response", exc_info=1) - raise CodeMessageException(400, "Unable to parse SAML2 response: %s" % (e,)) - - if saml2_auth.not_signed: - raise CodeMessageException(400, "SAML2 response was not signed") - - if "uid" not in saml2_auth.ava: - raise CodeMessageException(400, "uid not in SAML2 response") - - username = saml2_auth.ava["uid"][0] - - displayName = saml2_auth.ava.get("displayName", [None])[0] - return self._sso_auth_handler.on_successful_auth( - username, request, relay_state, user_display_name=displayName - ) + return self._saml_handler.handle_saml_response(request) diff --git a/synapse/server.py b/synapse/server.py index dbb35c7227..1bc8c08b58 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -194,8 +194,8 @@ class HomeServer(object): "sendmail", "registration_handler", "account_validity_handler", + "saml2_handler", "event_client_serializer", - "saml_client", ] REQUIRED_ON_MASTER_STARTUP = ["user_directory_handler", "stats_handler"] @@ -525,13 +525,13 @@ class HomeServer(object): def build_account_validity_handler(self): return AccountValidityHandler(self) - def build_event_client_serializer(self): - return EventClientSerializer(self) + def build_saml2_handler(self): + from synapse.handlers.saml2_handler import Saml2Handler - def build_saml_client(self): - from saml2.client import Saml2Client + return Saml2Handler(self) - return Saml2Client(self.config.saml2_sp_config) + def build_event_client_serializer(self): + return EventClientSerializer(self) def remove_pusher(self, app_id, push_key, user_id): return self.get_pusherpool().remove_pusher(app_id, push_key, user_id) -- cgit 1.5.1 From 36f4953dec97ec1650b7c0bb75905ed907a8cac1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jun 2019 23:50:55 +0100 Subject: Add support for tracking SAML2 sessions. This allows us to correctly handle `allow_unsolicited: False`. --- synapse/config/saml2_config.py | 20 +++++++++++++++++++- synapse/handlers/saml2_handler.py | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 3 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 463b5fdd68..965a97837f 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -27,7 +27,7 @@ class SAML2Config(Config): return try: - check_requirements('saml2') + check_requirements("saml2") except DependencyException as e: raise ConfigError(e.message) @@ -43,6 +43,11 @@ class SAML2Config(Config): if config_path is not None: self.saml2_sp_config.load_file(config_path) + # session lifetime: in milliseconds + self.saml2_session_lifetime = self.parse_duration( + saml2_config.get("saml_session_lifetime", "5m") + ) + def _default_saml_config_dict(self): import saml2 @@ -87,6 +92,13 @@ class SAML2Config(Config): # remote: # - url: https://our_idp/metadata.xml # + # # By default, the user has to go to our login page first. If you'd like to + # # allow IdP-initiated login, set 'allow_unsolicited: True' in an 'sp' + # # section: + # # + # #sp: + # # allow_unsolicited: True + # # # # The rest of sp_config is just used to generate our metadata xml, and you # # may well not need it, depending on your setup. Alternatively you # # may need a whole lot more detail - see the pysaml2 docs! @@ -110,6 +122,12 @@ class SAML2Config(Config): # # separate pysaml2 configuration file: # # # config_path: "%(config_dir_path)s/sp_conf.py" + # + # # the lifetime of a SAML session. This defines how long a user has to + # # complete the authentication process, if allow_unsolicited is unset. + # # The default is 5 minutes. + # # + # # saml_session_lifetime: 5m """ % { "config_dir_path": config_dir_path } diff --git a/synapse/handlers/saml2_handler.py b/synapse/handlers/saml2_handler.py index 880e6a625f..b06d3f172e 100644 --- a/synapse/handlers/saml2_handler.py +++ b/synapse/handlers/saml2_handler.py @@ -14,6 +14,7 @@ # limitations under the License. import logging +import attr import saml2 from saml2.client import Saml2Client @@ -29,6 +30,12 @@ class Saml2Handler: self._saml_client = Saml2Client(hs.config.saml2_sp_config) self._sso_auth_handler = SSOAuthHandler(hs) + # a map from saml session id to Saml2SessionData object + self._outstanding_requests_dict = {} + + self._clock = hs.get_clock() + self._saml2_session_lifetime = hs.config.saml2_session_lifetime + def handle_redirect_request(self, client_redirect_url): """Handle an incoming request to /login/sso/redirect @@ -43,6 +50,9 @@ class Saml2Handler: relay_state=client_redirect_url ) + now = self._clock.time_msec() + self._outstanding_requests_dict[reqid] = Saml2SessionData(creation_time=now) + for key, value in info["headers"]: if key == "Location": return value @@ -63,9 +73,15 @@ class Saml2Handler: resp_bytes = parse_string(request, "SAMLResponse", required=True) relay_state = parse_string(request, "RelayState", required=True) + # expire outstanding sessions before parse_authn_request_response checks + # the dict. + self.expire_sessions() + try: saml2_auth = self._saml_client.parse_authn_request_response( - resp_bytes, saml2.BINDING_HTTP_POST + resp_bytes, + saml2.BINDING_HTTP_POST, + outstanding=self._outstanding_requests_dict, ) except Exception as e: logger.warning("Exception parsing SAML2 response", exc_info=1) @@ -77,10 +93,29 @@ class Saml2Handler: if "uid" not in saml2_auth.ava: raise CodeMessageException(400, "uid not in SAML2 response") - username = saml2_auth.ava["uid"][0] + self._outstanding_requests_dict.pop(saml2_auth.in_response_to, None) + username = saml2_auth.ava["uid"][0] displayName = saml2_auth.ava.get("displayName", [None])[0] return self._sso_auth_handler.on_successful_auth( username, request, relay_state, user_display_name=displayName ) + + def expire_sessions(self): + expire_before = self._clock.time_msec() - self._saml2_session_lifetime + to_expire = set() + for reqid, data in self._outstanding_requests_dict.items(): + if data.creation_time < expire_before: + to_expire.add(reqid) + for reqid in to_expire: + logger.debug("Expiring session id %s", reqid) + del self._outstanding_requests_dict[reqid] + + +@attr.s +class Saml2SessionData: + """Data we track about SAML2 sessions""" + + # time the session was created, in milliseconds + creation_time = attr.ib() -- cgit 1.5.1 From 28db0ae5377ca8e7133957008ba35b49432636ee Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 27 Jun 2019 00:37:41 +0100 Subject: cleanups --- synapse/config/saml2_config.py | 19 ++++-- synapse/handlers/saml2_handler.py | 121 -------------------------------------- synapse/handlers/saml_handler.py | 121 ++++++++++++++++++++++++++++++++++++++ synapse/server.py | 8 +-- 4 files changed, 138 insertions(+), 131 deletions(-) delete mode 100644 synapse/handlers/saml2_handler.py create mode 100644 synapse/handlers/saml_handler.py (limited to 'synapse/handlers') diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 965a97837f..6a8161547a 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -83,6 +83,12 @@ class SAML2Config(Config): # so it is not normally necessary to specify them unless you need to # override them. # + # Once SAML support is enabled, a metadata file will be exposed at + # https://:/_matrix/saml2/metadata.xml, which you may be able to + # use to configure your SAML IdP with. Alternatively, you can manually configure + # the IdP to use an ACS location of + # https://:/_matrix/saml2/authn_response. + # #saml2_config: # sp_config: # # point this to the IdP's metadata. You can use either a local file or @@ -93,13 +99,14 @@ class SAML2Config(Config): # - url: https://our_idp/metadata.xml # # # By default, the user has to go to our login page first. If you'd like to - # # allow IdP-initiated login, set 'allow_unsolicited: True' in an 'sp' - # # section: - # # - # #sp: - # # allow_unsolicited: True + # # allow IdP-initiated login, set 'allow_unsolicited: True' in a + # # 'service.sp' section: # # - # # The rest of sp_config is just used to generate our metadata xml, and you + # #service: + # # sp: + # # allow_unsolicited: True + # + # # The examples below are just used to generate our metadata xml, and you # # may well not need it, depending on your setup. Alternatively you # # may need a whole lot more detail - see the pysaml2 docs! # diff --git a/synapse/handlers/saml2_handler.py b/synapse/handlers/saml2_handler.py deleted file mode 100644 index b06d3f172e..0000000000 --- a/synapse/handlers/saml2_handler.py +++ /dev/null @@ -1,121 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2019 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import logging - -import attr -import saml2 -from saml2.client import Saml2Client - -from synapse.api.errors import CodeMessageException -from synapse.http.servlet import parse_string -from synapse.rest.client.v1.login import SSOAuthHandler - -logger = logging.getLogger(__name__) - - -class Saml2Handler: - def __init__(self, hs): - self._saml_client = Saml2Client(hs.config.saml2_sp_config) - self._sso_auth_handler = SSOAuthHandler(hs) - - # a map from saml session id to Saml2SessionData object - self._outstanding_requests_dict = {} - - self._clock = hs.get_clock() - self._saml2_session_lifetime = hs.config.saml2_session_lifetime - - def handle_redirect_request(self, client_redirect_url): - """Handle an incoming request to /login/sso/redirect - - Args: - client_redirect_url (bytes): the URL that we should redirect the - client to when everything is done - - Returns: - bytes: URL to redirect to - """ - reqid, info = self._saml_client.prepare_for_authenticate( - relay_state=client_redirect_url - ) - - now = self._clock.time_msec() - self._outstanding_requests_dict[reqid] = Saml2SessionData(creation_time=now) - - for key, value in info["headers"]: - if key == "Location": - return value - - # this shouldn't happen! - raise Exception("prepare_for_authenticate didn't return a Location header") - - def handle_saml_response(self, request): - """Handle an incoming request to /_matrix/saml2/authn_response - - Args: - request (SynapseRequest): the incoming request from the browser. We'll - respond to it with a redirect. - - Returns: - Deferred[none]: Completes once we have handled the request. - """ - resp_bytes = parse_string(request, "SAMLResponse", required=True) - relay_state = parse_string(request, "RelayState", required=True) - - # expire outstanding sessions before parse_authn_request_response checks - # the dict. - self.expire_sessions() - - try: - saml2_auth = self._saml_client.parse_authn_request_response( - resp_bytes, - saml2.BINDING_HTTP_POST, - outstanding=self._outstanding_requests_dict, - ) - except Exception as e: - logger.warning("Exception parsing SAML2 response", exc_info=1) - raise CodeMessageException(400, "Unable to parse SAML2 response: %s" % (e,)) - - if saml2_auth.not_signed: - raise CodeMessageException(400, "SAML2 response was not signed") - - if "uid" not in saml2_auth.ava: - raise CodeMessageException(400, "uid not in SAML2 response") - - self._outstanding_requests_dict.pop(saml2_auth.in_response_to, None) - - username = saml2_auth.ava["uid"][0] - displayName = saml2_auth.ava.get("displayName", [None])[0] - - return self._sso_auth_handler.on_successful_auth( - username, request, relay_state, user_display_name=displayName - ) - - def expire_sessions(self): - expire_before = self._clock.time_msec() - self._saml2_session_lifetime - to_expire = set() - for reqid, data in self._outstanding_requests_dict.items(): - if data.creation_time < expire_before: - to_expire.add(reqid) - for reqid in to_expire: - logger.debug("Expiring session id %s", reqid) - del self._outstanding_requests_dict[reqid] - - -@attr.s -class Saml2SessionData: - """Data we track about SAML2 sessions""" - - # time the session was created, in milliseconds - creation_time = attr.ib() diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py new file mode 100644 index 0000000000..03a0ac4384 --- /dev/null +++ b/synapse/handlers/saml_handler.py @@ -0,0 +1,121 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import logging + +import attr +import saml2 +from saml2.client import Saml2Client + +from synapse.api.errors import CodeMessageException +from synapse.http.servlet import parse_string +from synapse.rest.client.v1.login import SSOAuthHandler + +logger = logging.getLogger(__name__) + + +class SamlHandler: + def __init__(self, hs): + self._saml_client = Saml2Client(hs.config.saml2_sp_config) + self._sso_auth_handler = SSOAuthHandler(hs) + + # a map from saml session id to Saml2SessionData object + self._outstanding_requests_dict = {} + + self._clock = hs.get_clock() + self._saml2_session_lifetime = hs.config.saml2_session_lifetime + + def handle_redirect_request(self, client_redirect_url): + """Handle an incoming request to /login/sso/redirect + + Args: + client_redirect_url (bytes): the URL that we should redirect the + client to when everything is done + + Returns: + bytes: URL to redirect to + """ + reqid, info = self._saml_client.prepare_for_authenticate( + relay_state=client_redirect_url + ) + + now = self._clock.time_msec() + self._outstanding_requests_dict[reqid] = Saml2SessionData(creation_time=now) + + for key, value in info["headers"]: + if key == "Location": + return value + + # this shouldn't happen! + raise Exception("prepare_for_authenticate didn't return a Location header") + + def handle_saml_response(self, request): + """Handle an incoming request to /_matrix/saml2/authn_response + + Args: + request (SynapseRequest): the incoming request from the browser. We'll + respond to it with a redirect. + + Returns: + Deferred[none]: Completes once we have handled the request. + """ + resp_bytes = parse_string(request, "SAMLResponse", required=True) + relay_state = parse_string(request, "RelayState", required=True) + + # expire outstanding sessions before parse_authn_request_response checks + # the dict. + self.expire_sessions() + + try: + saml2_auth = self._saml_client.parse_authn_request_response( + resp_bytes, + saml2.BINDING_HTTP_POST, + outstanding=self._outstanding_requests_dict, + ) + except Exception as e: + logger.warning("Exception parsing SAML2 response", exc_info=1) + raise CodeMessageException(400, "Unable to parse SAML2 response: %s" % (e,)) + + if saml2_auth.not_signed: + raise CodeMessageException(400, "SAML2 response was not signed") + + if "uid" not in saml2_auth.ava: + raise CodeMessageException(400, "uid not in SAML2 response") + + self._outstanding_requests_dict.pop(saml2_auth.in_response_to, None) + + username = saml2_auth.ava["uid"][0] + displayName = saml2_auth.ava.get("displayName", [None])[0] + + return self._sso_auth_handler.on_successful_auth( + username, request, relay_state, user_display_name=displayName + ) + + def expire_sessions(self): + expire_before = self._clock.time_msec() - self._saml2_session_lifetime + to_expire = set() + for reqid, data in self._outstanding_requests_dict.items(): + if data.creation_time < expire_before: + to_expire.add(reqid) + for reqid in to_expire: + logger.debug("Expiring session id %s", reqid) + del self._outstanding_requests_dict[reqid] + + +@attr.s +class Saml2SessionData: + """Data we track about SAML2 sessions""" + + # time the session was created, in milliseconds + creation_time = attr.ib() diff --git a/synapse/server.py b/synapse/server.py index 1bc8c08b58..9e28dba2b1 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -194,7 +194,7 @@ class HomeServer(object): "sendmail", "registration_handler", "account_validity_handler", - "saml2_handler", + "saml_handler", "event_client_serializer", ] @@ -525,10 +525,10 @@ class HomeServer(object): def build_account_validity_handler(self): return AccountValidityHandler(self) - def build_saml2_handler(self): - from synapse.handlers.saml2_handler import Saml2Handler + def build_saml_handler(self): + from synapse.handlers.saml_handler import SamlHandler - return Saml2Handler(self) + return SamlHandler(self) def build_event_client_serializer(self): return EventClientSerializer(self) -- cgit 1.5.1 From 01d0f8e701b4c2ddd04eee1a26edef952c0ac558 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 28 Jun 2019 15:17:15 +0100 Subject: Don't update the ratelimiter before sending a 3PID invite This would cause emails being sent, but Synapse responding with a 429 when creating the event. The client would then retry, and with bad timing the same scenario would happen again. Some testing I did ended up sending me 10 emails for one single invite because of this. --- synapse/handlers/room_member.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 4d6e883802..c860acf970 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -676,7 +676,7 @@ class RoomMemberHandler(object): # We need to rate limit *before* we send out any 3PID invites, so we # can't just rely on the standard ratelimiting of events. - yield self.base_handler.ratelimit(requester) + yield self.base_handler.ratelimit(requester, update=False) can_invite = yield self.third_party_event_rules.check_threepid_can_be_invited( medium, address, room_id -- cgit 1.5.1 From 15d9fc31bd549e2b9c04f96a0d3e8938c1bdc6a5 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 28 Jun 2019 16:04:05 +0100 Subject: Only ratelimit when sending the email If we do the opposite, an event can arrive after or while sending the email and the 3PID invite event will get ratelimited. --- synapse/handlers/room_member.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index c860acf970..66b05b4732 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -676,7 +676,7 @@ class RoomMemberHandler(object): # We need to rate limit *before* we send out any 3PID invites, so we # can't just rely on the standard ratelimiting of events. - yield self.base_handler.ratelimit(requester, update=False) + yield self.base_handler.ratelimit(requester) can_invite = yield self.third_party_event_rules.check_threepid_can_be_invited( medium, address, room_id @@ -823,6 +823,7 @@ class RoomMemberHandler(object): "sender": user.to_string(), "state_key": token, }, + ratelimit=False, txn_id=txn_id, ) -- cgit 1.5.1 From 915280f1edec3ddfe6261940d91ef451f207ed15 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 1 Jul 2019 10:22:42 +0100 Subject: Fixup comment --- synapse/handlers/presence.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 3edd359985..c80dc2eba0 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -1021,12 +1021,19 @@ class PresenceEventSource(object): if from_key == max_token: # This is necessary as due to the way stream ID generators work # we may get updates that have a stream ID greater than the max - # token. This is usually fine, as it just means that we may send - # down some presence updates multiple times. However, we need to - # be careful that the sync stream actually does make some - # progress, otherwise clients will end up tight looping calling - # /sync due to it returning the same token repeatedly. Hence - # this guard. C.f. #5503. + # token (e.g. max_token is N but stream generator may return + # results for N+2, due to N+1 not having finished being + # persisted yet). + # + # This is usually fine, as it just means that we may send down + # some presence updates multiple times. However, we need to be + # careful that the sync stream either actually does make some + # progress or doesn't return, otherwise clients will end up + # tight looping calling /sync due to it immediately returning + # the same token repeatedly. + # + # Hence this guard where we just return nothing so that the sync + # doesn't return. C.f. #5503. defer.returnValue(([], max_token)) presence = self.get_presence_handler() -- cgit 1.5.1 From 3bcb13edd098ae634946d213472a2caf5134b9a8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Jul 2019 12:13:22 +0100 Subject: Address review comments --- synapse/handlers/saml_handler.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 03a0ac4384..a1ce6929cf 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -18,7 +18,7 @@ import attr import saml2 from saml2.client import Saml2Client -from synapse.api.errors import CodeMessageException +from synapse.api.errors import SynapseError from synapse.http.servlet import parse_string from synapse.rest.client.v1.login import SSOAuthHandler @@ -84,14 +84,16 @@ class SamlHandler: outstanding=self._outstanding_requests_dict, ) except Exception as e: - logger.warning("Exception parsing SAML2 response", exc_info=1) - raise CodeMessageException(400, "Unable to parse SAML2 response: %s" % (e,)) + logger.warning("Exception parsing SAML2 response: %s", e) + raise SynapseError(400, "Unable to parse SAML2 response: %s" % (e,)) if saml2_auth.not_signed: - raise CodeMessageException(400, "SAML2 response was not signed") + logger.warning("SAML2 response was not signed") + raise SynapseError(400, "SAML2 response was not signed") if "uid" not in saml2_auth.ava: - raise CodeMessageException(400, "uid not in SAML2 response") + logger.warning("SAML2 response lacks a 'uid' attestation") + raise SynapseError(400, "uid not in SAML2 response") self._outstanding_requests_dict.pop(saml2_auth.in_response_to, None) -- cgit 1.5.1