diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7e41c605c..0d66d4e4d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,30 +25,24 @@ If you want to contribute to the repository, follow these steps: 1. Fork the repo. 2. Install dependencies: `npm install` 3. Build the code: `npm run build` -4. Verify the build before beginning your changes: `npm run test-unit` +4. Verify the build before beginning your changes: `npm test` 2. Develop and test your code changes. -3. Travis-CI will run the tests for all services once your changes are merged. 4. Add a test for your changes. Only refactoring and documentation changes require no new tests. -5. Make the test pass. +5. Make sure all the tests pass. +2. Check code style with `npm run lint` and fix any issues. 6. Commit your changes. Remember the follow the correct commit message guidelines. 7. Push to your fork and submit a pull request. +3. Travis-CI will run the tests once your changes are merged. 8. Be sure to sign the CLA. ## Tests -Out of the box, `npm test` runs linting, unit tests, and integration tests (which require credentials). -To run only the unit tests (sufficient for most cases), use `npm run test-unit`. - -To run the integration tests, you need to provide credentials to the integration test framework. -The integration test framework will skip integration tests for any service that does not have credentials, - -To provide credentials for the integration tests, copy `test/resources/auth.example.js` to `test/resources/auth.js` -and fill in credentials for the service(s) you wish to test. +`npm test` runs the full suite of unit tests. To run only specific tests, invoke jest with the `--testNamePattern` or `-t` flag: ``` -npm run jest -- /test/integration -t "resources key" +npm run jest -- /test/unit -t "resources key" ``` This will only run tests that match the test name pattern you provide. diff --git a/auth/token-managers/jwt-token-manager.ts b/auth/token-managers/jwt-token-manager.ts index 336305f19..de5f612dd 100644 --- a/auth/token-managers/jwt-token-manager.ts +++ b/auth/token-managers/jwt-token-manager.ts @@ -53,6 +53,7 @@ export class JwtTokenManager { protected requestWrapperInstance: RequestWrapper; private tokenInfo: any; private expireTime: number; + private refreshTime: number; /** * Create a new [[JwtTokenManager]] instance. @@ -97,6 +98,12 @@ export class JwtTokenManager { return this.tokenInfo[this.tokenName]; }); } else { + // If refresh needed, kick one off + if (this.tokenNeedsRefresh()) { + this.requestToken().then(tokenResponse => { + this.saveTokenInfo(tokenResponse.result); + }); + } // 2. use valid, managed token return Promise.resolve(this.tokenInfo[this.tokenName]); } @@ -142,8 +149,7 @@ export class JwtTokenManager { } /** - * Check if currently stored token is "expired" - * i.e. past the window to request a new token + * Check if currently stored token is expired * * @private * @returns {boolean} @@ -156,7 +162,28 @@ export class JwtTokenManager { } const currentTime = getCurrentTime(); - return expireTime < currentTime; + return expireTime <= currentTime; + } + + /** + * Check if currently stored token should be refreshed + * i.e. past the window to request a new token + * + * @private + * @returns {boolean} + */ + private tokenNeedsRefresh(): boolean { + const { refreshTime } = this; + const currentTime = getCurrentTime(); + + if (refreshTime && refreshTime > currentTime) { + return false; + } + + // Update refreshTime to 60 seconds from now to avoid redundant refreshes + this.refreshTime = currentTime + 60; + + return true; } /** @@ -175,36 +202,27 @@ export class JwtTokenManager { throw new Error(err); } - this.expireTime = this.calculateTimeForNewToken(accessToken); - this.tokenInfo = extend({}, tokenResponse); - } - - /** - * Decode the access token and calculate the time to request a new token. - * - * A time buffer prevents the edge case of the token expiring before the request could be made. - * The buffer will be a fraction of the total time to live - we are using 80% - * - * @param accessToken - JSON Web Token received from the service - * @private - * @returns {void} - */ - private calculateTimeForNewToken(accessToken): number { // the time of expiration is found by decoding the JWT access token // exp is the time of expire and iat is the time of token retrieval - let timeForNewToken; const decodedResponse = jwt.decode(accessToken); - if (decodedResponse) { - const { exp, iat } = decodedResponse; - const fractionOfTtl = 0.8; - const timeToLive = exp - iat; - timeForNewToken = exp - (timeToLive * (1.0 - fractionOfTtl)); - } else { + if (!decodedResponse) { const err = 'Access token recieved is not a valid JWT' logger.error(err); throw new Error(err); } - return timeForNewToken; + const { exp, iat } = decodedResponse; + // There are no required claims in JWT + if (!exp || !iat) { + this.expireTime = 0; + this.refreshTime = 0; + } else { + const fractionOfTtl = 0.8; + const timeToLive = exp - iat; + this.expireTime = exp; + this.refreshTime = exp - (timeToLive * (1.0 - fractionOfTtl)); + } + + this.tokenInfo = extend({}, tokenResponse); } } diff --git a/test/unit/iam-token-manager.test.js b/test/unit/iam-token-manager.test.js index 9a20671d4..2bc330ca1 100644 --- a/test/unit/iam-token-manager.test.js +++ b/test/unit/iam-token-manager.test.js @@ -20,6 +20,7 @@ RequestWrapper.mockImplementation(() => { }); const ACCESS_TOKEN = '9012'; +const CURRENT_ACCESS_TOKEN = '1234'; const IAM_RESPONSE = { result: { access_token: ACCESS_TOKEN, @@ -62,14 +63,11 @@ describe('iam_token_manager_v1', function() { const requestMock = jest.spyOn(instance, 'requestToken'); const currentTokenInfo = { - access_token: '1234', - refresh_token: '5678', - token_type: 'Bearer', - expires_in: 3600, - expiration: Math.floor(Date.now() / 1000), + access_token: CURRENT_ACCESS_TOKEN, }; instance.tokenInfo = currentTokenInfo; + instance.expireTime = Math.floor(Date.now() / 1000) - 1; mockSendRequest.mockImplementation(parameters => Promise.resolve(IAM_RESPONSE)); @@ -79,64 +77,48 @@ describe('iam_token_manager_v1', function() { done(); }); - it('should use a valid access token if one is stored', async done => { + it('should refresh an access token whose refreshTime has passed', async done => { const instance = new IamTokenManager({ apikey: 'abcd-1234' }); const requestMock = jest.spyOn(instance, 'requestToken'); const currentTokenInfo = { - access_token: ACCESS_TOKEN, - refresh_token: '5678', - token_type: 'Bearer', - expires_in: 3600, - expiration: Math.floor(Date.now() / 1000) + 3000, + access_token: CURRENT_ACCESS_TOKEN, }; instance.tokenInfo = currentTokenInfo; - instance.timeToLive = currentTokenInfo.expires_in; - instance.expireTime = currentTokenInfo.expiration; - - const token = await instance.getToken(); - expect(token).toBe(ACCESS_TOKEN); - expect(requestMock).not.toHaveBeenCalled(); - done(); - }); + instance.expireTime = Math.floor(Date.now() / 1000) + 60; + instance.refreshTime = Math.floor(Date.now() / 1000) - 1; - it('should refresh an access token without expires_in field', async done => { - const instance = new IamTokenManager({ apikey: 'abcd-1234' }); - const requestMock = jest.spyOn(instance, 'requestToken'); - - const currentTokenInfo = { - access_token: '1234', - refresh_token: '5678', - token_type: 'Bearer', - expiration: Math.floor(Date.now() / 1000), - }; - - instance.tokenInfo = currentTokenInfo; + const requestTokenSpy = jest + .spyOn(instance, 'requestToken') + .mockImplementation(() => Promise.resolve({ result: { access_token: ACCESS_TOKEN } })); mockSendRequest.mockImplementation(parameters => Promise.resolve(IAM_RESPONSE)); const token = await instance.getToken(); - expect(token).toBe(ACCESS_TOKEN); + expect(token).toBe(CURRENT_ACCESS_TOKEN); expect(requestMock).toHaveBeenCalled(); + expect(requestTokenSpy).toHaveBeenCalled(); + + requestTokenSpy.mockRestore(); done(); }); - it('should request a new token when refresh token does not have expiration field', async done => { + it('should use a valid access token if one is stored', async done => { const instance = new IamTokenManager({ apikey: 'abcd-1234' }); + const requestMock = jest.spyOn(instance, 'requestToken'); const currentTokenInfo = { - access_token: '1234', - refresh_token: '5678', - token_type: 'Bearer', + access_token: ACCESS_TOKEN, }; instance.tokenInfo = currentTokenInfo; - - mockSendRequest.mockImplementation(parameters => Promise.resolve(IAM_RESPONSE)); + instance.expireTime = Math.floor(Date.now() / 1000) + 60 * 60; + instance.refreshTime = Math.floor(Date.now() / 1000) + 48 * 60; const token = await instance.getToken(); expect(token).toBe(ACCESS_TOKEN); + expect(requestMock).not.toHaveBeenCalled(); done(); }); diff --git a/test/unit/jwt-token-manager.test.js b/test/unit/jwt-token-manager.test.js index 96df50fcf..443725c46 100644 --- a/test/unit/jwt-token-manager.test.js +++ b/test/unit/jwt-token-manager.test.js @@ -9,6 +9,7 @@ function getCurrentTime() { } const ACCESS_TOKEN = 'abc123'; +const CURRENT_ACCESS_TOKEN = 'abc123'; describe('JWT Token Manager', () => { it('should initialize base variables', () => { @@ -33,7 +34,7 @@ describe('JWT Token Manager', () => { const decodeSpy = jest .spyOn(jwt, 'decode') - .mockImplementation(token => ({ iat: 0, exp: 100 })); + .mockImplementation(token => ({ iat: 10, exp: 100 })); const requestTokenSpy = jest .spyOn(instance, 'requestToken') @@ -51,14 +52,14 @@ describe('JWT Token Manager', () => { done(); }); - it('should request a token if token is stored but expired', async done => { + it('should request a token if token is stored but needs refresh', async done => { const instance = new JwtTokenManager(); - instance.tokenInfo.access_token = '987zxc'; + instance.tokenInfo.access_token = CURRENT_ACCESS_TOKEN; const saveTokenInfoSpy = jest.spyOn(instance, 'saveTokenInfo'); const decodeSpy = jest .spyOn(jwt, 'decode') - .mockImplementation(token => ({ iat: 0, exp: 100 })); + .mockImplementation(token => ({ iat: 10, exp: 100 })); const requestTokenSpy = jest .spyOn(instance, 'requestToken') @@ -68,7 +69,7 @@ describe('JWT Token Manager', () => { expect(requestTokenSpy).toHaveBeenCalled(); expect(saveTokenInfoSpy).toHaveBeenCalled(); expect(decodeSpy).toHaveBeenCalled(); - expect(token).toBe(ACCESS_TOKEN); + expect(token).toBe(CURRENT_ACCESS_TOKEN); saveTokenInfoSpy.mockRestore(); decodeSpy.mockRestore(); @@ -132,6 +133,7 @@ describe('JWT Token Manager', () => { const instance = new JwtTokenManager(); instance.tokenInfo.access_token = ACCESS_TOKEN; instance.expireTime = getCurrentTime() + 1000; + instance.refreshTime = getCurrentTime() + 800; const token = await instance.getToken(); expect(token).toBe(ACCESS_TOKEN); done(); @@ -175,19 +177,40 @@ describe('JWT Token Manager', () => { }); }); + describe('tokenNeedsRefresh', () => { + it('should return true if current time is past refresh time', () => { + const instance = new JwtTokenManager(); + instance.refreshTime = getCurrentTime() - 1000; + + expect(instance.tokenNeedsRefresh()).toBe(true); + }); + + it('should return false if current time has not reached refresh time', () => { + const instance = new JwtTokenManager(); + instance.refreshTime = getCurrentTime() + 1000; + + expect(instance.tokenNeedsRefresh()).toBe(false); + }); + + it('should return true if refresh time has not been set', () => { + const instance = new JwtTokenManager(); + expect(instance.tokenNeedsRefresh()).toBe(true); + }); + }); + describe('saveTokenInfo', () => { it('should save information to object state', () => { const instance = new JwtTokenManager(); - - const expireTime = 100; - instance.calculateTimeForNewToken = jest.fn(token => expireTime); + const decodeSpy = jest + .spyOn(jwt, 'decode') + .mockImplementation(token => ({ iat: 10, exp: 100 })); const tokenResponse = { access_token: ACCESS_TOKEN }; instance.saveTokenInfo(tokenResponse); - expect(instance.expireTime).toBe(expireTime); + expect(instance.expireTime).toBe(100); expect(instance.tokenInfo).toEqual(tokenResponse); - expect(instance.calculateTimeForNewToken).toHaveBeenCalledWith(ACCESS_TOKEN); + decodeSpy.mockRestore(); }); it('should throw an error when access token is undefined', () => { @@ -203,9 +226,12 @@ describe('JWT Token Manager', () => { const instance = new JwtTokenManager(); const decodeSpy = jest .spyOn(jwt, 'decode') - .mockImplementation(token => ({ iat: 0, exp: 100 })); + .mockImplementation(token => ({ iat: 100, exp: 200 })); + + const tokenResponse = { access_token: ACCESS_TOKEN }; - expect(instance.calculateTimeForNewToken(ACCESS_TOKEN)).toBe(80); + instance.saveTokenInfo(tokenResponse); + expect(instance.refreshTime).toBe(180); decodeSpy.mockRestore(); }); @@ -213,5 +239,19 @@ describe('JWT Token Manager', () => { const instance = new JwtTokenManager(); expect(() => instance.calculateTimeForNewToken()).toThrow(); }); + + it('should gracefully handle a jwt without exp or iat claims', () => { + const instance = new JwtTokenManager(); + const decodeSpy = jest + .spyOn(jwt, 'decode') + .mockImplementation(token => ({ foo: 0, bar: 100 })); + + const tokenResponse = { access_token: ACCESS_TOKEN }; + + instance.saveTokenInfo(tokenResponse); + expect(instance.expireTime).toBe(0); + expect(instance.tokenInfo).toEqual(tokenResponse); + decodeSpy.mockRestore(); + }); }); });