Hebilicious / authjs-nuxt

AuthJS edge-compatible authentication Nuxt module.
https://authjs-nuxt.pages.dev/
MIT License
249 stars 28 forks source link

callback_url never stored in cookies #23

Closed dutsik closed 1 year ago

dutsik commented 1 year ago

Environment

node: 18.16.1

Reproduction

I am using signin helper with custom callback_url like this:

  useAuth().signIn('auth0', {callbackUrl:"/activity/hausfuehrung-chesa-laudenbacher", redirect:true}, options)

but this callback never triggered. instead you get to a default home page.

Describe the bug

I can assume that it should be stored in cookie before we do an actual redirect to auth provider page. but it is not... it is basically overwriten while copying cookie headers in utility function respondWithResponse and only the last cookie is stored packages/authjs-nuxt/src/runtime/utils.ts#L83

i beleive that the straight forward fix would be smth like this:

event.node.res.setHeader(key, value); => event.node.res.appendHeader(key, value);

Additional context

No response

Logs

No response

Hebilicious commented 1 year ago

Hi!

This is a bigger issue than I realised and I've opened an issue to track it there https://github.com/unjs/h3/issues/439 Your approach wouldn't work in all runtimes ... So I think to fix it there, we should introduce https://www.npmjs.com/package/set-cookie-parser

Response support has landed in h3 and it will be out in the next release : https://github.com/unjs/h3/pull/436 I think we should change this to appendHeader in h3 as well.

Hebilicious commented 1 year ago

@dutsik could you try #24 and let me know if it fixes your issue ?

dutsik-p commented 1 year ago

0.1.5 worked as expected 0.1.6 introduced a regression. so it doesnot work :(

Hebilicious commented 1 year ago

0.1.5 worked as expected 0.1.6 introduced a regression. so it doesnot work :(

Yes this got fixed in 0.1.7, sorry about that.

dutsik-p commented 1 year ago

no it doesnot fix the issue. this code that was removed in 0.1.6

  const cookieHeader = response.headers.get("set-cookie")
  if (cookieHeader) {
    const cookieString = setCookieParser.splitCookiesString(cookieHeader)
    event.node.res.setHeader("set-cookie", cookieString)

was solving the issue.

Set-Cookie header is just one header with all the cookie combined in it. you cannot loop through setting this header, you have to combine them and set or if you wish to loop through you need to append.

Hebilicious commented 1 year ago

@dutsik this code is still present https://github.com/Hebilicious/authjs-nuxt/blob/3bb9d0c999645eff762336bf5069c52565aa153e/packages/authjs-nuxt/src/runtime/utils.ts#L83

Are you still experiencing the issue ? Btw we just landed https://github.com/unjs/h3/pull/445 in h3 so we'll get better support with the next release.

dutsik commented 1 year ago

No it doesn’t work

I guess you need to revert back the part that process cookie headers separately.

Or loop through the ‘response.headers.entries()’ instead of ‘response.headers’, but I am not familiar with that part

Hebilicious commented 1 year ago

The logic is exactly the same and work on my end, can you please provide me with a reproduction repo/stackblitz? There could be a bug with h3 splitCookieString, which would explain the different behaviour between 0.1.5 and the releases after.

I just pushed 0.1.8, let me know if that fixes it for you @dutsik.

dutsik commented 1 year ago

I created the sandbox for you. expected behavior: when you click on signing the redirect cookie should be set correctly.

Here is what we have:

the problem is not with split cookie, but with the loop. output into the console key values, i am getting 'set-cookie' key at least twice with different values and only the last one used...

Hebilicious commented 1 year ago

@dutsik Thank you so much, I've been able to reproduce the issue locally. It will be fixed in 0.1.9, and I pushed a similar fix to H3. Since a Header object can have multiple "set-cookie" headers, and they can also have multiple values, we need to split the header value AND to append it to the h3 headers.

{                                                                                                                                            
  key: 'set-cookie',
  value: 'next-auth.callback-url=http%3A%2F%2Flocalhost%3A3000; Path=/; HttpOnly; SameSite=Lax'
}
{                                                                                                                                             
  key: 'set-cookie',
  value: 'next-auth.session-token=eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIn0..nCX-VfrdV14IYume.pvsRZwx4cWnjNPLk4DjrvBi6jmwGyidtQ-9Uu8FFOv8CmkipG9OU8JonZWKmZgNGVgxAqlRnAziM5xtfDK-licWy8A7Ypre7fcGRwDEKh1Qugp7zM7tPt57Oe3iUsKUn6CjSRvMJuueeUa5aosQVtFWEeSG29TZnfBpnsejIMEmHjCWAAcS-mKqw6NbLKmbhPPb4HB8DFJdyFiNnZOQ6KWBmIn_Q51Ztf68AFklYN1jEwboGfIM1jMJzozSaccr19i4cgukjr3cTqA.958IyzXoMOvoUoUn8cxiNQ; Path=/; Expires=Sat, 19 Aug 2023 06:35:56 GMT; HttpOnly; SameSite=Lax'
}

This is now fixed in 0.1.9