feat: refactor refresh token (#955)

* feat: refactor refresh token * chore: tsc build * fix: typo function path * fix: update test case * fix: update test case * chore: snyk support node 8.x
ali-sdk · Jun 8, 2021 · b0c6771 · b0c6771
1 parent 2073ddd
commit b0c6771
Show file tree

Hide file tree

Showing 13 changed files with 69 additions and 133 deletions.
diff --git a/README.md b/README.md
@@ -321,6 +321,9 @@ options:
 - accessKeySecret {String} access secret you create
 - [stsToken] {String} used by temporary authorization, detail [see](https://www.alibabacloud.com/help/doc-detail/32077.htm)
 - [refreshSTSToken] {Function} used by auto set `stsToken`、`accessKeyId`、`accessKeySecret` when sts info expires. return value must be object contains `stsToken`、`accessKeyId`、`accessKeySecret`
+- [refreshSTSTokenInterval] {number} use time (ms) of refresh STSToken interval it should be 
+  less than sts info expire interval, default is 300000ms(5min)
+  when sts info expires. return value must be object contains `stsToken`、`accessKeyId`、`accessKeySecret`
 - [bucket] {String} the default bucket you want to access
   If you don't have any bucket, please use `putBucket()` create one first.
 - [endpoint] {String} oss region domain. It takes priority over `region`. Set as extranet domain name, intranet domain name, accelerated domain name, etc. according to different needs. please see [endpoints](https://www.alibabacloud.com/help/doc-detail/31837.htm)
@@ -387,12 +390,14 @@ const store = new OSS({
   accessKeySecret: 'your STS secret',
   stsToken: 'your STS token',
   refreshSTSToken: async () => {
+    const info = await fetch('you sts server');
     return {
-      accessKeyId: 'your refreshed STS key',
-      accessKeySecret: 'your refreshed STS secret',
-      stsToken: 'your refreshed STS stoken'
+      accessKeyId: info.accessKeyId,
+      accessKeySecret: info.accessKeySecret,
+      stsToken: info.stsToken
     }
-  }
+  },
+  refreshSTSTokenInterval: 300000
 });
 ```
 

diff --git a/example/config/webpack.dev.conf.js b/example/config/webpack.dev.conf.js
@@ -47,7 +47,7 @@ module.exports = {
   devServer: {
     contentBase: path.resolve(__dirname, '../dist'),
     port: 3000,
-    host: '0.0.0.0',
+    host: '127.0.0.1',
     open: true, // open browser auto
     index: 'index.html', // like HtmlWebpackPlugin
     inline: true, // default:true

diff --git a/lib/browser/client.js b/lib/browser/client.js
@@ -14,6 +14,7 @@ const { encoder } = require('../common/utils/encoder');
 const { getReqUrl } = require('../common/client/getReqUrl');
 const { setSTSToken } = require('../common/utils/setSTSToken');
 const { retry } = require('../common/utils/retry');
+const { isFunction } = require('../common/utils/isFunction');
 
 const globalHttpAgent = new AgentKeepalive();
 
@@ -214,6 +215,9 @@ proto.request = async function (params) {
 };
 
 async function request(params) {
+  if (this.options.stsToken && isFunction(this.options.refreshSTSToken)) {
+    await setSTSToken.call(this);
+  }
   const reqParams = createRequest.call(this, params);
   if (!this.options.useFetch) {
     reqParams.params.mode = 'disable-fetch';
@@ -241,16 +245,6 @@ async function request(params) {
   }
 
   if (err) {
-    if (err.status === 403 && err.code === 'InvalidAccessKeyId' &&
-     this.options.accessKeyId.startsWith('STS.') &&
-     typeof this.options.refreshSTSToken === 'function') {
-      // prevent infinite loop, only trigger once within 10 seconds
-      if (!this._setOptions || Date.now() - this._setOptions > 10000) {
-        this._setOptions = Date.now();
-        await setSTSToken.call(this);
-        return this.request(params);
-      }
-    }
     throw err;
   }
 
@@ -259,7 +253,7 @@ async function request(params) {
     result.data = parseData;
   }
   return result;
-};
+}
 
 proto._getResource = function _getResource(params) {
   let resource = '/';

diff --git a/lib/client.js b/lib/client.js
@@ -17,6 +17,7 @@ const { encoder } = require('./common/utils/encoder');
 const { getReqUrl } = require('./common/client/getReqUrl');
 const { setSTSToken } = require('./common/utils/setSTSToken');
 const { retry } = require('./common/utils/retry');
+const { isFunction } = require('./common/utils/isFunction');
 
 const globalHttpAgent = new AgentKeepalive();
 const globalHttpsAgent = new HttpsAgentKeepalive();
@@ -186,6 +187,9 @@ proto.request = async function (params) {
 };
 
 async function request(params) {
+  if (this.options.stsToken && isFunction(this.options.refreshSTSToken)) {
+    await setSTSToken.call(this);
+  }
   const reqParams = createRequest.call(this, params);
   let result;
   let reqErr;
@@ -209,19 +213,6 @@ async function request(params) {
       await sendToWormhole(result.res);
     }
 
-    if (err.status === 403 && err.code === 'InvalidAccessKeyId' &&
-      this.options.accessKeyId.startsWith('STS.') &&
-      typeof this.options.refreshSTSToken === 'function') {
-      // prevent infinite loop, only trigger once within 10 seconds
-      if (!this._setOptions || Date.now() - this._setOptions > 10000) {
-        this._setOptions = Date.now();
-        await setSTSToken.call(this);
-        if (!params.stream) {
-          return this.request(params);
-        }
-      }
-    }
-
     if (err.name === 'ResponseTimeoutError') {
       err.message = `${err.message.split(',')[0]}, please increase the timeout or use multipartDownload.`;
     }

diff --git a/lib/common/client/initOptions.js b/lib/common/client/initOptions.js
@@ -23,9 +23,10 @@ module.exports = function (options) {
   if (!options || !options.accessKeyId || !options.accessKeySecret) {
     throw new Error('require accessKeyId, accessKeySecret');
   }
-  if (options.stsToken && !options.refreshSTSToken) {
+  if (options.stsToken && !options.refreshSTSToken && !options.refreshSTSTokenInterval) {
     console.warn(
-      "It's recommended to set `refreshSTSToken` to refresh stsToken、accessKeyId、accessKeySecret automatically when sts info expires"
+      "It's recommended to set 'refreshSTSToken' and 'refreshSTSTokenInterval' to refresh" +
+      ' stsToken、accessKeyId、accessKeySecret automatically when sts token has expired'
     );
   }
   if (options.bucket) {
@@ -44,6 +45,7 @@ module.exports = function (options) {
       sldEnable: false,
       headerEncoding: 'utf-8',
       refreshSTSToken: null,
+      refreshSTSTokenInterval: 60000 * 5,
       retryMax: 0
     },
     options

diff --git a/lib/common/utils/isFunction.ts b/lib/common/utils/isFunction.ts
@@ -0,0 +1,3 @@
+export const isFunction = (v) : boolean => {
+  return typeof v === 'function';
+};
diff --git a/lib/common/utils/setSTSToken.d.ts b/lib/common/utils/setSTSToken.d.ts
diff --git a/lib/common/utils/setSTSToken.js b/lib/common/utils/setSTSToken.js
diff --git a/lib/common/utils/setSTSToken.ts b/lib/common/utils/setSTSToken.ts
@@ -2,13 +2,23 @@ import { formatObjKey } from './formatObjKey';
 
 export async function setSTSToken(this: any) {
   if (!this.options) this.options = {};
-  let credentials = await this.options.refreshSTSToken();
-  credentials = formatObjKey(credentials, 'firstLowerCase');
-  if (credentials.securityToken) {
-    credentials.stsToken = credentials.securityToken;
+
+  const now = new Date();
+  if (this.stsTokenFreshTime) {
+    if (+now - this.stsTokenFreshTime >= this.options.refreshSTSTokenInterval) {
+      this.stsTokenFreshTime = now;
+      let credentials = await this.options.refreshSTSToken();
+      credentials = formatObjKey(credentials, 'firstLowerCase');
+      if (credentials.securityToken) {
+        credentials.stsToken = credentials.securityToken;
+      }
+      checkCredentials(credentials);
+      Object.assign(this.options, credentials);
+    }
+  } else {
+    this.stsTokenFreshTime = now;
   }
-  checkCredentials(credentials);
-  Object.assign(this.options, credentials);
+  return null;
 }
 
 function checkCredentials(obj) {

diff --git a/package.json b/package.json
@@ -21,8 +21,8 @@
   },
   "scripts": {
     "build-change-log": "standard-version",
-    "test": "mocha -t 60000 -r thunk-mocha -r should -r dotenv/config test/node/*.test.js test/node/**/*.test.js",
-    "test-cov": "nyc --reporter=lcov node_modules/.bin/_mocha -t 60000 -r thunk-mocha -r should test/node/*.test.js test/node/**/*.test.js",
+    "test": "npm run tsc && mocha -t 60000 -r thunk-mocha -r should -r dotenv/config test/node/*.test.js test/node/**/*.test.js",
+    "test-cov": "npm run tsc && nyc --reporter=lcov node_modules/.bin/_mocha -t 60000 -r thunk-mocha -r should test/node/*.test.js test/node/**/*.test.js",
     "jshint": "jshint .",
     "autod": "autod",
     "build-test": "MINIFY=1 node browser-build.js > test/browser/build/aliyun-oss-sdk.min.js && node -r dotenv/config task/browser-test-build.js > test/browser/build/tests.js",
@@ -111,7 +111,7 @@
     "request": "^2.88.0",
     "should": "^11.0.0",
     "sinon": "^1.17.7",
-    "snyk": "^1.520.0",
+    "snyk": "1.454.0",
     "standard-version": "^8.0.1",
     "stream-equal": "^1.1.0",
     "thunk-mocha": "^1.0.3",

diff --git a/test/browser/browser.test.js b/test/browser/browser.test.js
@@ -1,6 +1,3 @@
-/* eslint-disable @typescript-eslint/no-unused-vars */
-
-/* eslint no-await-in-loop: [0] */
 const assert = require('assert');
 // var oss = require('../');
 // var oss = OSS.Wrapper;

diff --git a/test/node/sts.test.js b/test/node/sts.test.js
@@ -1,33 +1,18 @@
-/**
- * Copyright(c) ali-sdk and other contributors.
- * MIT Licensed
- *
- * Authors:
- *   rockuw <rockuw@gmail.com> (https://github.com/rockuw)
- */
-
-
-/**
- * Module dependencies.
- */
-
 const assert = require('assert');
 const utils = require('./utils');
 const sts = require('../..').STS;
 const OSS = require('../..');
-const { formatObjKey } = require('../../lib/common/utils/formatObjKey');
 const config = require('../config').oss;
 const stsConfig = require('../config').sts;
 const mm = require('mm');
-const { obj2xml } = require('../../lib/common/utils/obj2xml');
 
 describe('test/sts.test.js', () => {
   const { prefix } = utils;
   describe('assumeRole()', () => {
     it('should assume role', async () => {
       const stsClient = sts(stsConfig);
       const result = await stsClient.assumeRole(stsConfig.roleArn);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
     });
 
     it('should assume role with policy', async () => {
@@ -45,7 +30,7 @@ describe('test/sts.test.js', () => {
         Version: '1'
       };
       const result = await stsClient.assumeRole(stsConfig.roleArn, policy);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
     });
 
     it('should assume role with policy string', async () => {
@@ -64,7 +49,7 @@ describe('test/sts.test.js', () => {
         "Version": "1"
       }`;
       const result = await stsClient.assumeRole(stsConfig.roleArn, policy);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
     });
 
     it('should handle error in assume role', async () => {
@@ -94,7 +79,7 @@ describe('test/sts.test.js', () => {
     it('should list objects using STS', async () => {
       const stsClient = sts(stsConfig);
       let result = await stsClient.assumeRole(stsConfig.roleArn);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
 
       const ossClient = new OSS({
         region: config.region,
@@ -106,13 +91,13 @@ describe('test/sts.test.js', () => {
 
       const name = `${prefix}ali-sdk/oss/sts-put1.js`;
       result = await ossClient.put(name, __filename);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
 
       result = await ossClient.list({
         'max-keys': 10
       });
 
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
     });
 
     it('should delete multi objects using STS', async () => {
@@ -132,7 +117,7 @@ describe('test/sts.test.js', () => {
       };
 
       let result = await stsClient.assumeRole(stsConfig.roleArn, policy);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
 
       let ossClient = new OSS({
         region: config.region,
@@ -145,10 +130,10 @@ describe('test/sts.test.js', () => {
       const name1 = `${prefix}ali-sdk/oss/sts-put1.js`;
       const name2 = `${prefix}ali-sdk/oss/sts-put2.js`;
       result = await ossClient.put(name1, __filename);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
 
       result = await ossClient.put(name2, __filename);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
 
       try {
         await ossClient.deleteMulti([name1, name2]);
@@ -171,7 +156,7 @@ describe('test/sts.test.js', () => {
       };
 
       result = await stsClient.assumeRole(stsConfig.roleArn, policy);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
 
       ossClient = new OSS({
         region: config.region,
@@ -182,7 +167,7 @@ describe('test/sts.test.js', () => {
       });
 
       result = await ossClient.deleteMulti([name1, name2]);
-      assert.equal(result.res.status, 200);
+      assert.strictEqual(result.res.status, 200);
     });
   });
 
@@ -191,10 +176,14 @@ describe('test/sts.test.js', () => {
 
     let store;
     before(async () => {
-      let { credentials } = await stsClient.assumeRole(stsConfig.roleArn);
-      credentials = formatObjKey(credentials, 'firstLowerCase');
+      const { credentials } = await stsClient.assumeRole(stsConfig.roleArn);
       const testRefreshSTSTokenConf = {
-        ...credentials,
+        region: config.region,
+        accessKeyId: credentials.AccessKeyId,
+        accessKeySecret: credentials.AccessKeySecret,
+        stsToken: credentials.SecurityToken,
+        bucket: stsConfig.bucket,
+        refreshSTSTokenInterval: 1000
       };
       store = new OSS(testRefreshSTSTokenConf);
     });
@@ -205,20 +194,12 @@ describe('test/sts.test.js', () => {
           const { credentials } = await stsClient.assumeRole(stsConfig.roleArn);
           return credentials;
         };
-        // 模拟sts过期错误
-        const errData = {
-          Test: {
-            Code: 'InvalidAccessKeyId'
-          }
-        };
-        mm.error(store.urllib, 'request', {
-          status: 403,
-          headers: {},
-          data: obj2xml(errData)
-        });
-        const res = await store.listBuckets();
-        assert.strictEqual(res.res.status, 200);
-        assert(res.buckets && Array.isArray(res.buckets));
+        const ak = store.options.accessKeyId;
+        await store.listBuckets();
+        assert.strictEqual(ak, store.options.accessKeyId);
+        await utils.sleep(2000);
+        await store.listBuckets();
+        assert.notStrictEqual(ak, store.options.accessKeyId);
       } catch (error) {
         assert(false, error);
       }