Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Oct 16, 2024

What do these changes do?

Apparently Firefox handles cookies differently than Chrome and Safari. It keeps cookies with the same name but different domains.
Even after invalidating the session key for the cookies. You login into Firefox open a new style dynamic service and authentication fails. Apparently it send out old cookie, the one with domain osparc.io (which is invalid) instead of the one with .osparc.io domain. This causes a 401 reply when opening UUID.services.osparc.io.

To avoid issues for users, the session cookie is being renamed.

Related issue/s

How to test

Dev-ops checklist

@GitHK GitHK marked this pull request as ready for review October 16, 2024 09:00
@sonarqubecloud
Copy link

@GitHK GitHK added this to the MartinKippenberger milestone Oct 16, 2024
@GitHK GitHK self-assigned this Oct 16, 2024
@codecov
Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.1%. Comparing base (cafbf96) to head (bf40ca7).
Report is 638 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6544      +/-   ##
=========================================
+ Coverage    84.5%   88.1%    +3.5%     
=========================================
  Files          10    1548    +1538     
  Lines         214   63350   +63136     
  Branches       25    2059    +2034     
=========================================
+ Hits          181   55836   +55655     
- Misses         23    7195    +7172     
- Partials       10     319     +309     
Flag Coverage Δ
integrationtests 64.7% <ø> (?)
unittests 86.1% <100.0%> (+1.5%) ⬆️

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

Files with missing lines Coverage Δ
...ings-library/src/settings_library/utils_session.py 44.4% <100.0%> (ø)

... and 1497 files with indirect coverage changes

@YuryHrytsuk
Copy link
Contributor

YuryHrytsuk commented Oct 16, 2024

This will break a few e2e ops tests because we explicitly rely on Cookie name. We need to update these tests as soon as this fix is rolled out.

Furthermore, this needs to be done in stages (master --> stag --> prod)

I will take care of it

@YuryHrytsuk
Copy link
Contributor

YuryHrytsuk commented Oct 16, 2024

This will break a few e2e ops tests because we explicitly rely on Cookie name. We need to update these tests as soon as this fix is rolled out.

Furthermore, this needs to be done in stages (master --> stag --> prod)

I will take care of it

I wonder if there are other places that explicitly rely on this cookie name. I hope there are no such 3rd parties

@GitHK
Copy link
Contributor Author

GitHK commented Oct 16, 2024

This will break a few e2e ops tests because we explicitly rely on Cookie name. We need to update these tests as soon as this fix is rolled out.
Furthermore, this needs to be done in stages (master --> stag --> prod)
I will take care of it

I wonder if there are other places that explicitly rely on this cookie name. I hope there are no such 3rd parties

inside the simcore codebase there are no other places. And we have tests to avoid cookie being out of sync that fail.
regarding other repos I would not know

@GitHK GitHK merged commit 55c967d into ITISFoundation:master Oct 16, 2024
57 checks passed
@GitHK GitHK deleted the pr-osparc-rename-cookie-for-ffx branch October 16, 2024 10:08
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.

4 participants