BeforeLogin could provide the auth used to login

[SebC99]

Is your feature request related to a problem? Please describe.
We let users create their account through various providers (email, facebook, google, etc.) and sometimes they forget which they used.
So when they try to login in the app after a while, they can try with an email/password and we would like to tell them they should use Facebook instead.

Describe the solution you'd like
The beforeLogin could be called even if a login failed, and we could have a status:failed and the authData provided by the client to login (for example the email), so that we could check if that user is a facebook user instead and return a specific error.
With the actual implementation, we can't do that, and the login will just failed without triggering the beforeLogin, and the user won't be able to know if he used a wrong password or a wrong provider.

Describe alternatives you've considered
The only way to do it for now, is to try to login, catch the error on client side, request a cloudfunction to check the user authentication method, and display the correct error message

[dplewis]

I believe what you are looking for it here you can pass the authData into userResult into the beforeLogin trigger. Would you like to do a PR?

[SebC99]

Not really as this is called only if the login succeeds... so it won’t help explaining a failure reason

[SebC99]

@dplewis it looks more like #4524 which has been closed

[dplewis]

This line from the closed PR

I think that the definition of beforeLogin is not before the login process begins, but before it is completed

I assumed this is how beforeLogin worked

@omairvaiyani You know more about this than I do.

[omairvaiyani]

That's correct as authentication/credential verification must take place, otherwise the beforeLogin hook will be called on incorrect login attempts - which whilst it would help with the use-case outlined by @SebC99, will otherwise cause problems for use-cases that require a valid attempt. The beforeLogin hook could be altered to trigger on failed attempts, but even with a new property on the request to show that it's a failed attempt would still be a breaking change for current developers. Consider a PR if this use-case is really important, there may be alternative options instead of introducing a breaking change.

[dplewis]

@omairvaiyani Thank you for the quick reply, it was really helpful. I can see why incorrect login attempts are skipped because we assume the user is logged before the trigger.

A new trigger on failure would prevent a breaking change

[SebC99]

you mean a beforeLoginAttempt one?

[dplewis]

but even with a new property on the request to show that it's a failed attempt would still be a breaking change for current developers.

I don't think a new property would break current apps. I could be wrong here

[SebC99]

I don't think a new property would break current apps. I could be wrong here

I think it could: a new authSuccess boolean property would be great (with a authData one to show which authData led to the failure) but then all users using the actual beforeLogin trigger would have to test this property to ensure their code is not run for non-signed user.
Moreover, with a authSuccess:false the trigger wouldn't be able to be called with a user in the object argument, so this undefined object could break their code too.
But it could be a simple warning in the changelog.

[omairvaiyani]

I don't think a new property would break current apps. I could be wrong here

Unfortunately it would be classed as a breaking change for the reasons @SebC99 has outlined - even if you warn developers in the changelog, the change would still break their app according to semver. parse-server recently bumped to v4 so it might be a little premature to go again - a different hook or a feature flag might be the quickest option here.

[SebC99]

I can try to make a PR but I don’t understand why there’s 2 calls to beforeLogin, one in RestWrite.js and one in the UserRouter.js?