diff --git a/featureflags/graph/actions.py b/featureflags/graph/actions.py index 39e52e5..dc7ce58 100644 --- a/featureflags/graph/actions.py +++ b/featureflags/graph/actions.py @@ -39,10 +39,11 @@ async def sign_in( conn: SAConnection, session: UserSession, ldap: BaseLDAP, -) -> bool: +) -> tuple[bool, str | None]: assert username and password, "Username and password are required" - if not await ldap.check_credentials(username, password): - return False + is_success, error_msg = await ldap.check_credentials(username, password) + if not is_success: + return False, error_msg user_id = await get_auth_user(username, conn=conn) @@ -68,7 +69,7 @@ async def sign_in( ) session.sign_in(user_id, expiration_time) - return True + return True, None @track diff --git a/featureflags/graph/graph.py b/featureflags/graph/graph.py index 9a884eb..8c0b16a 100644 --- a/featureflags/graph/graph.py +++ b/featureflags/graph/graph.py @@ -505,7 +505,7 @@ async def sing_in(ctx: dict, options: dict) -> AuthResult: return AuthResult("Password is required") async with ctx[GraphContext.DB_ENGINE].acquire() as conn: - success = await actions.sign_in( + is_success, error_msg = await actions.sign_in( username, password, conn=conn, @@ -513,8 +513,7 @@ async def sing_in(ctx: dict, options: dict) -> AuthResult: ldap=ctx[GraphContext.LDAP_SERVICE], ) - error = "Invalid username or password" if not success else None - return AuthResult(error) + return AuthResult(error_msg) @pass_context diff --git a/featureflags/services/ldap.py b/featureflags/services/ldap.py index 46b3272..c1839b8 100644 --- a/featureflags/services/ldap.py +++ b/featureflags/services/ldap.py @@ -3,7 +3,10 @@ from string import Template import ldap3 -from ldap3.core.exceptions import LDAPBindError +from ldap3.core.exceptions import ( + LDAPException, + LDAPInvalidCredentialsResult, +) from featureflags.utils import escape_dn_chars @@ -19,7 +22,7 @@ async def check_credentials( *, connect_timeout: int = 5, receive_timeout: int = 5, - ) -> bool: + ) -> tuple[bool, str | None]: raise NotImplementedError() @@ -34,8 +37,8 @@ async def check_credentials( *, connect_timeout: int = 5, receive_timeout: int = 5, - ) -> bool: - return self.user_is_bound + ) -> tuple[bool, str | None]: + return self.user_is_bound, None class LDAP(BaseLDAP): @@ -50,12 +53,13 @@ async def check_credentials( *, connect_timeout: int = 5, receive_timeout: int = 5, - ) -> bool: + ) -> tuple[bool, str | None]: server = ldap3.Server(self._host, connect_timeout=connect_timeout) dn_tpl = Template(self._base_dn) dn = dn_tpl.safe_substitute(user=escape_dn_chars(user)) + error_msg = None try: with ldap3.Connection( server=server, @@ -68,8 +72,15 @@ async def check_credentials( "LDAP -> Who am I: %s", connection.extend.standard.who_am_i(), ) - except LDAPBindError: + except LDAPException as e: user_is_bound = False - log.info("LDAP -> Bind error") + if type(e) is LDAPInvalidCredentialsResult: + error_msg = "Invalid username or password" + else: + try: + error_msg = getattr(e, "message") + except AttributeError: + error_msg = str(e) + log.error(f"LDAP -> Bind error: {error_msg}") - return user_is_bound + return user_is_bound, error_msg diff --git a/lets.yaml b/lets.yaml index 57c5b6c..50ce773 100644 --- a/lets.yaml +++ b/lets.yaml @@ -82,7 +82,7 @@ commands: - build-test - postgres-test cmd: | - docker-compose run --rm test + docker compose run --rm test postgres: description: Run postgres @@ -90,7 +90,7 @@ commands: postgres-test: description: Run postgres test db - cmd: docker-compose up -d postgres-test + cmd: docker compose up -d postgres-test apply-migrations-dev: description: Apply migrations to local postgres