LDAP stopped working after update (master branch), PR provided

Frappe Framework Bug
, , ,
1

After performing bench update trying to login via LDAP results in freezing login window with constant Verifying... message. Previous update was on May 31st (apparently Frappe v11.1.32).

Current version:
ERPNext: v11.1.46 (master)
Frappe Framework: v11.1.41 (master)

screen

Before last update it was working fine.
Except that similar freeze was seen for one specific user.

BUG FILLED

UPDATE

Frappe stack trace on LDAP login attempt:

Request Error
Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/app.py", line 61, in application
    response = frappe.handler.handle()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/handler.py", line 21, in handle
    data = execute_cmd(cmd)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/handler.py", line 56, in execute_cmd
    return frappe.call(method, **frappe.form_dict)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 1036, in call
    return fn(*args, **newargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 206, in login
    user = ldap.authenticate(frappe.as_unicode(args.usr), frappe.as_unicode(args.pwd))
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 172, in authenticate
    return self.create_or_update_user(self.convert_ldap_entry_to_dict(user), groups=groups)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 123, in create_or_update_user
    self.sync_roles(user, groups)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 89, in sync_roles
    lower_groups = [g.lower() for g in additional_groups]
TypeError: 'NoneType' object is not iterable
[ERROR] 2019-07-12 15:51:19,926 | /home/frappe/frappe-bench/apps/frappe/frappe/utils/error.py:
Could not take error snapshot: startswith first arg must be str or a tuple of str, not bytes
Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/app.py", line 61, in application
    response = frappe.handler.handle()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/handler.py", line 21, in handle
    data = execute_cmd(cmd)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/handler.py", line 56, in execute_cmd
    return frappe.call(method, **frappe.form_dict)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 1036, in call
    return fn(*args, **newargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 206, in login
    user = ldap.authenticate(frappe.as_unicode(args.usr), frappe.as_unicode(args.pwd))
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 172, in authenticate
    return self.create_or_update_user(self.convert_ldap_entry_to_dict(user), groups=groups)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 123, in create_or_update_user
    self.sync_roles(user, groups)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/ldap_settings/ldap_settings.py", line 89, in sync_roles
    lower_groups = [g.lower() for g in additional_groups]
TypeError: 'NoneType' object is not iterable

BUG

additional_groups is not tested for being None befor being iterated. See here:

2

So replacing this

def sync_roles(self, user, additional_groups=None):

with this

def sync_roles(self, user, additional_groups=[]):

should fix it.

4

There are three places where group var should be changed to [] for it to work.

diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py
index cfe2a26..1c9fd0c 100644
--- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py
+++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py
@@ -78,7 +78,7 @@ class LDAPSettings(Document):
                        setattr(user, key, value)
                user.save(ignore_permissions=True)

-       def sync_roles(self, user, additional_groups=None):
+       def sync_roles(self, user, additional_groups=[]):

                current_roles = set([d.role for d in user.get("roles")])

@@ -99,7 +99,7 @@ class LDAPSettings(Document):

                user.remove_roles(*roles_to_remove)

-       def create_or_update_user(self, user_data, groups=None):
+       def create_or_update_user(self, user_data, groups=[]):
                user = None
                if frappe.db.exists("User", user_data['email']):
                        user = frappe.get_doc("User", user_data['email'])
@@ -160,7 +160,7 @@ class LDAPSettings(Document):
                        # only try and connect as the user, once we have their fqdn entry.
                        self.connect_to_ldap(base_dn=user.entry_dn, password=password)

-                       groups = None
+                       groups = []
                        if self.ldap_group_field:
                                groups = getattr(user, self.ldap_group_field).values
5

Are you guys going to make the PR?

6

I’ll make one.

1 Like
7

PULL REQUEST

8

@rmeyer, apparently codacy doesn’t like function argument to have empty array as default value. Fixed with None check, should be fine now.

1 Like