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

Prepending Stale Cookies to requests after 10.3 #22751

Closed
iomedico-beyer opened this issue Jul 12, 2022 · 20 comments · Fixed by #23438
Closed

Prepending Stale Cookies to requests after 10.3 #22751

iomedico-beyer opened this issue Jul 12, 2022 · 20 comments · Fixed by #23438
Assignees
Labels
Reproducible Can be reproduced topic: cookies 🍪 topic: cy.origin Problems or enhancements related to cy.origin command type: bug

Comments

@iomedico-beyer
Copy link

iomedico-beyer commented Jul 12, 2022

Current behavior

We use cy.session+cy.origin (see code below) to login with Keycloak which is hosted on a different domain. This always worked well until updating from Cypress 10.2 to 10.3.

After the first tests, the session seems broken, although Cypress logs "Validate Session: valid". Authentification is no longer established. When I add a cy.wait(1000) between visit and contains in the test code of it('starts again' I can see that the browser is at the login screen again. There is also this error:

Timed out after waiting 60000ms for your remote page to load on origin(s):
- http://localhost:4200
A cross-origin request for https://login… was detected.

Same happens even for the first test if I rerun all tests using the "Run All Tests" button.

Desired behavior

Cypress should be able to store and recover the session (cookies?), as it already did before 10.3.

Test code to reproduce

In support/commands.ts:

Cypress.Commands.add('cachedLogin', () => {
    const args = { user };
    cy.session(
        args,
        () => {
            cy.visit('/');
            cy.origin(KEYCLOAK_DOMAIN, { args }, ({ user }) => {
                cy.get('#username').type(user.username);
                cy.get('#password').type(user.password);
                cy.get('#kc-login').click();
            });
            cy.url().should('contain', Cypress.config('baseUrl'));
        },
        {
            validate() {
                cy.request('/').its('status').should('eq', 200);
            },
        }
    );
});

In Cypress-issue-22751.cy.ts:

describe('issue 22751 reproducer', () => {
    beforeEach(() => {
        cy.cachedLogin();
    });

    it('starts', () => {
        cy.visit('/');
        cy.contains('Start');
    });

    it('starts again', () => {
        cy.visit('/');
        // cy.wait(1000); // add line to see that browser is at login screen
        cy.contains('Start'); // fails on Cypress 10.3; "Timed out after waiting 60000ms for your remote page to load on origin(s):"
    });
});

Cypress Version

10.3, 10.3.1, 10.4

Other

Tested with Chrome, Edge, Electron.

@nagash77 nagash77 assigned nagash77 and chrisbreiding and unassigned nagash77 Jul 12, 2022
@cypress-bot cypress-bot bot added the stage: investigating Someone from Cypress is looking into this label Jul 12, 2022
@chrisbreiding
Copy link
Contributor

Right now there doesn't seem to be enough information to reproduce the problem on our end, but it's possible this is fixed by #22594, which will be in the next release. If possible, could you try your tests against the latest pre-release version of Cypress. Here are instructions on how to install it. You can use this commit.

@cypress-bot cypress-bot bot added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: investigating Someone from Cypress is looking into this labels Jul 13, 2022
@iomedico-beyer
Copy link
Author

@chrisbreiding Thanks, but I think I’ll just wait for the next release and report back afterwards. There were seven Cypress releases in June, so I suppose the next one is already waiting around the corner.

@chrisbreiding
Copy link
Contributor

Should be out early next week, so it won't be too long. Look forward to hearing if it fixes your issue.

@mjhenkes mjhenkes assigned mjhenkes and unassigned chrisbreiding Jul 19, 2022
@mjhenkes
Copy link
Member

@iomedico-beyer, 10.3.1 released today, could you give it a try?

@iomedico-beyer
Copy link
Author

@iomedico-beyer, 10.3.1 released today, could you give it a try?

I did. Same problem.

@cypress-bot cypress-bot bot added stage: investigating Someone from Cypress is looking into this and removed stage: awaiting response Potential fix was proposed; awaiting response labels Jul 20, 2022
@mjhenkes
Copy link
Member

@iomedico-beyer, I'm sorry to hear 10.3.1 didn't address your issue. I will bet that your issue was caused by this change #20685, and this is where i'll need your help. I'm sure that some how cookies are not being passed correctly in your circumstance. Would it be possible for you to put together a reproduction that we could test against? Or baring that could you look into what cookies aren't getting passed and how they're being set?

@cypress-bot cypress-bot bot added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: investigating Someone from Cypress is looking into this labels Jul 20, 2022
@sebkasanzew
Copy link

I have a similar problem and did some investigation on what's changed between v10.2 and v10.3 for my test case.

I found that a POST request in its request header has a cookie length of 8696 Bytes in v10.3.
The same request from the frontend without cypress (or with v10.2) has a length of 4347 Bytes in its cookie.

The content of the cookie in cypress 10.3.0 and 10.3.1 has the same value twice in it, separated by a space.

Not sure if this is related to this issue, or if it's a new one.

@iomedico-beyer
Copy link
Author

10.3.1 (and probably also 10.3) stores two additional cookies which interestingly have no value. But removing them seems to work for me!

        cy.origin(KEYCLOAK_DOMAIN, { args }, ({ user }) => {
            cy.get('#username').type(user.username);
            cy.get('#password').type(user.password);
            cy.get('#kc-login').click();
            cy.getCookies().clearCookie('KC_RESTART'); // workaround for issue 22751
            cy.getCookies().clearCookie('KEYCLOAK_REMEMBER_ME'); // workaround 22751
        });

@mjhenkes
Copy link
Member

@sebkasanzew @iomedico-beyer Thanks for the debugging! I'm sure we must be storing cookies that we shouldn't. Would there be any way you guys could provide a reproduction? It will be hard for us to debug the issue without code to work off. 🤔

@iomedico-beyer
Copy link
Author

iomedico-beyer commented Jul 22, 2022

@mjhenkes I want to give you something to play with, but run into a strange problem.

https://edc6.iomedico.de/
cypress22751
cypress22751Pass

Expected: 403, because authenticated but not authorized.

But coming from that domain cy.get('#username') fails, though it matches when trying manually in interactive mode (cypress open). Any idea? (using Cypress 10.3.1)

And Cypress says “The Snapshot is Missing. Displaying Current State Of The Dom.”

@agebhardtsbauer
Copy link

agebhardtsbauer commented Jul 22, 2022

I am also experiencing this issue

Edit1:

I have also confirmed that my session is storing 3 additional cookies in 10.3+ when compared with 10.2

I have duplicates of JSESSIONID, NSC_dmjfouqpsubm, and SREINGRESS

Our application implements a third party SSO from IAM

@cypress-bot cypress-bot bot added stage: investigating Someone from Cypress is looking into this and removed stage: awaiting response Potential fix was proposed; awaiting response labels Jul 22, 2022
@mjhenkes
Copy link
Member

@iomedico-beyer, I setup a sample repo https://github.com/mjhenkes/issue-22751 to check what you were seeing. I think the issue that your running into is that in prod, your login is on the same origin as your site, where as in development using keycloak the login is on a different origin. It seems that in this instance cy.origin is having trouble creating a spec bridge for the same origin as top, this is indicative of a problem but a different problem than the cookie issue.

@iomedico-beyer
Copy link
Author

@mjhenkes I see. Should I report that cy.origin problem? And regarding the original one (22751): Can you work with this (when removing cy.origin) or do you need a HTTP 200 after login?

@mjhenkes
Copy link
Member

mjhenkes commented Jul 26, 2022

@iomedico-beyer, I can take care of logging the new issue. Unfortunately, to recreate this issue i need to have the auth provider to be on a different origin from top and to run through the cy.origin command, other wise cookies will function as normal 😅.

logged: #22942

@mjhenkes
Copy link
Member

I tried testing the basic keycloak docker example, offered by keycloak, but it worked fine. I think it's just too simple.

@mjhenkes mjhenkes removed their assignment Aug 2, 2022
@AtofStryker
Copy link
Contributor

My guess is this is related to the appending logic we have in the request, and possibly doubly set it in the response. @iomedico-beyer I have quite a few issues to go through, but would you maybe be able to get a reproduction together? I have a reproduction issue with keycloak instructions from a few iterations ago for issue #22282 which might serve as a good base. Once that is available, I can quickly verify that the appending logic is causing/not causing issues.

@iomedico-beyer
Copy link
Author

My guess is this is related to the appending logic we have in the request, and possibly doubly set it in the response. @iomedico-beyer I have quite a few issues to go through, but would you maybe be able to get a reproduction together? I have a reproduction issue with keycloak instructions from a few iterations ago for issue #22282 which might serve as a good base. Once that is available, I can quickly verify that the appending logic is causing/not causing issues.

Sorry, but I am currently very busy with other stuff. Maybe @sebkasanzew or @agebhardtsbauer?

@sebkasanzew
Copy link

@iomedico-beyer I'll try to build a repo with a reproducible error tomorrow, but can't promise anything.

sebkasanzew added a commit to sebkasanzew/cypress-issue-22751 that referenced this issue Aug 6, 2022
@sebkasanzew
Copy link

@AtofStryker Here is a repo with the reproducible error https://github.com/Sebkasanzew/cypress-issue-22751.

It was easier than I thought 😅. Thanks for the template @iomedico-beyer .

@AtofStryker
Copy link
Contributor

@sebkasanzew I was able to clone your reproduction and was able to reproduce the issue. I am pretty sure I know what the issue is, as we prepend updated cookies to the request and do not remove the stale values. This also explains why the cookie size is just about double on the request. We likely need to adapt this logic to override the previous cookie values, which should lead to expected behavior.

@AtofStryker AtofStryker added type: bug Reproducible Can be reproduced topic: cy.origin Problems or enhancements related to cy.origin command labels Aug 8, 2022
@cypress-bot cypress-bot bot added stage: routed to e2e-auth and removed stage: investigating Someone from Cypress is looking into this labels Aug 8, 2022
@AtofStryker AtofStryker removed their assignment Aug 8, 2022
@AtofStryker AtofStryker self-assigned this Aug 17, 2022
@mschile mschile added triage and removed triage labels Aug 18, 2022
@AtofStryker AtofStryker linked a pull request Aug 25, 2022 that will close this issue
4 tasks
@emilyrohrbough emilyrohrbough changed the title cy.session broken after updating from 10.2 to 10.3 Prepending Stale Cookies to requests after 10.3 Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reproducible Can be reproduced topic: cookies 🍪 topic: cy.origin Problems or enhancements related to cy.origin command type: bug
Projects
None yet
8 participants