codeant-ai-for-open-source[bot] commented on code in PR #36742:
URL: https://github.com/apache/superset/pull/36742#discussion_r2631839194
##########
superset/security/manager.py:
##########
@@ -143,11 +144,35 @@ def pre_delete(self, item: Model) -> None:
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()
Review Comment:
**Suggestion:** The exclusion list is fetched by calling
`SupersetSecurityManager.get_exclude_users_from_lists()` on the base class,
which bypasses any overrides in a custom security manager subclass, so
instances that customize this hook will not have their logic respected by the
`/api/v1/security/users/` filter. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
current_app.appbuilder.sm.get_exclude_users_from_lists()
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The code invokes the base class method directly, bypassing any overrides
provided by the actual configured security manager instance. That means custom
security manager subclasses won't have their logic respected. Calling the
configured instance (current_app.appbuilder.sm.get_exclude_users_from_lists())
ensures subclass overrides run and fixes a real behavior bug.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/manager.py
**Line:** 160:160
**Comment:**
*Logic Error: The exclusion list is fetched by calling
`SupersetSecurityManager.get_exclude_users_from_lists()` on the base class,
which bypasses any overrides in a custom security manager subclass, so
instances that customize this hook will not have their logic respected by the
`/api/v1/security/users/` filter.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
