Description
TL;DR
The isVerified
property derived from fromUserInfoAPI
is always unconditionally overridden by fromIdTokenPayload
.
Background
The userInfoMap
config for a Custom Provider is always given default values for the fromIdTokenPayload
and fromUserInfoAPI
objects. Because the developer's input values are always spread into these default objects, it's impossible to remove the default values by passing undefined
or {}
to fromIdTokenPayload
or fromUserInfoAPI
. And, of course, it's impossible to remove these defaults by excluding those properties from the Custom Provider's config. (Technically, a developer could to pass { [key: string]: undefined | null }
to override the defaults. But this is unintuitive, so it's unlikely that any developer would ever do this naturally.)
Because fromIdTokenPayload
and fromUserInfoAPI
are always given default values, the two checks that try to derive the isVerified
value will always run. This, in turn, means that fromUserInfoAPI.emailVerified
's value will always be overridden by fromIdTokenPayload.isVerified
. This produces a confusing/buggy DX. (Yes, developers can technically circumvent this problem as I mentioned earlier. But the only way they would know how to do that is by scrutinizing the source code. So the workaround I mentioned is not valid/practical for users.)
It seems that we might need to put some logic in place to prevent this issue from happening. Ideally, if a user only supplies a fromUserInfoAPI
object, then that should be the sole source of truth for deriving user information. Or, if a user only supplies a fromIdTokenPayload
object, then that should be the sole source of truth.
Potential Solutions
Here are some non-exhaustive ideas about how to solve this problem:
-
Keep the default values, and write documentation to bring clarity to developers. The existing OAuth Documentation would need to be updated to tell developers to pass
{ [key: string]: undefined | null }
to whicheveruserInfoMap
config they don't want to use. Admittedly, this produces an undesirable developer experience, and it also leaks the implementation details ofsupertokens-node
. -
Remove the default values, and write documentation to show devs good default values. In this case, the documentation's Node.js code block would stay the same. (The code block would not need to supply an object of
undefined
s tofromIdTokenPayload
, as would be required if the first solution was pursued.) And the page's documentation would be updated to suggest good default values forfromIdTokenPayload
and/orfromUserInfoAPI
. This would likely be a (small) breaking change; but it would produce a more intuitive developer experience and savesupertokens-node
from having to change its logic. -
Tinker with the conditional statements. Currently, the
userId
andemail
checks do not perform variable assignment ifundefined
is returned from the object property lookup (i.e.,accessField
). Perhaps something similar could be done forisVerified
? However, it's worth noting that relying on this logic could end badly if a Custom Provider does somehow provide a value for these properties that the developer does not want to use. -
Provide a way for developers to cancel the default values. This could be something like
excludeDefaultsForUserInfoMap: true
. However, this would bloat the Custom Provider config's options more, and it still might be something that leaks implementation details.
I'm sure there are other valid solutions as well. But this is probably a concern worth addressing in some way or another to prevent developers from experiencing unexpected behavior.