Skip to content

Commit c89676d

Browse files
committed
fab_auth_manager: allow get_user method to return the user authenticated via Kerberos
The issue this PR fixes was initially discussed in #39683. @jijoj-hmetrix and I noticed that, starting from Airflow 2.8.0, Kerberos authentication does not seem to work with the stable API. Even when a user provides a valid Kerberos ticket, that the whole gssapi authentication dance is successful, and that the user has the required permissions, the API returns a 403 response. ```console $ curl --negotiate -u: -s --service-name airflow https://airflow-test.xxxx.com/api/v1/pools | jq . { "detail": null, "status": 403, "title": "Forbidden", "type": "https://airflow.apache.org/docs/apache-airflow/2.10.2/stable-rest-api-ref.html#section/Errors/PermissionDenied" } ``` I found that [`airflow.providers.fab.auth_manager.fab_auth_manager.FabAuthManager.get_user`](https://github.com/apache/airflow/blob/baf2b3cb4453d44ff00598a3b0c42d432a7203f9/providers/src/airflow/providers/fab/auth_manager/fab_auth_manager.py#L185-L189) relies on flask-login's [current_user](https://github.com/maxcountryman/flask-login/blob/main/src/flask_login/utils.py#L25) to get the currently logged in user from the session. However, the Kerberos auth backend stores the authenticated user [in `g`](https://github.com/brouberol/airflow/blob/main/providers/src/airflow/providers/fab/auth_manager/api/auth/backend/kerberos_auth.py#L136) and not in the session. This patch allows the current user to be pulled either from `g` or the session, which allows the API to detect the user authenticated via Kerberos, and then link it to Fab permissions. Here's an examle from an instance running with the patch, with a admin user associated with a User account with Admin permissions: ```console $ curl --negotiate -u: -s --service-name airflow https://airflow-test.xxx.com/api/v1/pools { "pools": [ { "deferred_slots": 0, "description": "Default pool", "include_deferred": false, "name": "default_pool", "occupied_slots": 0, "open_slots": 128, "queued_slots": 0, "running_slots": 0, "scheduled_slots": 0, "slots": 128 } ], "total_entries": 1 } ``` I accompany the change with a small unit test. Signed-off-by: Balthazar Rouberol <brouberol@wikimedia.org>
1 parent 089aa63 commit c89676d

2 files changed

Lines changed: 36 additions & 5 deletions

File tree

providers/src/airflow/providers/fab/auth_manager/fab_auth_manager.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
import packaging.version
2727
from connexion import FlaskApi
28-
from flask import Blueprint, url_for
28+
from flask import Blueprint, g, url_for
2929
from packaging.version import Version
3030
from sqlalchemy import select
3131
from sqlalchemy.orm import Session, joinedload
@@ -183,9 +183,20 @@ def get_user_display_name(self) -> str:
183183
return f"{first_name} {last_name}".strip()
184184

185185
def get_user(self) -> User:
186-
"""Return the user associated to the user in session."""
186+
"""
187+
Return the user associated to the user in session.
188+
189+
Attempt to find the current user in g.user, as defined by the kerberos authentication backend.
190+
If no such user is found, return the `current_user` local proxy object, linked to the user session.
191+
192+
"""
187193
from flask_login import current_user
188194

195+
# If a user has gone through the Kerberos dance, the kerberos authentication manager
196+
# has linked it with a User model, stored in g.user, and not the session.
197+
if current_user.is_anonymous and getattr(g, "user", None) is not None and not g.user.is_anonymous:
198+
return g.user
199+
189200
return current_user
190201

191202
def init(self) -> None:

providers/tests/fab/auth_manager/test_fab_auth_manager.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@
1616
# under the License.
1717
from __future__ import annotations
1818

19+
from contextlib import contextmanager
1920
from itertools import chain
2021
from typing import TYPE_CHECKING
2122
from unittest import mock
2223
from unittest.mock import Mock
2324

2425
import pytest
25-
from flask import Flask
26+
from flask import Flask, g
2627

2728
from airflow.exceptions import AirflowConfigException, AirflowException
2829

@@ -72,6 +73,13 @@
7273
}
7374

7475

76+
@contextmanager
77+
def user_set(app, user):
78+
g.user = user
79+
yield
80+
g.user = None
81+
82+
7583
@pytest.fixture
7684
def auth_manager():
7785
return FabAuthManager(None)
@@ -114,12 +122,24 @@ def test_get_user_display_name(
114122
assert auth_manager.get_user_display_name() == expected
115123

116124
@mock.patch("flask_login.utils._get_user")
117-
def test_get_user(self, mock_current_user, auth_manager):
125+
def test_get_user(self, mock_current_user, minimal_app_for_auth_api, auth_manager):
118126
user = Mock()
119127
user.is_anonymous.return_value = True
120128
mock_current_user.return_value = user
129+
with minimal_app_for_auth_api.app_context():
130+
assert auth_manager.get_user() == user
121131

122-
assert auth_manager.get_user() == user
132+
@mock.patch("flask_login.utils._get_user")
133+
def test_get_user_from_flask_g(self, mock_current_user, minimal_app_for_auth_api, auth_manager):
134+
session_user = Mock()
135+
session_user.is_anonymous = True
136+
mock_current_user.return_value = session_user
137+
138+
flask_g_user = Mock()
139+
flask_g_user.is_anonymous = False
140+
with minimal_app_for_auth_api.app_context():
141+
with user_set(minimal_app_for_auth_api, flask_g_user):
142+
assert auth_manager.get_user() == flask_g_user
123143

124144
@pytest.mark.db_test
125145
@mock.patch.object(FabAuthManager, "get_user")

0 commit comments

Comments
 (0)