Skip to content

Commit

Permalink
Fix security manager inheritance in fab provider (#36538)
Browse files Browse the repository at this point in the history
  • Loading branch information
vincbeck committed Jan 3, 2024
1 parent 59c76cf commit 2093b6f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 14 deletions.
15 changes: 3 additions & 12 deletions airflow/providers/fab/auth_manager/fab_auth_manager.py
Expand Up @@ -17,7 +17,6 @@
# under the License.
from __future__ import annotations

import warnings
from functools import cached_property
from pathlib import Path
from typing import TYPE_CHECKING, Container
Expand All @@ -43,7 +42,7 @@
GroupCommand,
)
from airflow.configuration import conf
from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning
from airflow.exceptions import AirflowException
from airflow.models import DagModel
from airflow.providers.fab.auth_manager.cli_commands.definition import (
ROLES_COMMANDS,
Expand Down Expand Up @@ -334,20 +333,12 @@ def security_manager(self) -> FabAirflowSecurityManagerOverride:
from airflow.providers.fab.auth_manager.security_manager.override import (
FabAirflowSecurityManagerOverride,
)
from airflow.www.security_manager import AirflowSecurityManagerV2

sm_from_config = self.appbuilder.get_app.config.get("SECURITY_MANAGER_CLASS")
if sm_from_config:
if not issubclass(sm_from_config, AirflowSecurityManagerV2):
raise Exception(
"""Your CUSTOM_SECURITY_MANAGER must extend AirflowSecurityManagerV2,
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.",
AirflowProviderDeprecationWarning,
raise Exception(
"""Your CUSTOM_SECURITY_MANAGER must extend FabAirflowSecurityManagerOverride."""
)
return sm_from_config(self.appbuilder)

Expand Down
30 changes: 28 additions & 2 deletions tests/providers/fab/auth_manager/test_fab_auth_manager.py
Expand Up @@ -63,8 +63,12 @@ def auth_manager():


@pytest.fixture
def auth_manager_with_appbuilder():
flask_app = Flask(__name__)
def flask_app():
return Flask(__name__)


@pytest.fixture
def auth_manager_with_appbuilder(flask_app):
appbuilder = init_appbuilder(flask_app)
return FabAuthManager(appbuilder)

Expand Down Expand Up @@ -355,6 +359,28 @@ def test_is_authorized_view(self, access_view, user_permissions, expected_result
def test_security_manager_return_fab_security_manager_override(self, auth_manager_with_appbuilder):
assert isinstance(auth_manager_with_appbuilder.security_manager, FabAirflowSecurityManagerOverride)

@pytest.mark.db_test
def test_security_manager_return_custom_provided(self, flask_app, auth_manager_with_appbuilder):
class TestSecurityManager(FabAirflowSecurityManagerOverride):
pass

flask_app.config["SECURITY_MANAGER_CLASS"] = TestSecurityManager
assert isinstance(auth_manager_with_appbuilder.security_manager, TestSecurityManager)

@pytest.mark.db_test
def test_security_manager_wrong_inheritance_raise_exception(
self, flask_app, auth_manager_with_appbuilder
):
class TestSecurityManager:
pass

flask_app.config["SECURITY_MANAGER_CLASS"] = TestSecurityManager

with pytest.raises(
Exception, match="Your CUSTOM_SECURITY_MANAGER must extend FabAirflowSecurityManagerOverride."
):
auth_manager_with_appbuilder.security_manager

@pytest.mark.db_test
def test_get_url_login_when_auth_view_not_defined(self, auth_manager_with_appbuilder):
with pytest.raises(AirflowException, match="`auth_view` not defined in the security manager."):
Expand Down

0 comments on commit 2093b6f

Please sign in to comment.