fix(sql lab): SQL Lab access restrictions not applied to default schema

[diegomedina248]

SUMMARY

The schema level restrictions are not working properly in SQL Lab, when the default schema is used. Some DB Engines allow users to execute a query without specifying the Schema, in which default Schema will be queried.

In the permission check, we're assigning the schema from two sources: the query itself, or if it's not present, the schema selected on the left hand side in SQL Lab. It's the second scenario that's not properly checked, because, if the user has schema permission for the one selected, and if the query can be executed in the engine without specifying the schema explicitly, then the permission check will allow said execution, since there's a permission for the schema.

This PR alters the order of checks, so that we validate that the default schema actually contains the table we're trying to query, and then check the permission on said datasource.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

https://user-images.githubusercontent.com/17252075/172245754-0186181c-5bbb-46cd-8ae5-c104e0d6d965.mov

After:

https://user-images.githubusercontent.com/17252075/172245791-cda848a5-aa53-41ea-a53a-477e0b5a3269.mov

TESTING INSTRUCTIONS

1. Create a new user role with the following permissions:

• can read on SavedQuery
• can write on SavedQuery
• can read on Database
• can read on Query
• can sqllab on Superset
• can sqllab history on Superset
• can sqllab viz on Superset
• can sql json on Superset
• can sqllab table viz on Superset
• can validate sql json on Superset
• can activate on TabStateView
• can get on TabStateView
• can put on TabStateView
• can delete query on TabStateView
• can post on TabStateView
• can delete on TabStateView
• menu access on SQL Lab
• menu access on SQL Editor
• schema access on [examples].[information_schema]

2. Create a new user and assign the above role to it.
3. Log in with that user.
4. Go to SQL Lab
5. Select examples as database and information_schema as schema

Execute these two queries:

select * from birth_names

select * from public.birth_names

Ensure both fail with a forbidden error.

ADDITIONAL INFORMATION

• [ ] Has associated issue:
• [ ] 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

[codecov[bot]]

Codecov Report

Merging #20285 (2e63d50) into master (6b0bb80) will decrease coverage by 11.60%. The diff coverage is 75.00%.

:exclamation: Current head 2e63d50 differs from pull request most recent head 3c7ae32. Consider uploading reports for the commit 3c7ae32 to get more accurate results

@@             Coverage Diff             @@
##           master   #20285       +/-   ##
===========================================
- Coverage   66.83%   55.23%   -11.61%     
===========================================
  Files        1750     1750               
  Lines       65894    65899        +5     
  Branches     7017     7017               
===========================================
- Hits        44041    36399     -7642     
- Misses      20067    27714     +7647     
  Partials     1786     1786               

Flag	Coverage Δ
hive	53.96% <58.33%> (+0.01%)	:arrow_up:
mysql	?
postgres	?
presto	53.82% <58.33%> (+0.01%)	:arrow_up:
python	58.80% <75.00%> (-24.19%)	:arrow_down:
sqlite	?
unit	51.03% <66.66%> (-0.01%)	:arrow_down:

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

Impacted Files	Coverage Δ
superset/security/manager.py	64.58% <75.00%> (-30.97%)	:arrow_down:
superset/utils/dashboard_import_export.py	0.00% <0.00%> (-100.00%)	:arrow_down:
superset/key_value/commands/update.py	0.00% <0.00%> (-88.89%)	:arrow_down:
superset/key_value/commands/delete.py	0.00% <0.00%> (-85.30%)	:arrow_down:
superset/key_value/commands/delete_expired.py	0.00% <0.00%> (-80.77%)	:arrow_down:
superset/dashboards/commands/importers/v0.py	15.62% <0.00%> (-76.25%)	:arrow_down:
superset/datasets/commands/update.py	25.30% <0.00%> (-68.68%)	:arrow_down:
superset/datasets/commands/create.py	29.41% <0.00%> (-68.63%)	:arrow_down:
superset/datasets/commands/importers/v0.py	24.03% <0.00%> (-67.45%)	:arrow_down:
superset/reports/commands/execute.py	24.45% <0.00%> (-67.16%)	:arrow_down:
... and 276 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6b0bb80...3c7ae32. Read the comment docs.

[rusackas]

@eschutho if you wouldn't mind checking/accepting/amending the change requests, that would be much appreciated. Looks like this'll need a little rebase action as well :D

[eschutho]

I just checked with @yousoph on whether we should be fetching from the schema in the dropdown when not specified in the sql statement, which we're not doing.

[rusackas]

Fixed via https://github.com/apache/superset/pull/23356 I believe (hat tip to @eschutho for the reference)