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

HttpOnly cookies (and other cookie parameters) are lost after a refresh takes place on the server #3

Open
jskitz opened this issue Apr 1, 2024 · 7 comments

Comments

@jskitz
Copy link

jskitz commented Apr 1, 2024

Hello again @JiProchazka, I'm having a bit of a security issue that seems to only happen when the server does the token refresh and sets the cookies in the browser. Let me try to describe what I think is happening:

  1. I login to my service, and the access and refresh tokens are set like so (don't worry about keys, it's running locally):
Screen Shot 2024-04-01 at 11 13 23 AM
  1. I click to a new route, which means that the server is going to make initial API calls. However, it's going to result in a 401, and so the server is going to perform the token refresh. Upon that refresh, I realized that now the access and refresh tokens no longer have the HttpOnly flag set, and so now those tokens can be leaked on the client side. I discovered this by accident because I was console logging all my cookies to debug another problem, and all of a sudden I started seeing my access and refresh token logged in the client. Here is what the cookies change to:
Screen Shot 2024-04-01 at 11 20 20 AM

You will also notice that SameSite: Lax and the expiration is removed as well. I'm guessing that whatever the issue is with the HttpOnly flag is the same issue with SameSite and the expiration.

I'm currently debugging this, and will update if I find a fix. But thought I'd flag for you as well in case you know more quickly than I on how to fix it. Thanks!

@JiProchazka
Copy link
Owner

Hi Jason,

you are probably setting the cookies wrong on the backend when the refresh token is set. It is completely on the backend side and it has nothing to do with nuxt-cookies-auth - as this library on consumes what server sends.

@jskitz
Copy link
Author

jskitz commented Apr 1, 2024

Hi @JiProchazka,

The first screen shot is directly what comes out of the server. I can't imagine that the server starts sending non HttpOnly cookies, removes SameSite and the Expiration time. That would make little sense given all of these are simple server settings. It's literally a Django setting whether cookies are sent HttpOnly and SameSite Lax.

And of course it works until I change routes, and the Nuxt server app performs a refresh and sets the cookies. At that point, the cookies have been changed. Perhaps it's an issue with Nuxt? I'm still debugging, but will let you know if I make some headway.

Thanks for the quick response.

@jskitz
Copy link
Author

jskitz commented Apr 3, 2024

I've confirmed that the cookies coming out of the server are correct by logging this line:

const cookies = (res.headers.get("set-cookie") || "").split(",")

But after this code is executed:

for (const cookie of cookies) {
appendResponseHeader(event, "set-cookie", cookie)
}

which is exactly what Nuxt says you should do to attach the cookies, the attributes for HttpOnly and SameSite are removed when looking at them back in the browser. And so, this means that the tokens are exposed to client side JavaScript at that point.

I'm going to see if there is a way to control what Nuxt does with the cookie when executing appendResponseHeader, because this is what is definitely causing it. So seems like an issue with Nuxt. But if HttpOnly is lost on these cookies coming from the server, it's less interesting to use cookie based auth because then client side javascript can access it anyway.

Anyway, feel free to close. I'm confident this is a Nuxt problem.

Thanks again!

@jskitz
Copy link
Author

jskitz commented Apr 3, 2024

Actually the above was wrong. The problem is actually in this line of code after further debugging:

const cookies = (res.headers.get('set-cookie') || '').split(',')

This is not an accurate way to split cookies because the expires=Thu, 04 Apr 2024 01:03:45 GMT; line will actually prematurely split the cookies on the comma after Thu, and so this ends up stripping out some of the more useful flags. I think it's sort of lucky that this all works, but this is definitely the problem. I'm trying to see if there is a better way to split that. But it will probably require a regex. Anyway, this is definitely the issue.

Thanks for listening!

@jskitz
Copy link
Author

jskitz commented Apr 3, 2024

Here is a solution that works to fix the issue:

Replace this line:

        const cookies = (res.headers.get('set-cookie') || '').split(',')

with this block instead, using regex:

        const regex = /, (?=[^;]*; expires=)/
        const cookies = (res.headers.get('set-cookie') || '').split(regex)

Hope that's helpful.

@sdezza
Copy link
Contributor

sdezza commented Apr 29, 2024

Hello @jskitz ,
would you mind sharing your repo? Also working whith Django and Nuxt but it's really not easy :/

@jskitz
Copy link
Author

jskitz commented Apr 30, 2024

Hello @sdezza, I can't share my repository because it's a closed source, contract project I'm working on. However, here is an article that is very very close to what I'm doing on the Django side of things. I plan to write my own article on this soon, but haven't had the chance to finish it.

I'm also using Djoser for the user management / JWTs in my project. It's not perfect, but it gives you a lot of user management REST calls immediately, and is easy to extend and overload to fit your specific needs.

Hope this helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants