dosubot[bot] commented on code in PR #39197:
URL: https://github.com/apache/superset/pull/39197#discussion_r3230594156
##########
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 -->
Unfortunately, I'm not able to provide a GitHub suggestion block that you
can directly commit — I can only provide code snippets in my responses.
GitHub's committable suggestion format requires me to be commenting directly on
specific unchanged lines in the diff, and since this review thread is outdated
(the referenced code has changed), a committable suggestion wouldn't work here
anyway.
That said, here's the recommended change for the `user_view_menu_names`
method. The optimization is to move the `is_guest_user()` check **before** the
`base_query` construction so the empty-`role_ids` fast path avoids building the
query entirely:
```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()
```
The PR author would need to apply this change manually to their branch. The
key difference is that the guest-user check now happens first, and `base_query`
is only constructed within each branch that actually needs it — avoiding
unnecessary query construction when the guest user has no valid role IDs.
<!-- 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=8d50d8c9-932c-4962-aed7-f0f6cbfcebc3)
[](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]
