Skip to content

Commit

Permalink
feat: Pace token refresh requests to avoid rate-limiting (#79)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mike Kistler authored Feb 14, 2020
1 parent b6d5e6e commit d908c0d
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 88 deletions.
18 changes: 6 additions & 12 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
70 changes: 44 additions & 26 deletions auth/token-managers/jwt-token-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class JwtTokenManager {
protected requestWrapperInstance: RequestWrapper;
private tokenInfo: any;
private expireTime: number;
private refreshTime: number;

/**
* Create a new [[JwtTokenManager]] instance.
Expand Down Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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}
Expand All @@ -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;
}

/**
Expand All @@ -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);
}
}
58 changes: 20 additions & 38 deletions test/unit/iam-token-manager.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ RequestWrapper.mockImplementation(() => {
});

const ACCESS_TOKEN = '9012';
const CURRENT_ACCESS_TOKEN = '1234';
const IAM_RESPONSE = {
result: {
access_token: ACCESS_TOKEN,
Expand Down Expand Up @@ -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));

Expand All @@ -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();
});

Expand Down
64 changes: 52 additions & 12 deletions test/unit/jwt-token-manager.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ function getCurrentTime() {
}

const ACCESS_TOKEN = 'abc123';
const CURRENT_ACCESS_TOKEN = 'abc123';

describe('JWT Token Manager', () => {
it('should initialize base variables', () => {
Expand All @@ -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')
Expand All @@ -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')
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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', () => {
Expand All @@ -203,15 +226,32 @@ 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();
});

it('should throw an error if token is not a valid jwt', () => {
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();
});
});
});

0 comments on commit d908c0d

Please sign in to comment.