From cc4d0de32042aea1de63c1fcb813085d065a13ad Mon Sep 17 00:00:00 2001 From: Antonin Cezard Date: Tue, 25 Jul 2023 11:12:27 +0200 Subject: [PATCH] feat: Add Oauth test and catch early In some cases the promise rejection is not handled at all by refreshToken, since it was not really clear what was happening it seemed safer to add an early check --- .../cozy-stack-client/src/CozyStackClient.js | 2 +- .../src/CozyStackClient.spec.js | 4 +-- packages/cozy-stack-client/src/OAuthClient.js | 31 ++++++++++++---- .../cozy-stack-client/src/OAuthClient.spec.js | 35 +++++++++++++++++++ packages/cozy-stack-client/src/errors.js | 4 ++- 5 files changed, 65 insertions(+), 11 deletions(-) diff --git a/packages/cozy-stack-client/src/CozyStackClient.js b/packages/cozy-stack-client/src/CozyStackClient.js index 678b054581..d64b49311e 100644 --- a/packages/cozy-stack-client/src/CozyStackClient.js +++ b/packages/cozy-stack-client/src/CozyStackClient.js @@ -230,7 +230,7 @@ class CozyStackClient { if ( errors.EXPIRED_TOKEN.test(e.message) || errors.INVALID_TOKEN.test(e.message) || - /invalid_token/.test(e) + errors.INVALID_TOKEN_ALT.test(e.message) ) { try { await this._promiseCache.exec( diff --git a/packages/cozy-stack-client/src/CozyStackClient.spec.js b/packages/cozy-stack-client/src/CozyStackClient.spec.js index 37425d7ebb..b057ce6a2f 100644 --- a/packages/cozy-stack-client/src/CozyStackClient.spec.js +++ b/packages/cozy-stack-client/src/CozyStackClient.spec.js @@ -489,7 +489,7 @@ describe('CozyStackClient', () => { ) }) - it(`should NOT try to refresh the current token when receiving a 'forbidden' response with 'invalid token' error in 'www-authenticate' (not expired token)`, async () => { + it(`should try to refresh the current token when receiving a 'forbidden' response with 'invalid token' error in 'www-authenticate' (not expired token)`, async () => { const client = new CozyStackClient(FAKE_INIT_OPTIONS) jest.spyOn(client, 'refreshToken') global.fetch.mockResponseOnce(() => { @@ -515,7 +515,7 @@ describe('CozyStackClient', () => { } catch (e) { expect(e).toBeInstanceOf(FetchError) expect(e.message).toBe('Invalid token') - expect(client.refreshToken).not.toHaveBeenCalled() + expect(client.refreshToken).toHaveBeenCalled() const token = client.getAccessToken() expect(token).toEqual(FAKE_INIT_OPTIONS.token) } diff --git a/packages/cozy-stack-client/src/OAuthClient.js b/packages/cozy-stack-client/src/OAuthClient.js index 10d35354a3..96323349fe 100644 --- a/packages/cozy-stack-client/src/OAuthClient.js +++ b/packages/cozy-stack-client/src/OAuthClient.js @@ -494,8 +494,12 @@ class OAuthClient extends CozyStackClient { * @returns {Promise} A promise that resolves with a new AccessToken object */ async refreshToken() { - if (!this.isRegistered()) throw new NotRegisteredException() - if (!this.token) throw new Error('No token to refresh') + if (!this.isRegistered()) { + throw new NotRegisteredException() + } + if (!this.token) { + throw new Error('No token to refresh') + } const data = { grant_type: 'refresh_token', @@ -504,7 +508,7 @@ class OAuthClient extends CozyStackClient { client_secret: this.oauthOptions.clientSecret } try { - const result = await super.fetchJSON( + const result = await this.fetchJSONWithCurrentToken( 'POST', '/auth/access_token', this.dataToQueryString(data), @@ -512,6 +516,17 @@ class OAuthClient extends CozyStackClient { headers: { 'Content-Type': 'application/x-www-form-urlencoded' } } ) + // Have to catch early to check for revocation because it seems the error is not caught by the catch below in some cases (e.g. when the client is not registered, even if it should be) + .catch(async e => { + if (await this.checkForRevocation()) { + throw new Error( + 'Client has been revoked. Please authenticate again.' + ) + } else { + throw e + } + }) + const newToken = new AccessToken({ refresh_token: this.token.refreshToken, ...result @@ -528,11 +543,13 @@ class OAuthClient extends CozyStackClient { if ( errors.EXPIRED_TOKEN.test(e.message) || errors.INVALID_TOKEN.test(e.message) || - /invalid_token/.test(e) + errors.INVALID_TOKEN_ALT.test(e.message) ) { - // If the client has been revoked, then checkForRevocation will throw the error - // if not, then throw the real error - this.checkForRevocation() + const revoked = await this.checkForRevocation() + + if (revoked) { + throw new Error('Client has been revoked. Please authenticate again.') + } } throw e } diff --git a/packages/cozy-stack-client/src/OAuthClient.spec.js b/packages/cozy-stack-client/src/OAuthClient.spec.js index a9c72a1ce0..d4dffecec2 100644 --- a/packages/cozy-stack-client/src/OAuthClient.spec.js +++ b/packages/cozy-stack-client/src/OAuthClient.spec.js @@ -1,5 +1,6 @@ import OAuthClient from './OAuthClient' import AccessToken from './AccessToken' +import { FetchError } from './errors' const CLIENT_INIT_OPTIONS = { uri: 'http://cozy.tools:8080', @@ -280,6 +281,40 @@ describe('OAuthClient', () => { expect(client.getAccessToken()).toBe('accessToken-abcd') }) }) + + describe('getRefreshToken', () => { + it('should call onRevocation', async () => { + client = new OAuthClient(REGISTERED_CLIENT_INIT_OPTIONS) + client.setToken({ + tokenType: 'type', + accessToken: 'accessToken-abcd', + refreshToken: 'refresh-789', + scope: 'io.cozy.todos' + }) + client.fetchJSONWithCurrentToken = jest + .fn() + .mockImplementationOnce(async () => { + throw new FetchError('Invalid token', 'Invalid token') + }) + .mockImplementation(async () => { + throw new Error('Client not found') + }) + + const revocationSpy = jest.spyOn(client, 'onRevocationChange') + const refreshSpy = jest.spyOn(client, 'refreshToken') + + try { + await client.fetchInformation() + } catch (error) { + expect(refreshSpy).toHaveBeenCalled() + expect(revocationSpy).toHaveBeenCalledWith(true) + } + + // Clean up our spies + refreshSpy.mockRestore() + revocationSpy.mockRestore() + }) + }) }) const FAKE_DATA_COZY = JSON.stringify({ diff --git a/packages/cozy-stack-client/src/errors.js b/packages/cozy-stack-client/src/errors.js index f340ffb799..f7b7ff2401 100644 --- a/packages/cozy-stack-client/src/errors.js +++ b/packages/cozy-stack-client/src/errors.js @@ -1,11 +1,13 @@ const EXPIRED_TOKEN = /Expired token/ const CLIENT_NOT_FOUND = /Client not found/ const INVALID_TOKEN = /Invalid JWT token/ +const INVALID_TOKEN_ALT = /Invalid token/ export default { EXPIRED_TOKEN, CLIENT_NOT_FOUND, - INVALID_TOKEN + INVALID_TOKEN, + INVALID_TOKEN_ALT } const getWwwAuthenticateErrorMessage = response => {