ktmud commented on code in PR #21515:
URL: https://github.com/apache/superset/pull/21515#discussion_r977793428
##########
superset/security/manager.py:
##########
@@ -1702,6 +1702,22 @@ def on_permission_view_after_delete(
:param target: The mapped instance being persisted
"""
+ @staticmethod
+ def get_exclude_users_from_lists() -> List[str]:
+ """
+ Override to dynamically identify a list of usernames to exclude from
+ all UI dropdown lists, owners, created_by filters etc...
+
+ It will exclude all users from the all endpoints of the form
+ ``/api/v1/<modelview>/related/<column>``
+
+ Optionally you can also exclude them using the
`EXCLUDE_USERS_FROM_LISTS`
+ config setting.
+
+ :return: A list of usernames
+ """
+ return []
Review Comment:
Should this be a list of ids instead of usernames? Internal filtering should
preferably use IDs as they are more determinant.
##########
superset/views/filters.py:
##########
@@ -48,3 +52,31 @@ def apply(self, query: Query, value: Optional[Any]) -> Query:
user_model.username.ilike(like_value),
)
)
+
+
+class BaseFilterRelatedUsers(BaseFilter): # pylint:
disable=too-few-public-methods
+
+ """
+ Filter to apply on related users. Will exclude users in
EXCLUDE_USERS_FROM_LISTS
+
+ Use in the api by adding something like:
+ ```
+ filter_rel_fields = {
+ "owners": [["id", BaseFilterRelatedUsers, lambda: []]],
+ "created_by": [["id", BaseFilterRelatedUsers, lambda: []]],
+ }
+ ```
+ """
+
+ name = lazy_gettext("username")
+ arg_name = "username"
+
+ def apply(self, query: Query, value: Optional[Any]) -> Query:
+ user_model = security_manager.user_model
+ exclude_users = (
+ security_manager.get_exclude_users_from_lists()
+ if current_app.config["EXCLUDE_USERS_FROM_LISTS"] is None
+ else current_app.config["EXCLUDE_USERS_FROM_LISTS"]
Review Comment:
I'm still not sure if we need two config values. I understand setting a
hard-coded `EXCLUDE_USERS_FROM_LISTS` is easy but so is setting it in a custom
security manager.
Even if we do want to keep `EXCLUDE_USERS_FROM_LISTS`, you can use it in the
default `get_exclude_users_from_lists` so if users set
`get_exclude_users_from_lists`, `EXCLUDE_USERS_FROM_LISTS` will be ignored.
This prevents them from changing both configs and bite themselves down the road.
--
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]
