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

feat: embedded dashboard core #17530

Merged
merged 14 commits into from
Jan 26, 2022
Merged

feat: embedded dashboard core #17530

merged 14 commits into from
Jan 26, 2022

Conversation

suddjian
Copy link
Member

@suddjian suddjian commented Nov 23, 2021

SUMMARY

This PR implements the core of the embedded dashboards feature as described in #17187.

All commits in this branch have been individually reviewed, so it should be merge-able without requiring detailed review of a large change set.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

If you're at Preset, we have a demo host app that can help with testing: https://github.com/preset-io/embedded-demo. Just run that, and you'll be exercising this code.

If you're not at Preset, testing this will be more involved until more features, docs, and supporting utils are written. You'll need to build a host app that embeds the new dashboard view, fetches a guest token from the new api, and sends the guest token to the frontend using the postMessage and MessageChannel browser apis. Future PRs will add an SDK, documentation, UI, and a demo host app to make this easier.

Other things to QA could be making sure that access rights still work correctly outside the embed feature, but we already have automated tests for this.

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

Lily Kuang and others added 2 commits November 16, 2021 10:31
@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #17530 (cf26feb) into master (4675ca3) will decrease coverage by 0.45%.
The diff coverage is 66.37%.

❗ Current head cf26feb differs from pull request most recent head e3dc5ee. Consider uploading reports for the commit e3dc5ee to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17530      +/-   ##
==========================================
- Coverage   66.35%   65.89%   -0.46%     
==========================================
  Files        1570     1588      +18     
  Lines       61772    62351     +579     
  Branches     6243     6273      +30     
==========================================
+ Hits        40991    41089      +98     
- Misses      19182    19641     +459     
- Partials     1599     1621      +22     
Flag Coverage Δ
hive ?
mysql 81.27% <67.87%> (-0.82%) ⬇️
postgres 81.32% <67.87%> (-0.82%) ⬇️
presto ?
python 81.41% <67.87%> (-1.18%) ⬇️
sqlite 81.01% <67.87%> (-0.82%) ⬇️

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

Impacted Files Coverage Δ
...rt-controls/src/operators/rollingWindowOperator.ts 100.00% <ø> (ø)
...ui-chart-controls/src/operators/utils/constants.ts 100.00% <ø> (ø)
.../superset-ui-core/src/connection/SupersetClient.ts 100.00% <ø> (ø)
...superset-ui-core/src/query/types/PostProcessing.ts 100.00% <ø> (ø)
.../src/time-format/TimeFormatterRegistrySingleton.ts 100.00% <ø> (ø)
...ackages/superset-ui-core/src/utils/featureFlags.ts 100.00% <ø> (ø)
...s/legacy-plugin-chart-country-map/src/countries.ts 100.00% <ø> (ø)
...src/BigNumber/BigNumberWithTrendline/buildQuery.ts 9.09% <ø> (ø)
...hart-echarts/src/MixedTimeseries/transformProps.ts 0.00% <0.00%> (ø)
...chart-echarts/src/Timeseries/Area/controlPanel.tsx 33.33% <0.00%> (-6.67%) ⬇️
... and 133 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 4675ca3...e3dc5ee. Read the comment docs.

suddjian and others added 3 commits December 9, 2021 10:12
* generate an embed token

* improve existing tests

* add some auth setup, and rename token

* fix the stuff for compatibility with external request loaders

* docs, standard jwt claims, tweaks

* black

* lint

* tests, and safer token decoding

* linting

* type annotation

* prettier

* add feature flag

* quiet pylint

* apparently typing is a problem again

* Make guest role name configurable

* fake being a non-anonymous user

* just one log entry

* customizable algo

* lint

* lint again

* 403 works now!

* get guest token from header instead of cookie

* Revert "403 works now!"

This reverts commit df2f49a.

* fix tests

* Revert "Revert "403 works now!""

This reverts commit 883dff3.

* rename method
@HeshamMeneisi
Copy link

Hello @suddjian I was looking into embedded dashboards and came by this. In your current design, would there be a way to use user-related data (from the JWT) as filters? Something like some_table.uid=external_user.uid where external_user is part of the JWT?

suddjian and others added 3 commits January 13, 2022 15:04
* create entry for embedded dashboard in webpack

* add cookies

* lint

* token message handshake

* guestTokenHeaderName

* use setupClient instead of calling configure

* rename the webpack chunk

* simplified handshake

* embedded entrypoint: render a proper app

* make the embedded page accept anonymous connections

* format

* lint

* fix test
# Conflicts:
#	superset-frontend/src/embedded/index.tsx
#	superset/views/core.py

* lint

* Update superset-frontend/src/embedded/index.tsx

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* comment out origins checks

* move embedded for core to dashboard

* pylint

* isort

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>
Lily Kuang and others added 2 commits January 18, 2022 14:44
* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* add more test

Co-authored-by: Lily Kuang <lily@preset.io>
* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* rls rules for guest tokens

* test guest token rls rules

* more flexible rls rules

* lint

* fix tests

* fix test

* defaults

* fix some tests

* fix some tests

* lint

Co-authored-by: Lily Kuang <lily@preset.io>
@suddjian suddjian marked this pull request as ready for review January 22, 2022 07:03
@suddjian suddjian changed the title wip: embedded dashboards feature branch feat: embedded dashboard core Jan 22, 2022
@suddjian
Copy link
Member Author

Hello @suddjian I was looking into embedded dashboards and came by this. In your current design, would there be a way to use user-related data (from the JWT) as filters? Something like some_table.uid=external_user.uid where external_user is part of the JWT?

Sorry for the delayed reply, I just saw this. Yes, the guest token can contain Row Level Security rules. More details available in the corresponding pr

Copy link
Member

@lilykuang lilykuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

superset/config.py Outdated Show resolved Hide resolved
Co-authored-by: Lily Kuang <lily@preset.io>
# specific language governing permissions and limitations
# under the License.
from enum import Enum
from typing import List, Optional, TypedDict, Union
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suddjian TypedDict was introduced in Python 3.8 ( https://www.python.org/dev/peps/pep-0589/ , https://docs.python.org/3/whatsnew/3.8.html#typing ). The minimal supported version of Python by Superset is 3.7 ( https://github.com/apache/superset/blob/master/setup.py#L173-L175 ). We have to express that in a different way or drop support for Python 3.7.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like add that it make pre-commit hook mypy fail on Python 3.7:

superset/security/guest_token.py:18: error: Module "typing" has no attribute "TypedDict"; maybe "_TypedDict"?
superset/security/guest_token.py:24: error: Unexpected keyword argument "total" for "__init_subclass__" of "object"
Found 2 errors in 1 file (checked 2 source files)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, I'll push a fix. Sorry for the inconvenience.

shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022
* feat(dashboard): embedded dashboard UI configuration (apache#17175) (apache#17450)

* setup embedded provider

* update ui configuration

* fix test

* feat: Guest token (for embedded dashboard auth) (apache#17517)

* generate an embed token

* improve existing tests

* add some auth setup, and rename token

* fix the stuff for compatibility with external request loaders

* docs, standard jwt claims, tweaks

* black

* lint

* tests, and safer token decoding

* linting

* type annotation

* prettier

* add feature flag

* quiet pylint

* apparently typing is a problem again

* Make guest role name configurable

* fake being a non-anonymous user

* just one log entry

* customizable algo

* lint

* lint again

* 403 works now!

* get guest token from header instead of cookie

* Revert "403 works now!"

This reverts commit df2f49a.

* fix tests

* Revert "Revert "403 works now!""

This reverts commit 883dff3.

* rename method

* correct import

* feat: entry for embedded dashboard (apache#17529)

* create entry for embedded dashboard in webpack

* add cookies

* lint

* token message handshake

* guestTokenHeaderName

* use setupClient instead of calling configure

* rename the webpack chunk

* simplified handshake

* embedded entrypoint: render a proper app

* make the embedded page accept anonymous connections

* format

* lint

* fix test
# Conflicts:
#	superset-frontend/src/embedded/index.tsx
#	superset/views/core.py

* lint

* Update superset-frontend/src/embedded/index.tsx

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* comment out origins checks

* move embedded for core to dashboard

* pylint

* isort

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* feat: Authorizing guest access to embedded dashboards (apache#17757)

* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* add more test

Co-authored-by: Lily Kuang <lily@preset.io>

* feat: Row Level Security rules for guest tokens (apache#17836)

* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* rls rules for guest tokens

* test guest token rls rules

* more flexible rls rules

* lint

* fix tests

* fix test

* defaults

* fix some tests

* fix some tests

* lint

Co-authored-by: Lily Kuang <lily@preset.io>

* SupersetClient guest token test

* Apply suggestions from code review

Co-authored-by: Lily Kuang <lily@preset.io>

Co-authored-by: Lily Kuang <lily@preset.io>
ofekisr pushed a commit to nielsen-oss/superset that referenced this pull request Feb 8, 2022
* feat(dashboard): embedded dashboard UI configuration (apache#17175) (apache#17450)

* setup embedded provider

* update ui configuration

* fix test

* feat: Guest token (for embedded dashboard auth) (apache#17517)

* generate an embed token

* improve existing tests

* add some auth setup, and rename token

* fix the stuff for compatibility with external request loaders

* docs, standard jwt claims, tweaks

* black

* lint

* tests, and safer token decoding

* linting

* type annotation

* prettier

* add feature flag

* quiet pylint

* apparently typing is a problem again

* Make guest role name configurable

* fake being a non-anonymous user

* just one log entry

* customizable algo

* lint

* lint again

* 403 works now!

* get guest token from header instead of cookie

* Revert "403 works now!"

This reverts commit df2f49a.

* fix tests

* Revert "Revert "403 works now!""

This reverts commit 883dff3.

* rename method

* correct import

* feat: entry for embedded dashboard (apache#17529)

* create entry for embedded dashboard in webpack

* add cookies

* lint

* token message handshake

* guestTokenHeaderName

* use setupClient instead of calling configure

* rename the webpack chunk

* simplified handshake

* embedded entrypoint: render a proper app

* make the embedded page accept anonymous connections

* format

* lint

* fix test
# Conflicts:
#	superset-frontend/src/embedded/index.tsx
#	superset/views/core.py

* lint

* Update superset-frontend/src/embedded/index.tsx

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* comment out origins checks

* move embedded for core to dashboard

* pylint

* isort

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* feat: Authorizing guest access to embedded dashboards (apache#17757)

* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* add more test

Co-authored-by: Lily Kuang <lily@preset.io>

* feat: Row Level Security rules for guest tokens (apache#17836)

* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* rls rules for guest tokens

* test guest token rls rules

* more flexible rls rules

* lint

* fix tests

* fix test

* defaults

* fix some tests

* fix some tests

* lint

Co-authored-by: Lily Kuang <lily@preset.io>

* SupersetClient guest token test

* Apply suggestions from code review

Co-authored-by: Lily Kuang <lily@preset.io>

Co-authored-by: Lily Kuang <lily@preset.io>
bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022
* feat(dashboard): embedded dashboard UI configuration (apache#17175) (apache#17450)

* setup embedded provider

* update ui configuration

* fix test

* feat: Guest token (for embedded dashboard auth) (apache#17517)

* generate an embed token

* improve existing tests

* add some auth setup, and rename token

* fix the stuff for compatibility with external request loaders

* docs, standard jwt claims, tweaks

* black

* lint

* tests, and safer token decoding

* linting

* type annotation

* prettier

* add feature flag

* quiet pylint

* apparently typing is a problem again

* Make guest role name configurable

* fake being a non-anonymous user

* just one log entry

* customizable algo

* lint

* lint again

* 403 works now!

* get guest token from header instead of cookie

* Revert "403 works now!"

This reverts commit df2f49a.

* fix tests

* Revert "Revert "403 works now!""

This reverts commit 883dff3.

* rename method

* correct import

* feat: entry for embedded dashboard (apache#17529)

* create entry for embedded dashboard in webpack

* add cookies

* lint

* token message handshake

* guestTokenHeaderName

* use setupClient instead of calling configure

* rename the webpack chunk

* simplified handshake

* embedded entrypoint: render a proper app

* make the embedded page accept anonymous connections

* format

* lint

* fix test
# Conflicts:
#	superset-frontend/src/embedded/index.tsx
#	superset/views/core.py

* lint

* Update superset-frontend/src/embedded/index.tsx

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* comment out origins checks

* move embedded for core to dashboard

* pylint

* isort

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* feat: Authorizing guest access to embedded dashboards (apache#17757)

* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* add more test

Co-authored-by: Lily Kuang <lily@preset.io>

* feat: Row Level Security rules for guest tokens (apache#17836)

* helper methods and dashboard access

* guest token dashboard authz

* adjust csrf exempt list

* eums don't work that way

* Remove unnecessary import

* move row level security tests to their own file

* a bit of refactoring

* add guest token security tests

* refactor tests

* clean imports

* variable names can be too long apparently

* missing argument to get_user_roles

* don't redefine builtins

* remove unused imports

* fix test import

* default to global user when getting roles

* missing import

* mock it

* test get_user_roles

* infer g.user for ease of tests

* remove redundant check

* tests for guest user security manager fns

* use algo to get rid of warning messages

* tweaking access checks

* fix guest token security tests

* missing imports

* more tests

* more testing and also some small refactoring

* move validation out of parsing

* fix dashboard access check again

* rls rules for guest tokens

* test guest token rls rules

* more flexible rls rules

* lint

* fix tests

* fix test

* defaults

* fix some tests

* fix some tests

* lint

Co-authored-by: Lily Kuang <lily@preset.io>

* SupersetClient guest token test

* Apply suggestions from code review

Co-authored-by: Lily Kuang <lily@preset.io>

Co-authored-by: Lily Kuang <lily@preset.io>
@lilykuang lilykuang deleted the embedded branch August 30, 2022 16:23
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/XXL 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants