ferruzzi commented on code in PR #35926:
URL: https://github.com/apache/airflow/pull/35926#discussion_r1409879281
##########
.pre-commit-config.yaml:
##########
@@ -525,6 +524,7 @@ repos:
^airflow/providers/apache/spark/hooks/|
^airflow/providers/apache/spark/operators/|
^airflow/providers/exasol/hooks/exasol.py$|
+ ^airflow/providers/fab/auth_manager/security_manager/|
Review Comment:
[Non-blocking] Interesting. I wonder why so many are exempt, we should
maybe work on getting this list trimmed down in another PR.
##########
setup.cfg:
##########
@@ -103,6 +103,7 @@ install_requires =
# NOTE! When you change the value here, you also have to update
flask-appbuilder[oauth] in setup.py
flask-appbuilder==4.3.9
flask-caching>=1.5.0
+ # We should be able to remove flask-login. It is still used in core
Airflow but in tests. Looks feasible.
Review Comment:
It's not entirely removed, just moved into the provider, right?
##########
docs/apache-airflow-providers-fab/index.rst:
##########
@@ -0,0 +1,92 @@
+
+.. 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.
+
+``apache-airflow-providers-fab``
+================================
+
+
+.. toctree::
+ :hidden:
+ :maxdepth: 1
+ :caption: Basics
+
+ Home <self>
+ Changelog <changelog>
+ Security <security>
+
+.. toctree::
+ :hidden:
+ :maxdepth: 1
+ :caption: Guides
+
+ Auth manager <auth-manager>
+
+.. toctree::
+ :hidden:
+ :maxdepth: 1
+ :caption: Resources
+
+ Python API <_api/airflow/providers/fab/index>
+ PyPI Repository <https://pypi.org/project/apache-airflow-providers-fab/>
+ Installing from sources <installing-providers-from-sources>
+
+
+Package apache-airflow-providers-fab
+------------------------------------
+
+`Flask App Builder <https://flask-appbuilder.readthedocs.io/>`__
+
+
+Release: 1.0.0
+
+Provider package
+----------------
+
+This is a provider package for optional Airflow components using ``Flask App
Builder``.
+All classes for this provider package are in ``airflow.providers.fab`` python
module.
+
+Installation
+------------
+
+You can install this package on top of an existing Airflow 2 installation (see
``Requirements`` below)
+for the minimum Airflow version supported) via ``pip install
apache-airflow-providers-fab``
Review Comment:
Not sure what happened here, I think this was your intent
```suggestion
You can install this package on top of an existing Airflow 2 installation
(see ``Requirements`` below
for the minimum Airflow version supported) via ``pip install
apache-airflow-providers-fab``
```
##########
airflow/providers/fab/auth_manager/security_manager/override.py:
##########
@@ -0,0 +1,2668 @@
+#
Review Comment:
I ran this file and the old
`airflow/auth/managers/fab/security_manager/override.py` though a diff, for
easier review: https://editor.mergely.com/kNuiVsA7
Changes are mainly to import paths and comments.
##########
airflow/providers/fab/auth_manager/security_manager/override.py:
##########
@@ -0,0 +1,2668 @@
+#
+# 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 datetime
+import itertools
+import logging
+import os
+import random
+import uuid
+import warnings
+from typing import TYPE_CHECKING, Any, Callable, Collection, Container,
Iterable, Sequence
+
+import jwt
+import re2
+from flask import flash, g, session
+from flask_appbuilder import const
+from flask_appbuilder.const import (
+ AUTH_DB,
+ AUTH_LDAP,
+ AUTH_OAUTH,
+ AUTH_OID,
+ AUTH_REMOTE_USER,
+ LOGMSG_ERR_SEC_ADD_REGISTER_USER,
+ LOGMSG_ERR_SEC_AUTH_LDAP,
+ LOGMSG_ERR_SEC_AUTH_LDAP_TLS,
+ LOGMSG_WAR_SEC_LOGIN_FAILED,
+ LOGMSG_WAR_SEC_NOLDAP_OBJ,
+ MICROSOFT_KEY_SET_URL,
+)
+from flask_appbuilder.models.sqla import Base
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from flask_appbuilder.security.registerviews import (
+ RegisterUserDBView,
+ RegisterUserOAuthView,
+ RegisterUserOIDView,
+)
+from flask_appbuilder.security.views import (
+ AuthDBView,
+ AuthLDAPView,
+ AuthOAuthView,
+ AuthOIDView,
+ AuthRemoteUserView,
+ RegisterUserModelView,
+)
+from flask_babel import lazy_gettext
+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, literal, or_, select
+from sqlalchemy.exc import MultipleResultsFound
+from sqlalchemy.orm import Session, joinedload
+from werkzeug.security import check_password_hash, generate_password_hash
+
+from airflow.auth.managers.utils.fab import get_method_from_fab_action_map
+from airflow.configuration import conf
+from airflow.exceptions import AirflowException,
AirflowProviderDeprecationWarning, RemovedInAirflow3Warning
+from airflow.models import DagBag, DagModel
+from airflow.providers.fab.auth_manager.models import (
+ Action,
+ Permission,
+ RegisterUser,
+ Resource,
+ Role,
+ User,
+ assoc_permission_role,
+)
+from airflow.providers.fab.auth_manager.models.anonymous_user import
AnonymousUser
+from airflow.providers.fab.auth_manager.security_manager.constants import
EXISTING_ROLES
+from airflow.providers.fab.auth_manager.views.permissions import (
+ ActionModelView,
+ PermissionPairModelView,
+ ResourceModelView,
+)
+from airflow.providers.fab.auth_manager.views.roles_list import
CustomRoleModelView
+from airflow.providers.fab.auth_manager.views.user import (
+ CustomUserDBModelView,
+ CustomUserLDAPModelView,
+ CustomUserOAuthModelView,
+ CustomUserOIDModelView,
+ CustomUserRemoteUserModelView,
+)
+from airflow.providers.fab.auth_manager.views.user_edit import (
+ CustomResetMyPasswordView,
+ CustomResetPasswordView,
+ CustomUserInfoEditView,
+)
+from airflow.providers.fab.auth_manager.views.user_stats import
CustomUserStatsChartView
+from airflow.security import permissions
+from airflow.utils.session import NEW_SESSION, provide_session
+from airflow.www.extensions.init_auth_manager import get_auth_manager
+from airflow.www.security_manager import AirflowSecurityManagerV2
+from airflow.www.session import AirflowDatabaseSessionInterface
+
+if TYPE_CHECKING:
+ from airflow.auth.managers.base_auth_manager import ResourceMethod
+
+log = logging.getLogger(__name__)
+
+# This is the limit of DB user sessions that we consider as "healthy". If you
have more sessions that this
+# number then we will refuse to delete sessions that have expired and old user
sessions when resetting
+# user's password, and raise a warning in the UI instead. Usually when you
have that many sessions, it means
+# that there is something wrong with your deployment - for example you have an
automated API call that
+# continuously creates new sessions. Such setup should be fixed by reusing
sessions or by periodically
+# purging the old sessions by using `airflow db clean` command.
+MAX_NUM_DATABASE_USER_SESSIONS = 50000
+
+
+class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
+ """
+ This security manager overrides the default AirflowSecurityManager
security manager.
+
+ This security manager is used only if the auth manager FabAuthManager is
used. It defines everything in
+ the security manager that is needed for the FabAuthManager to work. Any
operation specific to
+ the AirflowSecurityManager should be defined here instead of
AirflowSecurityManager.
+
+ :param appbuilder: The appbuilder.
+ """
+
+ auth_view = None
+ """ The obj instance for authentication view """
+ registeruser_view = None
+ """ The obj instance for registering user view """
+ user_view = None
+ """ The obj instance for user view """
+
+ """ Models """
+ user_model = User
+ role_model = Role
+ action_model = Action
+ resource_model = Resource
+ permission_model = Permission
+
+ """ Views """
+ authdbview = AuthDBView
+ """ Override if you want your own Authentication DB view """
+ authldapview = AuthLDAPView
+ """ Override if you want your own Authentication LDAP view """
+ authoidview = AuthOIDView
+ """ Override if you want your own Authentication OID view """
+ authoauthview = AuthOAuthView
+ """ Override if you want your own Authentication OAuth view """
+ authremoteuserview = AuthRemoteUserView
+ """ Override if you want your own Authentication REMOTE_USER view """
+ registeruserdbview = RegisterUserDBView
+ """ Override if you want your own register user db view """
+ registeruseroidview = RegisterUserOIDView
+ """ Override if you want your own register user OpenID view """
+ registeruseroauthview = RegisterUserOAuthView
+ """ Override if you want your own register user OAuth view """
+ actionmodelview = ActionModelView
+ permissionmodelview = PermissionPairModelView
+ rolemodelview = CustomRoleModelView
+ registeruser_model = RegisterUser
+ registerusermodelview = RegisterUserModelView
+ resourcemodelview = ResourceModelView
+ userdbmodelview = CustomUserDBModelView
+ resetmypasswordview = CustomResetMyPasswordView
+ resetpasswordview = CustomResetPasswordView
+ userinfoeditview = CustomUserInfoEditView
+ userldapmodelview = CustomUserLDAPModelView
+ useroauthmodelview = CustomUserOAuthModelView
+ userremoteusermodelview = CustomUserRemoteUserModelView
+ useroidmodelview = CustomUserOIDModelView
+ userstatschartview = CustomUserStatsChartView
+
Review Comment:
In the following block: I see that you just used it how it was in the old
file (with some corrections), but I feel like the comment should be above the
assignment it is describing. I'll leave it up to you though, I don't feel too
strongly about it.
##########
docs/apache-airflow-providers-fab/index.rst:
##########
@@ -0,0 +1,92 @@
+
+.. 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.
+
+``apache-airflow-providers-fab``
+================================
+
+
+.. toctree::
+ :hidden:
+ :maxdepth: 1
+ :caption: Basics
+
+ Home <self>
+ Changelog <changelog>
+ Security <security>
+
+.. toctree::
+ :hidden:
+ :maxdepth: 1
+ :caption: Guides
+
+ Auth manager <auth-manager>
+
+.. toctree::
+ :hidden:
+ :maxdepth: 1
+ :caption: Resources
+
+ Python API <_api/airflow/providers/fab/index>
+ PyPI Repository <https://pypi.org/project/apache-airflow-providers-fab/>
+ Installing from sources <installing-providers-from-sources>
+
+
+Package apache-airflow-providers-fab
+------------------------------------
+
+`Flask App Builder <https://flask-appbuilder.readthedocs.io/>`__
+
+
+Release: 1.0.0
+
+Provider package
+----------------
+
+This is a provider package for optional Airflow components using ``Flask App
Builder``.
+All classes for this provider package are in ``airflow.providers.fab`` python
module.
Review Comment:
```suggestion
All classes for this provider package are in the ``airflow.providers.fab``
python module.
```
--
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
