This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new 3c5f5b50a4 Remove `sqla` module from security manager (#35557)
3c5f5b50a4 is described below

commit 3c5f5b50a4e7459d5edd60c40fb95db87142be99
Author: Vincent <97131062+vincb...@users.noreply.github.com>
AuthorDate: Thu Nov 9 12:41:04 2023 -0500

    Remove `sqla` module from security manager (#35557)
---
 .../auth/managers/fab/security_manager/override.py | 44 +++++++++-
 airflow/www/fab_security/sqla/__init__.py          | 16 ----
 airflow/www/fab_security/sqla/manager.py           | 99 ----------------------
 airflow/www/security_manager.py                    |  4 +-
 airflow/www/utils.py                               | 17 ++--
 airflow/www/views.py                               |  2 +-
 6 files changed, 56 insertions(+), 126 deletions(-)

diff --git a/airflow/auth/managers/fab/security_manager/override.py 
b/airflow/auth/managers/fab/security_manager/override.py
index fe4c407277..ba5ca1709f 100644
--- a/airflow/auth/managers/fab/security_manager/override.py
+++ b/airflow/auth/managers/fab/security_manager/override.py
@@ -49,7 +49,7 @@ from flask_jwt_extended import JWTManager, current_user as 
current_user_jwt
 from flask_login import LoginManager
 from itsdangerous import want_bytes
 from markupsafe import Markup
-from sqlalchemy import and_, func, inspect, or_, select
+from sqlalchemy import and_, func, inspect, literal, or_, select
 from sqlalchemy.exc import MultipleResultsFound
 from sqlalchemy.orm import Session, joinedload
 from werkzeug.security import check_password_hash, generate_password_hash
@@ -60,6 +60,7 @@ from airflow.auth.managers.fab.models import (
     RegisterUser,
     Resource,
     Role,
+    User,
     assoc_permission_role,
 )
 from airflow.auth.managers.fab.models.anonymous_user import AnonymousUser
@@ -95,7 +96,6 @@ from airflow.www.session import 
AirflowDatabaseSessionInterface
 
 if TYPE_CHECKING:
     from airflow.auth.managers.base_auth_manager import ResourceMethod
-    from airflow.auth.managers.fab.models import User
     from airflow.www.fab_security.manager import BaseSecurityManager
 
 log = logging.getLogger(__name__)
@@ -126,6 +126,7 @@ class 
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
     """ The obj instance for user view """
 
     """ Models """
+    user_model = User
     role_model = Role
     action_model = Action
     resource_model = Resource
@@ -403,6 +404,10 @@ class 
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
                 category="Security",
             )
 
+    @property
+    def get_session(self):
+        return self.appbuilder.get_session
+
     def create_login_manager(self) -> LoginManager:
         """Create the login manager."""
         lm = LoginManager(self.appbuilder.app)
@@ -1123,6 +1128,41 @@ class 
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
         if deleted_count:
             self.log.info("Deleted %s faulty permissions", deleted_count)
 
+    def permission_exists_in_one_or_more_roles(
+        self, resource_name: str, action_name: str, role_ids: list[int]
+    ) -> bool:
+        """
+        Efficiently check if a certain permission exists on a list of role 
ids; used by `has_access`.
+
+        :param resource_name: The view's name to check if exists on one of the 
roles
+        :param action_name: The permission name to check if exists
+        :param role_ids: a list of Role ids
+        :return: Boolean
+        """
+        q = (
+            self.appbuilder.get_session.query(self.permission_model)
+            .join(
+                assoc_permission_role,
+                and_(self.permission_model.id == 
assoc_permission_role.c.permission_view_id),
+            )
+            .join(self.role_model)
+            .join(self.action_model)
+            .join(self.resource_model)
+            .filter(
+                self.resource_model.name == resource_name,
+                self.action_model.name == action_name,
+                self.role_model.id.in_(role_ids),
+            )
+            .exists()
+        )
+        # Special case for MSSQL/Oracle (works on PG and MySQL > 8)
+        if self.appbuilder.get_session.bind.dialect.name in ("mssql", 
"oracle"):
+            return 
self.appbuilder.get_session.query(literal(True)).filter(q).scalar()
+        return self.appbuilder.get_session.query(q).scalar()
+
+    def perms_include_action(self, perms, action_name):
+        return any(perm.action and perm.action.name == action_name for perm in 
perms)
+
     def init_role(self, role_name, perms) -> None:
         """
         Initialize the role with actions and related resources.
diff --git a/airflow/www/fab_security/sqla/__init__.py 
b/airflow/www/fab_security/sqla/__init__.py
deleted file mode 100644
index 13a83393a9..0000000000
--- a/airflow/www/fab_security/sqla/__init__.py
+++ /dev/null
@@ -1,16 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
diff --git a/airflow/www/fab_security/sqla/manager.py 
b/airflow/www/fab_security/sqla/manager.py
deleted file mode 100644
index cdd6e44770..0000000000
--- a/airflow/www/fab_security/sqla/manager.py
+++ /dev/null
@@ -1,99 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-from __future__ import annotations
-
-import logging
-
-from sqlalchemy import and_, literal
-
-from airflow.auth.managers.fab.models import (
-    Action,
-    Permission,
-    RegisterUser,
-    Resource,
-    Role,
-    User,
-    assoc_permission_role,
-)
-from airflow.www.fab_security.manager import BaseSecurityManager
-
-log = logging.getLogger(__name__)
-
-
-class SecurityManager(BaseSecurityManager):
-    """
-    Responsible for authentication, registering security views, role and 
permission auto management.
-
-    If you want to change anything just inherit and override, then
-    pass your own security manager to AppBuilder.
-    """
-
-    user_model = User
-    """ Override to set your own User Model """
-    role_model = Role
-    """ Override to set your own Role Model """
-    action_model = Action
-    resource_model = Resource
-    permission_model = Permission
-    registeruser_model = RegisterUser
-
-    def __init__(self, appbuilder, **kwargs):
-        """
-        Class constructor.
-
-        :param appbuilder: F.A.B AppBuilder main object
-        """
-        super().__init__(appbuilder)
-
-    @property
-    def get_session(self):
-        return self.appbuilder.get_session
-
-    def permission_exists_in_one_or_more_roles(
-        self, resource_name: str, action_name: str, role_ids: list[int]
-    ) -> bool:
-        """
-        Efficiently check if a certain permission exists on a list of role 
ids; used by `has_access`.
-
-        :param resource_name: The view's name to check if exists on one of the 
roles
-        :param action_name: The permission name to check if exists
-        :param role_ids: a list of Role ids
-        :return: Boolean
-        """
-        q = (
-            self.appbuilder.get_session.query(self.permission_model)
-            .join(
-                assoc_permission_role,
-                and_(self.permission_model.id == 
assoc_permission_role.c.permission_view_id),
-            )
-            .join(self.role_model)
-            .join(self.action_model)
-            .join(self.resource_model)
-            .filter(
-                self.resource_model.name == resource_name,
-                self.action_model.name == action_name,
-                self.role_model.id.in_(role_ids),
-            )
-            .exists()
-        )
-        # Special case for MSSQL/Oracle (works on PG and MySQL > 8)
-        if self.appbuilder.get_session.bind.dialect.name in ("mssql", 
"oracle"):
-            return 
self.appbuilder.get_session.query(literal(True)).filter(q).scalar()
-        return self.appbuilder.get_session.query(q).scalar()
-
-    def perms_include_action(self, perms, action_name):
-        return any(perm.action and perm.action.name == action_name for perm in 
perms)
diff --git a/airflow/www/security_manager.py b/airflow/www/security_manager.py
index ffa65ce71d..552add8453 100644
--- a/airflow/www/security_manager.py
+++ b/airflow/www/security_manager.py
@@ -67,7 +67,7 @@ from airflow.security.permissions import (
 from airflow.utils.log.logging_mixin import LoggingMixin
 from airflow.utils.session import NEW_SESSION, provide_session
 from airflow.www.extensions.init_auth_manager import get_auth_manager
-from airflow.www.fab_security.sqla.manager import SecurityManager
+from airflow.www.fab_security.manager import BaseSecurityManager
 from airflow.www.utils import CustomSQLAInterface
 
 EXISTING_ROLES = FAB_EXISTING_ROLES
@@ -78,7 +78,7 @@ if TYPE_CHECKING:
     from airflow.auth.managers.models.base_user import BaseUser
 
 
-class AirflowSecurityManagerV2(SecurityManager, LoggingMixin):
+class AirflowSecurityManagerV2(BaseSecurityManager, LoggingMixin):
     """Custom security manager, which introduces a permission model adapted to 
Airflow.
 
     It's named V2 to differentiate it from the obsolete 
airflow.www.security.AirflowSecurityManager.
diff --git a/airflow/www/utils.py b/airflow/www/utils.py
index 9d7728798e..8c2282a31c 100644
--- a/airflow/www/utils.py
+++ b/airflow/www/utils.py
@@ -49,6 +49,7 @@ from airflow.utils.helpers import alchemy_to_dict
 from airflow.utils.json import WebEncoder
 from airflow.utils.sqlalchemy import tuple_in_condition
 from airflow.utils.state import State, TaskInstanceState
+from airflow.www.extensions.init_auth_manager import get_auth_manager
 from airflow.www.forms import DateTimeWithTimezoneField
 from airflow.www.widgets import AirflowDateTimePickerWidget
 
@@ -60,7 +61,8 @@ if TYPE_CHECKING:
     from sqlalchemy.sql import Select
     from sqlalchemy.sql.operators import ColumnOperators
 
-    from airflow.www.fab_security.sqla.manager import SecurityManager
+    from airflow.www.extensions.init_appbuilder import AirflowAppBuilder
+
 
 TI = TaskInstance
 
@@ -927,7 +929,7 @@ class UIAlert:
         self.html = html
         self.message = Markup(message) if html else message
 
-    def should_show(self, securitymanager: SecurityManager) -> bool:
+    def should_show(self, appbuilder: AirflowAppBuilder) -> bool:
         """Determine if the user should see the message.
 
         The decision is based on the user's role. If ``AUTH_ROLE_PUBLIC`` is
@@ -935,12 +937,15 @@ class UIAlert:
         ``AUTH_ROLE_PUBLIC`` role.
         """
         if self.roles:
-            current_user = securitymanager.current_user
+            current_user = get_auth_manager().get_user()
             if current_user is not None:
-                user_roles = {r.name for r in 
securitymanager.current_user.roles}
-            elif "AUTH_ROLE_PUBLIC" in 
securitymanager.appbuilder.get_app.config:
+                if not hasattr(current_user, "roles"):
+                    # If the user does not contain "roles" in its model, 
return False
+                    return False
+                user_roles = {r.name for r in current_user.roles}
+            elif "AUTH_ROLE_PUBLIC" in appbuilder.get_app.config:
                 # If the current_user is anonymous, assign AUTH_ROLE_PUBLIC 
role (if it exists) to them
-                user_roles = 
{securitymanager.appbuilder.get_app.config["AUTH_ROLE_PUBLIC"]}
+                user_roles = {appbuilder.get_app.config["AUTH_ROLE_PUBLIC"]}
             else:
                 # Unable to obtain user role - default to not showing
                 return False
diff --git a/airflow/www/views.py b/airflow/www/views.py
index a7cc8c3c1b..691a11f2d5 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -970,7 +970,7 @@ class Airflow(AirflowBaseView):
         )
 
         dashboard_alerts = [
-            fm for fm in settings.DASHBOARD_UIALERTS if 
fm.should_show(get_airflow_app().appbuilder.sm)
+            fm for fm in settings.DASHBOARD_UIALERTS if 
fm.should_show(get_airflow_app().appbuilder)
         ]
 
         def _iter_parsed_moved_data_table_names():

Reply via email to