-
Notifications
You must be signed in to change notification settings - Fork 246
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
proxy cookies #333
Comments
It might make sense to have an option to "Proxy response cookies" but I have a comment for your "temporary fix". You will lose cookies already set on |
@rchl yes i know, but strangely res.getHeader('Set-Cookie') is always undefined even if i see double set-cookie in nuxt response in my browser to render the view and i don't understand why...so I make sure that the 'last one' Set-Cookie is always the right one... it's ugly, but it works for the moment |
I'm currently dealing with the same problem. My backend adds a Set-Cookie header to the response with a new access_token whenever it has expired. This causes issues with SSR because they aren't forwarded to the client. I've been working around this issue so far by making requests that require authentication from the client, but this takes away the smoothness of SSR. An issue with the solution provided above is that on subsequent requests Axios might also need to access the new cookies. The ideal solution would be to mimic the behavior of browsers when dealing with cookies. A possible solution I've been considering is to have a server-side plugin that updates the Axios cookies, and the Set-Cookie header on the response, whenever a Set-Cookie header is received. export default function ({ $axios, res }) {
$axios.onResponse((response) => {
// get the set-cookie header from the response
const setCookies = response.headers['set-cookie'];
if (setCookies) {
// parse the cookies axios uses
const cookies = parse($axios.defaults.headers.common.cookie);
// add cookies from setCookies to cookies
// set $axios.defaults.headers.common.cookie equal to the new cookie header
// merge the existing Set-Cookie header with setCookies
// set the cookies on the response, so the client gets them back
res.setHeader('Set-Cookie', setCookies);
}
});
} One possible issue with this solution is that it would proxy all cookies. So a potential filter could be added to not proxy cookies such as It would be nice to have this as an option, so SSR can be used to the fullest, without worrying about cookies not syncing. |
@SebastiaanYN :
It's not cookies .... |
You're right, ignore that. I think I managed to get a working plugin that updates cookies for Axios, and merges them on the response object. This allows sequential requests to have the correct cookies. // plugins/ssr-cookie-proxy.js
import { parse as parseCookie } from 'cookie';
function parseSetCookies(cookies) {
return cookies
.map(cookie => cookie.split(';')[0])
.reduce((obj, cookie) => ({
...obj,
...parseCookie(cookie),
}), {});
}
function serializeCookies(cookies) {
return Object
.entries(cookies)
.map(([name, value]) => `${name}=${encodeURIComponent(value)}`)
.join('; ');
}
function mergeSetCookies(oldCookies, newCookies) {
const cookies = new Map();
function add(setCookie) {
const cookie = setCookie.split(';')[0];
const name = Object.keys(parseCookie(cookie))[0];
cookies.set(name, cookie);
}
oldCookies.forEach(add);
newCookies.forEach(add);
return [...cookies.values()];
}
export default function ({ $axios, res }) {
$axios.onResponse((response) => {
const setCookies = response.headers['set-cookie'];
if (setCookies) {
// Combine the cookies set on axios with the new cookies and serialize them
const cookie = serializeCookies({
...parseCookie($axios.defaults.headers.common.cookie),
...parseSetCookies(setCookies),
});
$axios.defaults.headers.common.cookie = cookie; // eslint-disable-line no-param-reassign
// If the res already has a Set-Cookie header it should be merged
if (res.getHeader('Set-Cookie')) {
const newCookies = mergeSetCookies(
res.getHeader('Set-Cookie'),
setCookies,
);
res.setHeader('Set-Cookie', newCookies);
} else {
res.setHeader('Set-Cookie', setCookies);
}
}
});
} And then register it in plugins: [
{ src: '@/plugins/ssr-cookie-proxy.js', mode: 'server' },
], |
Your code manage signed cookies too ? @SebastiaanYN |
It copies the cookies over, by name, without modifying the values. So I would expect it to work, but you'd have to test it to be 100% certain. |
@SebastiaanYN Nice work! If you would like to make a PR adding it built-in supported it is more than welcome. Also, this may you inspire for a one-liner update solution. (originally taken from express) |
@pi0 I'll give it a go. The reason I don't directly concat the old cookies with the new is to prevent unnecessary headers. If you make multiple requests that all overwrite a certain cookie, that could increase the header size quite a bit. And it would then also come down to the order browsers read the headers, whether they set the right cookies. @usb248 I think this is caused by the |
@SebastiaanYN |
That's strange. I don't know what could cause that. As far as I know |
yes... i don't know why too. maybe @pi0 ? duplicate headers occurs on first call (when there is no session cookie) on a route which has an axios SSR request. nuxt response + axios response. I have for example a get axios request in my nuxtServerInit (on an internal API in express in servermiddleware) which start an anonyme user session. |
Maybe can try |
The same ... :/, even if in |
The problem seems to come from nuxt ?? :
an explanation @pi0 ? |
@usb248 Browser cookies are already loaded into |
So how to get header at the right moment 😆 @pi0 ? Sebastian’s code still doesn’t work for me. res.getHeader('Set-Cookie') is always undefined in ssr plugin |
I have same issue. In ssr plugin, res.getHeader('Set-Cookie') always returns undefined. @usb248 You found any solution for this? |
@usb248 @sbthemes Take a look, maybe it helps: https://stackoverflow.com/a/34011746/1360402 |
+1 This indeed is a significant problem. I'm using However, since $axios is not sending cookies in those calls in The solution by @SebastiaanYN helped, thanks a lot! |
@SebastiaanYN Your solution works. Thank you so much. This should be a standard feature of Nuxt. |
and then you can use |
It seems to me that all my problems were resolved by upgrading After the upgrade, I don't need |
@svitekpavel AFAICS this module has no functionality to forward the response headers (cookie headers) of requests made on the server-side. So your problem was likely something else and couldn't be related to this issue. Such functionality would be wrong in a general case as you don't want response headers of any random SSR request to be forwarded to the client. There might be specific cases where you'd want that if you ensure that you only do it for specific URLs and specific headers but that's why you have an option using response interceptor. So for example, if your server is making an axios request to, let's say github, and that response sets some cookies, you wouldn't want those cookies to be forwarded to the client that made a request to your server. That would just not make sense in the general case. I think this issue can basically be closed because there is solution available and IMO it wouldn't make sense to have such functionality built-in. |
@rchl How about domains whitelist, which will pass cookies to client? Application backend apis for example. |
That could exist but maybe someone would want to have a white-list for specific cookies only? |
@SebastiaanYN You're coping only cookie value here. but it should set other attributes of cookies too e.g, expiry, path, httponly etc. cookies.set(name, setCookie); |
What problem does this feature solve?
Handle cookie jar in order to automatically resend received cookies on every subsequent request (server-side).
Actually, axios doesn't resend cookie received (with set-cookie) by nuxt serverResponse (in a serverMiddleware for example).
It cause many problem in my nuxt app.
I have to do something like this (temporary fix) :
but in some case, set-cookie header is duplicated...
The text was updated successfully, but these errors were encountered: