Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In the session example code, should 'event.params.userId' be just 'userId'? #835

Open
johndunderhill opened this issue Aug 2, 2024 · 2 comments
Labels
solidstart Related to SolidStart

Comments

@johndunderhill
Copy link

📚 Subject area/topic

/solid-start/building-your-application/api-routes.mdx

📋 Page(s) affected (or suggested, for new content)

https://docs.solidjs.com/solid-start/building-your-application/api-routes#session-management

📋 Description of content that is out-of-date or incorrect

The narrative below the code example under Session management states 'you can see that the userId is read from the cookie and then used to look up the user in the store'. But that's not what I see here. I see that the userId is read from the cookie, and then checked if empty, generating an error if so. If it's not empty, then 'event.params.userId' (where does that come from?) is used to look up a user in a store and the IDs are compared. Seems like a different flow.

A subsequent example in the Session documentation (following the link below the narrtive) shows a getUser() function without a parameter that somehow locates appropriate session data and fishes a userId out of that. But this would require a session ID of some kind, which I don't see here. Unless that's SESSION_SECRET?? Where does it come from??

Is there any way this could be made a bit more clear, showing one step at a time what choices are available here, and what's required to use one of these approaches?

🖥️ Reproduction in StackBlitz (if reporting incorrect content or code samples)

No response

@johndunderhill johndunderhill added improve documentation pending review Awaiting review by team members. labels Aug 2, 2024
@LadyBluenotes LadyBluenotes removed their assignment Sep 25, 2024
@LadyBluenotes LadyBluenotes removed the pending review Awaiting review by team members. label Sep 25, 2024
@brenelz
Copy link
Collaborator

brenelz commented Sep 26, 2024

So looking at this I think the example is correct. It is basically making sure the cookie userId matches the event.params.userId. The event.params.userId would come from the url of the api

/user?userId=123

We could probably do a better job explaining the event. What do you think?

@johndunderhill
Copy link
Author

@brenelz I'd like to help make this example more clear, but I'm still confused about we're trying to show here. This example does not really appear to be concerned with 'session management.' There are no session IDs here, no server-side session store, nor any ancillary session state.

Rather, this example seems to be concerned with authenticating the user, somewhat confusingly using an endpoint that also returns user information. It might be slightly less confusing if the example retrieved some application data instead, after validating the user.

const userId = getCookie("userId");
if (!userId) {
  return new Response("Not logged in", { status: 401 });
}

Here we retrieve the user ID from a cookie. Per the narritive, I'll assume that this cookie is httpOnly, however absent any information to the contrary, I can't assume it is also Secure, nor can I assume the presence of any cross site request forgery (CSRF) protection. Hence this cookie is subject to theft.

Given the code, I must also assume that this is an ordinary cookie (as opposed to a cookie containing an encrypted payload, or a cryptographically signed user object, such as a JWT). Hence the cookie is also subject to forgery (say, using a script).

So in this example, do we intend to rely entirely on httpOnly to seucrely authenticate the user? Is this common practice in Solid Start?

const user = await store.getUser(event.params.userId);

If the user is attested to by the cookie, then why pass in the user ID at all, in the URL? What does this add?

I am certainly not an expert at this, but I'm working on a large project in Solid that I'm considering porting to Solid Start. I'm trying to understand what the best practice is for handling authentication in this environment. I have no pre-conceptions; I want to take the path of least resistance. Is this what everybody else is doing? Using a bare user ID in an httpOnly cookie?

@LadyBluenotes LadyBluenotes added solidstart Related to SolidStart and removed improve documentation labels Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solidstart Related to SolidStart
Projects
None yet
Development

No branches or pull requests

3 participants