You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "sfirke (via GitHub)" <gi...@apache.org> on 2024/09/24 20:44:20 UTC

[PR] feat(auth): when user is not logged in, failure to access a dashboard will redirect to login screen [superset]

sfirke opened a new pull request, #30380:
URL: https://github.com/apache/superset/pull/30380

   ### SUMMARY
   If a viewer is not logged in, or they are the public/anonymous user, and they click a link that takes them to a Superset dashboard that is _not_ public, they are currently told "You don't have access" and sent to the list of dashboards, where they will see only public ones listed.  This is often misleading: in most cases, the problem is not that they don't have access -- they just need to log in.
   
   After the PR, such cases are routed to the login screen, and after a successful login the users are sent back to the dashboard they were trying to access.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   **BEFORE**
   https://github.com/user-attachments/assets/053bbf0e-a2f5-4e2b-8498-47694ff213a6
   
   **AFTER**
   https://github.com/user-attachments/assets/fc78d404-0d63-4512-9ec4-c1e813b7a66b (ignore that it says "current" at the beginning)
   
   ### TESTING INSTRUCTIONS
   Create a dashboard with restricted access, try to view it while not logged in.
   
   ### ADDITIONAL INFORMATION
   Implements this feature request: https://github.com/apache/superset/discussions/22190 
   Replaces this stale PR: https://github.com/apache/superset/pull/23280
   
   I have the DASHBOARD_RBAC flag enabled as well as the Public role in use.  My code alterations are minimal so I don't think it will negatively affect deployments that differ from mine, but it would be good to have someone check.
   
   I'm not sure how to write tests for this but am open to it if someone can advise.


-- 
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

Re: [PR] feat(auth): when user is not logged in, failure to access a dashboard should redirect to login screen [superset]

Posted by "Ishankoradia (via GitHub)" <gi...@apache.org>.

Ishankoradia commented on PR #30380:
URL: https://github.com/apache/superset/pull/30380#issuecomment-3387807607

   @sfirke i have a public dashboard and its redirecting users to `/login` page even when the dashboard is tagged with a `Public` role.
   
   Does this PR handle this case too ?
   
   I am using oauth with superset 4.1.1


-- 
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

Re: [PR] feat(auth): when user is not logged in, failure to access a dashboard should redirect to login screen [superset]

Posted by "villebro (via GitHub)" <gi...@apache.org>.

villebro commented on code in PR #30380:
URL: https://github.com/apache/superset/pull/30380#discussion_r1775760734


##########
superset/views/core.py:
##########
@@ -792,9 +792,16 @@ def dashboard(
         try:
             dashboard.raise_for_access()
         except SupersetSecurityException as ex:
+            # anonymous users should get the login screen, others should go to dashboard list
+            if g.user is None or g.user.is_anonymous:
+                redirect_url = f"{appbuilder.get_url_for_login}?next={request.url}"

Review Comment:
   Great, nice finishing touch to this! 👍 



-- 
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

Re: [PR] feat(auth): when user is not logged in, failure to access a dashboard should redirect to login screen [superset]

Posted by "Ishankoradia (via GitHub)" <gi...@apache.org>.

Ishankoradia commented on PR #30380:
URL: https://github.com/apache/superset/pull/30380#issuecomment-3393141735

   Ahh gotcha !!! thank you so much @sfirke 


-- 
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