next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

Explain how to expose `user.id` to the client

Open t3dotgg opened this issue 2 years ago • 9 comments

Description 📓

I have at least 3 or 4 users a week coming into the T3 Discord to ask specifically about getting a "user's ID from the session object"

I've copy-pasted the snippet detailed in this issue so many times I've lost count. We're thinking of adding this by default to create-t3-app to help smooth out adoption of NextAuth

How to reproduce ☕️

https://github.com/t3-oss/create-t3-app/issues/176

Contributing 🙌🏽

Yes, I am willing to help implement this feature in a PR

t3dotgg avatar Jul 11 '22 21:07 t3dotgg

When I googled how to get the user ID from the next-auth client, I found a bunch of examples of doing exactly this. So it seems like enough people want the user ID stored in the session that it makes sense to do it by default.

akinnee avatar Jul 11 '22 23:07 akinnee

Thanks! There are a few things here.

First, https://github.com/t3-oss/create-t3-app/issues/176 would only make sense if NextAuth was initialized with an adapter (implying session: { strategy: "database"} ), but that is not our default currently. (There is no user object in case of a JWT strategy, only token, and token can already be overwritten by the user, so there is no good default here, without also changing the jwt callback's default.)

Our current default is to return a subset of fields that can be used to visually present a logged-in user. The user's id is not part of that list.

https://github.com/nextauthjs/next-auth/blob/5bd00f6ff156efa22f616fd3d047005293a470e1/packages/next-auth/src/core/lib/default-callbacks.ts#L12-L14

https://github.com/nextauthjs/next-auth/blob/5bd00f6ff156efa22f616fd3d047005293a470e1/packages/next-auth/src/core/routes/session.ts#L128-L135

The reason is that every exposed field to the client adds to the privacy/security concerns, and although I cannot dig up the right thread right now, but this was a deliberate decision by Iain, the original author of next-auth. The closest answer I could find is this https://github.com/nextauthjs/next-auth/issues/349#issuecomment-650742917.

But if we can make the documentation more clear on this here: https://next-auth.js.org/configuration/callbacks#session-callback I'm all for it!

cc @lluia something to consider for your upcoming changes.

balazsorban44 avatar Jul 12 '22 00:07 balazsorban44

@balazsorban44 I'd be willing to put money on the vast majority (as in >70%) of Next-Auth users needing userId from session. IMO it would be worth running a survey as the default experience here is rough.

We will be adding this in create-t3-app now as user IDs are essential for the production of an application

t3dotgg avatar Jul 12 '22 08:07 t3dotgg

Keep in mind that user.id only exists if you are using a database, and since this is not our default, it's more likely that user.id would not be utilized as much as outlined here. We, unfortunately, don't have the exact data to verify the 70%. A good estimate is to compare the number of next-auth downloads against all the adapters:

image

Source: npmtrends

This shows that adapters are still the minority.

As lib authors, we prefer secure/simple defaults (Eg.: moving to encrypted over signed JWTs by default, even if they might be slightly bigger or slower to encrypt/decrypt). We provide good ways to effectively hook into the process of exposing data through the session callback, and we will continue to do so. Since the issue is spawned from a template that is being used by users, I think it's easy to override the template to fit the need there, as it's already being done here: https://github.com/t3-oss/create-t3-app/pull/180

We want the users to be intentional about exposing fields that aren't necessary for a basic view.

I'm keeping this open until we document this exact use case better, but we agreed internally that we don't want to expose the user's id by default to the client for now.

balazsorban44 avatar Jul 12 '22 15:07 balazsorban44

I don't really see how the user being able to fetch their own ID is bad from a security perspective?

It not being a default because a user id just doesn't exist depending on the auth strategy makes sense though. 👍

akinnee avatar Jul 12 '22 22:07 akinnee

There are probably more examples, but if for some reason user IDs would have been sensitive information themselves (for example social security numbers), that might be an issue to expose by default.

balazsorban44 avatar Jul 13 '22 00:07 balazsorban44

There are probably more examples, but if for some reason user IDs would have been sensitive information themselves (for example social security numbers), that might be an issue to expose by default.

Well I sure hope nobody is using a ssn as a user id! 😆

akinnee avatar Jul 14 '22 20:07 akinnee

Hopefully :crossed_fingers:. You never know. :roll_eyes:

There might be other examples though, it was from the top of my head. I think MongoDB's ObjectId can also unveil some unwanted information as it consists of timestamps and a counter.

The point still stands, we - as a library - cannot assume that these values are safe to expose. The developer likely can.

balazsorban44 avatar Jul 19 '22 21:07 balazsorban44

It looks like this issue did not receive any activity for 60 days. It will be closed in 7 days if no further activity occurs. If you think your issue is still relevant, commenting will keep it open. Thanks!

stale[bot] avatar Sep 20 '22 16:09 stale[bot]

The point still stands, we - as a library - cannot assume that these values are safe to expose.

Just my 2 cents - you are absolutely right with that and to go with that decision I would prefer to do not get any property directly from the library.

To say that, it makes no sense for me to get an image nor the email nor the name. My suggestion would be just to return an (mostly) empty session or token record. Anything else should be added by the developer of the app.

In addition - maybe - the developers will read the documentation more carefully than wondering currently why just some properties are not there by default.

TomFreudenberg avatar Sep 26 '22 21:09 TomFreudenberg

the userId is already exposed in the image url

https://cdn.discordapp.com/avatars/490667823392096268/a_afad35cc47f411b98be2c32a754e3265.gif

anirkm avatar Jun 27 '23 13:06 anirkm