Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
962870f
Fix inheritance chain in security manager
vandonr-amz Aug 29, 2023
a8818c0
keep backcompat + add obsolete warning
vandonr-amz Aug 30, 2023
531824b
fix build/static checks
vandonr-amz Aug 30, 2023
9b0b0c4
rename moved SecurityManager to differentiate it from the obsolete class
vandonr-amz Aug 30, 2023
54d1857
fix class checking
vandonr-amz Aug 30, 2023
3884c86
put all FAB security manager code in one file
vandonr-amz Aug 30, 2023
7f1f186
fix imports to use V2
vandonr-amz Aug 30, 2023
44abbce
fix import in mock
vandonr-amz Aug 30, 2023
1ac0955
modify examples in doc to match new "recommendation"
vandonr-amz Aug 30, 2023
f337b2b
fix refs in doc
vandonr-amz Aug 30, 2023
c32e4f0
fix test by simplifying it
vandonr-amz Aug 30, 2023
717ac2b
fix mock (again)
vandonr-amz Aug 30, 2023
a2b83aa
Merge branch 'main' into vandonr/fab
vandonr-amz Aug 31, 2023
79b2879
fix static check
vandonr-amz Aug 31, 2023
7f9dde5
Merge branch 'main' into vandonr/fab
vandonr-amz Aug 31, 2023
606e5f7
move logic to get security manager class to auth manager
vandonr-amz Sep 1, 2023
1bec1ef
Merge remote-tracking branch 'apache/main' into vandonr/fab
vandonr-amz Sep 1, 2023
3e44211
fix tests
vandonr-amz Sep 1, 2023
3d62d71
return generic security manager in base
vandonr-amz Sep 5, 2023
e4caf25
Merge remote-tracking branch 'apache/main' into vandonr/fab
vandonr-amz Sep 5, 2023
6d3ba00
Merge branch 'main' into vandonr/fab
vandonr-amz Sep 5, 2023
e598bf5
fix test
vandonr-amz Sep 6, 2023
435bf52
add type annotation on init
vandonr-amz Sep 6, 2023
98f987d
Merge remote-tracking branch 'apache/main' into vandonr/fab
vandonr-amz Sep 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@

if TYPE_CHECKING:
from airflow.api_connexion.types import APIResponse, UpdateMask
from airflow.www.security import AirflowSecurityManager
from airflow.www.security_manager import AirflowSecurityManagerV2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since on the long term we will deprecate AirflowSecurityManager in favor of AirflowSecurityManagerV2 I am wondering if AirflowSecurityManagerV2 is a good name since this name will stay. Should we keep the name AirflowSecurityManager? It can confusing but we can assume this is only temporary? Or should we come up with another name? SimplifiedAirflowSecurityManager? I dont like it either xD.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary things still stay around for a pretty long time to give users time to migrate...
Idk, I'm open to suggestions, but we can always rename it once we remove the old one.



def _check_action_and_resource(sm: AirflowSecurityManager, perms: list[tuple[str, str]]) -> None:
def _check_action_and_resource(sm: AirflowSecurityManagerV2, perms: list[tuple[str, str]]) -> None:
"""
Check if the action or resource exists and otherwise raise 400.

This function is intended for use in the REST API because it raise 400
This function is intended for use in the REST API because it raises an HTTP error 400
"""
for action, resource in perms:
if not sm.get_action(action):
Expand Down
23 changes: 14 additions & 9 deletions airflow/auth/managers/base_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
from airflow.utils.log.logging_mixin import LoggingMixin

if TYPE_CHECKING:
from flask import Flask

from airflow.auth.managers.models.base_user import BaseUser
from airflow.cli.cli_config import CLICommand
from airflow.www.security import AirflowSecurityManager
from airflow.www.security_manager import AirflowSecurityManagerV2


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

def __init__(self):
self._security_manager: AirflowSecurityManager | None = None
def __init__(self, app: Flask) -> None:
self._security_manager: AirflowSecurityManagerV2 | None = None
self.app = app

@staticmethod
def get_cli_commands() -> list[CLICommand]:
Expand Down Expand Up @@ -80,22 +83,24 @@ def get_security_manager_override_class(self) -> type:
Return the security manager override class.

The security manager override class is responsible for overriding the default security manager
class airflow.www.security.AirflowSecurityManager with a custom implementation. This class is
essentially inherited from airflow.www.security.AirflowSecurityManager.
class airflow.www.security_manager.AirflowSecurityManagerV2 with a custom implementation.
This class is essentially inherited from airflow.www.security_manager.AirflowSecurityManagerV2.

By default, return an empty class.
By default, return the generic AirflowSecurityManagerV2.
"""
return object
from airflow.www.security_manager import AirflowSecurityManagerV2

return AirflowSecurityManagerV2

@property
def security_manager(self) -> AirflowSecurityManager:
def security_manager(self) -> AirflowSecurityManagerV2:
"""Get the security manager."""
if not self._security_manager:
raise AirflowException("Security manager not defined.")
return self._security_manager

@security_manager.setter
def security_manager(self, security_manager: AirflowSecurityManager):
def security_manager(self, security_manager: AirflowSecurityManagerV2):
"""
Set the security manager.

Expand Down
2 changes: 1 addition & 1 deletion airflow/auth/managers/fab/cli_commands/role_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from airflow.utils import cli as cli_utils
from airflow.utils.cli import suppress_logs_and_warning
from airflow.utils.providers_configuration_loader import providers_configuration_loaded
from airflow.www.security import EXISTING_ROLES
from airflow.www.security_manager import EXISTING_ROLES

if TYPE_CHECKING:
from airflow.auth.managers.fab.models import Action, Permission, Resource, Role
Expand Down
21 changes: 19 additions & 2 deletions airflow/auth/managers/fab/fab_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# under the License.
from __future__ import annotations

import warnings
from typing import TYPE_CHECKING

from airflow import AirflowException
Expand Down Expand Up @@ -90,8 +91,24 @@ def is_logged_in(self) -> bool:
def get_security_manager_override_class(self) -> type:
"""Return the security manager override."""
from airflow.auth.managers.fab.security_manager.override import FabAirflowSecurityManagerOverride

return FabAirflowSecurityManagerOverride
from airflow.www.security import AirflowSecurityManager

sm_from_config = self.app.config.get("SECURITY_MANAGER_CLASS")
if sm_from_config:
if not issubclass(sm_from_config, AirflowSecurityManager):
raise Exception(
"""Your CUSTOM_SECURITY_MANAGER must extend FabAirflowSecurityManagerOverride,
not FAB's own security manager."""
)
if not issubclass(sm_from_config, FabAirflowSecurityManagerOverride):
warnings.warn(
"Please make your custom security manager inherit from "
"FabAirflowSecurityManagerOverride instead of AirflowSecurityManager.",
DeprecationWarning,
)
return sm_from_config

return FabAirflowSecurityManagerOverride # default choice

def url_for(self, *args, **kwargs):
"""Wrapper to allow mocking without having to import at the top of the file."""
Expand Down
17 changes: 0 additions & 17 deletions airflow/auth/managers/fab/security_manager/modules/__init__.py

This file was deleted.

Loading