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 2025/12/18 16:49:36 UTC

(superset) branch fix/exclude-users-from-security-api created (now f28a6afc79)

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

beto pushed a change to branch fix/exclude-users-from-security-api
in repository https://gitbox.apache.org/repos/asf/superset.git


      at f28a6afc79 fix(security): apply EXCLUDE_USERS_FROM_LISTS to /api/v1/security/users/

This branch includes the following new commits:

     new f28a6afc79 fix(security): apply EXCLUDE_USERS_FROM_LISTS to /api/v1/security/users/

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.



(superset) 01/01: fix(security): apply EXCLUDE_USERS_FROM_LISTS to /api/v1/security/users/

Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

beto pushed a commit to branch fix/exclude-users-from-security-api
in repository https://gitbox.apache.org/repos/asf/superset.git

commit f28a6afc7967088bfc36b358be2e2ee0a5086c33
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Dec 18 11:38:26 2025 -0500

    fix(security): apply EXCLUDE_USERS_FROM_LISTS to /api/v1/security/users/
    
    The new user listing API introduced in #32882 did not respect the
    EXCLUDE_USERS_FROM_LISTS configuration, causing a regression where
    excluded users appeared in the API response.
    
    This adds an ExcludeUsersFilter to SupersetUserApi that uses the same
    exclusion logic as BaseFilterRelatedUsers for consistency.
    
    🤖 Generated with [Claude Code](https://claude.com/claude-code)
    
    Co-Authored-By: Claude Opus 4.5 <no...@anthropic.com>
---
 superset/security/manager.py                       |  27 +++-
 .../security/exclude_users_filter_test.py          | 151 +++++++++++++++++++++
 2 files changed, 177 insertions(+), 1 deletion(-)

diff --git a/superset/security/manager.py b/superset/security/manager.py
index 8490c049e3..048edc9bac 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -25,6 +25,7 @@ from typing import Any, Callable, cast, NamedTuple, Optional, TYPE_CHECKING
 
 from flask import current_app, Flask, g, Request
 from flask_appbuilder import Model
+from flask_appbuilder.models.filters import BaseFilter
 from flask_appbuilder.security.sqla.apis import RoleApi, UserApi
 from flask_appbuilder.security.sqla.manager import SecurityManager
 from flask_appbuilder.security.sqla.models import (
@@ -143,11 +144,35 @@ class SupersetRoleApi(RoleApi):
         item.permissions = []
 
 
+class ExcludeUsersFilter(BaseFilter):  # pylint: disable=too-few-public-methods
+    """
+    Filter to exclude users from listings based on EXCLUDE_USERS_FROM_LISTS config.
+
+    This filter is designed for use as a base_filter on user listing APIs.
+    It uses the same exclusion logic as BaseFilterRelatedUsers for consistency.
+    """
+
+    name = _("username")
+    arg_name = "username"
+
+    def apply(self, query: SqlaQuery, value: Any) -> SqlaQuery:
+        exclude_users = (
+            SupersetSecurityManager.get_exclude_users_from_lists()
+            if current_app.config["EXCLUDE_USERS_FROM_LISTS"] is None
+            else current_app.config["EXCLUDE_USERS_FROM_LISTS"]
+        )
+        if exclude_users:
+            return query.filter(User.username.not_in(exclude_users))
+
+        return query
+
+
 class SupersetUserApi(UserApi):
     """
-    Overriding the UserApi to be able to delete users
+    Overriding the UserApi to be able to delete users and filter excluded users
     """
 
+    base_filters = [["username", ExcludeUsersFilter, lambda: []]]
     search_columns = [
         "id",
         "roles",
diff --git a/tests/unit_tests/security/exclude_users_filter_test.py b/tests/unit_tests/security/exclude_users_filter_test.py
new file mode 100644
index 0000000000..6f521004a6
--- /dev/null
+++ b/tests/unit_tests/security/exclude_users_filter_test.py
@@ -0,0 +1,151 @@
+# 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 flask_appbuilder.models.sqla.interface import SQLAInterface
+from flask_appbuilder.security.sqla.models import User
+from pytest_mock import MockerFixture
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
+
+from superset.security.manager import ExcludeUsersFilter, SupersetSecurityManager
+
+
+def test_exclude_users_filter_no_exclusions(mocker: MockerFixture) -> None:
+    """
+    Test ExcludeUsersFilter when no users are configured for exclusion.
+
+    The query should be returned unmodified.
+    """
+    mocker.patch(
+        "superset.security.manager.current_app.config",
+        {"EXCLUDE_USERS_FROM_LISTS": None},
+    )
+    mocker.patch.object(
+        SupersetSecurityManager,
+        "get_exclude_users_from_lists",
+        return_value=[],
+    )
+
+    engine = create_engine("sqlite://")
+    Session = sessionmaker(bind=engine)  # noqa: N806
+    session = Session()
+    query = session.query(User)
+
+    filter_ = ExcludeUsersFilter("username", SQLAInterface(User))
+    filtered_query = filter_.apply(query, None)
+
+    assert filtered_query == query
+
+
+def test_exclude_users_filter_with_config(mocker: MockerFixture) -> None:
+    """
+    Test ExcludeUsersFilter when EXCLUDE_USERS_FROM_LISTS config is set.
+    """
+    mocker.patch(
+        "superset.security.manager.current_app.config",
+        {"EXCLUDE_USERS_FROM_LISTS": ["service_account", "automation_user"]},
+    )
+
+    engine = create_engine("sqlite://")
+    Session = sessionmaker(bind=engine)  # noqa: N806
+    session = Session()
+    query = session.query(User)
+
+    filter_ = ExcludeUsersFilter("username", SQLAInterface(User))
+    filtered_query = filter_.apply(query, None)
+
+    compiled_query = filtered_query.statement.compile(
+        engine,
+        compile_kwargs={"literal_binds": True},
+    )
+    query_str = str(compiled_query)
+
+    assert "ab_user.username NOT IN" in query_str
+    assert "service_account" in query_str
+    assert "automation_user" in query_str
+
+
+def test_exclude_users_filter_with_security_manager(mocker: MockerFixture) -> None:
+    """
+    Test ExcludeUsersFilter when EXCLUDE_USERS_FROM_LISTS is None
+    and security manager provides the exclusion list.
+    """
+    mocker.patch(
+        "superset.security.manager.current_app.config",
+        {"EXCLUDE_USERS_FROM_LISTS": None},
+    )
+    mocker.patch.object(
+        SupersetSecurityManager,
+        "get_exclude_users_from_lists",
+        return_value=["gamma", "guest"],
+    )
+
+    engine = create_engine("sqlite://")
+    Session = sessionmaker(bind=engine)  # noqa: N806
+    session = Session()
+    query = session.query(User)
+
+    filter_ = ExcludeUsersFilter("username", SQLAInterface(User))
+    filtered_query = filter_.apply(query, None)
+
+    compiled_query = filtered_query.statement.compile(
+        engine,
+        compile_kwargs={"literal_binds": True},
+    )
+    query_str = str(compiled_query)
+
+    assert "ab_user.username NOT IN" in query_str
+    assert "gamma" in query_str
+    assert "guest" in query_str
+
+
+def test_exclude_users_filter_config_takes_precedence(mocker: MockerFixture) -> None:
+    """
+    Test that EXCLUDE_USERS_FROM_LISTS config takes precedence over
+    security manager's get_exclude_users_from_lists method.
+    """
+    mocker.patch(
+        "superset.security.manager.current_app.config",
+        {"EXCLUDE_USERS_FROM_LISTS": ["config_user"]},
+    )
+    # This should NOT be called when config is set
+    get_exclude_mock = mocker.patch.object(
+        SupersetSecurityManager,
+        "get_exclude_users_from_lists",
+        return_value=["sm_user"],
+    )
+
+    engine = create_engine("sqlite://")
+    Session = sessionmaker(bind=engine)  # noqa: N806
+    session = Session()
+    query = session.query(User)
+
+    filter_ = ExcludeUsersFilter("username", SQLAInterface(User))
+    filtered_query = filter_.apply(query, None)
+
+    compiled_query = filtered_query.statement.compile(
+        engine,
+        compile_kwargs={"literal_binds": True},
+    )
+    query_str = str(compiled_query)
+
+    # Config user should be excluded
+    assert "config_user" in query_str
+    # SM user should NOT be in the query (config takes precedence)
+    assert "sm_user" not in query_str
+    # get_exclude_users_from_lists should not be called when config is set
+    get_exclude_mock.assert_not_called()