o-nikolas commented on code in PR #32525:
URL: https://github.com/apache/airflow/pull/32525#discussion_r1262901259


##########
airflow/configuration.py:
##########
@@ -1743,24 +1742,6 @@ def initialize_secrets_backends() -> 
list[BaseSecretsBackend]:
     return backend_list
 
 
-def initialize_auth_manager() -> BaseAuthManager:

Review Comment:
   Nice, glad to see this dropped from conf :) 



##########
airflow/auth/managers/fab/security_manager_override.py:
##########
@@ -0,0 +1,218 @@
+#
+# 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
+
+from functools import cached_property
+
+from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, 
AUTH_REMOTE_USER
+from flask_babel import lazy_gettext
+
+
+class FabAirflowSecurityManagerOverride:
+    """
+    This security manager overrides the default AirflowSecurityManager 
security manager.

Review Comment:
   Can you add a paragraph explaining the ways that this one is different from 
the default?



##########
airflow/auth/managers/fab/security_manager_override.py:
##########
@@ -0,0 +1,218 @@
+#
+# 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
+
+from functools import cached_property
+
+from flask_appbuilder.const import AUTH_DB, AUTH_LDAP, AUTH_OAUTH, AUTH_OID, 
AUTH_REMOTE_USER
+from flask_babel import lazy_gettext
+
+
+class FabAirflowSecurityManagerOverride:
+    """
+    This security manager overrides the default AirflowSecurityManager 
security manager.
+
+    This security manager is used only if the auth manager FabAuthManager is 
used.
+
+    :param appbuilder: The appbuilder.
+    :param actionmodelview: The obj instance for action model view.
+    :param authdbview: The class for auth db view.
+    :param authldapview: The class for auth ldap view.
+    :param authoauthview: The class for auth oauth view.
+    :param authoidview: The class for auth oid view.
+    :param authremoteuserview: The class for auth remote user view.
+    :param permissionmodelview: The class for permission model view.
+    :param registeruser_view: The class for register user view.
+    :param registeruserdbview: The class for register user db view.
+    :param registeruseroauthview: The class for register user oauth view.
+    :param registerusermodelview: The class for register user model view.
+    :param registeruseroidview: The class for register user oid view.
+    :param resetmypasswordview: The class for reset my password view.
+    :param resetpasswordview: The class for reset password view.
+    :param rolemodelview: The class for role model view.
+    :param userinfoeditview: The class for user info edit view.
+    :param userdbmodelview: The class for user db model view.
+    :param userldapmodelview: The class for user ldap model view.
+    :param useroauthmodelview: The class for user oauth model view.
+    :param useroidmodelview: The class for user oid model view.
+    :param userremoteusermodelview: The class for user remote user model view.
+    :param userstatschartview: The class for user stats chart view.
+    """
+
+    """ The obj instance for authentication view """
+    auth_view = None
+    """ The obj instance for user view """
+    user_view = None
+
+    def __init__(self, **kwargs):
+        super().__init__(**kwargs)
+
+        self.appbuilder = kwargs["appbuilder"]
+        self.actionmodelview = kwargs["actionmodelview"]
+        self.authdbview = kwargs["authdbview"]
+        self.authldapview = kwargs["authldapview"]
+        self.authoauthview = kwargs["authoauthview"]
+        self.authoidview = kwargs["authoidview"]
+        self.authremoteuserview = kwargs["authremoteuserview"]
+        self.permissionmodelview = kwargs["permissionmodelview"]
+        self.registeruser_view = kwargs["registeruser_view"]
+        self.registeruserdbview = kwargs["registeruserdbview"]
+        self.registeruseroauthview = kwargs["registeruseroauthview"]
+        self.registerusermodelview = kwargs["registerusermodelview"]
+        self.registeruseroidview = kwargs["registeruseroidview"]
+        self.resetmypasswordview = kwargs["resetmypasswordview"]
+        self.resetpasswordview = kwargs["resetpasswordview"]
+        self.rolemodelview = kwargs["rolemodelview"]
+        self.userinfoeditview = kwargs["userinfoeditview"]
+        self.userdbmodelview = kwargs["userdbmodelview"]
+        self.userldapmodelview = kwargs["userldapmodelview"]
+        self.useroauthmodelview = kwargs["useroauthmodelview"]
+        self.useroidmodelview = kwargs["useroidmodelview"]
+        self.userremoteusermodelview = kwargs["userremoteusermodelview"]
+        self.userstatschartview = kwargs["userstatschartview"]
+
+    def register_views(self):
+        """Register FAB auth manager related views."""
+        if not self.appbuilder.app.config.get("FAB_ADD_SECURITY_VIEWS", True):
+            return
+
+        if self.auth_user_registration:
+            if self.auth_type == AUTH_DB:
+                self.registeruser_view = self.registeruserdbview()
+            elif self.auth_type == AUTH_OID:
+                self.registeruser_view = self.registeruseroidview()
+            elif self.auth_type == AUTH_OAUTH:
+                self.registeruser_view = self.registeruseroauthview()
+            if self.registeruser_view:
+                self.appbuilder.add_view_no_menu(self.registeruser_view)
+
+        self.appbuilder.add_view_no_menu(self.resetpasswordview())
+        self.appbuilder.add_view_no_menu(self.resetmypasswordview())
+        self.appbuilder.add_view_no_menu(self.userinfoeditview())
+
+        if self.auth_type == AUTH_DB:
+            self.user_view = self.userdbmodelview
+            self.auth_view = self.authdbview()
+        elif self.auth_type == AUTH_LDAP:
+            self.user_view = self.userldapmodelview
+            self.auth_view = self.authldapview()
+        elif self.auth_type == AUTH_OAUTH:
+            self.user_view = self.useroauthmodelview
+            self.auth_view = self.authoauthview()
+        elif self.auth_type == AUTH_REMOTE_USER:
+            self.user_view = self.userremoteusermodelview
+            self.auth_view = self.authremoteuserview()
+        else:
+            self.user_view = self.useroidmodelview
+            self.auth_view = self.authoidview()
+
+        self.appbuilder.add_view_no_menu(self.auth_view)
+
+        # this needs to be done after the view is added, otherwise the 
blueprint
+        # is not initialized
+        if self.is_auth_limited:
+            self.limiter.limit(self.auth_rate_limit, 
methods=["POST"])(self.auth_view.blueprint)
+
+        self.user_view = self.appbuilder.add_view(
+            self.user_view,
+            "List Users",
+            icon="fa-user",
+            label=lazy_gettext("List Users"),
+            category="Security",
+            category_icon="fa-cogs",
+            category_label=lazy_gettext("Security"),
+        )
+
+        role_view = self.appbuilder.add_view(
+            self.rolemodelview,
+            "List Roles",
+            icon="fa-group",
+            label=lazy_gettext("List Roles"),
+            category="Security",
+            category_icon="fa-cogs",
+        )
+        role_view.related_views = [self.user_view.__class__]
+
+        if self.userstatschartview:
+            self.appbuilder.add_view(
+                self.userstatschartview,
+                "User's Statistics",
+                icon="fa-bar-chart-o",
+                label=lazy_gettext("User's Statistics"),
+                category="Security",
+            )
+        if self.auth_user_registration:
+            self.appbuilder.add_view(
+                self.registerusermodelview,
+                "User's Statistics",

Review Comment:
   Copy/paste? This should say something about Registration



##########
airflow/www/fab_security/manager.py:
##########
@@ -208,12 +206,6 @@ def oauth_tokengetter(token=None):
     userstatschartview = UserStatsChartView
     permissionmodelview = PermissionModelView
 
-    @cached_property
-    def resourcemodelview(self):

Review Comment:
   Just for my own understanding, this is being moved to the fab auth manager 
(in the security_manager_override). But all the code above is described as an 
Override, which to me overrides some default implementation. But here it looks 
like the default is being removed. So now I'm a little confused what other auth 
managers will do. Will it become actually required to override the security 
manager? Since there is no impl left here? Or with other auth managers use the 
fab security manager override class?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to