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

Allow setting cookies from other origins #197

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

Conversation

justinr1234
Copy link
Contributor

@justinr1234 justinr1234 commented Sep 2, 2020

This allows cookies to be sent from an origin other than the primary origin. This can be useful if you want to have multiple sites hosted from different sub-domains but using the same API. Additionally, in development mode, this allows you to use a separate server (such as Ionic dev server) to go against the same API.

There shouldn't be any security concerns as this by default uses the same origin. It only allows changes if you specifically want to whitelist a particular domain to allow cookies from there.

Pass in allowed values as a comma-separated string:

SESSION_ALLOWED_ORIGINS=http://localhost:8100,https://sub.mydomain.com

) || [])
);
}
if (req.isSameOrigin || origins.includes(req.get("Origin") || "")) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should rename isSameOrigin to isAllowedOrigin and move this logic there.

Also, please process environmental variables in the root scope, near the top of the file, rather than within the middleware itself. At the moment this replace/split has to be executed for every single request which is an unnecessary cost.

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.

2 participants