-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Allow Cookies to Be Set from Headers
#8409
Comments
In case someone brings it up, I'm aware that Next Auth is already looking at Svelte Auth. So in some sense, that does alleviate the needs of some of the people that need auth in Svelte land who haven't been able to get it due to middleware limitations. However, Next/Svelte Auth still lacks some features that other solutions have. And to force developers to use only one particular solution for auth isn't completely fair or reasonable. |
I'm sorry, I've read this a couple of times and I'm completely stumped. What are you trying to do that you can't currently do? You can certainly append your own |
Sorry @Rich-Harris! Currently, I'm trying to have a user login at export const actions: Actions = {
default: async (event) => {
// Form Data
const formData = await event.request.formData().then(Object.fromEntries);
const { email, password, mode } = formData;
// Validate Data
const errors: ActionData = {};
if (!email) errors.email = "Email is required";
else if (!validateEmail(email)) errors.email = "Email is invalid";
if (!password) errors.password = "Password is required";
else if (mode === "signup" && !validatePassword(password)) {
errors.password = "Password must contain at least 8 characters, including a number";
}
if (errors.email || errors.password) return fail(400, { errors });
// Attempt Sign In / Sign Up
const normalizedMode = mode === "signup" ? "signup" : "signin";
const { status, responseHeaders } = await SuperTokensHelpers[normalizedMode](email, password);
// Auth failed
if (status === "WRONG_CREDENTIALS_ERROR") {
return fail(401, { errors: { banner: "Incorrect email and password combination" } });
}
if (status === "EMAIL_ALREADY_EXISTS_ERROR") {
return fail(400, { errors: { email: "This email already exists. Please sign in instead." } });
}
// Auth succeeded
responseHeaders.set("Location", new URL(request.url).searchParams.get("returnUrl") || "/");
return new Response(null, { status: 302, statusText: "OK", headers: responseHeaders });
},
} However, according to the docs:
So the Does that help? |
Okay. I just saw #7611 (comment), so it seems like returning a In this case, it seems like the only available option would be to add something like |
Getting a My inclination would be to install set-cookie-parser and do it like this: import * as set_cookie_parser from 'set-cookie-parser';
// in the action...
const { status, responseHeaders } = await SuperTokensHelpers[normalizedMode](email, password);
// ...
// Auth succeeded
for (const str of set_cookie_parser.splitCookiesString(responseHeaders.get('set-cookie'))) {
const { name, value, ...options } = set_cookie_parser.parseString(str);
cookies.set(name, value, {...});
}
throw redirect(302, url.searchParams.get('returnUrl') ?? '/'); |
(If you know that there'll only be a single cookie in |
Nothing is set in stone yet; so I could probably suggest that implementation change if that's what you'd recommend. Part of the point of the POCs that I'm doing is to give them an idea of what the potential inputs and outputs would be for their functions so that people could use their stuff middleware-free in the future (which means nice integration with Svelte Kit I'm not sure what the case would be for other auth options; but for // in the action...
const { status, cookies, responseHeaders } = await SuperTokensHelpers[normalizedMode](email, password);
cookies.forEach(/* Call `event.cookies.set` for each cookie */);
Object.entries(responseHeaders).forEach(/* Call `event.setHeaders` for each header */); |
The Most headers that can have multiple values (like So yes, returning an array of cookies (whether that's a string or an object, perhaps one that matches the return value of |
Okay great! All of this is really helpful. Thanks for the insights! |
…kensOutput` After chatting with Rich Harris (creator of Svelte) on sveltejs/kit#8409, it seems that it's better to expose any necessary auth cookies _explicitly_ rather than wrap them in `Headers`. The short reasoning for this is that it provides more options to developers using SSR frameworks. (The previous implementation locked some developers out of using `SuperTokens` in the framework of their choice.) Note that the `SuperTokensOutput` utility class now exposes the `responseHeaders` as a `Map` instead of a `Headers` object in order to support frameworks (or servers) that do not have a proper `Headers` class that they can use. (Node.js doesn't quite support `Headers` out of the box yet. See the MDN compatibility table.)
Describe the problem
Currently, Svelte Kit does not support setting cookies via
Headers
. Instead, it requires cookies to be set viaevent.cookies
. In most cases, this isn't a problem. (It might even be a nicer experience for some.) However, there still may be use cases where the developer would need the freedom to set cookies fromHeaders
.In my case, I'm trying to encourage an auth solution (
SuperTokens
) to navigate away from requiring middleware for authentication (which unfortunately a lot of auth solutions do) and to move towards the simplicity of requesting the correctHeaders
and returning the appropriate responseHeaders
. (Typically, auth solutions really only need to take cookies from headers as inputs -- maybe with some extra data -- and return new cookies in/with headers as outputs -- if any outputs are needed.) This is huge because it satisfies #4654 in a way that seems to avoid going against Rich Harris's philosophies. (Or, if this is still contrary, it's at least less contrary than requiring interop with middleware.)SuperTokens
seems to be up for something like this (in the future), and I've been trying to make POCs to prove the usefulness of this approach. I have a rough POC for Remix, but the idea is currently incompatible with SvelteKit.There may be other cases where a library could return response
Headers
containing important cookie information. If that happens, SvelteKit has no way of helping the developers using this package.(Relates to #4375)
Describe the proposed solution
If there is absolutely no desire to allow
Set-Cookie
in theHeaders
returned in Svelte Kit land, then perhaps a newevent.cookies.setFromHeader
(or similarly named) method could be created. This ensures thatevent.cookies
still maintains control over how cookies are set in the browser, but it also enables Svelte Kit to support libraries that may need to return cookie information in the headers. (Again, if the ideal is reached for all auth solutions in the future and they all become more flexible by navigating away from middleware, then this will be inevitable.)Alternatives considered
An alternative is simply to allow developers to return their own
Response
s in Svelte Kit in all relevant use cases -- such as withaction
s. The reason I ran into this problem is that I was trying to redirect a previously-unauthenticated user back to their target page after they submitted a login form (viaSuperTokens
). But I couldn't set cookies in the response headers, nor could I return a customResponse
.Allowing developers to return a custom
Response
that sets the cookies would inherently solve this problem. (Remix
andSolidStart
already do this.) But I'm not sure how easy that would be for Svelte Kit. And even if it was easy, I imagine it would be awkward to support bothevent.setHeaders
+event.cookies
andreturn new Response()
.Importance
would make my life easier
Additional Information
I'm assuming that one thought which may come to mind is, "Why not have these auth solutions return custom headers and custom cookies instead of having them just return headers with cookies inside?" That's a fair question, but it's a little harder to justify. Just as there are inconsistent workarounds for
Set-Cookie
, so there is not yet a universal standard for a "Cookie Object", let alone a "Cookies Map". But there is a standard forHeaders
. Consequently, it seems more reasonable to take a headers-based approach in this case.The text was updated successfully, but these errors were encountered: