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

misc: Update response-middleware.ts cookie expiration date #29269

Closed
wants to merge 3 commits into from

Conversation

jj497
Copy link

@jj497 jj497 commented Apr 4, 2024

Additional details

Steps to test

How has the user experience changed?

PR Tasks

@CLAassistant
Copy link

CLAassistant commented Apr 4, 2024

CLA assistant check
All committers have signed the CLA.

@cypress-app-bot
Copy link
Collaborator

@jj497 jj497 changed the title Update response-middleware.ts cookie expiration date misc: Update response-middleware.ts cookie expiration date Apr 4, 2024
@jj497 jj497 force-pushed the patch-1 branch 2 times, most recently from 548d4d2 to 282d0c5 Compare April 4, 2024 20:12
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

@jj497 We'll need a reproduction of the original issue this is resolving as well as a test demonstrating the issue this is resolving before accepting.

@jj497
Copy link
Author

jj497 commented Apr 19, 2024

Success without cypress initial cookie
Expired Cookie Response
Attached is a screenshot of the failure in the Cypress test suite with the __cypress.initial cookie. As you can see it has the 'athena' domain and comes back with an expired value of '1970-01-01' on the document and the error "You session has timed out....Please login again."
Also attached is another screenshot of success on the same browser, but a different tab. The complete.esp document does not contain the __cypress.intitial cookie and login proceeds as expected.

Because the login is on a 3rd party partner (athena) I can't reproduce the issue myself on our own servers but I hope the attached screenshots are sufficient evidence of the issue.

@jennifer-shehane
Copy link
Member

@jj497 You'll need to also sign our CLA. Could you sign that?

@jj497
Copy link
Author

jj497 commented Apr 24, 2024

@jj497 You'll need to also sign our CLA. Could you sign that?

complete!

@jj497
Copy link
Author

jj497 commented May 5, 2024

@jennifer-shehane - this PR accidentally closed when I resynced the fork. Could you please help me reopen it?
My changes are here: https://github.com/jj497/cypress/blob/patch-1

@jennifer-shehane
Copy link
Member

@jj497 Reopened

@jennifer-shehane jennifer-shehane requested review from cacieprins and removed request for cacieprins May 13, 2024 19:41
@jennifer-shehane
Copy link
Member

@jj497 We'll still need a test that we can run that shows the bug this is fixing. We cannot accept this otherwise. Setting the cookie to a future date has other implications that will break other parts of the codebase.

@jennifer-shehane
Copy link
Member

Closing since the undesired behavior cannot be confirmed. This would not be a passive change to change this as well.

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.

__cypress.initial cookie expired date causing signin side-effects
4 participants