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

[SDK-3808] Optionally sign the session store cookie #419

Merged
merged 8 commits into from
Nov 16, 2022

Conversation

adamjmcgrath
Copy link
Contributor

Description

Add an option signSessionStoreCookie (default false) - when true and using a custom session store, the session store cookie will be signed. e.g.

// session store => {"123abc": { ... session ... } }

// before
Cookie: appSession=abc123

// after (with cryptographically secure signature)
Cookie: appSession=abc123.b1825178d1f21747e60d329c2cfc21a9c

The default session id is a cryptographically secure random value, but this is useful when you want to override genid and you are only generating a unique value, not a cryptographically secure random value.

Also adding a requireSignedSessionStoreCookie config option to allow zero down time migration to signed session store cookies.

fafa0e0 The first commit is just moving some code into another file to make it easier to share
d1c3dc0 This is the main commit that implements signed session store cookies

References

express-session docs
genid

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 the default branch

@adamjmcgrath adamjmcgrath added the review:medium Medium review label Nov 14, 2022
@adamjmcgrath adamjmcgrath requested a review from a team as a code owner November 14, 2022 10:42
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
lib/crypto.js Outdated Show resolved Hide resolved
lib/crypto.js Outdated
return !!JWS.verify(
flattenedJWSFromCookie(cookie, value, signature),
keystore,
{ algorithm: 'HS256', crit: ['b64'] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way to not duplicate these values that are already used in the header constant?

Copy link
Contributor Author

@adamjmcgrath adamjmcgrath Nov 16, 2022

Choose a reason for hiding this comment

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

👍 have moved these into constants

Also, noticed a typo - thanks

@adamjmcgrath adamjmcgrath merged commit 6641d36 into master Nov 16, 2022
@adamjmcgrath adamjmcgrath deleted the signed-store-cookie branch November 16, 2022 16:19
@adamjmcgrath adamjmcgrath mentioned this pull request Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:medium Medium review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants