diff --git a/airflow/contrib/auth/backends/ldap_auth.py b/airflow/contrib/auth/backends/ldap_auth.py index 13e1f23dec559..e0e94074d83ce 100644 --- a/airflow/contrib/auth/backends/ldap_auth.py +++ b/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): diff --git a/scripts/ci/ldif/example.com.ldif b/scripts/ci/ldif/example.com.ldif index e041e35b57eba..ec723b65bfda6 100644 --- a/scripts/ci/ldif/example.com.ldif +++ b/scripts/ci/ldif/example.com.ldif @@ -3,4 +3,4 @@ dc: example description: LDAP Example objectClass: dcObject objectClass: organization -o: example \ No newline at end of file +o: example diff --git a/scripts/ci/ldif/manager.example.com.ldif b/scripts/ci/ldif/manager.example.com.ldif index 976b8c8ab0ed6..bf85c59b060bb 100644 --- a/scripts/ci/ldif/manager.example.com.ldif +++ b/scripts/ci/ldif/manager.example.com.ldif @@ -1,3 +1,3 @@ dn: cn=Manager,dc=example,dc=com cn: Manager -objectClass: organizationalRole \ No newline at end of file +objectClass: organizationalRole diff --git a/scripts/ci/ldif/user1.example.com.ldif b/scripts/ci/ldif/user1.example.com.ldif deleted file mode 100644 index 69cc9a41c56e4..0000000000000 --- a/scripts/ci/ldif/user1.example.com.ldif +++ /dev/null @@ -1,5 +0,0 @@ -dn: uid=user1,dc=example,dc=com -objectClass: account -objectClass: simpleSecurityObject -uid: user1 -userPassword: user1 \ No newline at end of file diff --git a/scripts/ci/ldif/users.example.com.ldif b/scripts/ci/ldif/users.example.com.ldif new file mode 100644 index 0000000000000..72549ad576f41 --- /dev/null +++ b/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 diff --git a/scripts/ci/load_fixtures.sh b/scripts/ci/load_fixtures.sh index 48887ef8b8fcc..c547ebe70418b 100755 --- a/scripts/ci/load_fixtures.sh +++ b/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; diff --git a/tests/core.py b/tests/core.py index 705fa791edaf7..cce9c7ac0ee61 100644 --- a/tests/core.py +++ b/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()