You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2024/06/12 13:51:34 UTC
(airflow) branch main updated: Add CSRF protection to "/logout" (#40145)
This is an automated email from the ASF dual-hosted git repository.
potiuk 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 14deaa2f1f Add CSRF protection to "/logout" (#40145)
14deaa2f1f is described below
commit 14deaa2f1fb8d5dbe4d2e1d9adaa390c5e5efbf8
Author: Shahar Epstein <60...@users.noreply.github.com>
AuthorDate: Wed Jun 12 16:51:27 2024 +0300
Add CSRF protection to "/logout" (#40145)
---
.../fab/auth_manager/security_manager/override.py | 17 +++++++++++++++++
airflow/www/static/css/bootstrap-theme.css | 18 +++++++++++++++---
airflow/www/static/css/material-icons.css | 4 +++-
airflow/www/templates/appbuilder/navbar_right.html | 7 ++++++-
newsfragments/40145.significant.rst | 5 +++++
.../api_endpoints/test_role_and_permission_endpoint.py | 2 ++
tests/www/views/test_session.py | 4 ++--
7 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/airflow/providers/fab/auth_manager/security_manager/override.py b/airflow/providers/fab/auth_manager/security_manager/override.py
index e617b8e846..56336b4fb2 100644
--- a/airflow/providers/fab/auth_manager/security_manager/override.py
+++ b/airflow/providers/fab/auth_manager/security_manager/override.py
@@ -57,8 +57,10 @@ from flask_appbuilder.security.views import (
AuthOAuthView,
AuthOIDView,
AuthRemoteUserView,
+ AuthView,
RegisterUserModelView,
)
+from flask_appbuilder.views import expose
from flask_babel import lazy_gettext
from flask_jwt_extended import JWTManager, current_user as current_user_jwt
from flask_login import LoginManager
@@ -123,6 +125,21 @@ log = logging.getLogger(__name__)
MAX_NUM_DATABASE_USER_SESSIONS = 50000
+# The following class patches the logout method within AuthView, so it only supports POST method
+# to make CSRF protection effective. You could remove the patch and configure it when it is supported
+# natively by Flask-AppBuilder (https://github.com/dpgaspar/Flask-AppBuilder/issues/2248)
+
+
+class _ModifiedAuthView(AuthView):
+ @expose("/logout/", methods=["POST"])
+ def logout(self):
+ return super().logout()
+
+
+for auth_view in [AuthDBView, AuthLDAPView, AuthOAuthView, AuthOIDView, AuthRemoteUserView]:
+ auth_view.__bases__ = (_ModifiedAuthView,)
+
+
class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
"""
This security manager overrides the default AirflowSecurityManager security manager.
diff --git a/airflow/www/static/css/bootstrap-theme.css b/airflow/www/static/css/bootstrap-theme.css
index 371762d558..d409434cc5 100644
--- a/airflow/www/static/css/bootstrap-theme.css
+++ b/airflow/www/static/css/bootstrap-theme.css
@@ -2768,7 +2768,10 @@ tbody.collapse.in {
overflow: hidden;
background-color: #e5e5e5;
}
-.dropdown-menu > li > a {
+
+.dropdown-menu > li > a,
+.dropdown-menu > li > form > button
+{
display: block;
padding: 3px 20px;
clear: both;
@@ -2776,9 +2779,14 @@ tbody.collapse.in {
line-height: 1.428571429;
color: #51504f;
white-space: nowrap;
+ width: 100%;
+ max-width: 100%;
+ text-align: left;
}
.dropdown-menu > li > a:hover,
-.dropdown-menu > li > a:focus {
+.dropdown-menu > li > a:focus,
+.dropdown-menu > li > form > button:hover,
+.dropdown-menu > li > form > button:focus {
text-decoration: none;
color: #262626;
background-color: #f5f5f5;
@@ -3569,13 +3577,17 @@ select[multiple].input-group-sm > .input-group-btn > .btn {
box-shadow: none;
}
.navbar-nav .dropdown:hover > .dropdown-menu > li > a,
+ .navbar-nav .dropdown:hover > .dropdown-menu > li > form > button,
.navbar-nav .dropdown:hover > .dropdown-menu .dropdown-header,
.navbar-nav .dropdown:focus-within > .dropdown-menu > li > a,
+ .navbar-nav .dropdown:focus-within > .dropdown-menu > li > form > button,
.navbar-nav .dropdown:focus-within > .dropdown-menu .dropdown-header {
padding: 5px 15px 5px 25px;
}
.navbar-nav .dropdown:hover > .dropdown-menu > li > a,
- .navbar-nav .dropdown:focus-within > .dropdown-menu > li > a {
+ .navbar-nav .dropdown:hover > .dropdown-menu > li > form > button,
+ .navbar-nav .dropdown:focus-within > .dropdown-menu > li > a,
+ .navbar-nav .dropdown:focus-within > .dropdown-menu > li > form > button {
line-height: 20px;
}
.navbar-nav .dropdown:hover > .dropdown-menu > li > a:hover,
diff --git a/airflow/www/static/css/material-icons.css b/airflow/www/static/css/material-icons.css
index 9ab8255f90..55041c83b5 100644
--- a/airflow/www/static/css/material-icons.css
+++ b/airflow/www/static/css/material-icons.css
@@ -70,7 +70,9 @@
font-size: 48px;
}
-.dropdown-menu > li > a > .material-icons {
+.dropdown-menu > li > a > .material-icons,
+.dropdown-menu > li > form > button > .material-icons
+{
margin-right: 6px;
}
diff --git a/airflow/www/templates/appbuilder/navbar_right.html b/airflow/www/templates/appbuilder/navbar_right.html
index 4227d7a3cc..fb82a15191 100644
--- a/airflow/www/templates/appbuilder/navbar_right.html
+++ b/airflow/www/templates/appbuilder/navbar_right.html
@@ -83,7 +83,12 @@
<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="{{auth_manager.get_url_logout()}}"><span class="material-icons">exit_to_app</span>{{_("Log Out")}}</a></li>
+ <li>
+ <form method="POST" action="{{auth_manager.get_url_logout()}}">
+ <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
+ <button type="submit" class="btn btn-link"><span class="material-icons">exit_to_app</span>{{_("Log Out")}}</button>
+ </form>
+ </li>
</ul>
</li>
{% else %}
diff --git a/newsfragments/40145.significant.rst b/newsfragments/40145.significant.rst
new file mode 100644
index 0000000000..beedfc7746
--- /dev/null
+++ b/newsfragments/40145.significant.rst
@@ -0,0 +1,5 @@
+``/logout`` endpoint in FAB Auth Manager is now CSRF protected
+
+The ``/logout`` endpoint's method in FAB Auth Manager has been changed from ``GET`` to ``POST`` in all existing
+AuthViews (``AuthDBView``, ``AuthLDAPView``, ``AuthOAuthView``, ``AuthOIDView``, ``AuthRemoteUserView``), and
+now includes CSRF protection to enhance security and prevent unauthorized logouts.
diff --git a/tests/providers/fab/auth_manager/api_endpoints/test_role_and_permission_endpoint.py b/tests/providers/fab/auth_manager/api_endpoints/test_role_and_permission_endpoint.py
index 454fce467c..a91a434412 100644
--- a/tests/providers/fab/auth_manager/api_endpoints/test_role_and_permission_endpoint.py
+++ b/tests/providers/fab/auth_manager/api_endpoints/test_role_and_permission_endpoint.py
@@ -35,6 +35,8 @@ from tests.test_utils.api_connexion_utils import (
delete_user,
)
+pytestmark = pytest.mark.db_test
+
@pytest.fixture(scope="module")
def configured_app(minimal_app_for_auth_api):
diff --git a/tests/www/views/test_session.py b/tests/www/views/test_session.py
index aeb9c0ffee..0ec219aaeb 100644
--- a/tests/www/views/test_session.py
+++ b/tests/www/views/test_session.py
@@ -40,7 +40,7 @@ def test_session_inaccessible_after_logout(user_client):
session_cookie = get_session_cookie(user_client)
assert session_cookie is not None
- resp = user_client.get("/logout/")
+ resp = user_client.post("/logout/")
assert resp.status_code == 302
# Try to access /home with the session cookie from earlier
@@ -78,7 +78,7 @@ def test_session_id_rotates(app, user_client):
old_session_cookie = get_session_cookie(user_client)
assert old_session_cookie is not None
- resp = user_client.get("/logout/")
+ resp = user_client.post("/logout/")
assert resp.status_code == 302
patch_path = "airflow.providers.fab.auth_manager.security_manager.override.check_password_hash"