Repository: incubator-airflow Updated Branches: refs/heads/v1-10-test c01a12eda -> 7c5157a93
[AIRFLOW-2624] Fix webserver login as anonymous (cherry picked from commit 2fd9328b412841429acc288b1441c8351ee15e98) Signed-off-by: Bolke de Bruin <bo...@xs4all.nl> Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/7c5157a9 Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/7c5157a9 Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/7c5157a9 Branch: refs/heads/v1-10-test Commit: 7c5157a9366dfb118bccb0c900313ea46e5eb80f Parents: c01a12e Author: Kevin Yang <kevin.y...@airbnb.com> Authored: Thu Jun 14 18:20:46 2018 -0700 Committer: Bolke de Bruin <bo...@xs4all.nl> Committed: Wed Jun 27 22:17:08 2018 +0200 ---------------------------------------------------------------------- airflow/www/utils.py | 6 ++-- tests/www/test_utils.py | 85 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/7c5157a9/airflow/www/utils.py ---------------------------------------------------------------------- diff --git a/airflow/www/utils.py b/airflow/www/utils.py index 7d9c8a0..44fa5c4 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -246,8 +246,8 @@ def action_logging(f): """ @functools.wraps(f) def wrapper(*args, **kwargs): - # Only AnonymousUserMixin() does not have user attribute - if current_user and hasattr(current_user, 'user'): + # AnonymousUserMixin() has user attribute but its value is None. + if current_user and hasattr(current_user, 'user') and current_user.user: user = current_user.user.username else: user = 'anonymous' @@ -286,7 +286,7 @@ def notify_owner(f): dag = dagbag.get_dag(dag_id) task = dag.get_task(task_id) - if current_user and hasattr(current_user, 'username'): + if current_user and hasattr(current_user, 'user') and current_user.user: user = current_user.username else: user = 'anonymous' http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/7c5157a9/tests/www/test_utils.py ---------------------------------------------------------------------- diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py index d69041a..9d788e8 100644 --- a/tests/www/test_utils.py +++ b/tests/www/test_utils.py @@ -7,9 +7,9 @@ # to you under the Apache License, Version 2.0 (the # "License"); you may not use this file except in compliance # with the License. You may obtain a copy of the License at -# +# # http://www.apache.org/licenses/LICENSE-2.0 -# +# # Unless required by applicable law or agreed to in writing, # software distributed under the License is distributed on an # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -17,9 +17,12 @@ # specific language governing permissions and limitations # under the License. +import mock import unittest from xml.dom import minidom +from airflow.www import app as application + from airflow.www import utils @@ -109,6 +112,84 @@ class UtilsTest(unittest.TestCase): 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") + @mock.patch("airflow.settings.Session") + def test_action_logging_with_login_user(self, mocked_session, mocked_get_user): + fake_username = 'someone' + mocked_current_user = mock.MagicMock() + mocked_get_user.return_value = mocked_current_user + mocked_current_user.user.username = fake_username + mocked_session_instance = mock.MagicMock() + mocked_session.return_value = mocked_session_instance + + app = application.create_app(testing=True) + # Patching here to avoid errors in applicant.create_app + with mock.patch("airflow.models.Log") as mocked_log: + with app.test_request_context(): + @utils.action_logging + def some_func(): + pass + + some_func() + mocked_log.assert_called_once() + (args, kwargs) = mocked_log.call_args_list[0] + self.assertEqual('some_func', kwargs['event']) + self.assertEqual(fake_username, kwargs['owner']) + mocked_session_instance.add.assert_called_once() + + @mock.patch("flask_login._get_user") + @mock.patch("airflow.settings.Session") + def test_action_logging_with_invalid_user(self, mocked_session, mocked_get_user): + anonymous_username = 'anonymous' + + # When the user returned by flask login_manager._load_user + # is invalid. + mocked_current_user = mock.MagicMock() + mocked_get_user.return_value = mocked_current_user + mocked_current_user.user = None + mocked_session_instance = mock.MagicMock() + mocked_session.return_value = mocked_session_instance + + app = application.create_app(testing=True) + # Patching here to avoid errors in applicant.create_app + with mock.patch("airflow.models.Log") as mocked_log: + with app.test_request_context(): + @utils.action_logging + def some_func(): + pass + + some_func() + mocked_log.assert_called_once() + (args, kwargs) = mocked_log.call_args_list[0] + self.assertEqual('some_func', kwargs['event']) + self.assertEqual(anonymous_username, kwargs['owner']) + mocked_session_instance.add.assert_called_once() + + # flask_login.current_user would be AnonymousUserMixin + # when there's no user_id in the flask session. + @mock.patch("airflow.settings.Session") + def test_action_logging_with_anonymous_user(self, mocked_session): + anonymous_username = 'anonymous' + + mocked_session_instance = mock.MagicMock() + mocked_session.return_value = mocked_session_instance + + app = application.create_app(testing=True) + # Patching here to avoid errors in applicant.create_app + with mock.patch("airflow.models.Log") as mocked_log: + with app.test_request_context(): + @utils.action_logging + def some_func(): + pass + + some_func() + mocked_log.assert_called_once() + (args, kwargs) = mocked_log.call_args_list[0] + self.assertEqual('some_func', kwargs['event']) + self.assertEqual(anonymous_username, kwargs['owner']) + mocked_session_instance.add.assert_called_once() + if __name__ == '__main__': unittest.main()
