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

Include cookie-related CORS config in passport-example #3864

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peey
Copy link

@peey peey commented Apr 2, 2021

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other: update an example

Current behavior

Example works without problems, but implicitly relies on same-origin cookie sending behaviour which isn't always the case in real implementations.

New behavior

Makes the reliance on same-origin cookie behaviour explicit and add sample CORS configuration that while not strictly needed in the example, are useful when adapting the example to real world scenario. Makes the example a tad bit more "complete".

Other Info

Adding this is perhaps important because the configuration affects a library (express-session) that users may not directly work with, so may not have knowledge of correct configuration. Moreover, the library itself is centric to express so it doesn't offer socket.io configuration information, so this might be the best place to put this.

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.

1 participant