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

feat: add support to dynamic cookie options generation #1027

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lincond
Copy link

@lincond lincond commented Feb 26, 2025

This patch introduces support for dynamic cookie generation in express-session. Instead of only accepting a static object for the options.cookie parameter, the patch allows developers to supply a function that returns a cookie options object. This enables dynamic generation of cookie settings on a per-request basis.

Changes Made:

- JSDoc Update:
The type for options.cookie has been updated from Object to Object|Function to reflect the new functionality

- Dynamic Cookie Options Resolution:
In the session middleware, when generating a new session, the code now checks if cookieOptions is a function. If it is, it calls the function with the current request (req) as an argument to retrieve the cookie options. Otherwise, it uses the static options as before.

req.session.cookie = new Cookie(
  typeof cookieOptions === 'function' ? cookieOptions(req) : cookieOptions
);

- Path Mismatch Check:
The cookie path validation now also resolves the cookie options dynamically to ensure that the generated cookie options are used in the check.

var resolvedCookieOptions = typeof cookieOptions === 'function' ? cookieOptions(req) : cookieOptions;
if (originalPath.indexOf(resolvedCookieOptions.path || '/') !== 0) {
  // Handle pathname mismatch
  debug('pathname mismatch');
  next();
  return;
}

Motivation and Use Case:

This change was made to address scenarios where the cookie configuration (e.g., the cookie path) needs to be determined dynamically based on the incoming request. For example, in multi-tenant applications where different URL paths should result in different cookie configurations, this enhancement allows the session middleware to generate the correct cookie settings on the fly.

Testing:

To test this change, you can set the options.cookie to a function that returns a cookie options object based on the request. Verify that:

  • When a function is provided, it correctly generates the cookie settings for each request.
  • When a static object is provided, the behavior remains unchanged.

Feel free to provide any feedback or suggestions for further improvements.

@bjohansebas
Copy link
Member

I like the idea, although for it to be accepted, you need to add tests to verify the behaviors.

@lincond
Copy link
Author

lincond commented Feb 26, 2025

I like the idea, although for it to be accepted, you need to add tests to verify the behaviors.

I'll work on it!

@lincond
Copy link
Author

lincond commented Feb 26, 2025

@bjohansebas Added tests to validate dynamic cookie generation via a function for the cookie option.

@raphendyr
Copy link

As an observation, this is targeting v1/master and there is now some work on v2 with breaking changes. While this is not such a thing, it still might be good to know.

I started to prototype idea to put the whole cookie stuff behind class/api/something, so that people could swap it over or reimplement for custom needs. The prototype is here: raphendyr@85c9bd9

When working on that, I keep this PR's need in mind too (where this merged or not).

@lincond
Copy link
Author

lincond commented Mar 17, 2025

@raphendyr good observation. This feature is already in use in one of the projects I work on and is working well.

As I didn't know how long until v2 then I submitted it straight to v1 as it would be great to install the library with this functionality straight from npm as soon as possible.

Your idea of separating the logic from the cookie sounds interesting. If you need any help porting this functionality to v2 I'm happy to help.

@lincond
Copy link
Author

lincond commented Mar 17, 2025

@bjohansebas added an example of how to use the callback as cookie options to README.md in addition to the tests. If you think you need anything else, let me know

@lincond lincond changed the title feat: add support to dynamic cookie generation feat: add support to dynamic cookie options generation Mar 17, 2025
@bjohansebas
Copy link
Member

I guess it could go into version 1 and be backported to the v2 branch if this gets merged, What do you think, @UlisesGascon?

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.

3 participants