[dove/feathers-authentication] Access token not removed on logout when user is not found

[claustres]

Steps to reproduce

Create an OAuth based app which reauthenticates the user using reAuthenticate() in the index when an access token is available. In the app add a try/catch block like this:

try {
        const response = await app.reAuthenticate()
        ...
      } catch (error) {
        // This ensure an old token is not kept when the user has been deleted
        if (error.code === 404) await app.logout()
        ...
      }

Now delete the user in the database and try to reauthenticate, the authentication fails probably because it is still using the old token:
error: api/authentication - Method: create: No record found for id '62c5b5921c535f8f68440c17'

Expected behavior

The access token referring to the old user should be deleted so that a new authentication is performed from scratch. It seems that the remove operation performed by logout() does not work as the user is not found and the token is not removed as well.

I agree this is a race use case but as with OAuth users should not be persistent it could be possible. Typically to be GDPR compliant you should be forced to purge users in the DB on a regular basis.

Not sure if this is a bug or expected behavior but this code worked with Feathers v3. Maybe it is now expected to call api.authentication.removeAccessToken() explicitely as using logout() is not sufficient ?

Actual behavior

The token is not deleted.

System configuration

Module versions (especially the part that's not working): 5.0.0-pre.28

NodeJS version: 16.14.12

[daffl]

So there was a legit problem with the error being cached which has been fixed in #2892 but I'm not sure if the access token should be removed on all errors. For example, any 5xx error might only be temporary as well as e.g. a 429 (Too Many Requests) in which case retrying with the existing access token could work.

[claustres]

I agree we should filter which error requires the token being removed. Typically in the issue use case a 404 should remove the token.

[daffl]

That makes sense. #2894 will remove the access token for all unrecoverable 400 errors. I think for other errors it is up to the developer if they want to remove the access token or not.