Skip to content

Commit

Permalink
Merge pull request #18 from passport-next/issue-9
Browse files Browse the repository at this point in the history 
Remove node monkeypatching; closes #9 #15
  • Loading branch information
@rwky
rwky authored Apr 30, 2019
2 parents 8a9dae2 + 3f1b6db commit 7827422
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 105 deletions.
20 changes: 1 addition & 19 deletions lib/framework/connect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,23 @@
*/
/* 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
*/

// 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;
};
9 changes: 0 additions & 9 deletions lib/middleware/authenticate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 = [];

Expand Down
9 changes: 9 additions & 0 deletions lib/middleware/initialize.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
};
};
103 changes: 26 additions & 77 deletions test/http/request.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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' };
Expand All @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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';

Expand All @@ -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';
Expand All @@ -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();
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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
Expand All @@ -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', () => {
Expand Down

0 comments on commit 7827422

Please sign in to comment.