fix: Redirect to login on unauthorised access for Dashboard.

[dheeraj281]

SUMMARY

As per Feature request #22190 , When a user is trying to access the dashboard link it says you don't have access to this Dashboard instead of redirecting user to the login page. This was happening due to issue with dashboard rbac feature.

As part of fix, when dashboard rbac feature is enabled. and DashboardAccessDeniedError is raised after all the checks, it will check if user is logged in or not in the callback function and it will redirect user to login accordingly.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• [x] Has associated issue: #22190
• [ ] Required feature flags:
• [ ] Changes UI
• [ ] Includes DB Migration (follow approval process in SIP-59)
  • [ ] Migration is atomic, supports rollback & is backwards-compatible
  • [ ] Confirm DB migration upgrade and downgrade tested
  • [ ] Runtime estimates and downtime expectations provided
• [ ] Introduces new feature or API
• [ ] Removes existing feature or API

[sfirke]

My testing, applying this patch to 2.1.0rc1 and with DASHBOARD_RBAC enabled:

• Seems to work if I delete my 'Public' user
• When I leave my Public user enabled, which I need, this will not work

Talking with @dheeraj281 in Slack it sounds like the AccessDeniedError exception in the case with the Public role configured is getting raised elsewhere. Anyone have an idea how to get this behavior for that case, too?

If that should be addressed separately I can open a different issue for that.

[codecov[bot]]

Codecov Report

Merging #23280 (a8cb024) into master (7d4aee9) will increase coverage by 0.01%. The diff coverage is 68.01%.

:exclamation: Current head a8cb024 differs from pull request most recent head ec91ab7. Consider uploading reports for the commit ec91ab7 to get more accurate results

@@            Coverage Diff             @@
##           master   #23280      +/-   ##
==========================================
+ Coverage   67.51%   67.53%   +0.01%     
==========================================
  Files        1900     1907       +7     
  Lines       73318    73468     +150     
  Branches     7935     7976      +41     
==========================================
+ Hits        49498    49613     +115     
- Misses      21787    21803      +16     
- Partials     2033     2052      +19     

Flag	Coverage Δ
hive	52.77% <75.92%> (+0.02%)	:arrow_up:
mysql	78.44% <87.96%> (+0.01%)	:arrow_up:
postgres	78.50% <87.96%> (+0.01%)	:arrow_up:
presto	52.70% <75.92%> (+0.04%)	:arrow_up:
python	82.27% <88.88%> (+0.01%)	:arrow_up:
sqlite	76.97% <86.11%> (+0.01%)	:arrow_up:
unit	52.48% <50.00%> (-0.04%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...t-frontend/src/dashboard/actions/dashboardState.js	55.76% <0.00%> (+1.30%)	:arrow_up:
superset-frontend/src/dashboard/actions/hydrate.js	2.08% <0.00%> (+0.21%)	:arrow_up:
...src/dashboard/components/DashboardBuilder/state.ts	70.37% <ø> (-2.05%)	:arrow_down:
...ard/components/FiltersBadge/DetailsPanel/index.tsx	82.22% <ø> (ø)
.../components/FiltersBadge/FilterIndicator/index.tsx	87.50% <ø> (ø)
...nd/src/dashboard/components/FiltersBadge/index.tsx	86.11% <ø> (ø)
...Filters/FilterBar/FiltersDropdownContent/index.tsx	20.00% <0.00%> (-5.00%)	:arrow_down:
...s/FilterBar/FiltersOutOfScopeCollapsible/index.tsx	16.66% <0.00%> (-3.34%)	:arrow_down:
...rontend/src/dashboard/containers/DashboardPage.tsx	8.79% <0.00%> (-0.10%)	:arrow_down:
...ontrols/DndColumnSelectControl/DndFilterSelect.tsx	42.27% <ø> (ø)
... and 79 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[nytai]

@sfirke the reason for that is when using a Public role then there is a current user, it's just stubbed. It's a bit tricky because the Public role exposes content to the public, not logged in user. How do you handle that? It's hard to know wether the user doesn't have access because they aren't logged in or because you don't want this content exposed. Additionally, redirecting users to a login page could result in a poor "public" experience

edit: nvm, looks like this code isn't as general as I thought it was

[DerLinne]

Hi! Is here any chance to support the review from our side? We would like to see this functionality working in superset - als Michel mentioned here: https://github.com/apache/superset/issues/25794

So if we can support, please give us a hands up!

[rusackas]

CC @eschutho @yousoph as this might have some interaction with embedded auth features in development.

[sfirke]

@DerLinne thanks for the offer of support! I am remain eager for this feature too. Can you try testing it thoroughly on your deployment and sharing any feedback, for starters?

[rusackas]

@sadpandajoe might be able to help in testing here too?

@dheeraj281 this needs a bit of a rebase to resolve conflicts, if you don't mind.

[sfirke]

ping @dheeraj281 @dheeraj-jaiswal-lowes - are you still interested in working on this fix? The project would still benefit from it!

[dheeraj281]

@sfirke Sure, I will take a look.

[sfirke]

I think I have a solution that works for me. Starting with the current master branch and none of the changes in this PR, I edited where the redirect goes after access check fails when user is anonymous. I replaced https://github.com/apache/superset/blob/master/superset/views/core.py#L794-L799 with:

        except SupersetSecurityException as ex:
            # anonymous users should get the login screen, others should go to dashboard list
            redirect_url = f"{appbuilder.get_url_for_login}?next={request.url}" if g.user is None or g.user.is_anonymous else "/dashboard/list/"
            warn_msg = "This dashboard does not allow public access." if g.user is None or g.user.is_anonymous else utils.error_msg_from_exception(ex)
            return redirect_with_flash(
                url=redirect_url,
                message=warn_msg,
                category="danger",
            )

Thoughts from folks in this thread? I have DASHBOARD_RBAC enabled and a Public role enabled, I'd want to know that it works for people not using those features.