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

Issue 227 - allow custom session id generation #252

Merged
merged 2 commits into from
Jun 29, 2021

Conversation

nholik
Copy link
Contributor

@nholik nholik commented Jun 27, 2021

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

This commit address issue
227
allowing an implementation of genid to be used when a custom session store is used. This is fully compatible with the existing implementation and aligns with how expression-session allows for overriding the way session ids are generated. Note because of how session ids are generated in this library, it is not possible to simply fall into the custom store's genid function.

In implementing these changes, the following was done:

  • Moved the default session id generation into config to make it clear what said default is
  • Added a new parameter genid compatible with how it works for express-session
  • Updated the appsession code to take advantage of the provided value and fallback to the default.
  • Added unit tests validating the genid parameter being used when specified and not having any effect on the encrypted cookie
  • Reordered the spread operators in the appsession custom store tests so that overrides are respected. This changed no other existing tests.
  • Updated documentation with the additional, optional parameter along with links to the compatible usage in express-session.

References

See discussion in this issue #227
Also https://github.com/expressjs/session#genid

Testing

Tested with node 14

  • 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

This commit address [issue
227](auth0#227)
allowing an implementation of genid to be used when a custom session
store is used.  This is fully compatibile with the existing
implementation and aligns with how expression-session allows for
overriding the way session ids are generated. Note because of how
session ids are generated in this library, it is not possible to simply
fall into the custom store's genid function.

In implementing these changes, the following was done:
- Moved the default session id generation into config to make it clear
what said default is
- Added a new parameter genid compatible with how it works for
express-session
- Updated the appsession code to take advantage of the provided value and
fallback to the default.
- Added unit tests validating the genid parameter being used when
specified and not having any effect on the encrypted cookie
implementation.
- Reordered the spread operators in the appsession custom store tests
so that overrides are respected.  This changed no other existing tests.
- Updated documentation with the additional, optional parameter along
with links to the compatabile usage in express-session.
@nholik nholik requested a review from a team as a code owner June 27, 2021 17:47
@adamjmcgrath adamjmcgrath merged commit fc5fcb7 into auth0:master Jun 29, 2021
@adamjmcgrath
Copy link
Contributor

Thanks @nholik!

@nholik nholik deleted the enhancement-custom-id-gen-issue-227 branch June 29, 2021 15:11
@adamjmcgrath adamjmcgrath mentioned this pull request Jul 14, 2021
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