From 3f1b6dbe7d720e0735e8c4e4be63858b336d40d7 Mon Sep 17 00:00:00 2001 From: Maya Date: Mon, 29 Apr 2019 17:36:06 +0200 Subject: [PATCH] Remove node monkeypatching; closes #9 --- lib/framework/connect.js | 20 +------ lib/middleware/authenticate.js | 9 --- lib/middleware/initialize.js | 9 +++ test/http/request.test.js | 103 +++++++++------------------------ 4 files changed, 36 insertions(+), 105 deletions(-) diff --git a/lib/framework/connect.js b/lib/framework/connect.js index 77f587cf..cd625249 100644 --- a/lib/framework/connect.js +++ b/lib/framework/connect.js @@ -3,18 +3,14 @@ */ /* eslint-disable camelcase, no-proto, no-shadow */ -const http = require('http'); const initialize = require('../middleware/initialize'); const authenticate = require('../middleware/authenticate'); -const IncomingMessageExt = require('../http/request'); /** * 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 @@ -22,22 +18,8 @@ const IncomingMessageExt = require('../http/request'); // eslint-disable-next-line no-multi-assign, func-names exports = module.exports = function () { - // HTTP extensions. - exports.__monkeypatchNode(); - return { initialize, authenticate, }; }; - -exports.__monkeypatchNode = function __monkeypatchNode() { - http.IncomingMessage.prototype.logIn = IncomingMessageExt.logIn; - http.IncomingMessage.prototype.login = http.IncomingMessage.prototype.logIn; - - http.IncomingMessage.prototype.logOut = IncomingMessageExt.logOut; - http.IncomingMessage.prototype.logout = http.IncomingMessage.prototype.logOut; - - http.IncomingMessage.prototype.isAuthenticated = IncomingMessageExt.isAuthenticated; - http.IncomingMessage.prototype.isUnauthenticated = IncomingMessageExt.isUnauthenticated; -}; diff --git a/lib/middleware/authenticate.js b/lib/middleware/authenticate.js index 0357b3a0..84dcaaa8 100644 --- a/lib/middleware/authenticate.js +++ b/lib/middleware/authenticate.js @@ -5,10 +5,7 @@ /* eslint-disable camelcase, no-proto, no-shadow */ const http = require('http'); -const IncomingMessageExt = require('../http/request'); const AuthenticationError = require('../errors/authenticationerror'); -const connect = require('../framework/connect'); - /** * Authenticates requests. @@ -96,12 +93,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) { - connect.__monkeypatchNode(); - } - - // accumulator for failures from each strategy in the chain const failures = []; diff --git a/lib/middleware/initialize.js b/lib/middleware/initialize.js index 1de02a62..17e159b4 100644 --- a/lib/middleware/initialize.js +++ b/lib/middleware/initialize.js @@ -40,6 +40,8 @@ * @api public */ +const IncomingMessageExt = require('../http/request'); + /* eslint-disable no-proto, no-shadow */ module.exports = function initialize(passport) { return function initialize(req, res, next) { @@ -51,6 +53,13 @@ module.exports = function initialize(passport) { req._passport.session = req.session[passport._key]; } + req.logIn = IncomingMessageExt.logIn; + req.login = IncomingMessageExt.logIn; + req.logOut = IncomingMessageExt.logOut; + req.logout = IncomingMessageExt.logOut; + req.isAuthenticated = IncomingMessageExt.isAuthenticated; + req.isUnauthenticated = IncomingMessageExt.isUnauthenticated; + next(); }; }; diff --git a/test/http/request.test.js b/test/http/request.test.js index 1d14ee14..ee6e33af 100644 --- a/test/http/request.test.js +++ b/test/http/request.test.js @@ -5,14 +5,19 @@ const http = require('http'); const Passport = require('../..').Passport; +const initialize = require('../../lib/middleware/initialize'); -require('../../lib/framework/connect').__monkeypatchNode(); - +function setup() { + const passport = new Passport(); + const req = new http.IncomingMessage(); + const middleware = initialize(passport); + middleware(req, {}, () => {}); + return { passport, req }; +} describe('http.ServerRequest', () => { describe('prototoype', () => { - const req = new http.IncomingMessage(); - + const { req } = setup(); it('should be extended with login', () => { expect(req.login).to.be.an('function'); expect(req.login).to.equal(req.logIn); @@ -34,13 +39,8 @@ describe('http.ServerRequest', () => { describe('#login', () => { describe('not establishing a session', () => { - const passport = new Passport(); - - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; + const { req } = setup(); req._passport.session = {}; - let error; before((done) => { @@ -77,14 +77,9 @@ describe('http.ServerRequest', () => { }); describe('not establishing a session and setting custom user property', () => { - const passport = new Passport(); - passport._userProperty = 'currentUser'; - - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; + const { req, passport } = setup(); req._passport.session = {}; - + passport._userProperty = 'currentUser'; let error; before((done) => { @@ -126,11 +121,7 @@ describe('http.ServerRequest', () => { }); describe('not establishing a session and invoked without a callback', () => { - const passport = new Passport(); - - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; + const { req } = setup(); req._passport.session = {}; const user = { id: '1', username: 'root' }; @@ -156,8 +147,7 @@ describe('http.ServerRequest', () => { }); describe('not establishing a session, without passport.initialize() middleware', () => { - const req = new http.IncomingMessage(); - + const { req } = setup(); let error; before((done) => { @@ -189,16 +179,10 @@ describe('http.ServerRequest', () => { }); describe('establishing a session', () => { - const passport = new Passport(); + const { req, passport } = setup(); passport.serializeUser((user, done) => { done(null, user.id); }); - - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; - req._passport.session = {}; - let error; before((done) => { @@ -234,17 +218,12 @@ describe('http.ServerRequest', () => { }); describe('establishing a session and setting custom user property', () => { - const passport = new Passport(); + const { req, passport } = setup(); passport.serializeUser((user, done) => { done(null, user.id); }); passport._userProperty = 'currentUser'; - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; - req._passport.session = {}; - let error; before((done) => { @@ -285,16 +264,12 @@ describe('http.ServerRequest', () => { }); describe('encountering an error when serializing to session', () => { - const passport = new Passport(); + const { req, passport } = setup(); + req._passport.session = {}; passport.serializeUser((user, done) => { done(new Error('something went wrong')); }); - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; - req._passport.session = {}; - let error; before((done) => { @@ -329,28 +304,12 @@ describe('http.ServerRequest', () => { }); }); - describe('establishing a session, without passport.initialize() middleware', () => { - const req = new http.IncomingMessage(); - const user = { id: '1', username: 'root' }; - - it('should throw an exception', () => { - expect(() => { - req.login(user, () => {}); - }).to.throw(Error, 'passport.initialize() middleware not in use'); - }); - }); - describe('establishing a session, but not passing a callback argument', () => { - const passport = new Passport(); + const { req, passport } = setup(); passport.serializeUser((user, done) => { done(null, user.id); }); - const req = new http.IncomingMessage(); - req._passport = {}; - req._passport.instance = passport; - req._passport.session = {}; - const user = { id: '1', username: 'root' }; it('should throw an exception', () => { @@ -364,12 +323,8 @@ describe('http.ServerRequest', () => { describe('#logout', () => { describe('existing session', () => { - const passport = new Passport(); - - const req = new http.IncomingMessage(); + const { req } = setup(); req.user = { id: '1', username: 'root' }; - req._passport = {}; - req._passport.instance = passport; req._passport.session = {}; req._passport.session.user = '1'; @@ -394,12 +349,8 @@ describe('http.ServerRequest', () => { }); describe('existing session and clearing custom user property', () => { - const passport = new Passport(); - - const req = new http.IncomingMessage(); + const { req } = setup(); req.currentUser = { id: '1', username: 'root' }; - req._passport = {}; - req._passport.instance = passport; req._passport.instance._userProperty = 'currentUser'; req._passport.session = {}; req._passport.session.user = '1'; @@ -425,7 +376,7 @@ describe('http.ServerRequest', () => { }); describe('existing session, without passport.initialize() middleware', () => { - const req = new http.IncomingMessage(); + const { req } = setup(); req.user = { id: '1', username: 'root' }; req.logout(); @@ -447,7 +398,7 @@ describe('http.ServerRequest', () => { describe('#isAuthenticated', () => { describe('with a user', () => { - const req = new http.IncomingMessage(); + const { req } = setup(); req.user = { id: '1', username: 'root' }; it('should be authenticated', () => { @@ -459,10 +410,8 @@ describe('http.ServerRequest', () => { }); describe('with a user set on custom property', () => { - const req = new http.IncomingMessage(); + const { req } = setup(); req.currentUser = { id: '1', username: 'root' }; - req._passport = {}; - req._passport.instance = {}; req._passport.instance._userProperty = 'currentUser'; it('should be authenticated', () => { @@ -474,7 +423,7 @@ describe('http.ServerRequest', () => { }); describe('without a user', () => { - const req = new http.IncomingMessage(); + const { req } = setup(); it('should not be authenticated', () => { // eslint-disable-next-line no-unused-expressions @@ -485,7 +434,7 @@ describe('http.ServerRequest', () => { }); describe('with a null user', () => { - const req = new http.IncomingMessage(); + const { req } = setup(); req.user = null; it('should not be authenticated', () => {