Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLLab page access with insufficient permissions is not redirecting #31513

Open
3 tasks done
callum-jones19 opened this issue Dec 18, 2024 · 1 comment
Open
3 tasks done
Labels
authentication:access-control Rlated to access control sqllab Namespace | Anything related to the SQL Lab

Comments

@callum-jones19
Copy link

callum-jones19 commented Dec 18, 2024

Bug description

The bug involves a user without the admin or sql_lab roles being allowed to access the SQLLab editor page. When on this page, they are unable to do anything, as the backend correctly refuses to send through any data to an unauthenticated account. However, the user is not redirected off this page as intended, and instead is presented with an Unexpected Error header. To achieve this bug:

  1. Log into Superset with a non-administrator account who does not have the sql_lab role, but who can access the create dataset page.
  2. Click on the Datasets header button to go to the /tablemodelview/list/ URL.
  3. Click on the "+ Dataset" button to go to the /dataset/add page.
  4. Click on the link embedded in the empty dataset source page that says "create dataset from SQL query".

The expected behavior here (as occurred in previous Superset versions) is that the user gets redirected to the home page with an "Access denied" toast notifying them of their insufficient permissions. The observed behavior is that they stay on this page, which is marked with an Unexpected Error header.

Attached below are the screenshots as the error occurs/appears in the master branch, using the docker-compose-non-dev.yml docker compose file. That is, the errors displayed in the screenshots are what appears in production.

Screenshots/recordings

Screenshot From 2024-12-19 09-03-31
Screenshot From 2024-12-19 09-03-20
Screenshot From 2024-12-19 09-01-32

Superset version

master / latest-dev

Python version

3.10

Node version

16

Browser

Firefox

Additional context

I narrowed the bug down specifically to commit 5ab1e7e, to the changes made in the following file: superset-frontend/src/features/datasets/AddDataset/DatasetPanel/MessageContent.tsx. Specifically, changing the span that onClick redirected the page to /superset/sqllab into a SPA react-router link. This SPA <Link> component appears to skip the permissions check, allowing the user onto the SQLLab page, although I am unsure why. This can be verified by refreshing the broken SQLLab page, or reverting the <Link> into a <a> element, which causes Superset to correctly re-route the user to the home page.

The unexpected error in the frontend (if viewed in development mode) reveals that the SQLLab editor page is throwing an error due a requisite prop being undefined. This happens because the SQLLab page is displayed but no data is sent through from the backend due to insufficient permissions, so the data is undefined.

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
Copy link

dosubot bot commented Dec 18, 2024

Hey @callum-jones19! I'm here to help you with your Superset issue. Let's work together to solve this bug and get everything back on track.

The issue you're experiencing is related to how the <Link> component from react-router-dom handles routing in a Single Page Application (SPA) context. The <Link> component is used for client-side navigation and does not inherently perform permission checks. This is because it bypasses the server-side request that would typically enforce these checks when using a standard <a> element, which triggers a full page reload and server-side permission validation.

In your case, the change in MessageContent.tsx from a <span> with an onClick handler to a <Link> component allows users to navigate to the SQLLab page without triggering the server-side permission checks that would normally occur with a full page reload. This is why users without the necessary permissions are able to access the page initially, but encounter errors when the backend refuses to send data.

To resolve this, you can consider reverting the <Link> component back to a standard <a> element, which will ensure that the server-side permission checks are enforced upon navigation. Alternatively, you could implement additional client-side logic to check permissions before allowing navigation via the <Link> component.

This behavior aligns with the known characteristics of the <Link> component in Superset, which is used for internal navigation and does not perform permission checks by itself [1].

To continue talking to Dosu, mention @dosu.


Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other

@dosubot dosubot bot added authentication:access-control Rlated to access control sqllab Namespace | Anything related to the SQL Lab labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication:access-control Rlated to access control sqllab Namespace | Anything related to the SQL Lab
Projects
None yet
Development

No branches or pull requests

1 participant