When either data_profiler_filter or superuser_filter aren't defined,…

airflow/contrib/auth/backends/ldap_auth.py

@@ -52,20 +52,15 @@ def get_ldap_connection(dn=None, password=None):
 
     return conn
 
+
 def group_contains_user(conn, search_base, group_filter, user_name_attr, username):
-    if not search_base or not group_filter:
-        LOG.debug("Skipping group check for %s %s", search_base, group_filter)
-        # Normally, would return false here. Return true to maintain backwards compatibility with legacy
-        # behavior, which always returned true for superuser and data profiler.
-        return True
+    search_filter = '(&({0}))'.format(group_filter)
+    if not conn.search(search_base, search_filter, attributes=[user_name_attr]):
+        LOG.warn("Unable to find group for %s %s", search_base, search_filter)
     else:
-        search_filter = '(&({0}))'.format(group_filter)
-        if not conn.search(search_base, search_filter, attributes=[user_name_attr]):
-            LOG.warn("Unable to find group for %s %s", search_base, search_filter)
-        else:
-            for resp in conn.response:
-                if resp.has_key('attributes') and resp['attributes'].get(user_name_attr)[0] == username:
-                    return True
+        for resp in conn.response:
+            if 'attributes' in resp and resp['attributes'].get(user_name_attr)[0] == username:
+                return True
     return False
 
 
@@ -76,10 +71,24 @@ def __init__(self, user):
         # Load and cache superuser and data_profiler settings.
         conn = get_ldap_connection(configuration.get("ldap", "bind_user"), configuration.get("ldap", "bind_password"))
         try:
-            self.superuser = group_contains_user(conn, configuration.get("ldap", "basedn"), configuration.get("ldap", "superuser_filter"), configuration.get("ldap", "user_name_attr"), user.username)
-            self.data_profiler = group_contains_user(conn, configuration.get("ldap", "basedn"), configuration.get("ldap", "data_profiler_filter"), configuration.get("ldap", "user_name_attr"), user.username)
+            self.superuser = group_contains_user(conn,
+                                                 configuration.get("ldap", "basedn"),
+                                                 configuration.get("ldap", "superuser_filter"),
+                                                 configuration.get("ldap", "user_name_attr"),
+                                                 user.username)
+        except AirflowConfigException:
+            self.superuser = True
+            LOG.debug("Missing configuration for superuser settings.  Skipping.")
+
+        try:
+            self.data_profiler = group_contains_user(conn,
+                                                     configuration.get("ldap", "basedn"),
+                                                     configuration.get("ldap", "data_profiler_filter"),
+                                                     configuration.get("ldap", "user_name_attr"),
+                                                     user.username)
         except AirflowConfigException:
-            LOG.debug("Missing configuration for superuser/data profiler settings. Skipping.")
+            self.data_profiler = True
+            LOG.debug("Missing configuration for dataprofiler settings. Skipping")
 
     @staticmethod
     def try_login(username, password):

scripts/ci/ldif/example.com.ldif

@@ -3,4 +3,4 @@ dc: example
 description: LDAP Example
 objectClass: dcObject
 objectClass: organization
-o: example
+o: example

scripts/ci/ldif/manager.example.com.ldif

@@ -1,3 +1,3 @@
 dn: cn=Manager,dc=example,dc=com
 cn: Manager
-objectClass: organizationalRole
+objectClass: organizationalRole

scripts/ci/ldif/users.example.com.ldif

@@ -0,0 +1,21 @@
+# create all users
+
+dn: uid=user1,dc=example,dc=com
+objectClass: account
+objectClass: simpleSecurityObject
+uid: user1
+userPassword: user1
+
+dn: uid=dataprofiler,dc=example,dc=com
+objectClass: account
+objectClass: simpleSecurityObject
+uid: dataprofiler
+userPassword: dataprofiler
+description: dataprofiler
+
+dn: uid=superuser,dc=example,dc=com
+objectClass: account
+objectClass: simpleSecurityObject
+uid: superuser
+userPassword: superuser
+description: superuser

scripts/ci/load_fixtures.sh

@@ -3,12 +3,13 @@ set -o verbose
 
 DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
 FIXTURES_DIR="$DIR/ldif"
+LOAD_ORDER=("example.com.ldif" "manager.example.com.ldif" "users.example.com.ldif")
 
 load_fixture () {
   ldapadd -x -H ldap://127.0.0.1:3890/ -D "cn=Manager,dc=example,dc=com" -w insecure -f $1
 }
 
-for FIXTURE in `ls ${FIXTURES_DIR}`
+for FILE in "${LOAD_ORDER[@]}"
 do
-  load_fixture "${FIXTURES_DIR}/${FIXTURE}"
+  load_fixture "${FIXTURES_DIR}/${FILE}"
 done;

tests/core.py

@@ -994,6 +994,23 @@ def test_unauthorized(self):
         response = self.app.get("/admin/airflow/landing_times")
         self.assertEqual(response.status_code, 302)
 
+    def test_no_filter(self):
+        response = self.login('user1', 'user1')
+        assert 'Data Profiling' in response.data.decode('utf-8')
+        assert 'Connections' in response.data.decode('utf-8')
+
+    def test_with_filters(self):
+        configuration.conf.set('ldap', 'superuser_filter',
+                               'description=superuser')
+        configuration.conf.set('ldap', 'data_profiler_filter',
+                               'description=dataprofiler')
+
+        response = self.login('dataprofiler', 'dataprofiler')
+        assert 'Data Profiling' in response.data.decode('utf-8')
+
+        response = self.login('superuser', 'superuser')
+        assert 'Connections' in response.data.decode('utf-8')
+
     def tearDown(self):
         configuration.test_mode()
         session = Session()