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

Robust handling of PFS cookies #3962

Merged
merged 18 commits into from
Apr 26, 2024
Merged

Robust handling of PFS cookies #3962

merged 18 commits into from
Apr 26, 2024

Conversation

nl0
Copy link
Member

@nl0 nl0 commented Apr 23, 2024

Description

Lazily (re)set PFS cookie when required

TODO

  • Changelog entry (skip if change is not significant to end users, e.g. docs only)

@nl0 nl0 changed the title PFS cookie Robust handling of PFS cookies Apr 25, 2024
@nl0 nl0 requested a review from fiskus April 25, 2024 09:09
Copy link

codecov bot commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.48%. Comparing base (7773230) to head (831c446).
Report is 7 commits behind head on master.

❗ Current head 831c446 differs from pull request most recent head fee9d7d. Consider uploading reports for the commit fee9d7d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3962      +/-   ##
==========================================
- Coverage   90.91%   84.48%   -6.43%     
==========================================
  Files          35       40       +5     
  Lines        5952     3488    -2464     
==========================================
- Hits         5411     2947    -2464     
  Misses        541      541              
Flag Coverage Δ
api-python ?
lambda 84.48% <ø> (?)

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.

Copy link
Member

@fiskus fiskus left a comment

Choose a reason for hiding this comment

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

For some reason, it still doesn't work for me
Screenshot from 2024-04-25 15-53-52

catalog/app/utils/PFSCookieManager.tsx Show resolved Hide resolved
catalog/app/utils/PFSCookieManager.tsx Outdated Show resolved Hide resolved
@nl0
Copy link
Member Author

nl0 commented Apr 25, 2024

For some reason, it still doesn't work for me

  1. you need an updated registry for that to work (PR in review)
  2. it won't work on localhost bc the cookie has strict policy -- it only works for the same (sub?)domain

fiskus
fiskus previously approved these changes Apr 25, 2024
fiskus
fiskus previously approved these changes Apr 26, 2024
catalog/app/utils/PFSCookieManager.tsx Show resolved Hide resolved
@nl0 nl0 marked this pull request as ready for review April 26, 2024 10:32
This reverts commit 5ce2fe3.
@nl0 nl0 requested a review from fiskus April 26, 2024 10:37
@nl0 nl0 added this pull request to the merge queue Apr 26, 2024
Merged via the queue into master with commit 6883c26 Apr 26, 2024
32 checks passed
@nl0 nl0 deleted the pfs-cookie branch April 26, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants