Merge pull request #138 from jaredhanson/state-opt

Automatic state support

lib/state/pkcesession.js

@@ -39,13 +39,14 @@ PKCESessionStore.prototype.store = function(req, verifier, state, meta, callback
   if (!req.session) { return callback(new Error('OAuth 2.0 authentication requires session support when using state. Did you forget to use express-session middleware?')); }
 
   var key = this._key;
-  var state = {
+  var sstate = {
     handle: uid(24),
     code_verifier: verifier
   };
+  if (state) { sstate.state = state; }
   if (!req.session[key]) { req.session[key] = {}; }
-  req.session[key].state = state;
-  callback(null, state.handle);
+  req.session[key].state = sstate;
+  callback(null, sstate.handle);
 };
 
 /**
@@ -81,7 +82,7 @@ PKCESessionStore.prototype.verify = function(req, providedState, callback) {
     return callback(null, false, { message: 'Invalid authorization request state.' });
   }
 
-  return callback(null, state.code_verifier);
+  return callback(null, state.code_verifier, state.state);
 };
 
 // Expose constructor.

lib/state/store.js

@@ -0,0 +1,88 @@
+var uid = require('uid2');
+
+/**
+ * Creates an instance of `SessionStore`.
+ *
+ * This is the state store implementation for the OAuth2Strategy used when
+ * the `state` option is enabled.  It generates a random state and stores it in
+ * `req.session` and verifies it when the service provider redirects the user
+ * back to the application.
+ *
+ * This state store requires session support.  If no session exists, an error
+ * will be thrown.
+ *
+ * Options:
+ *
+ *   - `key`  The key in the session under which to store the state
+ *
+ * @constructor
+ * @param {Object} options
+ * @api public
+ */
+function SessionStore(options) {
+  if (!options.key) { throw new TypeError('Session-based state store requires a session key'); }
+  this._key = options.key;
+}
+
+/**
+ * Store request state.
+ *
+ * This implementation simply generates a random string and stores the value in
+ * the session, where it will be used for verification when the user is
+ * redirected back to the application.
+ *
+ * @param {Object} req
+ * @param {Function} callback
+ * @api protected
+ */
+SessionStore.prototype.store = function(req, state, meta, callback) {
+  if (!req.session) { return callback(new Error('OAuth 2.0 authentication requires session support when using state. Did you forget to use express-session middleware?')); }
+
+  var key = this._key;
+  var sstate = {
+    handle: uid(24)
+  };
+  if (state) { sstate.state = state; }
+  if (!req.session[key]) { req.session[key] = {}; }
+  req.session[key].state = sstate;
+  callback(null, sstate.handle);
+};
+
+/**
+ * Verify request state.
+ *
+ * This implementation simply compares the state parameter in the request to the
+ * value generated earlier and stored in the session.
+ *
+ * @param {Object} req
+ * @param {String} providedState
+ * @param {Function} callback
+ * @api protected
+ */
+SessionStore.prototype.verify = function(req, providedState, callback) {
+  if (!req.session) { return callback(new Error('OAuth 2.0 authentication requires session support when using state. Did you forget to use express-session middleware?')); }
+
+  var key = this._key;
+  if (!req.session[key]) {
+    return callback(null, false, { message: 'Unable to verify authorization request state.' });
+  }
+
+  var state = req.session[key].state;
+  if (!state) {
+    return callback(null, false, { message: 'Unable to verify authorization request state.' });
+  }
+
+  delete req.session[key].state;
+  if (Object.keys(req.session[key]).length === 0) {
+    delete req.session[key];
+  }
+
+  if (state.handle !== providedState) {
+    return callback(null, false, { message: 'Invalid authorization request state.' });
+  }
+
+  return callback(null, true, state.state);
+};
+
+// Expose constructor.
+module.exports = SessionStore;

lib/strategy.js

@@ -7,9 +7,10 @@ var passport = require('passport-strategy')
   , util = require('util')
   , utils = require('./utils')
   , OAuth2 = require('oauth').OAuth2
-  , NullStateStore = require('./state/null')
-  , SessionStateStore = require('./state/session')
-  , PKCESessionStateStore = require('./state/pkcesession')
+  , NullStore = require('./state/null')
+  , NonceStore = require('./state/session')
+  , StateStore = require('./state/store')
+  , PKCEStateStore = require('./state/pkcesession')
   , AuthorizationError = require('./errors/authorizationerror')
   , TokenError = require('./errors/tokenerror')
   , InternalOAuthError = require('./errors/internaloautherror');
@@ -101,15 +102,15 @@ function OAuth2Strategy(options, verify) {
   this._pkceMethod = (options.pkce === true) ? 'S256' : options.pkce;
   this._key = options.sessionKey || ('oauth2:' + url.parse(options.authorizationURL).hostname);
 
-  if (options.store) {
+  if (options.store && typeof options.store == 'object') {
     this._stateStore = options.store;
+  } else if (options.store) {
+    this._stateStore = options.pkce ? new PKCEStateStore({ key: this._key }) : new StateStore({ key: this._key });
+  } else if (options.state) {
+    this._stateStore = options.pkce ? new PKCEStateStore({ key: this._key }) : new NonceStore({ key: this._key });
   } else {
-    if (options.state) {
-      this._stateStore = options.pkce ? new PKCESessionStateStore({ key: this._key }) : new SessionStateStore({ key: this._key });
-    } else {
-      if (options.pkce) { throw new TypeError('OAuth2Strategy requires `state: true` option when PKCE is enabled'); }
-      this._stateStore = new NullStateStore();
-    }
+    if (options.pkce) { throw new TypeError('OAuth2Strategy requires `state: true` option when PKCE is enabled'); }
+    this._stateStore = new NullStore();
   }
   this._trustProxy = options.proxy;
   this._passReqToCallback = options.passReqToCallback;
@@ -151,7 +152,8 @@ OAuth2Strategy.prototype.authenticate = function(req, options) {
   var meta = {
     authorizationURL: this._oauth2._authorizeUrl,
     tokenURL: this._oauth2._accessTokenUrl,
-    clientID: this._oauth2._clientId
+    clientID: this._oauth2._clientId,
+    callbackURL: callbackURL
   }
 
   if (req.query && req.query.code) {
@@ -250,7 +252,17 @@ OAuth2Strategy.prototype.authenticate = function(req, options) {
     }
 
     var state = options.state;
-    if (state) {
+    if (state && typeof state == 'string') {
+      // NOTE: In [email protected] and earlier, `state` could be passed as
+      //       an object.  However, it would result in an empty string being
+      //       serialized as the value of the query parameter by `url.format()`,
+      //       effectively ignoring the option.  This implies that `state` was
+      //       only functional when passed as a string value.
+      //
+      //       This fact is taken advantage of here to fall into the `else`
+      //       branch below when `state` is passed as an object.  In that case
+      //       the state will be automatically managed and persisted by the
+      //       state store.
       params.state = state;
 
       var parsed = url.parse(this._oauth2._authorizeUrl, true);
@@ -275,7 +287,9 @@ OAuth2Strategy.prototype.authenticate = function(req, options) {
       try {
         var arity = this._stateStore.store.length;
         if (arity == 5) {
-          this._stateStore.store(req, verifier, undefined, meta, stored);
+          this._stateStore.store(req, verifier, state, meta, stored);
+        } else if (arity == 4) {
+          this._stateStore.store(req, state, meta, stored);
         } else if (arity == 3) {
           this._stateStore.store(req, meta, stored);
         } else { // arity == 2

test/oauth2.pkce.test.js

@@ -89,6 +89,109 @@ describe('OAuth2Strategy', function() {
         expect(request.session['oauth2:www.example.com'].state.handle).to.equal(u.query.state);
         expect(request.session['oauth2:www.example.com'].state.code_verifier).to.have.length(43);
         expect(request.session['oauth2:www.example.com'].state.code_verifier).to.equal('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk');
+        expect(request.session['oauth2:www.example.com'].state.state).to.be.undefined;
+      });
+    });
+
+    describe('handling a request to be redirected for authorization with state', function() {
+      var request, url;
+
+      before(function(done) {
+        chai.passport.use(strategy)
+          .redirect(function(u) {
+            url = u;
+            done();
+          })
+          .req(function(req) {
+            request = req;
+            req.session = {};
+          })
+          .authenticate({ state: { returnTo: '/somewhere' }});
+      });
+
+      it('should be redirected', function() {
+        var u = uri.parse(url, true);
+        expect(u.query.state).to.have.length(24);
+        expect(u.query.code_challenge).to.have.length(43);
+        expect(u.query.code_challenge).to.equal('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM')
+        expect(u.query.code_challenge_method).to.equal('S256');
+      });
+
+      it('should save verifier in session', function() {
+        var u = uri.parse(url, true);
+        expect(request.session['oauth2:www.example.com'].state.handle).to.have.length(24);
+        expect(request.session['oauth2:www.example.com'].state.handle).to.equal(u.query.state);
+        expect(request.session['oauth2:www.example.com'].state.code_verifier).to.have.length(43);
+        expect(request.session['oauth2:www.example.com'].state.code_verifier).to.equal('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk');
+        expect(request.session['oauth2:www.example.com'].state.state).to.deep.equal({ returnTo: '/somewhere' });
+      });
+    });
+
+    describe('handling a request to be redirected for authorization with state set to boolean true', function() {
+      var request, url;
+
+      before(function(done) {
+        chai.passport.use(strategy)
+          .redirect(function(u) {
+            url = u;
+            done();
+          })
+          .req(function(req) {
+            request = req;
+            req.session = {};
+          })
+          .authenticate({ state: true });
+      });
+
+      it('should be redirected', function() {
+        var u = uri.parse(url, true);
+        expect(u.query.state).to.have.length(24);
+        expect(u.query.code_challenge).to.have.length(43);
+        expect(u.query.code_challenge).to.equal('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM')
+        expect(u.query.code_challenge_method).to.equal('S256');
+      });
+
+      it('should save verifier in session', function() {
+        var u = uri.parse(url, true);
+        expect(request.session['oauth2:www.example.com'].state.handle).to.have.length(24);
+        expect(request.session['oauth2:www.example.com'].state.handle).to.equal(u.query.state);
+        expect(request.session['oauth2:www.example.com'].state.code_verifier).to.have.length(43);
+        expect(request.session['oauth2:www.example.com'].state.code_verifier).to.equal('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk');
+        expect(request.session['oauth2:www.example.com'].state.state).to.equal(true);
+      });
+    });
+
+    describe('handling a request to be redirected for authorization with state set to boolean false', function() {
+      var request, url;
+
+      before(function(done) {
+        chai.passport.use(strategy)
+          .redirect(function(u) {
+            url = u;
+            done();
+          })
+          .req(function(req) {
+            request = req;
+            req.session = {};
+          })
+          .authenticate({ state: false });
+      });
+
+      it('should be redirected', function() {
+        var u = uri.parse(url, true);
+        expect(u.query.state).to.have.length(24);
+        expect(u.query.code_challenge).to.have.length(43);
+        expect(u.query.code_challenge).to.equal('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM')
+        expect(u.query.code_challenge_method).to.equal('S256');
+      });
+
+      it('should save verifier in session', function() {
+        var u = uri.parse(url, true);
+        expect(request.session['oauth2:www.example.com'].state.handle).to.have.length(24);
+        expect(request.session['oauth2:www.example.com'].state.handle).to.equal(u.query.state);
+        expect(request.session['oauth2:www.example.com'].state.code_verifier).to.have.length(43);
+        expect(request.session['oauth2:www.example.com'].state.code_verifier).to.equal('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk');
+        expect(request.session['oauth2:www.example.com'].state.state).to.be.undefined;
       });
     });
 
@@ -161,6 +264,48 @@ describe('OAuth2Strategy', function() {
       it('should supply info', function() {
         expect(info).to.be.an.object;
         expect(info.message).to.equal('Hello');
+        expect(info.state).to.be.undefined;
+      });
+
+      it('should remove state with verifier from session', function() {
+        expect(request.session['oauth2:www.example.com']).to.be.undefined;
+      });
+    });
+
+    describe('processing response to authorization request with state', function() {
+      var request
+        , user
+        , info;
+
+      before(function(done) {
+        chai.passport.use(strategy)
+          .success(function(u, i) {
+            user = u;
+            info = i;
+            done();
+          })
+          .req(function(req) {
+            request = req;
+
+            req.query = {};
+            req.query.code = 'SplxlOBeZQQYbYS6WxSbIA';
+            req.query.state = 'DkbychwKu8kBaJoLE5yeR5NK';
+            req.session = {};
+            req.session['oauth2:www.example.com'] = {};
+            req.session['oauth2:www.example.com']['state'] = { handle: 'DkbychwKu8kBaJoLE5yeR5NK', code_verifier: 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk', state: { returnTo: '/somewhere' } };
+          })
+          .authenticate();
+      });
+
+      it('should supply user', function() {
+        expect(user).to.be.an.object;
+        expect(user.id).to.equal('1234');
+      });
+
+      it('should supply info', function() {
+        expect(info).to.be.an.object;
+        expect(info.message).to.equal('Hello');
+        expect(info.state).to.deep.equal({ returnTo: '/somewhere' });
       });
 
       it('should remove state with verifier from session', function() {

test/oauth2.state.custom.test.js

@@ -6,6 +6,7 @@ var OAuth2Strategy = require('../lib/strategy')
   , uri = require('url');
 
 
+  // TODO: renam ethis file to oauth2.store.custom
 describe('OAuth2Strategy', function() {
 
   describe('with custom state store that accepts meta argument', function() {

test/oauth2.state.session.test.js

@@ -6,6 +6,7 @@ var OAuth2Strategy = require('../lib/strategy')
   , uri = require('url');
 
 
+  // TODO: rename this file to oauth2.state.nonce
 describe('OAuth2Strategy', function() {
 
   describe('using default session state store', function() {