Skip to content

feat: enable user impersonation in GSheets#14767

Merged
betodealmeida merged 1 commit into
apache:masterfrom
betodealmeida:gsheets_impersonation
May 22, 2021
Merged

feat: enable user impersonation in GSheets#14767
betodealmeida merged 1 commit into
apache:masterfrom
betodealmeida:gsheets_impersonation

Conversation

@betodealmeida
Copy link
Copy Markdown
Member

@betodealmeida betodealmeida commented May 22, 2021

SUMMARY

Allow impersonating the user when using GSheets.

Currently, if the user sets up a GSheets DB with credentials, the user has access to all spreadsheets from a given domain. With this change, enabling user impersonation will ensure that the user has access only to spreadsheets that they should have.

For example, I have the email roberto@dealmeida.net. If someones shares a spreadsheet with the "Dealmeida.net" organization I'll have access to it. But if someone shares a spreadsheets with "ceo@dealmeida.net", with this PR I won't be able to access it. Without the PR, or with user impersonation disabled, I would have access because the service account credentials have access to everything in the domain.

We had this set up at Lyft via a DB_CONNECTION_MUTATOR, but this is a cleaner and simpler approach.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

I tested manually with my domain, "dealmeida.net". I was able to access only the correct spreadsheets.

ADDITIONAL INFORMATION

  • Has associated issue:
  • 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
Copy link
Copy Markdown

codecov Bot commented May 22, 2021

Codecov Report

Merging #14767 (8ecda06) into master (d71b8b3) will decrease coverage by 0.25%.
The diff coverage is 55.55%.

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

@@            Coverage Diff             @@
##           master   #14767      +/-   ##
==========================================
- Coverage   77.62%   77.36%   -0.26%     
==========================================
  Files         962      962              
  Lines       49017    49026       +9     
  Branches     6155     6155              
==========================================
- Hits        38050    37930     -120     
- Misses      10763    10892     +129     
  Partials      204      204              
Flag Coverage Δ
hive ?
mysql 81.64% <55.55%> (-0.01%) ⬇️
postgres 81.66% <55.55%> (-0.01%) ⬇️
presto ?
python 81.70% <55.55%> (-0.49%) ⬇️
sqlite ?

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

Impacted Files Coverage Δ
.../CRUD/data/database/DatabaseModal/ExtraOptions.tsx 93.54% <ø> (ø)
superset/db_engine_specs/gsheets.py 73.33% <55.55%> (-26.67%) ⬇️
superset/db_engines/hive.py 0.00% <0.00%> (-82.15%) ⬇️
superset/db_engine_specs/hive.py 70.32% <0.00%> (-17.08%) ⬇️
superset/db_engine_specs/presto.py 83.36% <0.00%> (-6.53%) ⬇️
superset/db_engine_specs/sqlite.py 90.62% <0.00%> (-6.25%) ⬇️
superset/utils/celery.py 86.20% <0.00%> (-3.45%) ⬇️
superset/connectors/sqla/models.py 88.05% <0.00%> (-1.92%) ⬇️
superset/views/database/mixins.py 81.03% <0.00%> (-1.73%) ⬇️
superset/result_set.py 96.77% <0.00%> (-1.62%) ⬇️
... and 5 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 d71b8b3...f19a63d. Read the comment docs.

@betodealmeida betodealmeida merged commit 9e13476 into apache:master May 22, 2021
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request May 25, 2021
* master: (163 commits)
  fix(native-filters): Manage default value of filters by superset (apache#14785)
  fix: Additional ResultSet tests (apache#14741)
  chore: added BasicParametersMixin to Redshift (apache#14752)
  fix: make dataset list sort case insensitive (apache#14528)
  fix: use encodeURIComponent when getting table metadata (apache#14790)
  fix: ensure engine is outside parameters (apache#14787)
  database modal should close on connect with tab layout (apache#14771)
  feat(native-filters): add search all filter options (apache#14710)
  fix: extra query in Dashboard when native filter enabled (apache#14770)
  chore: Improves the native filters UI/UX - iteration 2 (apache#14753)
  fix(native filters): Fix explore state (apache#14779)
  fix(explore): DndColumnSelect not handling controls with "multi: false" (apache#14737)
  feat: Create BigQuery Parameters for DatabaseModal (apache#14721)
  feat: enable user impersonation in GSheets (apache#14767)
  fix: add DB should not say it's Postgres (apache#14766)
  Revert "fix(dashboard): multiple query trigger when native filter enabled (apache#14734)" (apache#14762)
  feat: save database with new dynamic form (apache#14583)
  fix: save non-parameter DBs (apache#14759)
  chore: Removes ColorSchemeControl.less (apache#14199)
  fix(explore): Icons width (apache#14717)
  ...
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 First shipped in 1.3.0 labels Mar 12, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
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 size/S 🚢 1.3.0 First shipped in 1.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants