From fb5ffd146a5a33820cfa7541e5ce09098f3d541a Mon Sep 17 00:00:00 2001 From: Joshua Carp Date: Thu, 4 Oct 2018 03:20:24 -0400 Subject: [PATCH] [AIRFLOW-3103][AIRFLOW-3147] Update flask-appbuilder (#3937) --- UPDATING.md | 16 ++++++++++++++++ .../auth/backends/github_enterprise_auth.py | 3 +++ airflow/contrib/auth/backends/google_auth.py | 3 +++ airflow/contrib/auth/backends/kerberos_auth.py | 5 ++++- airflow/contrib/auth/backends/ldap_auth.py | 5 ++++- airflow/contrib/auth/backends/password_auth.py | 5 ++++- airflow/default_login.py | 3 +++ airflow/www/utils.py | 8 ++++---- airflow/www/views.py | 2 +- airflow/www_rbac/decorators.py | 2 +- airflow/www_rbac/security.py | 6 +++--- .../templates/appbuilder/navbar_right.html | 2 +- setup.py | 4 ++-- tests/www/test_utils.py | 6 +++--- tests/www_rbac/test_security.py | 6 +++--- 15 files changed, 55 insertions(+), 21 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 1a250249d2cbf..74337f3fe88de 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -52,6 +52,22 @@ To delete a user: airflow users --delete --username jondoe ``` +### Custom auth backends interface change + +We have updated the version of flask-login we depend upon, and as a result any +custom auth backends might need a small change: `is_active`, +`is_authenticated`, and `is_anonymous` should now be properties. What this means is if +previously you had this in your user class + + def is_active(self): + return self.active + +then you need to change it like this + + @property + def is_active(self): + return self.active + ## Airflow 1.10 Installation and upgrading requires setting `SLUGIFY_USES_TEXT_UNIDECODE=yes` in your environment or diff --git a/airflow/contrib/auth/backends/github_enterprise_auth.py b/airflow/contrib/auth/backends/github_enterprise_auth.py index a0f23935b5434..e1b8b133425fe 100644 --- a/airflow/contrib/auth/backends/github_enterprise_auth.py +++ b/airflow/contrib/auth/backends/github_enterprise_auth.py @@ -41,14 +41,17 @@ class GHEUser(models.User): def __init__(self, user): self.user = user + @property def is_active(self): """Required by flask_login""" return True + @property def is_authenticated(self): """Required by flask_login""" return True + @property def is_anonymous(self): """Required by flask_login""" return False diff --git a/airflow/contrib/auth/backends/google_auth.py b/airflow/contrib/auth/backends/google_auth.py index 08b29e383a76a..bc7d552f59e93 100644 --- a/airflow/contrib/auth/backends/google_auth.py +++ b/airflow/contrib/auth/backends/google_auth.py @@ -42,14 +42,17 @@ class GoogleUser(models.User): def __init__(self, user): self.user = user + @property def is_active(self): """Required by flask_login""" return True + @property def is_authenticated(self): """Required by flask_login""" return True + @property def is_anonymous(self): """Required by flask_login""" return False diff --git a/airflow/contrib/auth/backends/kerberos_auth.py b/airflow/contrib/auth/backends/kerberos_auth.py index 4a019eb131f56..e3500c93170a5 100644 --- a/airflow/contrib/auth/backends/kerberos_auth.py +++ b/airflow/contrib/auth/backends/kerberos_auth.py @@ -71,14 +71,17 @@ def authenticate(username, password): return + @property def is_active(self): """Required by flask_login""" return True + @property def is_authenticated(self): """Required by flask_login""" return True + @property def is_anonymous(self): """Required by flask_login""" return False @@ -108,7 +111,7 @@ def load_user(userid, session=None): @provide_session def login(self, request, session=None): - if current_user.is_authenticated(): + if current_user.is_authenticated: flash("You are already logged in") return redirect(url_for('index')) diff --git a/airflow/contrib/auth/backends/ldap_auth.py b/airflow/contrib/auth/backends/ldap_auth.py index a949e89a77b9e..fefc389e489d3 100644 --- a/airflow/contrib/auth/backends/ldap_auth.py +++ b/airflow/contrib/auth/backends/ldap_auth.py @@ -235,14 +235,17 @@ def try_login(username, password): log.info("Password incorrect for user %s", username) raise AuthenticationError("Invalid username or password") + @property def is_active(self): """Required by flask_login""" return True + @property def is_authenticated(self): """Required by flask_login""" return True + @property def is_anonymous(self): """Required by flask_login""" return False @@ -273,7 +276,7 @@ def load_user(userid, session=None): @provide_session def login(self, request, session=None): - if current_user.is_authenticated(): + if current_user.is_authenticated: flash("You are already logged in") return redirect(url_for('admin.index')) diff --git a/airflow/contrib/auth/backends/password_auth.py b/airflow/contrib/auth/backends/password_auth.py index 879aaa142a4db..55f5daf8fdf76 100644 --- a/airflow/contrib/auth/backends/password_auth.py +++ b/airflow/contrib/auth/backends/password_auth.py @@ -71,14 +71,17 @@ def password(self, plaintext): def authenticate(self, plaintext): return check_password_hash(self._password, plaintext) + @property def is_active(self): """Required by flask_login""" return True + @property def is_authenticated(self): """Required by flask_login""" return True + @property def is_anonymous(self): """Required by flask_login""" return False @@ -137,7 +140,7 @@ def authenticate(session, username, password): @provide_session def login(self, request, session=None): - if current_user.is_authenticated(): + if current_user.is_authenticated: flash("You are already logged in") return redirect(url_for('admin.index')) diff --git a/airflow/default_login.py b/airflow/default_login.py index bf87bbc47fa76..e423199fe01a9 100644 --- a/airflow/default_login.py +++ b/airflow/default_login.py @@ -44,14 +44,17 @@ class DefaultUser(object): def __init__(self, user): self.user = user + @property def is_active(self): """Required by flask_login""" return True + @property def is_authenticated(self): """Required by flask_login""" return True + @property def is_anonymous(self): """Required by flask_login""" return False diff --git a/airflow/www/utils.py b/airflow/www/utils.py index e85bc5909ae27..6404d27f951af 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -73,8 +73,8 @@ class LoginMixin(object): def is_accessible(self): return ( not AUTHENTICATE or ( - not current_user.is_anonymous() and - current_user.is_authenticated() + not current_user.is_anonymous and + current_user.is_authenticated ) ) @@ -83,7 +83,7 @@ class SuperUserMixin(object): def is_accessible(self): return ( not AUTHENTICATE or - (not current_user.is_anonymous() and current_user.is_superuser()) + (not current_user.is_anonymous and current_user.is_superuser()) ) @@ -91,7 +91,7 @@ class DataProfilingMixin(object): def is_accessible(self): return ( not AUTHENTICATE or - (not current_user.is_anonymous() and current_user.data_profiling()) + (not current_user.is_anonymous and current_user.data_profiling()) ) diff --git a/airflow/www/views.py b/airflow/www/views.py index d9078caa39efa..0aef2281e7ccc 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -260,7 +260,7 @@ def data_profiling_required(f): def decorated_function(*args, **kwargs): if ( current_app.config['LOGIN_DISABLED'] or - (not current_user.is_anonymous() and current_user.data_profiling()) + (not current_user.is_anonymous and current_user.data_profiling()) ): return f(*args, **kwargs) else: diff --git a/airflow/www_rbac/decorators.py b/airflow/www_rbac/decorators.py index deb42abc1b588..8f962ef8406bb 100644 --- a/airflow/www_rbac/decorators.py +++ b/airflow/www_rbac/decorators.py @@ -32,7 +32,7 @@ def action_logging(f): @functools.wraps(f) def wrapper(*args, **kwargs): session = settings.Session() - if g.user.is_anonymous(): + if g.user.is_anonymous: user = 'anonymous' else: user = g.user.username diff --git a/airflow/www_rbac/security.py b/airflow/www_rbac/security.py index dc8cd9161af03..6bb67d4d8338e 100644 --- a/airflow/www_rbac/security.py +++ b/airflow/www_rbac/security.py @@ -195,7 +195,7 @@ def get_user_roles(self, user=None): """ if user is None: user = g.user - if user.is_anonymous(): + if user.is_anonymous: public_role = appbuilder.config.get('AUTH_ROLE_PUBLIC') return [appbuilder.security_manager.find_role(public_role)] \ if public_role else [] @@ -221,7 +221,7 @@ def get_accessible_dag_ids(self, username=None): if not username: username = g.user - if username.is_anonymous() or 'Public' in username.roles: + if username.is_anonymous or 'Public' in username.roles: # return an empty list if the role is public return set() @@ -245,7 +245,7 @@ def has_access(self, permission, view_name, user=None): """ if not user: user = g.user - if user.is_anonymous(): + if user.is_anonymous: return self.is_item_public(permission, view_name) return self._has_view_access(user, permission, view_name) diff --git a/airflow/www_rbac/templates/appbuilder/navbar_right.html b/airflow/www_rbac/templates/appbuilder/navbar_right.html index bf5aa43221884..d42f8e2e8a82c 100644 --- a/airflow/www_rbac/templates/appbuilder/navbar_right.html +++ b/airflow/www_rbac/templates/appbuilder/navbar_right.html @@ -47,7 +47,7 @@
    • -{% if not current_user.is_anonymous() %} +{% if not current_user.is_anonymous %}
  • {{g.user.get_full_name()}} diff --git a/setup.py b/setup.py index 5fc3403212135..eded3c4e50230 100644 --- a/setup.py +++ b/setup.py @@ -298,10 +298,10 @@ def do_setup(): 'croniter>=0.3.17, <0.4', 'dill>=0.2.2, <0.3', 'flask>=0.12.4, <0.13', - 'flask-appbuilder>=1.11.1, <2.0.0', + 'flask-appbuilder>=1.12, <2.0.0', 'flask-admin==1.4.1', 'flask-caching>=1.3.3, <1.4.0', - 'flask-login==0.2.11', + 'flask-login>=0.3, <0.5', 'flask-swagger==0.2.13', 'flask-wtf>=0.14.2, <0.15', 'funcsigs==1.0.0', diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py index a06d6b066a113..a824e58b09c31 100644 --- a/tests/www/test_utils.py +++ b/tests/www/test_utils.py @@ -117,8 +117,8 @@ def test_params_all(self): self.assertEqual('page=3&search=bash_&showPaused=False', utils.get_params(showPaused=False, page=3, search='bash_')) - # flask_login is loaded by calling flask_login._get_user. - @mock.patch("flask_login._get_user") + # flask_login is loaded by calling flask_login.utils._get_user. + @mock.patch("flask_login.utils._get_user") @mock.patch("airflow.settings.Session") def test_action_logging_with_login_user(self, mocked_session, mocked_get_user): fake_username = 'someone' @@ -143,7 +143,7 @@ def some_func(): self.assertEqual(fake_username, kwargs['owner']) mocked_session_instance.add.assert_called_once() - @mock.patch("flask_login._get_user") + @mock.patch("flask_login.utils._get_user") @mock.patch("airflow.settings.Session") def test_action_logging_with_invalid_user(self, mocked_session, mocked_get_user): anonymous_username = 'anonymous' diff --git a/tests/www_rbac/test_security.py b/tests/www_rbac/test_security.py index 67ea5b3035640..6e0b5726394da 100644 --- a/tests/www_rbac/test_security.py +++ b/tests/www_rbac/test_security.py @@ -109,7 +109,7 @@ def test_init_role_modelview(self): def test_get_user_roles(self): user = mock.MagicMock() - user.is_anonymous.return_value = False + user.is_anonymous = False roles = self.appbuilder.sm.find_role('Admin') user.roles = roles self.assertEqual(self.security_manager.get_user_roles(user), roles) @@ -144,7 +144,7 @@ def test_get_accessible_dag_ids(self, mock_get_user_roles, self.security_manager.init_role(role_name, role_vms, role_perms) role = self.security_manager.find_role(role_name) user.roles = [role] - user.is_anonymous.return_value = False + user.is_anonymous = False mock_get_all_permissions_views.return_value = {('can_dag_read', 'dag_id')} mock_get_user_roles.return_value = [role] @@ -154,7 +154,7 @@ def test_get_accessible_dag_ids(self, mock_get_user_roles, @mock.patch('airflow.www_rbac.security.AirflowSecurityManager._has_view_access') def test_has_access(self, mock_has_view_access): user = mock.MagicMock() - user.is_anonymous.return_value = False + user.is_anonymous = False mock_has_view_access.return_value = True self.assertTrue(self.security_manager.has_access('perm', 'view', user))