Skip to content

Commit

Permalink
[AIRFLOW-3103][AIRFLOW-3147] Update flask-appbuilder (apache#3937)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmcarp authored and galak75 committed Nov 23, 2018
1 parent b1e6605 commit 0ccddc3
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 23 deletions.
23 changes: 16 additions & 7 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@
This file documents any backwards-incompatible changes in Airflow and
assists users migrating to a new version.

## Airflow Master

### min_file_parsing_loop_time config option temporarily disabled

The scheduler.min_file_parsing_loop_time config option has been temporarily removed due to
some bugs.

## Airflow 1.10

Installation and upgrading requires setting `SLUGIFY_USES_TEXT_UNIDECODE=yes` in your environment or
Expand Down Expand Up @@ -119,6 +112,22 @@ elasticsearch_log_id_template = {{dag_id}}-{{task_id}}-{{execution_date}}-{{try_
elasticsearch_end_of_log_mark = end_of_log
```

### 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.9

### SSH Hook updates, along with new SSH Operator & SFTP Operator
Expand Down
3 changes: 3 additions & 0 deletions airflow/contrib/auth/backends/github_enterprise_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,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
Expand Down
3 changes: 3 additions & 0 deletions airflow/contrib/auth/backends/google_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,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
Expand Down
5 changes: 4 additions & 1 deletion airflow/contrib/auth/backends/kerberos_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,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
Expand Down Expand Up @@ -110,7 +113,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'))

Expand Down
5 changes: 4 additions & 1 deletion airflow/contrib/auth/backends/ldap_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,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
Expand Down Expand Up @@ -274,7 +277,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'))

Expand Down
5 changes: 4 additions & 1 deletion airflow/contrib/auth/backends/password_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'))

Expand Down
3 changes: 3 additions & 0 deletions airflow/default_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions airflow/www/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,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
)
)

Expand All @@ -81,15 +81,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())
)


Expand Down
2 changes: 1 addition & 1 deletion airflow/www/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,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:
Expand Down
2 changes: 1 addition & 1 deletion airflow/www_rbac/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion airflow/www_rbac/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def init_roles(appbuilder):


def is_view_only(user, appbuilder):
if user.is_anonymous():
if user.is_anonymous:
anonymous_role = appbuilder.sm.auth_role_public
return anonymous_role == 'Viewer'

Expand Down
2 changes: 1 addition & 1 deletion airflow/www_rbac/templates/appbuilder/navbar_right.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,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',
Expand Down
6 changes: 3 additions & 3 deletions tests/www/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,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'
Expand All @@ -142,7 +142,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'
Expand Down

0 comments on commit 0ccddc3

Please sign in to comment.