[AIRFLOW-3103][AIRFLOW-3147] Update flask-appbuilder (apache#3937)

Author: jmcarp

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

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

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

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'))
 

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'))
 

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'))
 

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

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,15 +83,15 @@ 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())
         )
 
 
 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())
         )
 
 

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:

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

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)
 

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>

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',

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'

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))