From 3d95405e5fe4ef1b795dbfd63a3532cac65b8cd4 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Tue, 5 Apr 2016 17:26:37 +0200 Subject: [PATCH 01/14] Introduce LDAP authentication --- synapse/config/homeserver.py | 3 +- synapse/config/ldap.py | 48 ++++++++++++++++++++++++++ synapse/python_dependencies.py | 1 + synapse/rest/client/v1/login.py | 60 +++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 synapse/config/ldap.py diff --git a/synapse/config/homeserver.py b/synapse/config/homeserver.py index acf74c8761..9a80ac39ec 100644 --- a/synapse/config/homeserver.py +++ b/synapse/config/homeserver.py @@ -30,13 +30,14 @@ from .saml2 import SAML2Config from .cas import CasConfig from .password import PasswordConfig from .jwt import JWTConfig +from .ldap import LDAPConfig class HomeServerConfig(TlsConfig, ServerConfig, DatabaseConfig, LoggingConfig, RatelimitConfig, ContentRepositoryConfig, CaptchaConfig, VoipConfig, RegistrationConfig, MetricsConfig, ApiConfig, AppServiceConfig, KeyConfig, SAML2Config, CasConfig, - JWTConfig, PasswordConfig,): + JWTConfig, LDAPConfig, PasswordConfig,): pass diff --git a/synapse/config/ldap.py b/synapse/config/ldap.py new file mode 100644 index 0000000000..86528139e2 --- /dev/null +++ b/synapse/config/ldap.py @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- +# Copyright 2015 Niklas Riekenbrauck +# +# 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. + +from ._base import Config + + +class LDAPConfig(Config): + def read_config(self, config): + ldap_config = config.get("ldap_config", None) + if ldap_config: + self.ldap_enabled = ldap_config.get("enabled", False) + self.ldap_server = ldap_config["server"] + self.ldap_port = ldap_config["port"] + self.ldap_search_base = ldap_config["search_base"] + self.ldap_search_property = ldap_config["search_property"] + self.ldap_email_property = ldap_config["email_property"] + self.ldap_full_name_property = ldap_config["full_name_property"] + else: + self.ldap_enabled = False + self.ldap_server = None + self.ldap_port = None + self.ldap_search_base = None + self.ldap_search_property = None + self.ldap_email_property = None + self.ldap_full_name_property = None + + def default_config(self, **kwargs): + return """\ + # ldap_config: + # server: "ldap://localhost" + # port: 389 + # search_base: "ou=Users,dc=example,dc=com" + # search_property: "cn" + # email_property: "email" + # full_name_property: "givenName" + """ diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index cf1414b4db..d6b6e82bd7 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -37,6 +37,7 @@ REQUIREMENTS = { "pysaml2>=3.0.0,<4.0.0": ["saml2>=3.0.0,<4.0.0"], "pymacaroons-pynacl": ["pymacaroons"], "pyjwt": ["jwt"], + "python-ldap": ["ldap"], } CONDITIONAL_REQUIREMENTS = { "web_client": { diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index d14ce3efa2..13720973be 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -36,6 +36,8 @@ import xml.etree.ElementTree as ET import jwt from jwt.exceptions import InvalidTokenError +import ldap + logger = logging.getLogger(__name__) @@ -47,6 +49,7 @@ class LoginRestServlet(ClientV1RestServlet): CAS_TYPE = "m.login.cas" TOKEN_TYPE = "m.login.token" JWT_TYPE = "m.login.jwt" + LDAP_TYPE = "m.login.ldap" def __init__(self, hs): super(LoginRestServlet, self).__init__(hs) @@ -56,6 +59,13 @@ class LoginRestServlet(ClientV1RestServlet): self.jwt_enabled = hs.config.jwt_enabled self.jwt_secret = hs.config.jwt_secret self.jwt_algorithm = hs.config.jwt_algorithm + self.ldap_enabled = hs.config.ldap_enabled + self.ldap_server = hs.config.ldap_server + self.ldap_port = hs.config.ldap_port + self.ldap_search_base = hs.config.ldap_search_base + self.ldap_search_property = hs.config.ldap_search_property + self.ldap_email_property = hs.config.ldap_email_property + self.ldap_full_name_property = hs.config.ldap_full_name_property self.cas_enabled = hs.config.cas_enabled self.cas_server_url = hs.config.cas_server_url self.cas_required_attributes = hs.config.cas_required_attributes @@ -64,6 +74,8 @@ class LoginRestServlet(ClientV1RestServlet): def on_GET(self, request): flows = [] + if self.ldap_enabled: + flows.append({"type": LoginRestServlet.LDAP_TYPE}) if self.jwt_enabled: flows.append({"type": LoginRestServlet.JWT_TYPE}) if self.saml2_enabled: @@ -107,6 +119,10 @@ class LoginRestServlet(ClientV1RestServlet): "uri": "%s%s" % (self.idp_redirect_url, relay_state) } defer.returnValue((200, result)) + elif self.ldap_enabled and (login_submission["type"] == + LoginRestServlet.JWT_TYPE): + result = yield self.do_ldap_login(login_submission) + defer.returnValue(result) elif self.jwt_enabled and (login_submission["type"] == LoginRestServlet.JWT_TYPE): result = yield self.do_jwt_login(login_submission) @@ -160,6 +176,50 @@ class LoginRestServlet(ClientV1RestServlet): defer.returnValue((200, result)) + @defer.inlineCallbacks + def do_ldap_login(self, login_submission): + if 'medium' in login_submission and 'address' in login_submission: + user_id = yield self.hs.get_datastore().get_user_id_by_threepid( + login_submission['medium'], login_submission['address'] + ) + if not user_id: + raise LoginError(403, "", errcode=Codes.FORBIDDEN) + else: + user_id = login_submission['user'] + + if not user_id.startswith('@'): + user_id = UserID.create( + user_id, self.hs.hostname + ).to_string() + + # FIXME check against LDAP Server!! + + auth_handler = self.handlers.auth_handler + user_exists = yield auth_handler.does_user_exist(user_id) + if user_exists: + user_id, access_token, refresh_token = ( + yield auth_handler.get_login_tuple_for_user_id(user_id) + ) + result = { + "user_id": user_id, # may have changed + "access_token": access_token, + "refresh_token": refresh_token, + "home_server": self.hs.hostname, + } + + else: + user_id, access_token = ( + yield self.handlers.registration_handler.register(localpart=user_id.localpart) + ) + result = { + "user_id": user_id, # may have changed + "access_token": access_token, + "home_server": self.hs.hostname, + } + + defer.returnValue((200, result)) + + @defer.inlineCallbacks def do_token_login(self, login_submission): token = login_submission['token'] From 7b9319b1c837991ab187e2f280ff267c35a7c653 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 13:02:49 +0200 Subject: [PATCH 02/14] move LDAP authentication to AuthenticationHandler --- synapse/handlers/auth.py | 54 ++++++++++++++++++++++++++++---- synapse/rest/client/v1/login.py | 55 --------------------------------- 2 files changed, 48 insertions(+), 61 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index d5d6faa85f..eeca820845 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -30,6 +30,8 @@ import simplejson import synapse.util.stringutils as stringutils +import ldap + logger = logging.getLogger(__name__) @@ -49,6 +51,15 @@ class AuthHandler(BaseHandler): self.sessions = {} self.INVALID_TOKEN_HTTP_STATUS = 401 + self.ldap_enabled = hs.config.ldap_enabled + self.ldap_server = hs.config.ldap_server + self.ldap_port = hs.config.ldap_port + self.ldap_search_base = hs.config.ldap_search_base + self.ldap_search_property = hs.config.ldap_search_property + self.ldap_email_property = hs.config.ldap_email_property + self.ldap_full_name_property = hs.config.ldap_full_name_property + + @defer.inlineCallbacks def check_auth(self, flows, clientdict, clientip): """ @@ -215,8 +226,8 @@ class AuthHandler(BaseHandler): if not user_id.startswith('@'): user_id = UserID.create(user_id, self.hs.hostname).to_string() - user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) - self._check_password(user_id, password, password_hash) + self._check_password(user_id, password) + defer.returnValue(user_id) @defer.inlineCallbacks @@ -340,8 +351,8 @@ class AuthHandler(BaseHandler): StoreError if there was a problem storing the token. LoginError if there was an authentication problem. """ - user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) - self._check_password(user_id, password, password_hash) + + self._check_password(user_id, password) logger.info("Logging in user %s", user_id) access_token = yield self.issue_access_token(user_id) @@ -407,12 +418,43 @@ class AuthHandler(BaseHandler): else: defer.returnValue(user_infos.popitem()) - def _check_password(self, user_id, password, stored_hash): + def _check_password(self, user_id, password): """Checks that user_id has passed password, raises LoginError if not.""" - if not self.validate_hash(password, stored_hash): + + if not (self._check_ldap_password(user_id, password) or self._check_local_password(user_id, password)): logger.warn("Failed password login for user %s", user_id) raise LoginError(403, "", errcode=Codes.FORBIDDEN) + def _check_local_password(self, user_id, password): + user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) + return not self.validate_hash(password, password_hash) + + def _check_ldap_password(self, user_id, password): + if not self.ldap_enabled: + return False + + logger.info("Authenticating %s with LDAP" % user_id) + try: + l = ldap.initialize("%s:%s" % (ldap_server, ldap_port)) + if self.ldap_tls: + logger.debug("Initiating TLS") + self._connection.start_tls_s() + + dn = "%s=%s, %s" % (ldap_search_property, user_id.localpart, ldap_search_base) + logger.debug("DN for LDAP authentication: %s" % dn) + + l.simple_bind_s(dn.encode('utf-8'), password.encode('utf-8')) + + if not self.does_user_exist(user_id): + user_id, access_token = ( + yield self.handlers.registration_handler.register(localpart=user_id.localpart) + ) + + return True + except ldap.LDAPError, e: + logger.info(e) + return False + @defer.inlineCallbacks def issue_access_token(self, user_id): access_token = self.generate_access_token(user_id) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 13720973be..da0fd2a8e0 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -36,8 +36,6 @@ import xml.etree.ElementTree as ET import jwt from jwt.exceptions import InvalidTokenError -import ldap - logger = logging.getLogger(__name__) @@ -49,7 +47,6 @@ class LoginRestServlet(ClientV1RestServlet): CAS_TYPE = "m.login.cas" TOKEN_TYPE = "m.login.token" JWT_TYPE = "m.login.jwt" - LDAP_TYPE = "m.login.ldap" def __init__(self, hs): super(LoginRestServlet, self).__init__(hs) @@ -59,13 +56,6 @@ class LoginRestServlet(ClientV1RestServlet): self.jwt_enabled = hs.config.jwt_enabled self.jwt_secret = hs.config.jwt_secret self.jwt_algorithm = hs.config.jwt_algorithm - self.ldap_enabled = hs.config.ldap_enabled - self.ldap_server = hs.config.ldap_server - self.ldap_port = hs.config.ldap_port - self.ldap_search_base = hs.config.ldap_search_base - self.ldap_search_property = hs.config.ldap_search_property - self.ldap_email_property = hs.config.ldap_email_property - self.ldap_full_name_property = hs.config.ldap_full_name_property self.cas_enabled = hs.config.cas_enabled self.cas_server_url = hs.config.cas_server_url self.cas_required_attributes = hs.config.cas_required_attributes @@ -74,8 +64,6 @@ class LoginRestServlet(ClientV1RestServlet): def on_GET(self, request): flows = [] - if self.ldap_enabled: - flows.append({"type": LoginRestServlet.LDAP_TYPE}) if self.jwt_enabled: flows.append({"type": LoginRestServlet.JWT_TYPE}) if self.saml2_enabled: @@ -176,49 +164,6 @@ class LoginRestServlet(ClientV1RestServlet): defer.returnValue((200, result)) - @defer.inlineCallbacks - def do_ldap_login(self, login_submission): - if 'medium' in login_submission and 'address' in login_submission: - user_id = yield self.hs.get_datastore().get_user_id_by_threepid( - login_submission['medium'], login_submission['address'] - ) - if not user_id: - raise LoginError(403, "", errcode=Codes.FORBIDDEN) - else: - user_id = login_submission['user'] - - if not user_id.startswith('@'): - user_id = UserID.create( - user_id, self.hs.hostname - ).to_string() - - # FIXME check against LDAP Server!! - - auth_handler = self.handlers.auth_handler - user_exists = yield auth_handler.does_user_exist(user_id) - if user_exists: - user_id, access_token, refresh_token = ( - yield auth_handler.get_login_tuple_for_user_id(user_id) - ) - result = { - "user_id": user_id, # may have changed - "access_token": access_token, - "refresh_token": refresh_token, - "home_server": self.hs.hostname, - } - - else: - user_id, access_token = ( - yield self.handlers.registration_handler.register(localpart=user_id.localpart) - ) - result = { - "user_id": user_id, # may have changed - "access_token": access_token, - "home_server": self.hs.hostname, - } - - defer.returnValue((200, result)) - @defer.inlineCallbacks def do_token_login(self, login_submission): From 92767dd70313c81c12b91bf35ed44044969b4ef6 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 16:57:54 +0200 Subject: [PATCH 03/14] add tls property --- synapse/config/ldap.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/synapse/config/ldap.py b/synapse/config/ldap.py index 86528139e2..9c14593a99 100644 --- a/synapse/config/ldap.py +++ b/synapse/config/ldap.py @@ -23,6 +23,7 @@ class LDAPConfig(Config): self.ldap_enabled = ldap_config.get("enabled", False) self.ldap_server = ldap_config["server"] self.ldap_port = ldap_config["port"] + self.ldap_tls = ldap_config.get("tls", False) self.ldap_search_base = ldap_config["search_base"] self.ldap_search_property = ldap_config["search_property"] self.ldap_email_property = ldap_config["email_property"] @@ -31,6 +32,7 @@ class LDAPConfig(Config): self.ldap_enabled = False self.ldap_server = None self.ldap_port = None + self.ldap_tls = False self.ldap_search_base = None self.ldap_search_property = None self.ldap_email_property = None @@ -39,10 +41,12 @@ class LDAPConfig(Config): def default_config(self, **kwargs): return """\ # ldap_config: - # server: "ldap://localhost" - # port: 389 - # search_base: "ou=Users,dc=example,dc=com" - # search_property: "cn" - # email_property: "email" - # full_name_property: "givenName" + # enabled: true + # server: "ldap://localhost" + # port: 389 + # tls: false + # search_base: "ou=Users,dc=example,dc=com" + # search_property: "cn" + # email_property: "email" + # full_name_property: "givenName" """ From 823b8be4b706b54457d6f1d8f1065ba37a14026d Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 16:58:50 +0200 Subject: [PATCH 04/14] add tls property and twist my head around twisted --- synapse/handlers/auth.py | 44 ++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index eeca820845..14a2a4d8b9 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -54,11 +54,13 @@ class AuthHandler(BaseHandler): self.ldap_enabled = hs.config.ldap_enabled self.ldap_server = hs.config.ldap_server self.ldap_port = hs.config.ldap_port + self.ldap_tls = hs.config.ldap_tls self.ldap_search_base = hs.config.ldap_search_base self.ldap_search_property = hs.config.ldap_search_property self.ldap_email_property = hs.config.ldap_email_property self.ldap_full_name_property = hs.config.ldap_full_name_property + self.hs = hs # FIXME better possibility to access registrationHandler later? @defer.inlineCallbacks def check_auth(self, flows, clientdict, clientip): @@ -352,7 +354,10 @@ class AuthHandler(BaseHandler): LoginError if there was an authentication problem. """ - self._check_password(user_id, password) + if not self._check_password(user_id, password): + logger.warn("Failed password login for user %s", user_id) + raise LoginError(403, "", errcode=Codes.FORBIDDEN) + logger.info("Logging in user %s", user_id) access_token = yield self.issue_access_token(user_id) @@ -418,42 +423,51 @@ class AuthHandler(BaseHandler): else: defer.returnValue(user_infos.popitem()) + @defer.inlineCallbacks def _check_password(self, user_id, password): - """Checks that user_id has passed password, raises LoginError if not.""" + defer.returnValue(not ((yield self._check_ldap_password(user_id, password)) or (yield self._check_local_password(user_id, password)))) - if not (self._check_ldap_password(user_id, password) or self._check_local_password(user_id, password)): - logger.warn("Failed password login for user %s", user_id) - raise LoginError(403, "", errcode=Codes.FORBIDDEN) + @defer.inlineCallbacks def _check_local_password(self, user_id, password): - user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) - return not self.validate_hash(password, password_hash) + try: + user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) + defer.returnValue(not self.validate_hash(password, password_hash)) + except: + defer.returnValue(False) + + @defer.inlineCallbacks def _check_ldap_password(self, user_id, password): if not self.ldap_enabled: - return False + logger.info("LDAP not configured") + defer.returnValue(False) logger.info("Authenticating %s with LDAP" % user_id) try: - l = ldap.initialize("%s:%s" % (ldap_server, ldap_port)) + ldap_url = "%s:%s" % (self.ldap_server, self.ldap_port) + logger.debug("Connecting LDAP server at %s" % ldap_url) + l = ldap.initialize(ldap_url) if self.ldap_tls: logger.debug("Initiating TLS") self._connection.start_tls_s() - dn = "%s=%s, %s" % (ldap_search_property, user_id.localpart, ldap_search_base) + local_name = UserID.from_string(user_id).localpart + + dn = "%s=%s, %s" % (self.ldap_search_property, local_name, self.ldap_search_base) logger.debug("DN for LDAP authentication: %s" % dn) l.simple_bind_s(dn.encode('utf-8'), password.encode('utf-8')) - if not self.does_user_exist(user_id): + if not (yield self.does_user_exist(user_id)): user_id, access_token = ( - yield self.handlers.registration_handler.register(localpart=user_id.localpart) + yield self.hs.get_handlers().registration_handler.register(localpart=local_name) ) - return True + defer.returnValue(True) except ldap.LDAPError, e: - logger.info(e) - return False + logger.info("LDAP error: %s" % e) + defer.returnValue(False) @defer.inlineCallbacks def issue_access_token(self, user_id): From 8f0e47fae81c314f8d6e664e60d5ce5b136d99d4 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 17:04:53 +0200 Subject: [PATCH 05/14] cleanup --- synapse/rest/client/v1/login.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index da0fd2a8e0..d14ce3efa2 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -107,10 +107,6 @@ class LoginRestServlet(ClientV1RestServlet): "uri": "%s%s" % (self.idp_redirect_url, relay_state) } defer.returnValue((200, result)) - elif self.ldap_enabled and (login_submission["type"] == - LoginRestServlet.JWT_TYPE): - result = yield self.do_ldap_login(login_submission) - defer.returnValue(result) elif self.jwt_enabled and (login_submission["type"] == LoginRestServlet.JWT_TYPE): result = yield self.do_jwt_login(login_submission) @@ -164,7 +160,6 @@ class LoginRestServlet(ClientV1RestServlet): defer.returnValue((200, result)) - @defer.inlineCallbacks def do_token_login(self, login_submission): token = login_submission['token'] From afff321e9a4d4a4d27841cc5cd737720d78dbffd Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 17:32:06 +0200 Subject: [PATCH 06/14] code style --- synapse/handlers/auth.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 14a2a4d8b9..37cbaa0b46 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -60,7 +60,7 @@ class AuthHandler(BaseHandler): self.ldap_email_property = hs.config.ldap_email_property self.ldap_full_name_property = hs.config.ldap_full_name_property - self.hs = hs # FIXME better possibility to access registrationHandler later? + self.hs = hs # FIXME better possibility to access registrationHandler later? @defer.inlineCallbacks def check_auth(self, flows, clientdict, clientip): @@ -425,8 +425,12 @@ class AuthHandler(BaseHandler): @defer.inlineCallbacks def _check_password(self, user_id, password): - defer.returnValue(not ((yield self._check_ldap_password(user_id, password)) or (yield self._check_local_password(user_id, password)))) - + defer.returnValue( + not ( + (yield self._check_ldap_password(user_id, password)) + or + (yield self._check_local_password(user_id, password)) + )) @defer.inlineCallbacks def _check_local_password(self, user_id, password): @@ -436,7 +440,6 @@ class AuthHandler(BaseHandler): except: defer.returnValue(False) - @defer.inlineCallbacks def _check_ldap_password(self, user_id, password): if not self.ldap_enabled: @@ -454,14 +457,18 @@ class AuthHandler(BaseHandler): local_name = UserID.from_string(user_id).localpart - dn = "%s=%s, %s" % (self.ldap_search_property, local_name, self.ldap_search_base) + dn = "%s=%s, %s" % ( + self.ldap_search_property, + local_name, + self.ldap_search_base) logger.debug("DN for LDAP authentication: %s" % dn) l.simple_bind_s(dn.encode('utf-8'), password.encode('utf-8')) if not (yield self.does_user_exist(user_id)): user_id, access_token = ( - yield self.hs.get_handlers().registration_handler.register(localpart=local_name) + handler = self.hs.get_handlers().registration_handler + yield handler.register(localpart=local_name) ) defer.returnValue(True) From 67f3a50e9ae68b66b465b5b4b86bc81da625d1e6 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 17:44:03 +0200 Subject: [PATCH 07/14] fix exception handling --- synapse/handlers/auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 37cbaa0b46..0e6b7c9e26 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -437,7 +437,7 @@ class AuthHandler(BaseHandler): try: user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) defer.returnValue(not self.validate_hash(password, password_hash)) - except: + except LoginError: defer.returnValue(False) @defer.inlineCallbacks @@ -473,7 +473,7 @@ class AuthHandler(BaseHandler): defer.returnValue(True) except ldap.LDAPError, e: - logger.info("LDAP error: %s" % e) + logger.warn("LDAP error: %s", e) defer.returnValue(False) @defer.inlineCallbacks From 875ed05bdcfdee72452a3eab196e3935a79e4004 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 17:48:36 +0200 Subject: [PATCH 08/14] fix pep8 --- synapse/handlers/auth.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 0e6b7c9e26..ee2b285cc1 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -358,7 +358,6 @@ class AuthHandler(BaseHandler): logger.warn("Failed password login for user %s", user_id) raise LoginError(403, "", errcode=Codes.FORBIDDEN) - logger.info("Logging in user %s", user_id) access_token = yield self.issue_access_token(user_id) refresh_token = yield self.issue_refresh_token(user_id) @@ -466,8 +465,8 @@ class AuthHandler(BaseHandler): l.simple_bind_s(dn.encode('utf-8'), password.encode('utf-8')) if not (yield self.does_user_exist(user_id)): + handler = self.hs.get_handlers().registration_handler user_id, access_token = ( - handler = self.hs.get_handlers().registration_handler yield handler.register(localpart=local_name) ) From 4c5e8adf8b326798ec71a1cc1caac49f63980ba8 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 17:56:12 +0200 Subject: [PATCH 09/14] conditionally import ldap --- synapse/handlers/auth.py | 7 +++++-- synapse/python_dependencies.py | 1 - 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index ee2b285cc1..12585abb1b 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -30,8 +30,6 @@ import simplejson import synapse.util.stringutils as stringutils -import ldap - logger = logging.getLogger(__name__) @@ -60,6 +58,9 @@ class AuthHandler(BaseHandler): self.ldap_email_property = hs.config.ldap_email_property self.ldap_full_name_property = hs.config.ldap_full_name_property + if self.ldap_enabled: + import ldap + self.hs = hs # FIXME better possibility to access registrationHandler later? @defer.inlineCallbacks @@ -445,6 +446,8 @@ class AuthHandler(BaseHandler): logger.info("LDAP not configured") defer.returnValue(False) + import ldap + logger.info("Authenticating %s with LDAP" % user_id) try: ldap_url = "%s:%s" % (self.ldap_server, self.ldap_port) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index d6b6e82bd7..cf1414b4db 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -37,7 +37,6 @@ REQUIREMENTS = { "pysaml2>=3.0.0,<4.0.0": ["saml2>=3.0.0,<4.0.0"], "pymacaroons-pynacl": ["pymacaroons"], "pyjwt": ["jwt"], - "python-ldap": ["ldap"], } CONDITIONAL_REQUIREMENTS = { "web_client": { From 3555a659ec5a78ef1dad2a9fb1e28d2fcb4f06b5 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 18:03:55 +0200 Subject: [PATCH 10/14] output ldap version for info and to pacify pep8 --- synapse/handlers/auth.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 12585abb1b..cae81dbd67 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -60,6 +60,8 @@ class AuthHandler(BaseHandler): if self.ldap_enabled: import ldap + logger.info("Import ldap version: %s", ldap.__version__) + self.hs = hs # FIXME better possibility to access registrationHandler later? From 27a0c21c38f83572f984b9556ab5740a91428caf Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 18:10:14 +0200 Subject: [PATCH 11/14] make tests for ldap more specific to not be fooled by Mocks --- synapse/handlers/auth.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index cae81dbd67..f3acdf00da 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -58,7 +58,7 @@ class AuthHandler(BaseHandler): self.ldap_email_property = hs.config.ldap_email_property self.ldap_full_name_property = hs.config.ldap_full_name_property - if self.ldap_enabled: + if self.ldap_enabled is True: import ldap logger.info("Import ldap version: %s", ldap.__version__) @@ -444,8 +444,8 @@ class AuthHandler(BaseHandler): @defer.inlineCallbacks def _check_ldap_password(self, user_id, password): - if not self.ldap_enabled: - logger.info("LDAP not configured") + if self.ldap_enabled is not True: + logger.debug("LDAP not configured") defer.returnValue(False) import ldap From 9c62fcdb688d889c6d3deffbc82ac4bbfbd4ffc4 Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 18:16:35 +0200 Subject: [PATCH 12/14] remove line --- synapse/handlers/auth.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index f3acdf00da..7c62f833ae 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -62,7 +62,6 @@ class AuthHandler(BaseHandler): import ldap logger.info("Import ldap version: %s", ldap.__version__) - self.hs = hs # FIXME better possibility to access registrationHandler later? @defer.inlineCallbacks From ed4d18f516385c2d367388aed00d13879273e99c Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Wed, 6 Apr 2016 18:30:11 +0200 Subject: [PATCH 13/14] fix check for failed authentication --- synapse/handlers/auth.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 7c62f833ae..7a13a8b11c 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -230,7 +230,9 @@ class AuthHandler(BaseHandler): if not user_id.startswith('@'): user_id = UserID.create(user_id, self.hs.hostname).to_string() - self._check_password(user_id, password) + if not (yield self._check_password(user_id, password)): + logger.warn("Failed password login for user %s", user_id) + raise LoginError(403, "", errcode=Codes.FORBIDDEN) defer.returnValue(user_id) @@ -356,7 +358,7 @@ class AuthHandler(BaseHandler): LoginError if there was an authentication problem. """ - if not self._check_password(user_id, password): + if not (yield self._check_password(user_id, password)): logger.warn("Failed password login for user %s", user_id) raise LoginError(403, "", errcode=Codes.FORBIDDEN) From 674379e673772e4235d3274eeaa7974cf10f110d Mon Sep 17 00:00:00 2001 From: Christoph Witzany Date: Thu, 7 Apr 2016 13:01:09 +0200 Subject: [PATCH 14/14] Add myself to AUTHORS.rst Signed-off-by: Christoph Witzany --- AUTHORS.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index 8711a6ae5c..3dcb1c2a89 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -57,3 +57,6 @@ Florent Violleau Niklas Riekenbrauck * Add JWT support for registration and login + +Christoph Witzany + * Add LDAP support for authentication