Should authentication-local strategy 'authenticate' method omit 'provider' when calling getEntity?

[Huggzorx]

In authentication-local/src/strategy.ts there is this code:

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

await this.comparePassword(result, password);

return {
      authentication: { strategy: this.name },
      [entity]: await this.getEntity(result, params)
};

The call to findEntity omits the provider, but the call to getEntity does not. Further, the getEntity method just returns the result (of findEntity) if provider is undefined, but calls the entity service (e.g. users) and passes on the provider along with the rest of the params if it is set.

This means that even though this is is essentially an internal call to a different service, the provider will be 'rest' if the authentication was triggered with a REST request.

Is there a rational behind this that I'm missing?

An example of how this could be a problem is if I have an after hook on my users service that removes certain fields (e.g. isVerified) when provider is external. If I extend the Authentication service to reject the authentication request if isVerified is false, this won't work because the authentication result will contain a user record that has been sanitised of the isVerified field.

I can work around this by extending the local authentication strategy but maybe I'm missing the reasoning behind it being this way?

Edit Actually, I see now that the reasoning behind this is that the user entity is returned along with the access token so in my example you wouldn't want to skip the removal of isVerified. I guess the answer in this case is to just extend the strategy and put the isVerified checks in the authenticate method rather than extending the authentication service and doing the check after the authentication?

[daffl]

Correct. For local authentication you can just override getEntityQuery to include isVerified: true.

[Huggzorx]

That makes sense, thanks

[matiaslopezd]

Hi @daffl I'm having a problem with authenticate function of LocalStrategy class, exactly here:

https://github.com/feathersjs/feathers/blob/892c9a3a599c19bb275d001b4e719f486140b97b/packages/authentication-local/src/strategy.ts#L129

In contrast with the previous line, in findEntity you omit provider param, but not in getEntity. This provoke that pass provider like rest when is in reality, server request.

https://github.com/feathersjs/feathers/blob/892c9a3a599c19bb275d001b4e719f486140b97b/packages/authentication-local/src/strategy.ts#L123

For example, I have in app.hook.js a hook over all request. This make some endpoints exceptions for validate request (or not) based on result of isProvider('external') common hook.

We are migrating our Rest API from v3 to v4 and this broke all because every login do a request over users service with a provider like rest.

I know that I can override authenticate function but this not follow the logic of provider param.

Again, thanks to the feathers team for this awesome web-framework and I'm still donating!

[daffl]

Yes, getEntity should return the entity for both cases. The first is for the external representation that will be sent with the authentication result. The second is internal for being able to compare the password.

[matiaslopezd]

Ok I understand that both return the entity, but is correct say that the provider is rest when in reality is server request?

I mean the first request to authentication endpoint is from rest or external, the second findEntity and third getEntity is from server not from rest. That flow it is not fulfilled with the current code when return entity without omit provider param, generating a internal request like external and also authenticated.

Authenticated param in context of a server request never needed. Because server not need know that is authenticated to himself.

In summary I believe that getEntity need omit provider and authenticated params by default to follow the logic of provider value.

[daffl]

The result from authenticate will be returned to the client so it is an external request. We e.g. can't return the password to the user after they authenticated.