Skip to content

Commit d3ce442

Browse files
vandonr-amzvandonr-amz
authored
Fix inheritance chain in security manager (#33901)
1 parent 48930bc commit d3ce442

File tree

25 files changed

+1588
-1746
lines changed

25 files changed

+1588
-1746
lines changed

‎airflow/api_connexion/endpoints/role_and_permission_endpoint.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@
4040

4141
if TYPE_CHECKING:
4242
from airflow.api_connexion.types import APIResponse, UpdateMask
43-
from airflow.www.security import AirflowSecurityManager
43+
from airflow.www.security_manager import AirflowSecurityManagerV2
4444

4545

46-
def _check_action_and_resource(sm: AirflowSecurityManager, perms: list[tuple[str, str]]) -> None:
46+
def _check_action_and_resource(sm: AirflowSecurityManagerV2, perms: list[tuple[str, str]]) -> None:
4747
"""
4848
Check if the action or resource exists and otherwise raise 400.
4949
50-
This function is intended for use in the REST API because it raise 400
50+
This function is intended for use in the REST API because it raises an HTTP error 400
5151
"""
5252
for action, resource in perms:
5353
if not sm.get_action(action):

‎airflow/auth/managers/base_auth_manager.py

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@
2424
from airflow.utils.log.logging_mixin import LoggingMixin
2525

2626
if TYPE_CHECKING:
27+
from flask import Flask
28+
2729
from airflow.auth.managers.models.base_user import BaseUser
2830
from airflow.cli.cli_config import CLICommand
29-
from airflow.www.security import AirflowSecurityManager
31+
from airflow.www.security_manager import AirflowSecurityManagerV2
3032

3133

3234
class BaseAuthManager(LoggingMixin):
@@ -36,8 +38,9 @@ class BaseAuthManager(LoggingMixin):
3638
Auth managers are responsible for any user management related operation such as login, logout, authz, ...
3739
"""
3840

39-
def __init__(self):
40-
self._security_manager: AirflowSecurityManager | None = None
41+
def __init__(self, app: Flask) -> None:
42+
self._security_manager: AirflowSecurityManagerV2 | None = None
43+
self.app = app
4144

4245
@staticmethod
4346
def get_cli_commands() -> list[CLICommand]:
@@ -80,22 +83,24 @@ def get_security_manager_override_class(self) -> type:
8083
Return the security manager override class.
8184
8285
The security manager override class is responsible for overriding the default security manager
83-
class airflow.www.security.AirflowSecurityManager with a custom implementation. This class is
84-
essentially inherited from airflow.www.security.AirflowSecurityManager.
86+
class airflow.www.security_manager.AirflowSecurityManagerV2 with a custom implementation.
87+
This class is essentially inherited from airflow.www.security_manager.AirflowSecurityManagerV2.
8588
86-
By default, return an empty class.
89+
By default, return the generic AirflowSecurityManagerV2.
8790
"""
88-
return object
91+
from airflow.www.security_manager import AirflowSecurityManagerV2
92+
93+
return AirflowSecurityManagerV2
8994

9095
@property
91-
def security_manager(self) -> AirflowSecurityManager:
96+
def security_manager(self) -> AirflowSecurityManagerV2:
9297
"""Get the security manager."""
9398
if not self._security_manager:
9499
raise AirflowException("Security manager not defined.")
95100
return self._security_manager
96101

97102
@security_manager.setter
98-
def security_manager(self, security_manager: AirflowSecurityManager):
103+
def security_manager(self, security_manager: AirflowSecurityManagerV2):
99104
"""
100105
Set the security manager.
101106

‎airflow/auth/managers/fab/cli_commands/role_command.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
from airflow.utils import cli as cli_utils
3030
from airflow.utils.cli import suppress_logs_and_warning
3131
from airflow.utils.providers_configuration_loader import providers_configuration_loaded
32-
from airflow.www.security import EXISTING_ROLES
32+
from airflow.www.security_manager import EXISTING_ROLES
3333

3434
if TYPE_CHECKING:
3535
from airflow.auth.managers.fab.models import Action, Permission, Resource, Role

‎airflow/auth/managers/fab/fab_auth_manager.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
# under the License.
1818
from __future__ import annotations
1919

20+
import warnings
2021
from typing import TYPE_CHECKING
2122

2223
from airflow import AirflowException
@@ -90,8 +91,24 @@ def is_logged_in(self) -> bool:
9091
def get_security_manager_override_class(self) -> type:
9192
"""Return the security manager override."""
9293
from airflow.auth.managers.fab.security_manager.override import FabAirflowSecurityManagerOverride
93-
94-
return FabAirflowSecurityManagerOverride
94+
from airflow.www.security import AirflowSecurityManager
95+
96+
sm_from_config = self.app.config.get("SECURITY_MANAGER_CLASS")
97+
if sm_from_config:
98+
if not issubclass(sm_from_config, AirflowSecurityManager):
99+
raise Exception(
100+
"""Your CUSTOM_SECURITY_MANAGER must extend FabAirflowSecurityManagerOverride,
101+
not FAB's own security manager."""
102+
)
103+
if not issubclass(sm_from_config, FabAirflowSecurityManagerOverride):
104+
warnings.warn(
105+
"Please make your custom security manager inherit from "
106+
"FabAirflowSecurityManagerOverride instead of AirflowSecurityManager.",
107+
DeprecationWarning,
108+
)
109+
return sm_from_config
110+
111+
return FabAirflowSecurityManagerOverride # default choice
95112

96113
def url_for(self, *args, **kwargs):
97114
"""Wrapper to allow mocking without having to import at the top of the file."""

‎airflow/auth/managers/fab/security_manager/modules/__init__.py

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)