Skip to content

Commit

Permalink
feat: Add Oauth test and catch early
Browse files Browse the repository at this point in the history
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
  • Loading branch information
acezard committed Jul 25, 2023
1 parent 3cc0101 commit cc4d0de
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 11 deletions.
2 changes: 1 addition & 1 deletion packages/cozy-stack-client/src/CozyStackClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions packages/cozy-stack-client/src/CozyStackClient.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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)
}
Expand Down
31 changes: 24 additions & 7 deletions packages/cozy-stack-client/src/OAuthClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -504,14 +508,25 @@ 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),
{
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
Expand All @@ -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
}
Expand Down
35 changes: 35 additions & 0 deletions packages/cozy-stack-client/src/OAuthClient.spec.js
Original file line number Diff line number Diff line change
@@ -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',
Expand Down Expand Up @@ -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({
Expand Down
4 changes: 3 additions & 1 deletion packages/cozy-stack-client/src/errors.js
Original file line number Diff line number Diff line change
@@ -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 => {
Expand Down

0 comments on commit cc4d0de

Please sign in to comment.