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

Duplicate Session Save Operations During Login with Session Regeneration #1017

Open
kapitalistn opened this issue Feb 23, 2024 · 0 comments
Open

Comments

@kapitalistn
Copy link

kapitalistn commented Feb 23, 2024

Issue Description:

In a NestJS application utilizing Express-Session for session management and Passport for authentication, I've encountered an issue where sessions are saved twice during the authentication process. This behavior traces back to the session regeneration process implemented in Passport's sessionmanager.js, intended to mitigate session fixation attacks. While session regeneration is a critical security measure, it leads to an additional, seemingly redundant session save operation due to express-session's automatic saving mechanism.

SessionManager.prototype.logIn = function(req, user, options, cb) {
  if (typeof options == 'function') {
    cb = options;
    options = {};
  }
  options = options || {};
  
  if (!req.session) { return cb(new Error('Login sessions require session support. Did you forget to use `express-session` middleware?')); }
  
  var self = this;
  var prevSession = req.session;
  
  // regenerate the session, which is good practice to help
  // guard against forms of session fixation
  req.session.regenerate(function(err) {
    if (err) {
      return cb(err);
    }
    
    self._serializeUser(user, req, function(err, obj) {
      if (err) {
        return cb(err);
      }
      if (options.keepSessionInfo) {
        merge(req.session, prevSession);
      }
      if (!req.session[self._key]) {
        req.session[self._key] = {};
      }
      // store user information in session, typically a user id
      req.session[self._key].user = obj;
      // save the session before redirection to ensure page
      // load does not happen before session is saved
      req.session.save(function(err) {
        if (err) {
          return cb(err);
        }
        cb();
      });
      })}
  );
}

This snippet illustrates the explicit call to req.session.save after session regeneration and user serialization, which is part of Passport's login process. Although necessary for security, this process, coupled with express-session's automatic save at the response cycle's end, results in the session being persisted twice.

Steps to Reproduce:

  1. Create a NestJS application with Express integration.
  2. Configure express-session and Passport for session management and authentication.
  3. Implement a login endpoint that authenticates a user and regenerates the session upon successful authentication.
  4. Observe the session save operations, which can be tracked via logging or breakpoints.

Expected Behavior:

The session should only be saved once after regeneration and any updates, without an additional automatic save operation triggered by express-session.

Actual Behavior:
The session undergoes two save operations: initially, after explicit regeneration and user serialization, and again, automatically, due to express-session detecting session changes.

Possible Implications:
This redundancy may lead to increased performance overhead and could complicate session management in high-traffic applications.

Environment:

  • NestJS version: 10.3.3
  • Passport version: 0.7.0
  • express-session version: 1.18.0
  • Node.js version: 20.11.0
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

No branches or pull requests

1 participant