[
https://issues.apache.org/jira/browse/AIRFLOW-3103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16637882#comment-16637882
]
ASF GitHub Bot commented on AIRFLOW-3103:
-----------------------------------------
ashb closed pull request #3937: [AIRFLOW-3103][AIRFLOW-3147] Update
flask-appbuilder
URL: https://github.com/apache/incubator-airflow/pull/3937
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/UPDATING.md b/UPDATING.md
index 1a250249d2..74337f3fe8 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 a0f23935b5..e1b8b13342 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 08b29e383a..bc7d552f59 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 4a019eb131..e3500c9317 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 a949e89a77..fefc389e48 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 879aaa142a..55f5daf8fd 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 bf87bbc47f..e423199fe0 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 e85bc5909a..6404d27f95 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 d9078caa39..0aef2281e7 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 deb42abc1b..8f962ef840 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 dc8cd9161a..6bb67d4d83 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 bf5aa43221..d42f8e2e8a 100644
--- a/airflow/www_rbac/templates/appbuilder/navbar_right.html
+++ b/airflow/www_rbac/templates/appbuilder/navbar_right.html
@@ -47,7 +47,7 @@
<!-- clock -->
<li><a id="clock"></a></li>
-{% if not current_user.is_anonymous() %}
+{% if not current_user.is_anonymous %}
<li class="dropdown">
<a class="dropdown-toggle" data-toggle="dropdown" href="#">
<span class="fa fa-user"></span> {{g.user.get_full_name()}}<b
class="caret"></b>
diff --git a/setup.py b/setup.py
index 5fc3403212..eded3c4e50 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 a06d6b066a..a824e58b09 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 67ea5b3035..6e0b572639 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))
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Update flask-login
> ------------------
>
> Key: AIRFLOW-3103
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3103
> Project: Apache Airflow
> Issue Type: Improvement
> Components: authentication
> Reporter: Josh Carp
> Priority: Minor
>
> Airflow uses a release of flask-login from 2014. Flask-login has fixed some
> bugs and added some features since then, so we should upgrade. Note:
> flask-appbuilder also pins to an old version of flask-login, so we'll have to
> update that library as well; PR submitted atÂ
> https://github.com/dpgaspar/Flask-AppBuilder/pull/811.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)