Merge pull request #848 from jaredhanson/no-monkeypatch

No monkeypatch

lib/authenticator.js

@@ -17,7 +17,6 @@ function Authenticator() {
   this._deserializers = [];
   this._infoTransformers = [];
   this._framework = null;
-  this._userProperty = 'user';
 
   this.init();
 }
@@ -29,7 +28,7 @@ function Authenticator() {
  */
 Authenticator.prototype.init = function() {
   this.framework(require('./framework/connect')());
-  this.use(new SessionStrategy(this.deserializeUser.bind(this)));
+  this.use(new SessionStrategy({ key: this._key }, this.deserializeUser.bind(this)));
   this._sm = new SessionManager({ key: this._key }, this.serializeUser.bind(this));
 };
 
@@ -128,8 +127,6 @@ Authenticator.prototype.framework = function(fw) {
  */
 Authenticator.prototype.initialize = function(options) {
   options = options || {};
-  this._userProperty = options.userProperty || 'user';
-
   return this._framework.initialize(this, options);
 };
 

lib/framework/connect.js

@@ -8,32 +8,15 @@ var initialize = require('../middleware/initialize')
  * Framework support for Connect/Express.
  *
  * This module provides support for using Passport with Express.  It exposes
- * middleware that conform to the `fn(req, res, next)` signature and extends
- * Node's built-in HTTP request object with useful authentication-related
- * functions.
+ * middleware that conform to the `fn(req, res, next)` signature.
  *
  * @return {Object}
  * @api protected
  */
 exports = module.exports = function() {
 
-  // HTTP extensions.
-  exports.__monkeypatchNode();
-
   return {
     initialize: initialize,
     authenticate: authenticate
   };
 };
-
-exports.__monkeypatchNode = function() {
-  var http = require('http');
-  var IncomingMessageExt = require('../http/request');
-
-  http.IncomingMessage.prototype.login =
-  http.IncomingMessage.prototype.logIn = IncomingMessageExt.logIn;
-  http.IncomingMessage.prototype.logout =
-  http.IncomingMessage.prototype.logOut = IncomingMessageExt.logOut;
-  http.IncomingMessage.prototype.isAuthenticated = IncomingMessageExt.isAuthenticated;
-  http.IncomingMessage.prototype.isUnauthenticated = IncomingMessageExt.isUnauthenticated;
-};

lib/http/request.js

@@ -1,10 +1,3 @@
-/**
- * Module dependencies.
- */
-//var http = require('http')
-//  , req = http.IncomingMessage.prototype;
-
-
 var req = exports = module.exports = {};
 
 /**
@@ -35,10 +28,7 @@ req.logIn = function(user, options, done) {
   }
   options = options || {};
 
-  var property = 'user';
-  if (this._passport && this._passport.instance) {
-    property = this._passport.instance._userProperty || 'user';
-  }
+  var property = this._userProperty || 'user';
   var session = (options.session === undefined) ? true : options.session;
 
   this[property] = user;
@@ -63,10 +53,7 @@ req.logIn = function(user, options, done) {
  */
 req.logout =
 req.logOut = function() {
-  var property = 'user';
-  if (this._passport && this._passport.instance) {
-    property = this._passport.instance._userProperty || 'user';
-  }
+  var property = this._userProperty || 'user';
 
   this[property] = null;
   if (this._passport) {
@@ -81,11 +68,7 @@ req.logOut = function() {
  * @api public
  */
 req.isAuthenticated = function() {
-  var property = 'user';
-  if (this._passport && this._passport.instance) {
-    property = this._passport.instance._userProperty || 'user';
-  }
-
+  var property = this._userProperty || 'user';
   return (this[property]) ? true : false;
 };
 

lib/middleware/authenticate.js

@@ -92,12 +92,6 @@ module.exports = function authenticate(passport, name, options, callback) {
   }
 
   return function authenticate(req, res, next) {
-    if (http.IncomingMessage.prototype.logIn
-        && http.IncomingMessage.prototype.logIn !== IncomingMessageExt.logIn) {
-      require('../framework/connect').__monkeypatchNode();
-    }
-
-
     // accumulator for failures from each strategy in the chain
     var failures = [];
 

lib/middleware/initialize.js

@@ -1,3 +1,9 @@
+/**
+ * Module dependencies.
+ */
+var IncomingMessageExt = require('../http/request');
+
+
 /**
  * Passport initialization.
  *
@@ -39,17 +45,24 @@
  * @return {Function}
  * @api public
  */
-module.exports = function initialize(passport) {
+module.exports = function initialize(passport, options) {
+  options = options || {};
 
   return function initialize(req, res, next) {
+    req.login =
+    req.logIn = IncomingMessageExt.logIn;
+    req.logout =
+    req.logOut = IncomingMessageExt.logOut;
+    req.isAuthenticated = IncomingMessageExt.isAuthenticated;
+    req.isUnauthenticated = IncomingMessageExt.isUnauthenticated;
+
+    if (options.userProperty) {
+      req._userProperty = options.userProperty;
+    }
+
     req._passport = {};
     req._passport.instance = passport;
 
-    if (req.session && req.session[passport._key]) {
-      // load data from existing session
-      req._passport.session = req.session[passport._key];
-    }
-
     next();
   };
 };

lib/sessionmanager.js

@@ -15,22 +15,23 @@ SessionManager.prototype.logIn = function(req, user, cb) {
     if (err) {
       return cb(err);
     }
-    if (!req._passport.session) {
-      req._passport.session = {};
-    }
-    req._passport.session.user = obj;
+    // TODO: Error if session isn't available here.
     if (!req.session) {
       req.session = {};
     }
-    req.session[self._key] = req._passport.session;
+    if (!req.session[self._key]) {
+      req.session[self._key] = {};
+    }
+    req.session[self._key].user = obj;
     cb();
   });
 }
 
 SessionManager.prototype.logOut = function(req, cb) {
-  if (req._passport && req._passport.session) {
-    delete req._passport.session.user;
+  if (req.session && req.session[this._key]) {
+    delete req.session[this._key].user;
   }
+
   cb && cb();
 }
 

lib/strategies/session.js

@@ -20,6 +20,7 @@ function SessionStrategy(options, deserializeUser) {
 
   Strategy.call(this);
   this.name = 'session';
+  this._key = options.key || 'passport';
   this._deserializeUser = deserializeUser;
 }
 
@@ -47,8 +48,8 @@ SessionStrategy.prototype.authenticate = function(req, options) {
 
   var self = this, 
       su;
-  if (req._passport.session) {
-    su = req._passport.session.user;
+  if (req.session[this._key]) {
+    su = req.session[this._key].user;
   }
 
   if (su || su === 0) {
@@ -60,10 +61,9 @@ SessionStrategy.prototype.authenticate = function(req, options) {
     this._deserializeUser(su, req, function(err, user) {
       if (err) { return self.error(err); }
       if (!user) {
-        delete req._passport.session.user;
+        delete req.session[self._key].user;
       } else {
-        // TODO: Remove instance access
-        var property = req._passport.instance._userProperty || 'user';
+        var property = req._userProperty || 'user';
         req[property] = user;
       }
       self.pass();

test/authenticator.middleware.test.js

@@ -35,8 +35,8 @@ describe('Authenticator', function() {
         expect(error).to.be.undefined;
       });
 
-      it('should set user property on authenticator', function() {
-        expect(passport._userProperty).to.equal('user');
+      it('should not set user property on request', function() {
+        expect(request._userProperty).to.be.undefined;
       });
 
       it('should not initialize namespace within session', function() {
@@ -48,10 +48,6 @@ describe('Authenticator', function() {
         expect(request._passport.instance).to.be.an.instanceOf(Authenticator);
         expect(request._passport.instance).to.equal(passport);
       });
-
-      it('should not expose session storage on internal request property', function() {
-        expect(request._passport.session).to.be.undefined;
-      });
     });
 
     describe('handling a request with custom user property', function() {
@@ -75,8 +71,8 @@ describe('Authenticator', function() {
         expect(error).to.be.undefined;
       });
 
-      it('should set user property on authenticator', function() {
-        expect(passport._userProperty).to.equal('currentUser');
+      it('should set user property on request', function() {
+        expect(request._userProperty).to.equal('currentUser');
       });
 
       it('should not initialize namespace within session', function() {
@@ -88,10 +84,6 @@ describe('Authenticator', function() {
         expect(request._passport.instance).to.be.an.instanceOf(Authenticator);
         expect(request._passport.instance).to.equal(passport);
       });
-
-      it('should not expose session storage on internal request property', function() {
-        expect(request._passport.session).to.be.undefined;
-      });
     });
 
   });
@@ -278,8 +270,9 @@ describe('Authenticator', function() {
 
             req._passport = {};
             req._passport.instance = {};
-            req._passport.session = {};
-            req._passport.session.user = '123456';
+            req.session = {};
+            req.session['passport'] = {};
+            req.session['passport'].user = '123456';
           })
           .next(function(err) {
             error = err;
@@ -298,8 +291,8 @@ describe('Authenticator', function() {
       });
 
       it('should maintain session', function() {
-        expect(request._passport.session).to.be.an('object');
-        expect(request._passport.session.user).to.equal('123456');
+        expect(request.session['passport']).to.be.an('object');
+        expect(request.session['passport'].user).to.equal('123456');
       });
     });