user entity is not getting removed on logout

[KrishnaPG]

Steps to reproduce

When logout is called on the client side, the server is removing the authentication object on the connection, but not removing the user entity. This is causing the server to return results even after logout

Expected behavior

The user entity should not exist anymore on the connection (as well the params in the hooks)

Actual behavior

The user entity exists and any hooks that check for it assume that the user is still logged in

System configuration

Using socket.io as the transport with authentication backed by Knex DB adapter for local + jwt strategies

Module versions: 4.5.2

NodeJS version: v12.9.1

Operating System: Windows 10

Browser Version: Chrome latest

Module Loader: npm 6.6.0

[daffl]

Do you have an example when this is causing issues?

• All channels will get removed on logout
• Any future request will still run through authentication and error since the information is missing

I don't disagree that it would still make sense to remove but I currently can't think of a reliable way to do this (since the entity name or authentication result can be anything).

[KrishnaPG]

@daffl you are right regarding the authenticate hook acting as gateway and hence rejecting the unauthenticated.

The problem for me came when I used a combination of authenticated + no-auth-required services. The exact scenario is like this: I was trying to create an audit log service that logs every other service call. I made this audit log as an internal-only and no-auth-required service, and put it as application level hook so that any call to any other service gets logged into the db. This is all fine, but I was also trying to log which call was made against which user id.

Taking the cue from feathers-authentication-hooks, I tried to determine the current user with below kind of code (sampled from the docs there):

      setField({
        from: 'params.user.id',
        as: 'params.query.id'
      })

And used couple of these (again from the docs there) in the authenticated services to restrict/filter the current user:

const setUserId = setField({
  from: 'params.user.id',
  as: 'data.userId'
});
const limitToUser = setField({
  from: 'params.user.id',
  as: 'params.query.userId'
});

All this started giving weird results. Took me a day to figure out what was going wrong: the params.user.id does not really reflect the underlying user.

To summarize, I understand it might be difficult to remove the user entity. The fundamental question then is:

• what is the most reliable way to determine who is making the service call i.e. the current user, in the hooks (for both authenticated and unauthenticated scenarios)?

I guess I would have to check connection.authentication && connection.user - but

1. would that work for all transports (e.g. REST), and
2. would it be reliable (i.e. likely to change in other versions etc.) ?

And since we are on that topic, another question on a slightly different tangent is:

• the audit log (i.e. log every other service call), Wondering if anyone has tackled this and any hooks/plugins etc. that already provide this kind of audit log functionality.

[KrishnaPG]

Also came across this scenario: consider a mixed-auth endpoint which shows some public records for unauthed users, and all private records for authed users. Below sequence of steps:

• visit the endpoint before login: it displays the public records
• now, login as user A and visit the same endpoint: it displays the private records of user A.
• now logoff. Client removes the JWT from its localstorage.
• next, visit the endpoint again. There is no JWT, which means no auth test done. But when the records are retrieved, the find/get will look at the params.user which still points to user A, hence shows the private records of user A.

This was the problem I was facing.

[daffl]

That shouldn't be possible because like I mentioned in my previous comment, external requests that require authentication will fail if there is no information available (even if the user is still set on the connection).

@KrishnaPG Thinking a little more about it, there is a fairly straightforward fix for this which is adding

const { entity } = this.configuration;

delete connection[entity];

to https://github.com/feathersjs/feathers/blob/master/packages/authentication/src/jwt.ts#L58

[KrishnaPG]

Thank you @daffl The problem is not with authenticated endpoints. It is with endpoints that support both authenticated and unauthenticated (for example, this scenario from the guide)

Wondering - why not just remove the whole connection object (kick the client, since they logged out) and force them to reconnect (which socket.io and the likes do anyway). Wouldn't it be clean approach?

The added advantage is: JWT can be tied to the connection (some UID specific to the connection is put in the payload and verified). Any other connections that try to use the same JWT will fail, and once the connection is removed JWT will automatically fail to verify. (Currently, JWTs are still valid even after the client is logged out, if someone has a copy of them, if I am not mistaken)

[daffl]

I wonder how you came across that guide. The docs are from two versions and several years ago and might not be applicable to the most recent version (which is at https://docs.feathersjs.com/).

Your suggestion of clearing the connection completely makes sense though, however, to invalidate JWTs properly a JWT blacklisting mechanism should be used.

[KrishnaPG]

I did more debugging, and the issue is occurring in a more common scenario than what I described earlier. Scenario is as simple as login, logout and then login as another user:

---

1. Local strategy auth with users database and service. Client connects with websockets. Users are looked up with eMails, but primary keys are user ids.
2. user hooks

 find: [
          authenticate('jwt'), 
          context => { 
               if (context.params.user) // limit the queries to user-id
                    context.params.query.id = context.params.user.id; 
           }
        ],

3. Login as user1 from the client using authenticate() method with local strategy (email + password). It will succeed
4. Logout
5. Try logging in with user2. This will not succeed

---

The problem is:

1. In the authenticate method of local strategy, it is making an internal service call to serivce("users").find() method

   feathers/packages/authentication-local/src/strategy.ts

   Line 124 in 859c601

   const result = await this.findEntity(username, omit(params, 'provider'));

2. For internal service calls the users.find hook authenticate('jwt') does not work or is ignored.

3. Now, when connection is fresh, the users.find() will give correct result, since there is no user on the connection or context.param.user; However, when there was an earlier user, and that user was disconnected or logout, but that user is still hanging around in the connection and context.param.user in that case the second users.find hook, shown above, will try to limit the query to that old user. As a result, I am seeing queries such as:

      find({ query: {  email: `user2-email', id: 'user1-id-from-old-connection'      } })

which ofcourse yields zero results, and there by auth failing.

Not sure how to resolve this situation. I am unable to progress and enforce queries to limit to the current logged in user. If I remove the second hook in the find method (context.params.query.id = context.params.user.id) login works fine - but the queries start to return all data (as expected but not desired).

[daffl]

This will be fixed via #1889

[KrishnaPG]

Thank you. Meanwhile, I have forced socket reconnect on the client-side whenever logout was triggered. It worked fine. Now login with user2 works after logout of user1 works.

The only problem now is, logout event is available on the feathers client, but JWT reauth failure event is not available. It would be great if event is raised on JWT reauth failure on the client side (as requested in #1858 ) so that full spectrum can be covered.

fClient.on('logout', reconnectSocket);
fClient.on('authFailure', reconnectSocket); //<-- this is not available currently

[KrishnaPG]

Never mind. Forcing socket reconnect from the client-side has its own set of problems, opening a pandora box. Guess its better to fix it on the server side with clean session / connection close.

Only point I could add is, please consider both scenarios:

1. User logged out, and logging-in
2. User is reauthenticating:
   • scenario 1: JWT renewal (for the same user)
   • scenario 2: JWT expired, and when prompted with login UI, the user entered entire different credentials and logged-in as different user altogether.

Hope #1889 works well in all the above scenarios.

[KrishnaPG]

Another case to cover (documenting for the sake of completeness):

3. User logs in as user1 and opens another tab in the same browser and logs in as another user user 2.

Effectively now both users (in two different tabs of same browser instance) are sharing the same JWT in the local storage. Any reAuth or service call from the old tab should be invalidated.