Skip to content

Commit

Permalink
fix: Update tests for unregistered error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
acezard committed Jul 26, 2023
1 parent cc4d0de commit dc3bc1a
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 40 deletions.
10 changes: 9 additions & 1 deletion packages/cozy-stack-client/src/CozyStackClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,15 @@ class CozyStackClient {
this._promiseCache = new PromiseCache()
}
isRevocationError(err) {
return err.message && errors.CLIENT_NOT_FOUND.test(err.message)
const message = err?.message

if (!message) return false

if (
errors.CLIENT_NOT_FOUND.test(err.message) ||
errors.UNREGISTERED_CLIENT.test(err.message)
)
return true
}
/**
* Creates a {@link DocumentCollection} instance.
Expand Down
24 changes: 11 additions & 13 deletions packages/cozy-stack-client/src/OAuthClient.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import CozyStackClient from './CozyStackClient'
import AccessToken from './AccessToken'
import logDeprecate from './logDeprecate'
import errors from './errors'
import errors, { FetchError } from './errors'
import logger from './logger'

/**
* @typedef {string} SessionCode
Expand Down Expand Up @@ -516,16 +517,6 @@ 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,
Expand All @@ -540,6 +531,12 @@ class OAuthClient extends CozyStackClient {

return newToken
} catch (e) {
if (this.isRevocationError(e)) {
this.onRevocationChange(true)
logger.warn('Client has been revoked. Please authenticate again.')
throw e
}

if (
errors.EXPIRED_TOKEN.test(e.message) ||
errors.INVALID_TOKEN.test(e.message) ||
Expand All @@ -548,7 +545,8 @@ class OAuthClient extends CozyStackClient {
const revoked = await this.checkForRevocation()

if (revoked) {
throw new Error('Client has been revoked. Please authenticate again.')
logger.warn('Client has been revoked. Please authenticate again.')
throw e
}
}
throw e
Expand Down Expand Up @@ -668,7 +666,7 @@ class OAuthClient extends CozyStackClient {
*
* @async
* @returns {Promise<boolean>} A Promise that resolves to `false` if client is still valid, or `true` if it has been revoked.
* @throws {Error} If an error occurs while fetching the client information.
* @throws {FetchError} If an error occurs while fetching the client information.
*/
async checkForRevocation() {
try {
Expand Down
5 changes: 3 additions & 2 deletions packages/cozy-stack-client/src/OAuthClient.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,10 @@ describe('OAuthClient', () => {
client.fetchJSONWithCurrentToken = jest
.fn()
.mockImplementationOnce(async () => {
throw new FetchError('Invalid token', 'Invalid token')
throw new FetchError({}, 'Invalid JWT token')
})
.mockImplementation(async () => {
throw new Error('Client not found')
throw new FetchError({}, 'the client must be registered')
})

const revocationSpy = jest.spyOn(client, 'onRevocationChange')
Expand All @@ -308,6 +308,7 @@ describe('OAuthClient', () => {
} catch (error) {
expect(refreshSpy).toHaveBeenCalled()
expect(revocationSpy).toHaveBeenCalledWith(true)
expect(error).toBeInstanceOf(FetchError)
}

// Clean up our spies
Expand Down
54 changes: 30 additions & 24 deletions packages/cozy-stack-client/src/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,61 +2,67 @@ const EXPIRED_TOKEN = /Expired token/
const CLIENT_NOT_FOUND = /Client not found/
const INVALID_TOKEN = /Invalid JWT token/
const INVALID_TOKEN_ALT = /Invalid token/
const UNREGISTERED_CLIENT = /the client must be registered/

export default {
EXPIRED_TOKEN,
CLIENT_NOT_FOUND,
INVALID_TOKEN,
INVALID_TOKEN_ALT
INVALID_TOKEN_ALT,
UNREGISTERED_CLIENT
}

const invalidTokenRegex = /invalid_token/
const expiredTokenRegex = /access_token_expired/

const getWwwAuthenticateErrorMessage = response => {
const invalidTokenRegex = /invalid_token/
const expiredTokenRegex = /access token expired/
const wwwAuthenticateHeader =
response.headers && response.headers.get('www-authenticate')
const wwwAuthenticateHeader = response.headers?.get('www-authenticate')

if (!wwwAuthenticateHeader) {
return undefined
}
if (!wwwAuthenticateHeader) return undefined

if (expiredTokenRegex.test(wwwAuthenticateHeader)) {
return 'Expired token'
}
if (expiredTokenRegex.test(wwwAuthenticateHeader)) return 'Expired token'

if (invalidTokenRegex.test(wwwAuthenticateHeader)) {
return 'Invalid token'
}
if (invalidTokenRegex.test(wwwAuthenticateHeader)) return 'Invalid token'
}

const getReasonMessage = (reason, wwwAuthenticateErrorMessage) => {
// As for now we only want to use `reason.error` over `reason.message` if it's an unregistered client error
// For other scenarios, we want to still use `reason.message` over `JSON.stringify(reason)` for better backward compatibility
const isUnregisteredError =
typeof reason.error === 'string' && UNREGISTERED_CLIENT.test(reason.error)
? reason.error
: undefined

return undefined
return (
isUnregisteredError ||
reason.message ||
wwwAuthenticateErrorMessage ||
(typeof reason === 'string' ? reason : JSON.stringify(reason))
)
}

export class FetchError extends Error {
constructor(response, reason) {
super()
if (Error.captureStackTrace) {
Error.captureStackTrace(this, this.constructor)
}
Error.captureStackTrace?.(this, this.constructor)

// WARN We have to hardcode this because babel doesn't play nice when extending Error
this.name = 'FetchError'
this.response = response
this.url = response.url
this.status = response.status
this.reason = reason

let wwwAuthenticateErrorMessage = getWwwAuthenticateErrorMessage(response)

if (reason === null) {
throw new Error(
`FetchError received a ${response.status} error without a Response Body when calling ${response.url}`
)
}

let wwwAuthenticateErrorMessage = getWwwAuthenticateErrorMessage(response)

Object.defineProperty(this, 'message', {
value:
reason.message ||
wwwAuthenticateErrorMessage ||
(typeof reason === 'string' ? reason : JSON.stringify(reason))
value: getReasonMessage(reason, wwwAuthenticateErrorMessage)
})
}
}

0 comments on commit dc3bc1a

Please sign in to comment.