fix(embedded): add row level security extra cache keys for guest users
SUMMARY
Cache keys from guest token row level security clauses were not being evaluated, causing data to leak across sessions via cache.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
In one session, create/use an guest_token without a rls clause, and a session with a rls clause with an additional cache key e.g.:
{ "clause": "publisher = '{{cache_key_wrapper('Nintendo')}}'" }
]
The same dashboard should have independent cache for each session, and the one with the rls clause should be unable to see unfiltered data.
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
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.
@michael-s-molina know anyone who would be well qualified to review this. I'm trying not to throw as much at @dpgaspar or @villebro though here I go mentioning them anyway ;)
Closing/reopening to kick-start CI.
@michael-s-molina know anyone who would be well qualified to review this. I'm trying not to throw as much at @dpgaspar or @villebro though here I go mentioning them anyway ;)
@rusackas We don't use embedded and RLS at Airbnb so you probably pinged the wrong person 😄
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 57.33%. Comparing base (
0ae1ca7) to head (802f360). Report is 5863 commits behind head on master.
:exclamation: There is a different number of reports uploaded between BASE (0ae1ca7) and HEAD (802f360). Click for more details.
HEAD has 3 uploads less than BASE
Flag BASE (0ae1ca7) HEAD (802f360) python 6 3
Additional details and impacted files
@@ Coverage Diff @@
## master #18924 +/- ##
==========================================
- Coverage 66.21% 57.33% -8.88%
==========================================
Files 1633 1746 +113
Lines 63210 70409 +7199
Branches 6409 6409
==========================================
- Hits 41852 40372 -1480
- Misses 19698 28377 +8679
Partials 1660 1660
| Flag | Coverage Δ | |
|---|---|---|
| hive | ∅ <100.00%> (∅) |
|
| mysql | ? |
|
| postgres | ? |
|
| presto | ? |
|
| python | ∅ <100.00%> (∅) |
|
| sqlite | ? |
|
| unit | ∅ <100.00%> (?) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Looks like there are still a lot of failing tests here. I'll convert it to draft for now, but it may be closed due to inactivity eventually if it isn't made mergeable.