adonisjs / auth

Official Authentication package for AdonisJS
https://docs.adonisjs.com/guides/auth/introduction
MIT License
187 stars 65 forks source link

`login` fails when `identifierKey` leads to a falsy value #240

Closed shaheer-2000 closed 4 months ago

shaheer-2000 commented 4 months ago

Package version

8.2.3

Describe the bug

When identifierKey leads to a falsy value (say user[identifierKey] === 0), login (as a result of getUserForLogin failing) fails with the exception Cannot login user. Value of "${identifierKey}" is not defined.

Is this intended? If not, ideally the check should be for undefined and null values.

https://github.com/adonisjs/auth/blob/71c09d118e478289f631a9cd1c55ee48a7e333b5/src/Guards/Base/index.ts#L128-L129

Reproduction repo

No response

thetutlage commented 4 months ago

Why the id will be falsy?

shaheer-2000 commented 4 months ago

@thetutlage although identifierKey should ideally be the primaryKey, it isn't guaranteed that this would be a non-zero value. I agree that ideally it should be auto-incrementing, 1-based

If it is expected to always be 1-based, non-zero value, then there should be a clear mention of it in documentation or the error thrown imo (hence why I asked if this was intentional)

thetutlage commented 4 months ago

Not sure why such an obvious thing should be mentioned clearly in the docs.

You are authenticating a user and your system cannot guarantee that user will have an id, then you should look into fixing that

shaheer-2000 commented 4 months ago

The user does have an id (integer) and that's 0, as per my previous comment. It isn't the system that's at fault here, but I see you've marked this as completed, so I'll assume this is intentional and id should always be 1-based.

Thanks for your time, and apologies for the non-issue.