From a9fb853e3ef69cecaca23f83207c0470199dcaa5 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Fri, 12 Sep 2025 09:21:37 +0100 Subject: [PATCH] fix: Bump FAB to 5.X (#33055) Co-authored-by: Joe Li --- UPDATING.md | 3 +- .../configuration/configuring-superset.mdx | 104 ------------------ .../configuration/configuring-superset.mdx | 104 ------------------ pyproject.toml | 2 +- requirements/base.txt | 8 +- requirements/development.txt | 5 +- superset-core/pyproject.toml | 2 +- superset/config.py | 1 - superset/dashboards/api.py | 6 +- superset/extensions/__init__.py | 5 +- superset/initialization/__init__.py | 4 +- superset/migrations/env.py | 4 +- superset/models/dynamic_plugins.py | 1 + superset/security/manager.py | 42 +++---- superset/tags/api.py | 6 +- superset/utils/core.py | 4 +- superset/views/base.py | 16 +-- superset/views/base_api.py | 6 +- tests/integration_tests/base_api_tests.py | 6 +- .../dashboards/dashboard_test_utils.py | 2 +- .../dynamic_plugins_tests.py | 4 +- tests/integration_tests/fixtures/users.py | 6 +- tests/integration_tests/queries/api_tests.py | 36 +++--- tests/integration_tests/security_tests.py | 22 ++-- tests/unit_tests/conftest.py | 2 +- tests/unit_tests/databases/api_test.py | 20 ++-- tests/unit_tests/security/manager_test.py | 6 +- 27 files changed, 100 insertions(+), 327 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 16d3373e1b3..f34c46debf0 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,7 +23,8 @@ This file documents any backwards-incompatible changes in Superset and assists people when migrating to a new version. ## Next -- [35062](https://github.com/apache/superset/pull/35062): Changed the function signature of `setupExtensions` to `setupCodeOverrides` with options as arguments. +- [33055](https://github.com/apache/superset/pull/33055): Upgrades Flask-AppBuilder to 5.0.0. The AUTH_OID authentication type has been deprecated and is no longer available as an option in Flask-AppBuilder. OpenID (OID) is considered a deprecated authentication protocol - if you are using AUTH_OID, you will need to migrate to an alternative authentication method such as OAuth, LDAP, or database authentication before upgrading. +- [35062](https://github.com/apache/superset/pull/35062): Changed the function signature of `setupExtensions` to `setupCodeOverrides` with options as arguments. - [34871](https://github.com/apache/superset/pull/34871): Fixed Jest test hanging issue from Ant Design v5 upgrade. MessageChannel is now mocked in test environment to prevent rc-overflow from causing Jest to hang. Test environment only - no production impact. - [34782](https://github.com/apache/superset/pull/34782): Dataset exports now include the dataset ID in their file name (similar to charts and dashboards). If managing assets as code, make sure to rename existing dataset YAMLs to include the ID (and avoid duplicated files). - [34536](https://github.com/apache/superset/pull/34536): The `ENVIRONMENT_TAG_CONFIG` color values have changed to support only Ant Design semantic colors. Update your `superset_config.py`: diff --git a/docs/docs/configuration/configuring-superset.mdx b/docs/docs/configuration/configuring-superset.mdx index 1a453adfe32..0a50b07fe69 100644 --- a/docs/docs/configuration/configuring-superset.mdx +++ b/docs/docs/configuration/configuring-superset.mdx @@ -363,110 +363,6 @@ CUSTOM_SECURITY_MANAGER = CustomSsoSecurityManager ] ``` -### Keycloak-Specific Configuration using Flask-OIDC - -If you are using Keycloak as OpenID Connect 1.0 Provider, the above configuration based on [`Authlib`](https://authlib.org/) might not work. In this case using [`Flask-OIDC`](https://pypi.org/project/flask-oidc/) is a viable option. - -Make sure the pip package [`Flask-OIDC`](https://pypi.org/project/flask-oidc/) is installed on the webserver. This was successfully tested using version 2.2.0. This package requires [`Flask-OpenID`](https://pypi.org/project/Flask-OpenID/) as a dependency. - -The following code defines a new security manager. Add it to a new file named `keycloak_security_manager.py`, placed in the same directory as your `superset_config.py` file. - -```python -from flask_appbuilder.security.manager import AUTH_OID -from superset.security import SupersetSecurityManager -from flask_oidc import OpenIDConnect -from flask_appbuilder.security.views import AuthOIDView -from flask_login import login_user -from urllib.parse import quote -from flask_appbuilder.views import ModelView, SimpleFormView, expose -from flask import ( - redirect, - request -) -import logging - -class OIDCSecurityManager(SupersetSecurityManager): - - def __init__(self, appbuilder): - super(OIDCSecurityManager, self).__init__(appbuilder) - if self.auth_type == AUTH_OID: - self.oid = OpenIDConnect(self.appbuilder.get_app) - self.authoidview = AuthOIDCView - -class AuthOIDCView(AuthOIDView): - - @expose('/login/', methods=['GET', 'POST']) - def login(self, flag=True): - sm = self.appbuilder.sm - oidc = sm.oid - - @self.appbuilder.sm.oid.require_login - def handle_login(): - user = sm.auth_user_oid(oidc.user_getfield('email')) - - if user is None: - info = oidc.user_getinfo(['preferred_username', 'given_name', 'family_name', 'email']) - user = sm.add_user(info.get('preferred_username'), info.get('given_name'), info.get('family_name'), - info.get('email'), sm.find_role('Gamma')) - - login_user(user, remember=False) - return redirect(self.appbuilder.get_url_for_index) - - return handle_login() - - @expose('/logout/', methods=['GET', 'POST']) - def logout(self): - oidc = self.appbuilder.sm.oid - - oidc.logout() - super(AuthOIDCView, self).logout() - redirect_url = request.url_root.strip('/') + self.appbuilder.get_url_for_login - - return redirect( - oidc.client_secrets.get('issuer') + '/protocol/openid-connect/logout?redirect_uri=' + quote(redirect_url)) -``` - -Then add to your `superset_config.py` file: - -```python -from keycloak_security_manager import OIDCSecurityManager -from flask_appbuilder.security.manager import AUTH_OID, AUTH_REMOTE_USER, AUTH_DB, AUTH_LDAP, AUTH_OAUTH -import os - -AUTH_TYPE = AUTH_OID -SECRET_KEY: 'SomethingNotEntirelySecret' -OIDC_CLIENT_SECRETS = '/path/to/client_secret.json' -OIDC_ID_TOKEN_COOKIE_SECURE = False -OIDC_OPENID_REALM: '' -OIDC_INTROSPECTION_AUTH_METHOD: 'client_secret_post' -CUSTOM_SECURITY_MANAGER = OIDCSecurityManager - -# Will allow user self registration, allowing to create Flask users from Authorized User -AUTH_USER_REGISTRATION = True - -# The default user self registration role -AUTH_USER_REGISTRATION_ROLE = 'Public' -``` - -Store your client-specific OpenID information in a file called `client_secret.json`. Create this file in the same directory as `superset_config.py`: - -```json -{ - "": { - "issuer": "https:///realms/", - "auth_uri": "https:///realms//protocol/openid-connect/auth", - "client_id": "https://", - "client_secret": "", - "redirect_uris": [ - "https:///oauth-authorized/" - ], - "userinfo_uri": "https:///realms//protocol/openid-connect/userinfo", - "token_uri": "https:///realms//protocol/openid-connect/token", - "token_introspection_uri": "https:///realms//protocol/openid-connect/token/introspect" - } -} -``` - ## LDAP Authentication FAB supports authenticating user credentials against an LDAP server. diff --git a/docs/versioned_docs/version-6.0.0/configuration/configuring-superset.mdx b/docs/versioned_docs/version-6.0.0/configuration/configuring-superset.mdx index 1a453adfe32..0a50b07fe69 100644 --- a/docs/versioned_docs/version-6.0.0/configuration/configuring-superset.mdx +++ b/docs/versioned_docs/version-6.0.0/configuration/configuring-superset.mdx @@ -363,110 +363,6 @@ CUSTOM_SECURITY_MANAGER = CustomSsoSecurityManager ] ``` -### Keycloak-Specific Configuration using Flask-OIDC - -If you are using Keycloak as OpenID Connect 1.0 Provider, the above configuration based on [`Authlib`](https://authlib.org/) might not work. In this case using [`Flask-OIDC`](https://pypi.org/project/flask-oidc/) is a viable option. - -Make sure the pip package [`Flask-OIDC`](https://pypi.org/project/flask-oidc/) is installed on the webserver. This was successfully tested using version 2.2.0. This package requires [`Flask-OpenID`](https://pypi.org/project/Flask-OpenID/) as a dependency. - -The following code defines a new security manager. Add it to a new file named `keycloak_security_manager.py`, placed in the same directory as your `superset_config.py` file. - -```python -from flask_appbuilder.security.manager import AUTH_OID -from superset.security import SupersetSecurityManager -from flask_oidc import OpenIDConnect -from flask_appbuilder.security.views import AuthOIDView -from flask_login import login_user -from urllib.parse import quote -from flask_appbuilder.views import ModelView, SimpleFormView, expose -from flask import ( - redirect, - request -) -import logging - -class OIDCSecurityManager(SupersetSecurityManager): - - def __init__(self, appbuilder): - super(OIDCSecurityManager, self).__init__(appbuilder) - if self.auth_type == AUTH_OID: - self.oid = OpenIDConnect(self.appbuilder.get_app) - self.authoidview = AuthOIDCView - -class AuthOIDCView(AuthOIDView): - - @expose('/login/', methods=['GET', 'POST']) - def login(self, flag=True): - sm = self.appbuilder.sm - oidc = sm.oid - - @self.appbuilder.sm.oid.require_login - def handle_login(): - user = sm.auth_user_oid(oidc.user_getfield('email')) - - if user is None: - info = oidc.user_getinfo(['preferred_username', 'given_name', 'family_name', 'email']) - user = sm.add_user(info.get('preferred_username'), info.get('given_name'), info.get('family_name'), - info.get('email'), sm.find_role('Gamma')) - - login_user(user, remember=False) - return redirect(self.appbuilder.get_url_for_index) - - return handle_login() - - @expose('/logout/', methods=['GET', 'POST']) - def logout(self): - oidc = self.appbuilder.sm.oid - - oidc.logout() - super(AuthOIDCView, self).logout() - redirect_url = request.url_root.strip('/') + self.appbuilder.get_url_for_login - - return redirect( - oidc.client_secrets.get('issuer') + '/protocol/openid-connect/logout?redirect_uri=' + quote(redirect_url)) -``` - -Then add to your `superset_config.py` file: - -```python -from keycloak_security_manager import OIDCSecurityManager -from flask_appbuilder.security.manager import AUTH_OID, AUTH_REMOTE_USER, AUTH_DB, AUTH_LDAP, AUTH_OAUTH -import os - -AUTH_TYPE = AUTH_OID -SECRET_KEY: 'SomethingNotEntirelySecret' -OIDC_CLIENT_SECRETS = '/path/to/client_secret.json' -OIDC_ID_TOKEN_COOKIE_SECURE = False -OIDC_OPENID_REALM: '' -OIDC_INTROSPECTION_AUTH_METHOD: 'client_secret_post' -CUSTOM_SECURITY_MANAGER = OIDCSecurityManager - -# Will allow user self registration, allowing to create Flask users from Authorized User -AUTH_USER_REGISTRATION = True - -# The default user self registration role -AUTH_USER_REGISTRATION_ROLE = 'Public' -``` - -Store your client-specific OpenID information in a file called `client_secret.json`. Create this file in the same directory as `superset_config.py`: - -```json -{ - "": { - "issuer": "https:///realms/", - "auth_uri": "https:///realms//protocol/openid-connect/auth", - "client_id": "https://", - "client_secret": "", - "redirect_uris": [ - "https:///oauth-authorized/" - ], - "userinfo_uri": "https:///realms//protocol/openid-connect/userinfo", - "token_uri": "https:///realms//protocol/openid-connect/token", - "token_introspection_uri": "https:///realms//protocol/openid-connect/token/introspect" - } -} -``` - ## LDAP Authentication FAB supports authenticating user credentials against an LDAP server. diff --git a/pyproject.toml b/pyproject.toml index ef848653092..9d83616441a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,7 @@ dependencies = [ "cryptography>=42.0.4, <45.0.0", "deprecation>=2.1.0, <2.2.0", "flask>=2.2.5, <3.0.0", - "flask-appbuilder>=4.8.1, <5.0.0", + "flask-appbuilder>=5.0.0,<6", "flask-caching>=2.1.0, <3", "flask-compress>=1.13, <2.0", "flask-talisman>=1.0.0, <2.0", diff --git a/requirements/base.txt b/requirements/base.txt index 4357d92701c..d62d7f09750 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -114,11 +114,9 @@ flask==2.3.3 # flask-session # flask-sqlalchemy # flask-wtf -flask-appbuilder==4.8.1 - # via - # apache-superset (pyproject.toml) - # apache-superset-core -flask-babel==2.0.0 +flask-appbuilder==5.0.0 + # via apache-superset (pyproject.toml) +flask-babel==3.1.0 # via flask-appbuilder flask-caching==2.3.1 # via apache-superset (pyproject.toml) diff --git a/requirements/development.txt b/requirements/development.txt index f9b292c8b95..978fecc8b69 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -208,12 +208,11 @@ flask==2.3.3 # flask-sqlalchemy # flask-testing # flask-wtf -flask-appbuilder==4.8.1 +flask-appbuilder==5.0.0 # via # -c requirements/base-constraint.txt # apache-superset - # apache-superset-core -flask-babel==2.0.0 +flask-babel==3.1.0 # via # -c requirements/base-constraint.txt # flask-appbuilder diff --git a/superset-core/pyproject.toml b/superset-core/pyproject.toml index 7c6a760caae..0511509caa1 100644 --- a/superset-core/pyproject.toml +++ b/superset-core/pyproject.toml @@ -42,7 +42,7 @@ classifiers = [ "Topic :: Software Development :: Libraries :: Python Modules", ] dependencies = [ - "flask-appbuilder>=4.5.3, <5.0.0", + "flask-appbuilder>=5.0.0,<6", ] [project.urls] diff --git a/superset/config.py b/superset/config.py index b41992fe1e8..9290a4123f5 100644 --- a/superset/config.py +++ b/superset/config.py @@ -350,7 +350,6 @@ FAB_API_SWAGGER_UI = True # AUTHENTICATION CONFIG # ---------------------------------------------------- # The authentication type -# AUTH_OID : Is for OpenID # AUTH_DB : Is for database (username/password) # AUTH_LDAP : Is for LDAP # AUTH_REMOTE_USER : Is for using REMOTE_USER from web server diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 83387ee3372..c811e5a868b 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -22,7 +22,7 @@ from io import BytesIO from typing import Any, Callable, cast from zipfile import is_zipfile, ZipFile -from flask import g, redirect, request, Response, send_file, url_for +from flask import current_app, g, redirect, request, Response, send_file, url_for from flask_appbuilder import permission_name from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface @@ -332,8 +332,8 @@ class DashboardRestApi(BaseSupersetModelRestApi): """Deterministic string representation of the API instance for etag_cache.""" # pylint: disable=consider-using-f-string return "Superset.dashboards.api.DashboardRestApi@v{}{}".format( - self.appbuilder.app.config["VERSION_STRING"], - self.appbuilder.app.config["VERSION_SHA"], + current_app.config["VERSION_STRING"], + current_app.config["VERSION_SHA"], ) @expose("/", methods=("GET",)) diff --git a/superset/extensions/__init__.py b/superset/extensions/__init__.py index ffe9c799678..a44d82f76af 100644 --- a/superset/extensions/__init__.py +++ b/superset/extensions/__init__.py @@ -20,7 +20,8 @@ from typing import Any, Callable, Optional import celery from flask import Flask -from flask_appbuilder import AppBuilder, SQLA +from flask_appbuilder import AppBuilder +from flask_appbuilder.utils.legacy import get_sqla_class from flask_caching.backends.base import BaseCache from flask_migrate import Migrate from flask_talisman import Talisman @@ -123,7 +124,7 @@ async_query_manager: AsyncQueryManager = LocalProxy( cache_manager = CacheManager() celery_app = celery.Celery() csrf = CSRFProtect() -db = SQLA() # pylint: disable=disallowed-name +db = get_sqla_class()() _event_logger: dict[str, Any] = {} encrypted_field_factory = EncryptedFieldFactory() event_logger = LocalProxy(lambda: _event_logger.get("event_logger")) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index cdecd96b92b..82296e0e37f 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -25,7 +25,7 @@ from typing import Any, Callable, TYPE_CHECKING import wtforms_json from colorama import Fore, Style from deprecation import deprecated -from flask import abort, Flask, redirect, request, session, url_for +from flask import abort, current_app, Flask, redirect, request, session, url_for from flask_appbuilder import expose, IndexView from flask_appbuilder.api import safe from flask_appbuilder.utils.base import get_safe_redirect @@ -290,7 +290,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods "Home", label=_("Home"), href="/superset/welcome/", - cond=lambda: bool(appbuilder.app.config["LOGO_TARGET_PATH"]), + cond=lambda: bool(current_app.config["LOGO_TARGET_PATH"]), ) appbuilder.add_view( diff --git a/superset/migrations/env.py b/superset/migrations/env.py index 295eb85d1f8..f6101543676 100755 --- a/superset/migrations/env.py +++ b/superset/migrations/env.py @@ -22,7 +22,7 @@ from alembic import context from alembic.operations.ops import MigrationScript from alembic.runtime.migration import MigrationContext from flask import current_app -from flask_appbuilder import Base +from flask_appbuilder import Model from sqlalchemy import engine_from_config, pool # this is the Alembic Config object, which provides @@ -45,7 +45,7 @@ if "sqlite" in DATABASE_URI: # Escape % chars in the database URI to avoid interpolation errors in ConfigParser escaped_uri = DATABASE_URI.replace("%", "%%") config.set_main_option("sqlalchemy.url", escaped_uri) -target_metadata = Base.metadata # pylint: disable=no-member +target_metadata = Model.metadata # pylint: disable=no-member # other values from the config, defined by the needs of env.py, diff --git a/superset/models/dynamic_plugins.py b/superset/models/dynamic_plugins.py index 62eb0421bec..51a2150ed06 100644 --- a/superset/models/dynamic_plugins.py +++ b/superset/models/dynamic_plugins.py @@ -21,6 +21,7 @@ from superset.models.helpers import AuditMixinNullable class DynamicPlugin(Model, AuditMixinNullable): + __tablename__ = "dynamic_plugin" id = Column(Integer, primary_key=True) name = Column(Text, unique=True, nullable=False) # key corresponds to viz_type from static plugins diff --git a/superset/security/manager.py b/superset/security/manager.py index b27874cdfa8..caa6d559c2b 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -805,7 +805,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods from superset.connectors.sqla.models import SqlaTable user_datasources.update( - self.get_session.query(SqlaTable) + self.session.query(SqlaTable) .filter(get_dataset_access_filters(SqlaTable)) .all() ) @@ -841,7 +841,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods def user_view_menu_names(self, permission_name: str) -> set[str]: base_query = ( - self.get_session.query(self.viewmenu_model.name) + self.session.query(self.viewmenu_model.name) .join(self.permissionview_model) .join(self.permission_model) .join(assoc_permissionview_role) @@ -949,7 +949,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods # datasource_access if perms := self.user_view_menu_names("datasource_access"): tables = ( - self.get_session.query(SqlaTable.schema) + self.session.query(SqlaTable.schema) .filter(SqlaTable.database_id == database.id) .filter(or_(SqlaTable.perm.in_(perms))) .distinct() @@ -1009,7 +1009,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods # datasource_access if perms := self.user_view_menu_names("datasource_access"): tables = ( - self.get_session.query(SqlaTable.schema) + self.session.query(SqlaTable.schema) .filter(SqlaTable.database_id == database.id) .filter(or_(SqlaTable.perm.in_(perms))) .distinct() @@ -1152,7 +1152,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods merge_pv("catalog_access", datasource.get_catalog_perm()) logger.info("Creating missing database permissions.") - databases = self.get_session.query(models.Database).all() + databases = self.session.query(models.Database).all() for database in databases: merge_pv("database_access", database.perm) @@ -1162,7 +1162,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods """ logger.info("Cleaning faulty perms") - pvms = self.get_session.query(PermissionView).filter( + pvms = self.session.query(PermissionView).filter( or_( PermissionView.permission # pylint: disable=singleton-comparison == None, # noqa: E711 @@ -1205,7 +1205,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods Gets list of all PVM """ pvms = ( - self.get_session.query(self.permissionview_model) + self.session.query(self.permissionview_model) .options( eagerload(self.permissionview_model.permission), eagerload(self.permissionview_model.view_menu), @@ -1220,7 +1220,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods definition """ role_from_permissions_names = self.builtin_roles.get(role_name, []) - all_pvms = self.get_session.query(PermissionView).all() + all_pvms = self.session.query(PermissionView).all() role_from_permissions = [] for pvm_regex in role_from_permissions_names: view_name_regex = pvm_regex[0] @@ -1237,7 +1237,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods """ Find a List of models by a list of ids, if defined applies `base_filter` """ - query = self.get_session.query(self.role_model).filter( + query = self.session.query(self.role_model).filter( self.role_model.id.in_(role_ids) ) return query.all() @@ -1510,7 +1510,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods ) # Clean database schema permissions schema_pvms = ( - self.get_session.query(self.permissionview_model) + self.session.query(self.permissionview_model) .join(self.permission_model) .join(self.viewmenu_model) .filter( @@ -1609,7 +1609,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods chart_table = Slice.__table__ # pylint: disable=no-member new_database_name = target.database_name datasets = ( - self.get_session.query(SqlaTable) + self.session.query(SqlaTable) .filter(SqlaTable.database_id == target.id) .all() ) @@ -1688,7 +1688,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods logger.warning( "Dataset has no database will retry with database_id to set permission" ) - database = self.get_session.query(Database).get(target.database_id) + database = self.session.query(Database).get(target.database_id) dataset_perm = self.get_dataset_perm( target.id, target.table_name, database.database_name ) @@ -2322,7 +2322,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods client_id=shortid()[:10], user_id=get_user_id(), ) - self.get_session.expunge(query) + self.session.expunge(query) if database and table or query: if query: @@ -2439,7 +2439,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods form_data and (dashboard_id := form_data.get("dashboardId")) and ( - dashboard_ := self.get_session.query(Dashboard) + dashboard_ := self.session.query(Dashboard) .filter(Dashboard.id == dashboard_id) .one_or_none() ) @@ -2472,7 +2472,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods form_data.get("type") != "NATIVE_FILTER" and (slice_id := form_data.get("slice_id")) and ( - slc := self.get_session.query(Slice) + slc := self.session.query(Slice) .filter(Slice.id == slice_id) .one_or_none() ) @@ -2546,7 +2546,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods need to be scoped """ return ( - self.get_session.query(self.user_model) + self.session.query(self.user_model) .filter(self.user_model.username == username) .one_or_none() ) @@ -2601,7 +2601,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods user_roles = [role.id for role in self.get_user_roles(g.user)] regular_filter_roles = ( - self.get_session.query(RLSFilterRoles.c.rls_filter_id) + self.session.query(RLSFilterRoles.c.rls_filter_id) .join(RowLevelSecurityFilter) .filter( RowLevelSecurityFilter.filter_type == RowLevelSecurityFilterType.REGULAR @@ -2609,18 +2609,18 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods .filter(RLSFilterRoles.c.role_id.in_(user_roles)) ) base_filter_roles = ( - self.get_session.query(RLSFilterRoles.c.rls_filter_id) + self.session.query(RLSFilterRoles.c.rls_filter_id) .join(RowLevelSecurityFilter) .filter( RowLevelSecurityFilter.filter_type == RowLevelSecurityFilterType.BASE ) .filter(RLSFilterRoles.c.role_id.in_(user_roles)) ) - filter_tables = self.get_session.query(RLSFilterTables.c.rls_filter_id).filter( + filter_tables = self.session.query(RLSFilterTables.c.rls_filter_id).filter( RLSFilterTables.c.table_id == table.id ) query = ( - self.get_session.query( + self.session.query( RowLevelSecurityFilter.id, RowLevelSecurityFilter.group_key, RowLevelSecurityFilter.clause, @@ -2827,7 +2827,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods if self.is_admin(): return - orig_resource = self.get_session.query(resource.__class__).get(resource.id) + orig_resource = self.session.query(resource.__class__).get(resource.id) owners = orig_resource.owners if hasattr(orig_resource, "owners") else [] if g.user.is_anonymous or g.user not in owners: diff --git a/superset/tags/api.py b/superset/tags/api.py index 2d92b2ff5c6..9d413ef51a5 100644 --- a/superset/tags/api.py +++ b/superset/tags/api.py @@ -17,7 +17,7 @@ import logging from typing import Any -from flask import request, Response +from flask import current_app, request, Response from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface from marshmallow import ValidationError @@ -146,8 +146,8 @@ class TagRestApi(BaseSupersetModelRestApi): """Deterministic string representation of the API instance for etag_cache.""" return ( "Superset.tags.api.TagRestApi@v" - f"{self.appbuilder.app.config['VERSION_STRING']}" - f"{self.appbuilder.app.config['VERSION_SHA']}" + f"{current_app.config['VERSION_STRING']}" + f"{current_app.config['VERSION_SHA']}" ) @expose("/", methods=("POST",)) diff --git a/superset/utils/core.py b/superset/utils/core.py index bb315f3694d..bd0c4e4d38e 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -69,9 +69,9 @@ import sqlalchemy as sa from cryptography.hazmat.backends import default_backend from cryptography.x509 import Certificate, load_pem_x509_certificate from flask import current_app as app, g, request -from flask_appbuilder import SQLA from flask_appbuilder.security.sqla.models import User from flask_babel import gettext as __ +from flask_sqlalchemy import SQLAlchemy from markupsafe import Markup from pandas.api.types import infer_dtype from pandas.core.dtypes.common import is_numeric_dtype @@ -613,7 +613,7 @@ def readfile(file_path: str) -> str | None: def generic_find_constraint_name( - table: str, columns: set[str], referenced: str, database: SQLA + table: str, columns: set[str], referenced: str, database: SQLAlchemy ) -> str | None: """Utility to find a constraint name in alembic migrations""" tbl = sa.Table( diff --git a/superset/views/base.py b/superset/views/base.py index 000ab652909..5744e73198a 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -37,7 +37,7 @@ from flask import ( ) from flask_appbuilder import BaseView, Model, ModelView from flask_appbuilder.actions import action -from flask_appbuilder.const import AUTH_OAUTH, AUTH_OID +from flask_appbuilder.const import AUTH_OAUTH from flask_appbuilder.forms import DynamicForm from flask_appbuilder.models.sqla.filters import BaseFilter from flask_appbuilder.security.sqla.models import User @@ -471,12 +471,6 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument ) frontend_config["AUTH_PROVIDERS"] = oauth_providers - if auth_type == AUTH_OID: - oid_providers = [] - for provider in appbuilder.sm.openid_providers: - oid_providers.append(provider) - frontend_config["AUTH_PROVIDERS"] = oid_providers - bootstrap_data = { "application_root": app.config["APPLICATION_ROOT"], "static_assets_prefix": app.config["STATIC_ASSETS_PREFIX"], @@ -603,9 +597,7 @@ class DeleteMixin: # pylint: disable=too-few-public-methods else: view_menu = security_manager.find_view_menu(item.get_perm()) pvs = ( - security_manager.get_session.query( - security_manager.permissionview_model - ) + db.session.query(security_manager.permissionview_model) .filter_by(view_menu=view_menu) .all() ) @@ -614,10 +606,10 @@ class DeleteMixin: # pylint: disable=too-few-public-methods self.post_delete(item) for pv in pvs: - security_manager.get_session.delete(pv) + db.session.delete(pv) if view_menu: - security_manager.get_session.delete(view_menu) + db.session.delete(view_menu) db.session.commit() # pylint: disable=consider-using-transaction diff --git a/superset/views/base_api.py b/superset/views/base_api.py index 682e8f86971..843c57aae44 100644 --- a/superset/views/base_api.py +++ b/superset/views/base_api.py @@ -657,15 +657,13 @@ class BaseSupersetModelRestApi(BaseSupersetApiMixin, ModelRestApi): # Create generic base filters with added request filter filters = self._get_distinct_filter(column_name, args.get("filter")) # Make the query - query_count = self.appbuilder.get_session.query( + query_count = db.session.query( func.count(distinct(getattr(self.datamodel.obj, column_name))) ) count = self.datamodel.apply_filters(query_count, filters).scalar() if count == 0: return self.response(200, count=count, result=[]) - query = self.appbuilder.get_session.query( - distinct(getattr(self.datamodel.obj, column_name)) - ) + query = db.session.query(distinct(getattr(self.datamodel.obj, column_name))) # Apply generic base filters with added request filter query = self.datamodel.apply_filters(query, filters) # Apply sort diff --git a/tests/integration_tests/base_api_tests.py b/tests/integration_tests/base_api_tests.py index 319a785b3a7..7792d83c0ff 100644 --- a/tests/integration_tests/base_api_tests.py +++ b/tests/integration_tests/base_api_tests.py @@ -55,10 +55,10 @@ class Model1Api(BaseSupersetModelRestApi): } -appbuilder.add_api(Model1Api) - - class TestOpenApiSpec(SupersetTestCase): + def setUp(self) -> None: + appbuilder.add_api(Model1Api) + def test_open_api_spec(self): """ API: Test validate OpenAPI spec diff --git a/tests/integration_tests/dashboards/dashboard_test_utils.py b/tests/integration_tests/dashboards/dashboard_test_utils.py index ecaf2cc024c..67973b52aa5 100644 --- a/tests/integration_tests/dashboards/dashboard_test_utils.py +++ b/tests/integration_tests/dashboards/dashboard_test_utils.py @@ -29,7 +29,7 @@ from tests.integration_tests.dashboards.consts import DEFAULT_DASHBOARD_SLUG_TO_ logger = logging.getLogger(__name__) -session = appbuilder.get_session +session = appbuilder.session def get_mock_positions(dashboard: Dashboard) -> dict[str, Any]: diff --git a/tests/integration_tests/dynamic_plugins_tests.py b/tests/integration_tests/dynamic_plugins_tests.py index d8f5aab7f44..3f4e649665c 100644 --- a/tests/integration_tests/dynamic_plugins_tests.py +++ b/tests/integration_tests/dynamic_plugins_tests.py @@ -26,7 +26,7 @@ class TestDynamicPlugins(SupersetTestCase): Dynamic Plugins: Responds not found when disabled """ self.login(ADMIN_USERNAME) - uri = "/dynamic-plugins/api" + uri = "/dynamic-plugins/list/" rv = self.client.get(uri) assert rv.status_code == 404 @@ -36,6 +36,6 @@ class TestDynamicPlugins(SupersetTestCase): Dynamic Plugins: Responds successfully when enabled """ self.login(ADMIN_USERNAME) - uri = "/dynamic-plugins/api" + uri = "/dynamic-plugins/list/" rv = self.client.get(uri) assert rv.status_code == 200 diff --git a/tests/integration_tests/fixtures/users.py b/tests/integration_tests/fixtures/users.py index af03d1d4954..0151cceeea3 100644 --- a/tests/integration_tests/fixtures/users.py +++ b/tests/integration_tests/fixtures/users.py @@ -49,9 +49,9 @@ def create_user_and_group( def cleanup(user, group): - security_manager.get_session.delete(user) - security_manager.get_session.delete(group) - security_manager.get_session.commit() + security_manager.session.delete(user) + security_manager.session.delete(group) + security_manager.session.commit() @pytest.fixture diff --git a/tests/integration_tests/queries/api_tests.py b/tests/integration_tests/queries/api_tests.py index c2a4715a440..db7b6b825e1 100644 --- a/tests/integration_tests/queries/api_tests.py +++ b/tests/integration_tests/queries/api_tests.py @@ -18,7 +18,6 @@ """Unit tests for Superset""" from datetime import datetime, timedelta -from unittest import mock import random import string @@ -453,21 +452,13 @@ class TestQueryApi(SupersetTestCase): db.session.delete(updated_query) db.session.commit() - @mock.patch("superset.sql_lab.cancel_query") - @mock.patch("superset.views.core.db.session") - def test_stop_query_not_found( - self, mock_superset_db_session, mock_sql_lab_cancel_query - ): + def test_stop_query_not_found(self): """ Handles stop query when the DB engine spec does not have a cancel query method (with invalid client_id). """ form_data = {"client_id": "foo2"} - query_mock = mock.Mock() - query_mock.return_value = None self.login(ADMIN_USERNAME) - mock_superset_db_session.query().filter_by().one_or_none = query_mock - mock_sql_lab_cancel_query.return_value = True rv = self.client.post( "/api/v1/query/stop", data=json.dumps(form_data), @@ -478,22 +469,25 @@ class TestQueryApi(SupersetTestCase): data = json.loads(rv.data.decode("utf-8")) assert data["message"] == "Query with client_id foo2 not found" - @mock.patch("superset.sql_lab.cancel_query") - @mock.patch("superset.views.core.db.session") - def test_stop_query(self, mock_superset_db_session, mock_sql_lab_cancel_query): + # @mock.patch("superset.sql_lab.cancel_query") + def test_stop_query(self): """ Handles stop query when the DB engine spec does not have a cancel query method. """ form_data = {"client_id": "foo"} - query_mock = mock.Mock() - query_mock.client_id = "foo" - query_mock.status = QueryStatus.RUNNING - self.login(ADMIN_USERNAME) - mock_superset_db_session.query().filter_by().one_or_none().return_value = ( - query_mock + admin = self.get_user("admin") + example_db = get_example_database() + query1 = self.insert_query( + example_db.id, + admin.id, + form_data["client_id"], + sql="SELECT col1, col2 from table1", + select_sql="SELECT col1, col2 from table1", + executed_sql="SELECT col1, col2 from table1 LIMIT 100", + changed_on=datetime.utcnow() - timedelta(days=1), ) - mock_sql_lab_cancel_query.return_value = True + self.login(ADMIN_USERNAME) rv = self.client.post( "/api/v1/query/stop", data=json.dumps(form_data), @@ -503,3 +497,5 @@ class TestQueryApi(SupersetTestCase): assert rv.status_code == 200 data = json.loads(rv.data.decode("utf-8")) assert data["result"] == "OK" + db.session.delete(query1) + db.session.commit() diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 88c0f5178dc..7aba9ffd341 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1959,13 +1959,11 @@ class TestSecurityManager(SupersetTestCase): class TestDatasources(SupersetTestCase): @patch("superset.security.SupersetSecurityManager.can_access_database") - @patch("superset.security.SupersetSecurityManager.get_session") - def test_get_user_datasources_admin( - self, mock_get_session, mock_can_access_database - ): + @patch("superset.security.SupersetSecurityManager.session") + def test_get_user_datasources_admin(self, mock_session, mock_can_access_database): Datasource = namedtuple("Datasource", ["database", "schema", "name"]) mock_can_access_database.return_value = True - mock_get_session.query.return_value.filter.return_value.all.return_value = [] + mock_session.query.return_value.filter.return_value.all.return_value = [] with mock.patch.object( SqlaTable, "get_all_datasources" @@ -1984,13 +1982,11 @@ class TestDatasources(SupersetTestCase): ] @patch("superset.security.SupersetSecurityManager.can_access_database") - @patch("superset.security.SupersetSecurityManager.get_session") - def test_get_user_datasources_gamma( - self, mock_get_session, mock_can_access_database - ): + @patch("superset.security.SupersetSecurityManager.session") + def test_get_user_datasources_gamma(self, mock_session, mock_can_access_database): Datasource = namedtuple("Datasource", ["database", "schema", "name"]) mock_can_access_database.return_value = False - mock_get_session.query.return_value.filter.return_value.all.return_value = [] + mock_session.query.return_value.filter.return_value.all.return_value = [] with mock.patch.object( SqlaTable, "get_all_datasources" @@ -2005,14 +2001,14 @@ class TestDatasources(SupersetTestCase): assert datasources == [] @patch("superset.security.SupersetSecurityManager.can_access_database") - @patch("superset.security.SupersetSecurityManager.get_session") + @patch("superset.security.SupersetSecurityManager.session") def test_get_user_datasources_gamma_with_schema( - self, mock_get_session, mock_can_access_database + self, mock_session, mock_can_access_database ): Datasource = namedtuple("Datasource", ["database", "schema", "name"]) mock_can_access_database.return_value = False - mock_get_session.query.return_value.filter.return_value.all.return_value = [ + mock_session.query.return_value.filter.return_value.all.return_value = [ Datasource("database1", "schema1", "table1"), Datasource("database1", "schema1", "table2"), ] diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index a33b0b7b20a..16a35cb5eb4 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -54,7 +54,7 @@ def get_session(mocker: MockerFixture) -> Callable[[], Session]: # patch session get_session = mocker.patch( - "superset.security.SupersetSecurityManager.get_session", + "superset.security.SupersetSecurityManager.session", ) get_session.return_value = in_memory_session # FAB calls get_session.get_bind() to get a handler to the engine diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py index 3712a9e96cc..c1d94c1b390 100644 --- a/tests/unit_tests/databases/api_test.py +++ b/tests/unit_tests/databases/api_test.py @@ -67,7 +67,7 @@ def test_filter_by_uuid( from superset.databases.api import DatabaseRestApi from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -135,7 +135,7 @@ def test_password_mask( from superset.databases.api import DatabaseRestApi from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -191,7 +191,7 @@ def test_database_connection( from superset.databases.api import DatabaseRestApi from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -355,7 +355,7 @@ def test_update_with_password_mask( from superset.databases.api import DatabaseRestApi from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -502,7 +502,7 @@ def test_delete_ssh_tunnel( from superset.databases.ssh_tunnel.models import SSHTunnel from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -580,7 +580,7 @@ def test_delete_ssh_tunnel_not_found( from superset.databases.ssh_tunnel.models import SSHTunnel from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -658,7 +658,7 @@ def test_apply_dynamic_database_filter( from superset.databases.api import DatabaseRestApi from superset.models.core import Database - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -753,7 +753,7 @@ def test_oauth2_happy_path( from superset.databases.api import DatabaseRestApi from superset.models.core import Database, DatabaseUserOAuth2Tokens - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -822,7 +822,7 @@ def test_oauth2_permissions( from superset.databases.api import DatabaseRestApi from superset.models.core import Database, DatabaseUserOAuth2Tokens - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member @@ -888,7 +888,7 @@ def test_oauth2_multiple_tokens( from superset.databases.api import DatabaseRestApi from superset.models.core import Database, DatabaseUserOAuth2Tokens - DatabaseRestApi.datamodel.session = session + DatabaseRestApi.datamodel._session = session # create table for databases Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py index 79d6afeda7e..db34a8edb98 100644 --- a/tests/unit_tests/security/manager_test.py +++ b/tests/unit_tests/security/manager_test.py @@ -450,7 +450,7 @@ def test_raise_for_access_chart_for_datasource_permission( when the user does not have access to the chart datasource """ sm = SupersetSecurityManager(appbuilder) - session = sm.get_session + session = sm.session engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member @@ -510,7 +510,7 @@ def test_raise_for_access_chart_on_admin( from superset.utils.core import override_user sm = SupersetSecurityManager(appbuilder) - session = sm.get_session + session = sm.session engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member @@ -547,7 +547,7 @@ def test_raise_for_access_chart_owner( when the user does not have access to the chart datasource """ sm = SupersetSecurityManager(appbuilder) - session = sm.get_session + session = sm.session engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member