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

SameSite cookie config not respected by skipSilentLogin cookie #315

Closed
andreasenberg opened this issue Jan 19, 2022 · 7 comments · Fixed by #317
Closed

SameSite cookie config not respected by skipSilentLogin cookie #315

andreasenberg opened this issue Jan 19, 2022 · 7 comments · Fixed by #317
Labels
bug Something isn't working

Comments

@andreasenberg
Copy link

Describe the problem

An application embedded within an iframe gets stuck in a redirect loop when attempting to do silent log in and user is not logged in at IDP.

The issue seems to be related to the fact that the skipSilentLogin cookie is missing a same site attribute and thus is defaulted to Lax by the browser. As the application is embedded within an iframe this cookie will be blocked and not sent to the application backend causing another silent log in attempt.

What was the expected behavior?

Silent log in should be attempted once and not on subsequent page requests.

I was expecting the skipSilentLogin cookie to honor the sameSite attribute from config.session.cookie.sameSite. That is unless there is a good reason not to, if so help me understand why.

I've forked the repository myself to see if changes to pickup the sameSite config for skipSilentLogin will solve this issue and it seems to be the case.

Reproduction

A reproduction would look something like this

  1. Have an application embedded within an iframe, in our case this is an app embedded within Teams.
  2. Make sure the user is not logged in at the IDP (as this will trigger additional attempts to log in)
  3. Access the application in a way that would trigger a silent log in and where it should end up on the same path after log in.
  4. Observe the redirects from the page in 3) -> auth endpoint (for silent log in) -> callback with error (as user is not logged in)
  5. A too many redirects error will be shown in the browser.

Environment

  • Version of this library used: 2.5.2
  • Any other relevant information you think would be useful: Issue detected in Chrome Version 97.0.4692.71 but I suspect all Chrome versions where SameSite is defaulted to Lax will result in this behaviour.
@antonhedstrom
Copy link

+1

@linuws
Copy link

linuws commented Jan 25, 2022

Yes, this would be great to get in place! Can we get a PR up with the mentioned fix in the original issue or do we have any other input from the core maintainers of the project? Thanks

@adamjmcgrath
Copy link
Contributor

Hi @linuws @antonhedstrom @andreasenberg

Have raised a pr #317

You can test it out by installing npm i "auth0/express-openid-connect#iframe-skip-silent-login"

@adamjmcgrath adamjmcgrath added the bug Something isn't working label Jan 26, 2022
@linuws
Copy link

linuws commented Jan 27, 2022

Hi @adamjmcgrath and thank you very much for looking into this. I have testet and verified that it worked. I can see the settings being present in the cookie and there is no redirect-loop anymore. 👍

@adamjmcgrath
Copy link
Contributor

Great - thanks @linuws! Once #317 is merged I'll do a release and update this thread.

@andreasenberg
Copy link
Author

Hi @adamjmcgrath. Thank you for looking into this, appreciate it! Looking forward to that release! 🙂

Thanks @linuws for your help testing the provided fix.

@adamjmcgrath
Copy link
Contributor

No problem - this got released in https://github.com/auth0/express-openid-connect/releases/tag/v2.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants