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 insecure cookie #151

Merged
merged 4 commits into from
Sep 15, 2020
Merged

Conversation

bjarkebm
Copy link
Contributor

@bjarkebm bjarkebm commented Aug 4, 2020

Description

Currently, the cookie set is always Secure unless running on localhost or when NODE_ENV is not production.
When deploying code to a non-https site that has NODE_ENV=production, setting the cookie will fail.
Our use-case is deploying PRs to a dynamically created environment, where we test that everything is working as in production (thus, we have NODE_ENV=production), but the environment is a random url, and so cannot have https (at least easily).

The proposed solution is to allow setting an environment variable that turns off setting the cookie as secure, AUTH0_ALLOW_INSECURE_COOKIE=true

Testing

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

src/utils/cookies.ts Outdated Show resolved Hide resolved
tests/utils/cookies.test.ts Outdated Show resolved Hide resolved
@bjarkebm
Copy link
Contributor Author

I decided to rearrange the tests a little to make it clearer when the tests are for production environment (all tests but one), and for when they are not-production (just one test)

@sandrinodimattia
Copy link
Member

Excellent, thank you! 🙇 Will be available in the next release.

@sandrinodimattia sandrinodimattia merged commit 885a76a into auth0:master Sep 15, 2020
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.

None yet

3 participants