This is an automated email from the ASF dual-hosted git repository.
ferruzzi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 2b0d88e450 Handle logout by auth manager (#32819)
2b0d88e450 is described below
commit 2b0d88e450f11af8e447864ca258142a6756126d
Author: Vincent <97131062+vincb...@users.noreply.github.com>
AuthorDate: Mon Jul 31 15:20:38 2023 -0400
Handle logout by auth manager (#32819)
* Handle logout by auth manager
---
airflow/auth/managers/base_auth_manager.py | 10 ++++------
airflow/auth/managers/fab/fab_auth_manager.py | 6 ++++++
airflow/www/auth.py | 2 +-
airflow/www/extensions/init_appbuilder.py | 4 ----
airflow/www/extensions/init_security.py | 4 +---
airflow/www/templates/appbuilder/navbar_right.html | 2 +-
tests/auth/managers/fab/test_fab_auth_manager.py | 11 +++++++++++
tests/www/views/test_session.py | 8 +-------
8 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/airflow/auth/managers/base_auth_manager.py
b/airflow/auth/managers/base_auth_manager.py
index a234efcaca..cb40ee9c87 100644
--- a/airflow/auth/managers/base_auth_manager.py
+++ b/airflow/auth/managers/base_auth_manager.py
@@ -41,32 +41,30 @@ class BaseAuthManager(LoggingMixin):
@abstractmethod
def get_user_name(self) -> str:
"""Return the username associated to the user in session."""
- ...
@abstractmethod
def get_user(self) -> BaseUser:
"""Return the user associated to the user in session."""
- ...
@abstractmethod
def get_user_id(self) -> str:
"""Return the user ID associated to the user in session."""
- ...
@abstractmethod
def is_logged_in(self) -> bool:
"""Return whether the user is logged in."""
- ...
@abstractmethod
def get_url_login(self, **kwargs) -> str:
"""Return the login page url."""
- ...
+
+ @abstractmethod
+ def get_url_logout(self) -> str:
+ """Return the logout page url."""
@abstractmethod
def get_url_user_profile(self) -> str | None:
"""Return the url to a page displaying info about the current user."""
- ...
def get_security_manager_override_class(self) -> type:
"""
diff --git a/airflow/auth/managers/fab/fab_auth_manager.py
b/airflow/auth/managers/fab/fab_auth_manager.py
index 085b6e997b..7a23953b35 100644
--- a/airflow/auth/managers/fab/fab_auth_manager.py
+++ b/airflow/auth/managers/fab/fab_auth_manager.py
@@ -70,6 +70,12 @@ class FabAuthManager(BaseAuthManager):
else:
return url_for(f"{self.security_manager.auth_view.endpoint}.login")
+ def get_url_logout(self):
+ """Return the logout page url."""
+ if not self.security_manager.auth_view:
+ raise AirflowException("`auth_view` not defined in the security
manager.")
+ return url_for(f"{self.security_manager.auth_view.endpoint}.logout")
+
def get_url_user_profile(self) -> str | None:
"""Return the url to a page displaying info about the current user."""
if not self.security_manager.user_view:
diff --git a/airflow/www/auth.py b/airflow/www/auth.py
index 46b645f197..9afe995054 100644
--- a/airflow/www/auth.py
+++ b/airflow/www/auth.py
@@ -54,7 +54,7 @@ def has_access(permissions: Sequence[tuple[str, str]] | None
= None) -> Callable
hostname=get_hostname()
if conf.getboolean("webserver", "EXPOSE_HOSTNAME")
else "redact",
- logout_url=appbuilder.get_url_for_logout,
+ logout_url=get_auth_manager().get_url_logout(),
),
403,
)
diff --git a/airflow/www/extensions/init_appbuilder.py
b/airflow/www/extensions/init_appbuilder.py
index 2537ea8bc7..11c358abb6 100644
--- a/airflow/www/extensions/init_appbuilder.py
+++ b/airflow/www/extensions/init_appbuilder.py
@@ -588,10 +588,6 @@ class AirflowAppBuilder:
def get_url_for_login_with(self, next_url: str | None = None) -> str:
return get_auth_manager().get_url_login(next_url=next_url)
- @property
- def get_url_for_logout(self):
- return url_for(f"{self.sm.auth_view.endpoint}.logout")
-
@property
def get_url_for_index(self):
return
url_for(f"{self.indexview.endpoint}.{self.indexview.default_view}")
diff --git a/airflow/www/extensions/init_security.py
b/airflow/www/extensions/init_security.py
index cdd4ec8862..ea3c2211c9 100644
--- a/airflow/www/extensions/init_security.py
+++ b/airflow/www/extensions/init_security.py
@@ -20,7 +20,6 @@ import logging
from importlib import import_module
from flask import g, redirect
-from flask_login import logout_user
from airflow.configuration import conf
from airflow.exceptions import AirflowConfigException, AirflowException
@@ -70,5 +69,4 @@ def init_check_user_active(app):
@app.before_request
def check_user_active():
if get_auth_manager().is_logged_in() and not g.user.is_active:
- logout_user()
- return redirect(get_auth_manager().get_url_login())
+ return redirect(get_auth_manager().get_url_logout())
diff --git a/airflow/www/templates/appbuilder/navbar_right.html
b/airflow/www/templates/appbuilder/navbar_right.html
index 2c16e09d96..8eec9f9fcf 100644
--- a/airflow/www/templates/appbuilder/navbar_right.html
+++ b/airflow/www/templates/appbuilder/navbar_right.html
@@ -78,7 +78,7 @@
<li><a href="{{user_profile_url}}"><span
class="material-icons">account_circle</span>{{_("Your Profile")}}</a></li>
<li role="separator" class="divider"></li>
{% endif %}
- <li><a href="{{appbuilder.get_url_for_logout}}"><span
class="material-icons">exit_to_app</span>{{_("Log Out")}}</a></li>
+ <li><a href="{{auth_manager.get_url_logout()}}"><span
class="material-icons">exit_to_app</span>{{_("Log Out")}}</a></li>
</ul>
</li>
{% else %}
diff --git a/tests/auth/managers/fab/test_fab_auth_manager.py
b/tests/auth/managers/fab/test_fab_auth_manager.py
index 96ddbc7f3e..c36ef8686d 100644
--- a/tests/auth/managers/fab/test_fab_auth_manager.py
+++ b/tests/auth/managers/fab/test_fab_auth_manager.py
@@ -99,6 +99,17 @@ class TestFabAuthManager:
auth_manager.get_url_login(next_url="next_url")
mock_url_for.assert_called_once_with("test_endpoint.login",
next="next_url")
+ def test_get_url_logout_when_auth_view_not_defined(self, auth_manager):
+ with pytest.raises(AirflowException, match="`auth_view` not defined in
the security manager."):
+ auth_manager.get_url_logout()
+
+ @mock.patch("airflow.auth.managers.fab.fab_auth_manager.url_for")
+ def test_get_url_logout(self, mock_url_for, auth_manager):
+ auth_manager.security_manager.auth_view = Mock()
+ auth_manager.security_manager.auth_view.endpoint = "test_endpoint"
+ auth_manager.get_url_logout()
+ mock_url_for.assert_called_once_with("test_endpoint.logout")
+
def test_get_url_user_profile_when_auth_view_not_defined(self,
auth_manager):
assert auth_manager.get_url_user_profile() is None
diff --git a/tests/www/views/test_session.py b/tests/www/views/test_session.py
index d8987a0178..822cf967fd 100644
--- a/tests/www/views/test_session.py
+++ b/tests/www/views/test_session.py
@@ -95,10 +95,4 @@ def test_check_active_user(app, user_client):
user.active = False
resp = user_client.get("/home")
assert resp.status_code == 302
- assert "/login" in resp.headers.get("Location")
-
- # And they were logged out
- user.active = True
- resp = user_client.get("/home")
- assert resp.status_code == 302
- assert "/login" in resp.headers.get("Location")
+ assert "/logout" in resp.headers.get("Location")
