apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62.8k stars 13.87k forks source link

feat(auth): when user is not logged in, failure to access a dashboard should redirect to login screen #30380

Closed sfirke closed 1 month ago

sfirke commented 1 month ago

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

note I've changed the error message since recording this

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.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.90%. Comparing base (76d897e) to head (d8ad102). Report is 774 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #30380 +/- ## =========================================== + Coverage 60.48% 83.90% +23.41% =========================================== Files 1931 533 -1398 Lines 76236 38524 -37712 Branches 8568 0 -8568 =========================================== - Hits 46114 32322 -13792 + Misses 28017 6202 -21815 + Partials 2105 0 -2105 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [hive](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.99% <0.00%> (-0.18%)` | :arrow_down: | | [javascript](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [mysql](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.88% <100.00%> (?)` | | | [postgres](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.95% <100.00%> (?)` | | | [presto](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.49% <0.00%> (-0.32%)` | :arrow_down: | | [python](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.90% <100.00%> (+20.41%)` | :arrow_up: | | [sqlite](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.40% <100.00%> (?)` | | | [unit](https://app.codecov.io/gh/apache/superset/pull/30380/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.64% <0.00%> (+3.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.