Skip to content

Commit

Permalink
Move is_active user property to FAB auth manager (#42042)
Browse files Browse the repository at this point in the history
  • Loading branch information
vincbeck authored Sep 12, 2024
1 parent 9a1f9e9 commit a094f91
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 23 deletions.
4 changes: 0 additions & 4 deletions airflow/auth/managers/models/base_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@
class BaseUser:
"""User model interface."""

@property
def is_active(self) -> bool:
return True

@abstractmethod
def get_id(self) -> str: ...

Expand Down
3 changes: 2 additions & 1 deletion airflow/providers/fab/auth_manager/fab_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ def init(self) -> None:

def is_logged_in(self) -> bool:
"""Return whether the user is logged in."""
return not self.get_user().is_anonymous
user = self.get_user()
return not user.is_anonymous and user.is_active

def is_authorized_configuration(
self,
Expand Down
2 changes: 0 additions & 2 deletions airflow/www/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
from airflow.www.extensions.init_security import (
init_api_auth,
init_cache_control,
init_check_user_active,
init_xframe_protection,
)
from airflow.www.extensions.init_session import init_airflow_session_interface
Expand Down Expand Up @@ -176,7 +175,6 @@ def create_app(config=None, testing=False):
init_xframe_protection(flask_app)
init_cache_control(flask_app)
init_airflow_session_interface(flask_app)
init_check_user_active(flask_app)
return flask_app


Expand Down
13 changes: 0 additions & 13 deletions airflow/www/extensions/init_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,8 @@
import logging
from importlib import import_module

from flask import redirect, request

from airflow.configuration import conf
from airflow.exceptions import AirflowConfigException, AirflowException
from airflow.www.extensions.init_auth_manager import get_auth_manager

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -73,13 +70,3 @@ def apply_cache_control(response):
return response

app.after_request(apply_cache_control)


def init_check_user_active(app):
@app.before_request
def check_user_active():
url_logout = get_auth_manager().get_url_logout()
if request.path == url_logout:
return
if get_auth_manager().is_logged_in() and not get_auth_manager().get_user().is_active:
return redirect(url_logout)
1 change: 1 addition & 0 deletions newsfragments/42042.significant.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Removed ``is_active`` property from ``BaseUser``. This property is longer used.
9 changes: 9 additions & 0 deletions tests/providers/fab/auth_manager/test_fab_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ def test_is_logged_in(self, mock_get_user, auth_manager):

assert auth_manager.is_logged_in() is False

@mock.patch.object(FabAuthManager, "get_user")
def test_is_logged_in_with_inactive_user(self, mock_get_user, auth_manager):
user = Mock()
user.is_anonymous.return_value = False
user.is_active.return_value = True
mock_get_user.return_value = user

assert auth_manager.is_logged_in() is False

@pytest.mark.parametrize(
"api_name, method, user_permissions, expected_result",
chain(
Expand Down
1 change: 0 additions & 1 deletion tests/test_utils/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ def no_op(*args, **kwargs):
"init_xframe_protection",
"init_airflow_session_interface",
"init_appbuilder",
"init_check_user_active",
]

@functools.wraps(f)
Expand Down
1 change: 0 additions & 1 deletion tests/www/views/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ def app(examples_dag_bag):
"init_jinja_globals",
"init_plugins",
"init_airflow_session_interface",
"init_check_user_active",
]
)
def factory():
Expand Down
1 change: 0 additions & 1 deletion tests/www/views/test_views_rate_limit.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ def app_with_rate_limit_one(examples_dag_bag):
"init_jinja_globals",
"init_plugins",
"init_airflow_session_interface",
"init_check_user_active",
]
)
def factory():
Expand Down

0 comments on commit a094f91

Please sign in to comment.