fix: allow user ID to be passed as a string

[sir-dunxalot]

Fixes/supersedes nextauthjs/next-auth#1644

Per the JWT spec (sub section), a token's sub is a string. Thus, if next-auth is calling getUser(sub) (example here), you're passing a string to getUser(). This PR addresses this incompatibility by allowing userId to be passed into the Prisma adapter's methods as either a string or a number.

[kripod]

Casting each userId parameter to Number instead of the other way around – String(userId) – doesn’t look good to me. IDs are often stored as strings, rather than numbers.

[sir-dunxalot]

@kripod I'm not sure how casting to a string would be compatible with the Prisma adapter and schema. The point of this PR is to enable strings and numbers to be passed as the userId and turn that value into a number for the Prisma adapter.

Here is the schema.prisma file, which shows user ID is stored as int:

Maybe you can clarify your comment?

Personally, I would like to use ccid or uuid, which would be strings, as the user ID but #38 shows that isn't possible today.

[kripod]

There are no runtime changes to be made. The current code already handles both string and number IDs correctly – only the types aren’t correct.

[sir-dunxalot]

I think I understand but I would like to make sure we're on the same page... Without these code changes, I encounter a silent error. Specifically, when next-auth gets sub from a JWT and calls getUser() like here, an error is thrown. However, because of this empty catch block the error is completely silent. As a result, account linking does not work and a user adding an account to their existing user will end up with a new user created for the new account.

Are you saying we should change, for example, async function getUser(userId: number) to async function getUser(userId: number | string). Then cast userId to string and the above bug won't be encountered? If so, do we need to cast the value at all? The only reason I am casting values is because of this feedback in the original PR.

If what I just wrote matches your feedback then I will update the PR accordingly.

Thanks!

[kripod]

Thanks for your feedback!

I wasn’t aware of the JWT case as I’ve been using a stored session approach all along. In my case, transforming only the types has worked flawlessly. As strings cannot be casted into IEEE 754 numbers without data loss, I would advise against changes like below:

const _userId = Number(userId);

Casting the other way (from numbers to strings) could work, though, but I’m not sure what the benefit of that would be:

const _userId = String(userId);

[sir-dunxalot]

@kripod Based on this comment do you still want me to update this PR for merging? If so, I will be happy to make the changes requested above.

[kripod]

@sir-dunxalot Since this PR was opened, #72 has landed, which suggests the use of string IDs instead of numbers. I think this is resolved now.