dosubot[bot] commented on code in PR #39197:
URL: https://github.com/apache/superset/pull/39197#discussion_r3230576152
##########
superset/security/manager.py:
##########
@@ -966,6 +966,21 @@ def user_view_menu_names(self, permission_name: str) ->
set[str]:
.join(self.role_model)
)
Review Comment:
<!-- Answer -->
Yes, I agree with the suggestion. The `base_query` (which constructs a
multi-join query object) is built unconditionally on lines 765–771
[[1]](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset/security/manager.py#L765-L771),
but in the guest-user branch when `role_ids` is empty, the method returns
`set()` immediately — meaning that query construction was wasted work.
Moving the `is_guest_user()` check (and the early return for empty
`role_ids`) **before** `base_query` avoids that unnecessary overhead. Here's a
patch:
```python
def user_view_menu_names(self, permission_name: str) -> set[str]:
# Guest users (embedded dashboards) have is_anonymous=False but no
# database identity, so querying by user_id returns nothing. Instead,
# resolve permissions directly from the roles attached to the guest
# token (typically the Public role).
if self.is_guest_user():
role_ids = [
role.id for role in g.user.roles if role and role.id is not None
]
if not role_ids:
return set()
base_query = (
self.get_session.query(self.viewmenu_model.name)
.join(self.permissionview_model)
.join(self.permission_model)
.join(assoc_permissionview_role)
.join(self.role_model)
)
view_menu_names = (
base_query.filter(self.role_model.id.in_(role_ids)).filter(
self.permission_model.name == permission_name
)
).all()
return {s.name for s in view_menu_names}
base_query = (
self.get_session.query(self.viewmenu_model.name)
.join(self.permissionview_model)
.join(self.permission_model)
.join(assoc_permissionview_role)
.join(self.role_model)
)
if not g.user.is_anonymous:
user_id = get_user_id()
user_roles_filter = or_(
exists().where(
(assoc_user_role.c.user_id == user_id)
& (assoc_user_role.c.role_id == self.role_model.id)
& (self.permission_model.name == permission_name)
),
exists().where(
(assoc_user_group.c.user_id == user_id)
& (assoc_user_group.c.group_id == self.group_model.id)
& (assoc_group_role.c.group_id == self.group_model.id)
& (assoc_group_role.c.role_id == self.role_model.id)
& (self.permission_model.name == permission_name)
),
)
view_menu_names = base_query.filter(user_roles_filter).all()
return {s.name for s in view_menu_names}
# Properly treat anonymous user
if public_role := self.get_public_role():
view_menu_names = (
base_query.filter(self.role_model.id == public_role.id).filter(
self.permission_model.name == permission_name
)
).all()
return {s.name for s in view_menu_names}
return set()
```
This way, when `is_guest_user()` is `True` and `role_ids` is empty, the
method returns immediately without ever constructing the `base_query`. When the
guest user *does* have roles, `base_query` is constructed only within that
branch. The non-guest paths remain unchanged.
<!-- Dosu Comment Footer -->
*To reply, just mention
[@dosu](https://go.dosu.dev/dosubot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=reply-with-mention&utm_term=apache%2Fsuperset).*
---
Docs are dead. Just use
[Dosu](https://dosu.dev?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-tagline&utm_term=apache%2Fsuperset).
[](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-feedback&utm_term=apache%2Fsuperset&message_id=8c1d74e1-ce5b-4710-995f-ad666690bb75)
[](https://github.dosu.com/apache/superset?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-ask-repo&utm_term=apache%2Fsuperset)
[](https://app.dosu.dev/signup?referrer=openSource&source=github-footer&utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-share-team&utm_term=apache%2Fsuperset)
--
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]
