-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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(jinja): current_user_email macro #27197
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #27197 +/- ##
==========================================
+ Coverage 69.46% 69.59% +0.12%
==========================================
Files 1905 1905
Lines 74580 74591 +11
Branches 8330 8330
==========================================
+ Hits 51806 51910 +104
+ Misses 20724 20631 -93
Partials 2050 2050
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thanks @Vitor-Avila for the PR. Regarding your comment,
this doesn't seem to make sense to us, i.e., the user information (including the ID, email, username, etc.) is being pulled from the Superset metadata database. |
hey @john-bodley let me know if you have any concerns with this implementation. This was also asked in this ticket #26808 I think it makes sense depending on your data structure, but happy to review if it introduces any security concerns. |
@Vitor-Avila there's no security or implementation concerns IMO, I just think you should remove the following,
from your PR description as said macro does actually need to ingest data from the Superset user table. |
@john-bodley ohh gotcha! My point there is that if you want to apply an RLS rule using the other macros, for example: employee = '{{current_username()}}' You have to have the Superset I updated the PR description -- let me know if you have any questions! |
a1b594f
to
40b0c78
Compare
@john-bodley I assumed that you're ok with us merging this, but let us know if there are any other questions. thanks! |
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
(cherry picked from commit 1d571ec)
SUMMARY
This PR adds a new Jinja macro:
{{current_user_email()}}
. This macro can be specially useful when applying RLS based on the logged in user, as it's typically easier for organizations to have the email address information available in their warehouse (as opposed to the Superset account'susername
orid
).Fixes #26808.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI changes.
TESTING INSTRUCTIONS
select '{{current_user_email()}}' as my_email;
Tests also added.
ADDITIONAL INFORMATION
ENABLE_TEMPLATE_PROCESSING