ktmud commented on code in PR #21515:
URL: https://github.com/apache/superset/pull/21515#discussion_r978940070
##########
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:
Also, I think most enterprise users would probably have to have their custom
security manager anyway. I understand a config value is easier to set up and
would do the work for most people; re-reploying Superset every time a new
service account needs to be added also doesn't sound like a big deal since it
is likely not frequent. But sooner or later, we'd need to migrate current FAB
views for user profiles to React and add proper REST API for suers and roles,
too. When that API is added, it'd be only natural to configure things like user
types in the Edit User page.
A robust and scalable user layer is critical to Superset's success among
enterprise users so it'd be prudent to be more future-proof here. Superset has
long suffered from not having a built-in user and access control layer, which
IMO had made the access control in Superset more complex than it needs to be
and hindered the advance of features like RBAC. Current overrides on top of FAB
are hacky and error-prone. Dynamic string-based permissions also feel
unreliable. Not the scope of this PR, but I really think there should be a
discussion on what the user and auth models in Superset would look like in the
future.
I'm not opposed to merging this PR as it since it gets the job done, just
thought in general this area needs a little bit more considerations.
--
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]
