Re: [I] Permission in PUBLIC role removes API permission from ADMIN role. [superset]

["FynnMazurkiewicz (via GitHub)" <gi...@apache.org>]

FynnMazurkiewicz commented on issue #24837:
URL: https://github.com/apache/superset/issues/24837#issuecomment-2501991590

   TLDR: **Patch available!**
   
   This bug is still present in version 4.0.1. The workaround suggested by @lbuchli did not work for me either.
   
   **Bug summary**
   Whenever you set the "can_read" permission of "Dashboard" as part of the public permissions, the API of that endpoint will start ignoring the JWT entirely and always assume that the public user makes the request. In fact, this bug not only occurs for this endpoint, but for all endpoints. As soon as any item is part of the public read permissions, the JWT is ignored. This breaks the JWT authorization, as the API ignores any given token and always assumes public user permissions. For example, the dashboard endpoint will always return only dashboards available to the public user, even if you supply a JWT Bearer token of a user that has access to more dashboards (such as the admin role).
   
   **Root cause**
   I have completed a root cause analysis of this bug. It is caused by an issue in FlaskAppBuilder, the underlying framework also built by the lovely @dpgaspar. [In the security decorator `_protect()`](https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/security/decorators.py#L98), that is called to set the authorization context on any endpoint, there is a special condition set for items that are considered public (meaning the public permissions can read them). I'll quote the relevant parts for discussion:
   
   ```python
   flask_appbuilder/security/decorators.py:98
   
   # Check if the resource is public
   if current_app.appbuilder.sm.is_item_public(permission_str, class_permission_name):
       return f(self, *args, **kwargs)
   
   # if no browser login then verify JWT
   if not (self.allow_browser_login or allow_browser_login):
       verify_jwt_in_request()
   
   # Verify resource access
   if current_app.appbuilder.sm.has_access(permission_str, class_permission_name):
       return f(self, *args, **kwargs)
   ```
   In this decorator, it is first checked whether the item is public and if it is, the endpoint function is immediately called - with no further JWT token handling. You can see the function call to `verify_jwt_in_request()` is only called when the public condition check fails.
   
   I am not sure if this can be considered a bug in FAB itself, so I won't open a bug issue there. I do see however, that this might cause issues a lot and I highly question this order is correct. @dpgaspar What do you think?
   
   **Patch**
   Until there is a proper fix implemented by @dpgaspar or any other maintainer, its possible to hack around this by attempting a JWT login by overwriting `is_item_public` in the SecurityManager and adding a call to `verify_jwt_in_request` there.
   ```python
   class MySecurityManager(SupersetSecurityManager):
       ...
       def is_item_public(self, permission_name, view_name):
           verify_jwt_in_request(optional=True) # Attempt to parse any existing JWT and fail silently
           return super().is_item_public(permission_name, view_name)
       ...
   ```
   Thanks to my employer https://www.dksr.city/en/ for donating A LOT of time to work on this and being able to donate my findings back to the open source community.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org