diff --git a/src/http.js b/src/http.js index 85f1e53b5..ab64c9e72 100644 --- a/src/http.js +++ b/src/http.js @@ -69,45 +69,44 @@ class Http { request.headers = Object.assign(this.defaultHeaders, request.headers); request.method = request.method || 'get'; - const promise = this.prepareRequest(request) - .then(() => this.requestExecutor.fetch(request)) - .then(Http.errorFilter) - .catch(error => { - // Handle oauth access token expiration - if (this.oauth && error && error.status === 401) { - this.oauth.clearCachedAccessToken(); - return this.oauth.introspectAccessToken() - .then(({ active }) => { - if (active === false) { - return this.http(uri, request, context); - } - // Access token is still active or other error happen, re-throw - throw error; - }); - } + let retriedOnAuthError = false; + const execute = () => { + const promise = this.prepareRequest(request) + .then(() => this.requestExecutor.fetch(request)) + .then(Http.errorFilter) + .catch(error => { + // Clear cached token then retry request one more time + if (this.oauth && error && error.status === 401 && !retriedOnAuthError) { + retriedOnAuthError = true; + this.oauth.clearCachedAccessToken(); + return execute(); + } - throw error; - }); + throw error; + }); - if (!this.cacheMiddleware) { - return promise; - } + if (!this.cacheMiddleware) { + return promise; + } + + const ctx = { + uri, // TODO: remove unused property. req.url should be the key. + isCollection: context.isCollection, + resources: context.resources, + req: request, + cacheStore: this.cacheStore + }; + return this.cacheMiddleware(ctx, () => { + if (ctx.res) { + return; + } - const ctx = { - uri, // TODO: remove unused property. req.url should be the key. - isCollection: context.isCollection, - resources: context.resources, - req: request, - cacheStore: this.cacheStore + return promise.then(res => ctx.res = res); + }) + .then(() => ctx.res); }; - return this.cacheMiddleware(ctx, () => { - if (ctx.res) { - return; - } - return promise.then(res => ctx.res = res); - }) - .then(() => ctx.res); + return execute(); } delete(uri, request, context) { diff --git a/src/oauth.js b/src/oauth.js index 0f8318237..0121a1587 100644 --- a/src/oauth.js +++ b/src/oauth.js @@ -57,34 +57,6 @@ class OAuth { }); } - introspectAccessToken() { - if (!this.accessToken) { - return Promise.reject(new Error('No accessToken in cache to be introspected.')); - } - - const endpoint = '/oauth2/v1/introspect'; - return this.getJwt(endpoint) - .then(jwt => { - const params = formatParams({ - token: this.accessToken, - token_type_hint: 'access_token', - client_assertion_type: 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer', - client_assertion: jwt - }); - return this.client.requestExecutor.fetch({ - url: `${this.client.baseUrl}${endpoint}`, - method: 'POST', - body: params, - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/x-www-form-urlencoded' - } - }); - }) - .then(Http.errorFilter) - .then(res => res.json()); - } - clearCachedAccessToken() { this.accessToken = null; } diff --git a/test/jest/http.test.js b/test/jest/http.test.js index 0b5600e98..9b24d8585 100644 --- a/test/jest/http.test.js +++ b/test/jest/http.test.js @@ -402,8 +402,8 @@ describe('Http class', () => { }); describe('Oauth', () => { - it('should get new token and retry request when accessToken is not active any more', () => { - expect.assertions(5); + it('should retry to get new token when response staus is 401', () => { + expect.assertions(4); requestExecutor = { fetch: jest.fn().mockImplementation((request) => { if (request.headers.Authorization === 'Bearer expired_token') { @@ -424,31 +424,55 @@ describe('Http class', () => { }), clearCachedAccessToken: jest.fn().mockImplementation(() => { oauth.accessToken = null; - }), - introspectAccessToken: jest.fn().mockResolvedValueOnce({ active: false }) + }) }; const http = new Http({ requestExecutor, oauth }); jest.spyOn(http, 'http'); return http.http('http://fakey.local') .then(res => { - expect(http.http).toHaveBeenCalledTimes(2); + expect(http.http).toHaveBeenCalledTimes(1); expect(oauth.getAccessToken).toHaveBeenCalledTimes(2); - expect(oauth.introspectAccessToken).toHaveBeenCalledTimes(1); expect(oauth.clearCachedAccessToken).toHaveBeenCalledTimes(1); expect(res.status).toEqual(200); }); }); - it('should throw error if status is 401, but access token is still active', () => { + it('should retry only one time when response staus is 401', () => { + expect.assertions(5); + requestExecutor = { + fetch: jest.fn().mockImplementation((request) => { + if (request.headers.Authorization === 'Bearer invalid_token') { + response.status = 401; + } else if (request.headers.Authorization === 'Bearer valid_token') { + response.status = 200; + } + return Promise.resolve(response); + }) + }; + const oauth = { + getAccessToken: jest.fn().mockResolvedValue({ access_token: 'invalid_token' }), + clearCachedAccessToken: jest.fn() + }; + const http = new Http({ requestExecutor, oauth }); + jest.spyOn(http, 'http'); + return http.http('http://fakey.local') + .catch(error => { + expect(http.http).toHaveBeenCalledTimes(1); + expect(oauth.getAccessToken).toHaveBeenCalledTimes(2); + expect(oauth.clearCachedAccessToken).toHaveBeenCalledTimes(1); + expect(error).toBeInstanceOf(OktaApiError); + expect(error.status).toEqual(401); + }); + }); + it('should throw error from oauth.getAccessToken', () => { expect.assertions(1); response.status = 401; const oauth = { - getAccessToken: jest.fn().mockResolvedValueOnce({ access_token: 'fake token' }), - introspectAccessToken: jest.fn().mockResolvedValueOnce({ active: true }) + getAccessToken: jest.fn().mockRejectedValueOnce(new Error('bad jwk')) }; const http = new Http({ requestExecutor, oauth }); return http.http('http://fakey.local') .catch(err => { - expect(err).toBeInstanceOf(OktaApiError); + expect(err.message).toEqual('bad jwk'); }); }); });