Merge pull request #5 from wenwei1202/refactoring

refactoring

README.en.md

@@ -24,7 +24,7 @@ Wechat Official Accounts version, see [passport-wechat-public](https://github.co
 - The Wechat authentication strategy authenticates users using a Wechat
 account and OAuth 2.0 tokens.  The strategy requires a `verify` callback, which
 accepts these credentials and calls `done` providing a user, `options` specifying an corp ID, corp secret, callback URL, and optionally state, scope. The last two are getAccessToken and saveAccessToken functions for access token, and both required.
-  
+
   `getAccessToken` and `saveAccessToken` are two functions for access token, since wechat has limitation for retrieving access token.For every authentication, it will try to get the access token via `getAccessToken` function,if can't get one it will hit the wechat api `/gettoken` to get a new one then save it via `saveAccessToken` function.
 
 ```
@@ -114,7 +114,7 @@ followers:
    "UserId":"USERID",
    "DeviceId":"DEVICEID"
 }
-``` 
+```
 
 unfolloers:
 

README.md

@@ -41,9 +41,10 @@ passport.use("wechat",new WechatPublicStrategy({
   },
   function getAccessToken(cb) { ... },
   function saveAccessToken(accessToken,cb){ ... }
-));
+))
 ```
 
+
 #### Authenticate Requests
 
 

index.js

@@ -1,20 +1,20 @@
 /**
  * Module dependencies.
  */
-var Strategy = require('./lib/strategy');
-var AccessToken = require('./lib/access_token');
+var Strategy = require('./lib/strategy')
+var AccessToken = require('./lib/access_token')
 
 /**
  * Expose `Strategy` directly from package.
  */
-exports = module.exports = Strategy;
+exports = module.exports = Strategy
 
 /**
  * Export constructors.
  */
-exports.Strategy = Strategy;
+exports.Strategy = Strategy
 
 /**
 * Export AccessToken
 */
-exports.AccessToken = AccessToken;
+exports.AccessToken = AccessToken

lib/access_token.js

@@ -1,10 +1,13 @@
-var AccessToken = function(access_token, expires_in) {
+var AccessToken = function(access_token, expires_in, create_at) {
   if (!(this instanceof AccessToken)) {
-    return new AccessToken(access_token, expires_in)
+    return new AccessToken(access_token, expires_in, create_at)
+  }
+  if (!access_token || !expires_in || !create_at) {
+    throw new Error('\'access_token\',  \'expires_in\' and \'create_at\' properties are required.')
   }
   this.access_token = access_token
   this.expires_in = expires_in
-  this.create_at = Date.now()
+  this.create_at = create_at
 }
 
 AccessToken.prototype.isExpired = function() {

lib/oauth.js

@@ -45,7 +45,7 @@ OAuth.prototype.getOAuthAccessToken = function(callback) {
     if (err) {
       return callback(err)
     }
-    const accessToken = new AccessToken(result.access_token, result.expires_in)
+    const accessToken = new AccessToken(result.access_token, result.expires_in, Date.now())
     self._saveAccessToken(accessToken)
     callback(null, accessToken)
   })

lib/utils.js

@@ -21,10 +21,10 @@ exports.originalURL = function(req, options) {
   }
   var trustProxy = options.proxy
 
-  var proto = (req.headers['x-forwarded-proto'] || '').toLowerCase()
-    , tls = req.connection.encrypted || (trustProxy && 'https' == proto.split(/\s*,\s*/)[0])
-    , host = (trustProxy && req.headers['x-forwarded-host']) || req.headers.host
-    , protocol = tls ? 'https' : 'http'
-    , path = req.url || ''
+  var proto = (req.headers['x-forwarded-proto'] || '').toLowerCase(),
+    tls = req.connection.encrypted || (trustProxy && 'https' == proto.split(/\s*,\s*/)[0]),
+    host = (trustProxy && req.headers['x-forwarded-host']) || req.headers.host,
+    protocol = tls ? 'https' : 'http',
+    path = req.url || ''
   return protocol + '://' + host + path
 }

package.json

@@ -36,4 +36,4 @@
     "coveralls": "^2.11.9",
     "mocha-lcov-reporter": "^1.2.0"
   }
-}
+}

test/access_token.test.js

@@ -4,30 +4,44 @@ const expect = chai.expect
 const AccessToken = require('../lib/access_token')
 
 
-describe('AccessToken ', () => {
-  describe('constructor ', () => {
-    it('should return the AccessToken as expected', () => {
-      const token = new AccessToken('token123', 7200)
+describe('AccessToken ', function() {
+  describe('constructor ', function() {
+    it('should return the AccessToken as expected', function() {
+      const token = new AccessToken('token123', 7200, Date.now())
       expect(token).to.have.property('access_token', 'token123')
-      expect(token).to.have.property('expires_in', 7200)
+      expect(token).to.have.property('expires_in', 7200, Date.now())
       expect(token).to.have.property('create_at')
     })
 
-    it('should return the AccessToken when "new" is missed', () => {
-      const token =  AccessToken('token123', 7200)
+    it('should return the AccessToken when "new" is missed', function() {
+      const token = AccessToken('token123', 7200, Date.now())
       expect(token).to.be.instanceof(AccessToken)
     })
+
+    it('should throw error when any of required arguments is null ', function() {
+      const args = [
+        ['token', 7200, null],
+        ['token', null, Date.now()],
+        [null, 7200, Date.now()]
+      ]
+      args.forEach((arg) => {
+        expect(function() {
+          AccessToken.apply(null, arg)
+        }).to.throw(Error)
+      })
+
+    })
   })
 
-  describe('isExpired function ', () => {
-    it('should return true if token is expired', () => {
-      const token = new AccessToken('token123', 7200)
+  describe('isExpired function ', function() {
+    it('should return true if token is expired', function() {
+      const token = new AccessToken('token123', 7200, Date.now())
       token.create_at -= 7201 * 1000
       expect(token.isExpired()).to.be.true
     })
 
-    it('should return false if token is valid', () => {
-      const token = new AccessToken('token123', 7200)
+    it('should return false if token is valid', function() {
+      const token = new AccessToken('token123', 7200, Date.now())
       expect(token.isExpired()).to.be.false
     })
   })

test/oauth.test.js

@@ -117,7 +117,7 @@ describe('OAuth ', () => {
 
   describe('getAccessToken ', () => {
     const getAccessToken = function(cb) {
-      cb(null, new AccessToken('token', 7200))
+      cb(null, new AccessToken('token', 7200, Date.now()))
     }
     const oauth = OAuth('corpId', 'corpSecret', getAccessToken, () => {})
     oauth.getOAuthAccessToken = function(cb) {
@@ -147,7 +147,7 @@ describe('OAuth ', () => {
   describe('getUserInfo ', () => {
     const oauth = OAuth('corpId', 'corpSecret', () => {}, () => {})
     const code = 'code'
-    const accessToken = new AccessToken('token', 7200)
+    const accessToken = new AccessToken('token', 7200, Date.now())
     it('should the authorize url ', () => {
       oauth.getUserInfo(accessToken, code, (err, profile) => {
         expect(profile).to.deep.equal({

test/strategy.test.js

@@ -190,7 +190,7 @@ describe('Strategy', function() {
     })
 
     it('should fetch info including openId if user has not subscribed', function(done) {
-      const strategy = new WechatStrategy(options, (profile) => {
+      const strategy = new WechatStrategy(options, (profile, verified) => {
         expect(profile.UserId).to.equal('OPENID')
         done()
       }, getAccessToken, () => {})
@@ -206,7 +206,7 @@ describe('Strategy', function() {
 
     it('should return error the code is invalid', function(done) {
       var err
-      const strategy = new WechatStrategy(options, (profile) => {
+      const strategy = new WechatStrategy(options, (profile, verified) => {
         expect(err.errcode).to.equal(40029)
         expect(err.errmsg).to.equal('invalid code')
         done()
@@ -226,7 +226,7 @@ describe('Strategy', function() {
     })
 
     it('should fail if UserId is null', function(done) {
-      const strategy = new WechatStrategy(options, () => {}, getAccessToken, () => {})
+      const strategy = new WechatStrategy(options, (profile, verified) => {}, getAccessToken, () => {})
       strategy._oauth.getUserInfo = getUserInfo
 
       chai.passport.use(strategy)
@@ -259,7 +259,7 @@ describe('Strategy', function() {
 
     it('should pass 3 arguments when passReqToCallback is true', (done) => {
       options.passReqToCallback = true
-      const strategy = new WechatStrategy(options, function() {
+      const strategy = new WechatStrategy(options, function(req, profile, verified) {
         expect(arguments.length).to.equal(3)
         done()
       }, getAccessToken, saveAccessToken)
@@ -274,7 +274,7 @@ describe('Strategy', function() {
 
     it('should pass 2 arguments when when passReqToCallback is false', (done) => {
       options.passReqToCallback = false
-      const strategy = new WechatStrategy(options, function(req, profile) {
+      const strategy = new WechatStrategy(options, function(req, profile, verified) {
         expect(arguments.length).to.equal(2)
         done()
       }, getAccessToken, saveAccessToken)
@@ -288,7 +288,7 @@ describe('Strategy', function() {
     })
     it('should get an error when verify function throw any error', (done) => {
       options.passReqToCallback = false
-      const strategy = new WechatStrategy(options, function(req, profile) {
+      const strategy = new WechatStrategy(options, function(req, profile, verified) {
         throw new Error('Verify Error')
       }, getAccessToken, saveAccessToken)
       strategy._oauth.getUserInfo = getUserInfo
@@ -388,11 +388,11 @@ describe('Strategy', function() {
   }
 
   function getAccessToken(cb) {
-    const token = new AccessToken('token123', 7200)
+    const token = new AccessToken('token123', 7200, Date.now())
     cb(null, token)
   }
 
-  function saveAccessToken() {
+  function saveAccessToken(accessToken) {
 
   }
 })