You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2024/05/29 22:08:28 UTC
(superset) branch impersonate_email_prefix updated (f1a2c266da -> 415bbf76a1)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch impersonate_email_prefix
in repository https://gitbox.apache.org/repos/asf/superset.git
discard f1a2c266da feat: impersonate with email prefix
new 415bbf76a1 feat: impersonate with email prefix
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (f1a2c266da)
\
N -- N -- N refs/heads/impersonate_email_prefix (415bbf76a1)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
RESOURCES/FEATURE_FLAGS.md | 1 +
superset/config.py | 18 ++++++++++--------
superset/models/core.py | 4 ++--
tests/unit_tests/models/core_test.py | 3 ++-
4 files changed, 15 insertions(+), 11 deletions(-)
(superset) 01/01: feat: impersonate with email prefix
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch impersonate_email_prefix
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 415bbf76a140fa25224eb31710f8aaff0a7594f3
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed May 29 17:26:37 2024 -0400
feat: impersonate with email prefix
---
RESOURCES/FEATURE_FLAGS.md | 1 +
superset/config.py | 18 +++---
superset/db_engine_specs/trino.py | 6 +-
superset/models/core.py | 15 +++--
tests/unit_tests/db_engine_specs/test_base.py | 1 +
tests/unit_tests/models/core_test.py | 89 +++++++++++++++++++++++++++
6 files changed, 116 insertions(+), 14 deletions(-)
diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md
index a8796084fb..950e652efa 100644
--- a/RESOURCES/FEATURE_FLAGS.md
+++ b/RESOURCES/FEATURE_FLAGS.md
@@ -50,6 +50,7 @@ These features are **finished** but currently being tested. They are usable, but
- ESTIMATE_QUERY_COST
- GLOBAL_ASYNC_QUERIES [(docs)](https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries)
- HORIZONTAL_FILTER_BAR
+- IMPERSONATE_WITH_EMAIL_PREFIX
- PLAYWRIGHT_REPORTS_AND_THUMBNAILS
- RLS_IN_SQLLAB
- SSH_TUNNELING [(docs)](https://superset.apache.org/docs/configuration/setup-ssh-tunneling)
diff --git a/superset/config.py b/superset/config.py
index aa8178d086..8d9ca66d8a 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -461,6 +461,8 @@ DEFAULT_FEATURE_FLAGS: dict[str, bool] = {
# Apply RLS rules to SQL Lab queries. This requires parsing and manipulating the
# query, and might break queries and/or allow users to bypass RLS. Use with care!
"RLS_IN_SQLLAB": False,
+ # When impersonating a user, use the email prefix instead of the username
+ "IMPERSONATE_WITH_EMAIL_PREFIX": False,
# Enable caching per impersonation key (e.g username) in a datasource where user
# impersonation is enabled
"CACHE_IMPERSONATION": False,
@@ -643,8 +645,8 @@ THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER, ExecutorType.SELENIUM]
# `THUMBNAIL_EXECUTE_AS`; the executor is only equal to the currently logged in
# user if the executor type is equal to `ExecutorType.CURRENT_USER`)
# and return the final digest string:
-THUMBNAIL_DASHBOARD_DIGEST_FUNC: (
- None | (Callable[[Dashboard, ExecutorType, str], str])
+THUMBNAIL_DASHBOARD_DIGEST_FUNC: None | (
+ Callable[[Dashboard, ExecutorType, str], str]
) = None
THUMBNAIL_CHART_DIGEST_FUNC: Callable[[Slice, ExecutorType, str], str] | None = None
@@ -1038,8 +1040,8 @@ SQLLAB_CTAS_NO_LIMIT = False
# else:
# return f'tmp_{schema}'
# Function accepts database object, user object, schema name and sql that will be run.
-SQLLAB_CTAS_SCHEMA_NAME_FUNC: (
- None | (Callable[[Database, models.User, str, str], str])
+SQLLAB_CTAS_SCHEMA_NAME_FUNC: None | (
+ Callable[[Database, models.User, str, str], str]
) = None
# If enabled, it can be used to store the results of long-running queries
@@ -1083,10 +1085,10 @@ UPLOADED_CSV_HIVE_NAMESPACE: str | None = None
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[[Database, models.User], list[str]] = ( # noqa: E731
- lambda database, user: [UPLOADED_CSV_HIVE_NAMESPACE]
- if UPLOADED_CSV_HIVE_NAMESPACE
- else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[[Database, models.User], list[str]] = (
+ lambda database, user: ( # noqa: E731
+ [UPLOADED_CSV_HIVE_NAMESPACE] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ )
)
# Values that should be treated as nulls for the csv uploads.
diff --git a/superset/db_engine_specs/trino.py b/superset/db_engine_specs/trino.py
index 600f236b48..51b517f865 100644
--- a/superset/db_engine_specs/trino.py
+++ b/superset/db_engine_specs/trino.py
@@ -233,7 +233,11 @@ class TrinoEngineSpec(PrestoBaseEngineSpec):
execute_thread = threading.Thread(
target=_execute,
- args=(execute_result, execute_event, current_app._get_current_object()), # pylint: disable=protected-access
+ args=(
+ execute_result,
+ execute_event,
+ current_app._get_current_object(),
+ ), # pylint: disable=protected-access
)
execute_thread.start()
diff --git a/superset/models/core.py b/superset/models/core.py
index b933c1694f..33c35961d2 100755
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -59,7 +59,7 @@ from sqlalchemy.pool import NullPool
from sqlalchemy.schema import UniqueConstraint
from sqlalchemy.sql import ColumnElement, expression, Select
-from superset import app, db_engine_specs
+from superset import app, db_engine_specs, is_feature_enabled
from superset.commands.database.exceptions import DatabaseInvalidError
from superset.constants import LRU_CACHE_MAX_SIZE, PASSWORD_MASK
from superset.databases.utils import make_url_safe
@@ -116,7 +116,9 @@ class ConfigurationMethod(StrEnum):
DYNAMIC_FORM = "dynamic_form"
-class Database(Model, AuditMixinNullable, ImportExportMixin): # pylint: disable=too-many-public-methods
+class Database(
+ Model, AuditMixinNullable, ImportExportMixin
+): # pylint: disable=too-many-public-methods
"""An ORM object that stores Database related information"""
__tablename__ = "dbs"
@@ -390,9 +392,7 @@ class Database(Model, AuditMixinNullable, ImportExportMixin): # pylint: disable
return (
username
if (username := get_username())
- else object_url.username
- if self.impersonate_user
- else None
+ else object_url.username if self.impersonate_user else None
)
@contextmanager
@@ -477,6 +477,11 @@ class Database(Model, AuditMixinNullable, ImportExportMixin): # pylint: disable
)
effective_username = self.get_effective_user(sqlalchemy_url)
+ if effective_username and is_feature_enabled("IMPERSONATE_WITH_EMAIL_PREFIX"):
+ user = security_manager.find_user(username=effective_username)
+ if user and user.email:
+ effective_username = user.email.split("@")[0]
+
oauth2_config = self.get_oauth2_config()
access_token = (
get_oauth2_access_token(
diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py
index 0950dcb439..14ccdda851 100644
--- a/tests/unit_tests/db_engine_specs/test_base.py
+++ b/tests/unit_tests/db_engine_specs/test_base.py
@@ -19,6 +19,7 @@
from __future__ import annotations
+import json
from textwrap import dedent
from typing import Any
diff --git a/tests/unit_tests/models/core_test.py b/tests/unit_tests/models/core_test.py
index e653eee716..2004ff482f 100644
--- a/tests/unit_tests/models/core_test.py
+++ b/tests/unit_tests/models/core_test.py
@@ -21,11 +21,13 @@ from datetime import datetime
import pytest
from pytest_mock import MockFixture
from sqlalchemy.engine.reflection import Inspector
+from sqlalchemy.engine.url import make_url
from superset.connectors.sqla.models import SqlaTable, TableColumn
from superset.models.core import Database
from superset.sql_parse import Table
from superset.utils import json
+from tests.unit_tests.conftest import with_feature_flags
def test_get_metrics(mocker: MockFixture) -> None:
@@ -289,3 +291,90 @@ def test_get_all_catalog_names(mocker: MockFixture) -> None:
assert database.get_all_catalog_names(force=True) == {"examples", "other"}
get_inspector.assert_called_with(ssh_tunnel=None)
+
+
+def test_get_sqla_engine(mocker: MockFixture) -> None:
+ """
+ Test `_get_sqla_engine`.
+ """
+ from superset.models.core import Database
+
+ user = mocker.MagicMock()
+ user.email = "alice.doe@example.org"
+ mocker.patch(
+ "superset.models.core.security_manager.find_user",
+ return_value=user,
+ )
+ mocker.patch("superset.models.core.get_username", return_value="alice")
+
+ create_engine = mocker.patch("superset.models.core.create_engine")
+
+ database = Database(
+ database_name="my_db",
+ sqlalchemy_uri="trino://",
+ )
+ database._get_sqla_engine(nullpool=False)
+
+ create_engine.assert_called_with(
+ make_url("trino:///"),
+ connect_args={"source": "Apache Superset"},
+ )
+
+
+def test_get_sqla_engine_user_impersonation(mocker: MockFixture) -> None:
+ """
+ Test user impersonation in `_get_sqla_engine`.
+ """
+ from superset.models.core import Database
+
+ user = mocker.MagicMock()
+ user.email = "alice.doe@example.org"
+ mocker.patch(
+ "superset.models.core.security_manager.find_user",
+ return_value=user,
+ )
+ mocker.patch("superset.models.core.get_username", return_value="alice")
+
+ create_engine = mocker.patch("superset.models.core.create_engine")
+
+ database = Database(
+ database_name="my_db",
+ sqlalchemy_uri="trino://",
+ impersonate_user=True,
+ )
+ database._get_sqla_engine(nullpool=False)
+
+ create_engine.assert_called_with(
+ make_url("trino:///"),
+ connect_args={"user": "alice", "source": "Apache Superset"},
+ )
+
+
+@with_feature_flags(IMPERSONATE_WITH_EMAIL_PREFIX=True)
+def test_get_sqla_engine_user_impersonation_email(mocker: MockFixture) -> None:
+ """
+ Test user impersonation in `_get_sqla_engine` with `username_from_email`.
+ """
+ from superset.models.core import Database
+
+ user = mocker.MagicMock()
+ user.email = "alice.doe@example.org"
+ mocker.patch(
+ "superset.models.core.security_manager.find_user",
+ return_value=user,
+ )
+ mocker.patch("superset.models.core.get_username", return_value="alice")
+
+ create_engine = mocker.patch("superset.models.core.create_engine")
+
+ database = Database(
+ database_name="my_db",
+ sqlalchemy_uri="trino://",
+ impersonate_user=True,
+ )
+ database._get_sqla_engine(nullpool=False)
+
+ create_engine.assert_called_with(
+ make_url("trino:///"),
+ connect_args={"user": "alice.doe", "source": "Apache Superset"},
+ )