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

Cookies are being lost after redirect using middleware #7484

Closed
1 task
i7N3 opened this issue Jun 26, 2023 · 7 comments
Closed
1 task

Cookies are being lost after redirect using middleware #7484

i7N3 opened this issue Jun 26, 2023 · 7 comments
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@i7N3
Copy link

i7N3 commented Jun 26, 2023

What version of astro are you using?

2.6.1

Are you using an SSR adapter? If so, which one?

@astrojs/node: 5.2.0

What package manager are you using?

npm or pnpm

What operating system are you using?

Monterey m1

What browser are you using?

Any

Describe the Bug

I am faced with the fact that the cookie is lost after redirecting.

How to reproduce?

const middleware = defineMiddleware(async (context, next) => {
        // ...
	if (isAuthSuccess) {
		context.cookies.set('jid', '<TOKEN>')
		return context.redirect(`/dashboard`)
	}
	return next()
})
export const onRequest = sequence(middleware)

When I log cookie value on /dashboard page with Astro.cookies.get(...).value it's empty.
If I remove the redirect statement, then it works.

Tested both: dev and preview

What's the expected result?

Cookies should be present after redirect.

Link to Minimal Reproducible Example

https://github.com/i7N3/astro-middleware-bug-reproduction/tree/54281b3bf6e42bc5becb041c9db83aeb368d5b16

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

This was previously fixed here: #7294

It shouldn't matter which adapter it is. Possible regression?

@matthewp
Copy link
Contributor

I would guess that the bug is specifically because of the redirect. https://github.com/i7N3/astro-middleware-bug-reproduction/blob/master/src/middleware.ts

@i7N3
Copy link
Author

i7N3 commented Jun 26, 2023

I would guess that the bug is specifically because of the redirect. https://github.com/i7N3/astro-middleware-bug-reproduction/blob/master/src/middleware.ts

I tested it as described and yes, it's because of redirect.

@medeirosjoaquim
Copy link

I think I'm facing the same thing, but I'm using the API, and it also seems to be the redirect. I'm setting cookies before redirect. After the redirect I should be able to get the cookie, once the redirected page loads, with Astro.cookies.get("/myroute"), right? But it's undefined. Don't know if it's a bug or I am missing something from the docs on how cookies and redirect should work.

@ematipico ematipico added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Jul 6, 2023
@asleepace
Copy link

@medeirosjoaquim I was able to get it working by using context.request.headers.get('cookie')

import { defineMiddleware } from "astro/middleware";

// server-side rendering
export const prerender = false;

// check if user is authenticated
const handleAuthentication = defineMiddleware((context, next) => {
  const astroCookies = context.cookies
  const requestCookies = context.request.headers.get('cookie')
  console.log("[middleware] astroCookies:", astroCookies)         // {} (bad)
  console.log("[middleware] requestCookies:", requestCookies)     // { someCookie: 'someValue' } (good)
  return next()
})

@ematipico ematipico self-assigned this Aug 22, 2023
@i7N3
Copy link
Author

i7N3 commented Sep 2, 2023

I found a way to redirect and prevent losing cookies

import { defineMiddleware, sequence } from 'astro/middleware';

export const middleware = defineMiddleware(async (context, next) => {
    console.log('pathname: ', context.url.pathname);

    if (context.url.pathname.includes('/login')) {
        return new Response(null, {
            status: 301,
            headers: {
                Location: '/dashboard',
                'Set-Cookie': 'jid=HelloWorld; Path=/;',
            },
        });
    }

    return next();
});

export const onRequest = sequence(middleware);

But using this approach I can't set multiple cookies at once:

context.cookies.set(key1, value1)
context.cookies.set(key2, value2)

@i7N3
Copy link
Author

i7N3 commented Sep 23, 2023

Fixed by #8612
Working demo

@i7N3 i7N3 closed this as completed Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

No branches or pull requests

5 participants