feat(embedded): add hook to allow superset admins to validate guest token parameters

[dmarkey]

SUMMARY

This allows the Superset admin to tighten up Guest token configuration with a validator hook. Any aspect can be validated but most likely the RLS Clause.

TESTING INSTRUCTIONS

Set GUEST_TOKEN_VALIDATOR_HOOK = lambda x: len(x["rls"]) == 1 and "tenant_id=" in x["rls"][0]["clause"] and the RLS clause in the guest token will need to contain "tenant_id="

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

[github-actions]

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.69%. Comparing base (76d897e) to head (8bcf43f).
Report is 1094 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #30132       +/-   ##
===========================================
+ Coverage   60.48%   83.69%   +23.20%     
===========================================
  Files        1931      529     -1402     
  Lines       76236    38421    -37815     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    32157    -13957     
+ Misses      28017     6264    -21753     
+ Partials     2105        0     -2105     

Flag	Coverage Δ
hive	48.86% <33.33%> (-0.30%)	⬇️
javascript	?
mysql	76.66% <100.00%> (?)
postgres	76.73% <100.00%> (?)
presto	53.39% <33.33%> (-0.41%)	⬇️
python	83.69% <100.00%> (+20.20%)	⬆️
sqlite	76.18% <100.00%> (?)
unit	60.44% <33.33%> (+2.81%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nytai]

LGTM

[rusackas]

Thanks @giftig and @dmarkey for dialing this in. @nytai already approved it, which is giving me an itchy trigger finger. If any of @giftig 's change requests are blockers, feel free to hit the "request changes" button to block the merge. Just make sure you're around to unblock it when the required adjustments are made ;) Thanks all :D

[giftig]

My comments aren't really blocking, just a small improvement which would be nice imo. It's an easy +1 for me if the tests can be separated like they were before but without the repetition of test fixtures, but it's not a big enough issue to block the PR.

[dmarkey]

Alright did another test refactor, broke out the tests again and moved them to their own class with the heavy dashboard fixture loaded in the class scope.

[giftig]

Minor question about noqa but LGTM

[dmarkey]

Not sure if I need to take action based on that failed cypress test?

[villebro]

@dmarkey nothing that you've caused - we're investigating it currently, but it seems like a flaky test that may be caused by a perf regression that's been recently introduced. We're looking into it, but for now we'll just keep restarting the CI check until it passes..

[dmarkey]

Thanks for merging! I guess it will be in 4.2.0?

[rusackas]

No, thank YOU!

It'll definitely be in 5.0, and if we do a 4.2 (TBD) it'll definitely be in there, too. Right now, @sadpandajoe is building 4.1 RCs. I've labeled this PR to get cherries, but I'm not sure if that'll happen or not at this point.