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 set in middleware missing on server actions #67814

Closed
viniciusbitt opened this issue Jul 16, 2024 · 6 comments · Fixed by #67924
Closed

Cookies set in middleware missing on server actions #67814

viniciusbitt opened this issue Jul 16, 2024 · 6 comments · Fixed by #67924
Labels
bug Issue was opened via the bug report template. locked Middleware Related to Next.js Middleware.

Comments

@viniciusbitt
Copy link

viniciusbitt commented Jul 16, 2024

Link to the code that reproduces this issue

https://github.com/viniciusbitt/nextjs-middleware-server-action-missing-cookie

To Reproduce

  1. Clear cookies
  2. Trigger server action
  3. Set cookie in middleware
  4. Try get the cookie on server action, but is undefined

Current vs. Expected behavior

I'm doing refresh token rotation in the middleware, and using server actions to validate if the user is logged in, I expected the cookie set on middleware to be visible to server actions

Provide environment information

Operating System:
  Platform: win32
  Arch: x64
  Version: Windows 11 Pro
  Available memory (MB): 32602
  Available CPU cores: 12
Binaries:
  Node: 20.12.2
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 15.0.0-canary.68 // Latest available version is detected (15.0.0-canary.68).
  eslint-config-next: N/A
  react: 19.0.0-rc.0
  react-dom: 19.0.0-rc.0
  typescript: 5.3.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Middleware

Which stage(s) are affected? (Select all that apply)

next dev (local), next build (local), next start (local)

Additional context

No response

@viniciusbitt viniciusbitt added the bug Issue was opened via the bug report template. label Jul 16, 2024
@github-actions github-actions bot added the Middleware Related to Next.js Middleware. label Jul 16, 2024
@icyJoseph
Copy link
Contributor

A bit tricky to see the issue in your example, because, straight up navigating to the landing page, sets a cookie too. That being said, I think something like this should work:

import { NextRequest, NextResponse } from "next/server";

export async function middleware(request: NextRequest) {
  const expires = new Date();
  expires.setMinutes(expires.getMinutes() + 60);

  const cookie = {
    httpOnly: true,
    name: "random_string",
    value: Math.random().toString(),
    expires,
  };

  request.cookies.set(cookie.name, cookie.value);

  let response = NextResponse.next({
    request: {
      headers: new Headers(request.headers),
    },
  });
  response.cookies.set(cookie);

  return response;
}

@viniciusbitt
Copy link
Author

@icyJoseph It's kind of difficult to reproduce, you have to simulate the "access_token" (random_string) cookie expiring when calling server action, that's why I said to clear the cookies in the first step.

I tested your example and it worked, but is it the right way to do this? Looks like I'm back to the issue #49442

@icyJoseph
Copy link
Contributor

icyJoseph commented Jul 17, 2024

I mean, I put it together from information present in the docs.

That link documents how to forward modified headers.

And that it is mentioned that you can set cookies on the incoming request:

For incoming requests, cookies comes with the following methods: get, getAll, set, and delete cookies. You can check for the existence of a cookie with has or remove all cookies with clear.

On the same page as the link above.

@Pbmangan
Copy link

I believe this issue is impacting supabase sever side auth on NextJS supabase/ssr#36

@cdgn-coding
Copy link

We are experiencing this issue in next 14.2.5 and below. Will this be merged to next 14? We cannot migrate to next 15.

Copy link
Contributor

github-actions bot commented Aug 7, 2024

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Aug 7, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2024
lubieowoce pushed a commit that referenced this issue Aug 23, 2024
If middleware targets a server action handler and sets or updates a
cookie, the newly updated cookie would not be reflected in the
`cookies()` response of the action handler

In #65008 we fixed a bug where cookies set in middleware were not
reflected in the `cookies()` call in a server component from the same
request. We did this by introducing a `x-middleware-set-cookie` header,
that signaled to downstream handlers that middleware had run on the
request & set a cookie. However this handling was only applied to the
sealed/read-only cookies. Cookies accessed from a server action use
`mutableCookies`, since those aren't frozen as a server action is
allowed to modify cookies.

This pulls the cookie merge handling into a function and applies the
merge to `mutableCookies`.

Fixes #67814
Closes NDX-95
lubieowoce pushed a commit that referenced this issue Aug 23, 2024
If middleware targets a server action handler and sets or updates a
cookie, the newly updated cookie would not be reflected in the
`cookies()` response of the action handler

In #65008 we fixed a bug where cookies set in middleware were not
reflected in the `cookies()` call in a server component from the same
request. We did this by introducing a `x-middleware-set-cookie` header,
that signaled to downstream handlers that middleware had run on the
request & set a cookie. However this handling was only applied to the
sealed/read-only cookies. Cookies accessed from a server action use
`mutableCookies`, since those aren't frozen as a server action is
allowed to modify cookies.

This pulls the cookie merge handling into a function and applies the
merge to `mutableCookies`.

Fixes #67814
Closes NDX-95
lubieowoce pushed a commit that referenced this issue Aug 28, 2024
If middleware targets a server action handler and sets or updates a
cookie, the newly updated cookie would not be reflected in the
`cookies()` response of the action handler

In #65008 we fixed a bug where cookies set in middleware were not
reflected in the `cookies()` call in a server component from the same
request. We did this by introducing a `x-middleware-set-cookie` header,
that signaled to downstream handlers that middleware had run on the
request & set a cookie. However this handling was only applied to the
sealed/read-only cookies. Cookies accessed from a server action use
`mutableCookies`, since those aren't frozen as a server action is
allowed to modify cookies.

This pulls the cookie merge handling into a function and applies the
merge to `mutableCookies`.

Fixes #67814
Closes NDX-95
lubieowoce pushed a commit that referenced this issue Sep 2, 2024
If middleware targets a server action handler and sets or updates a
cookie, the newly updated cookie would not be reflected in the
`cookies()` response of the action handler

In #65008 we fixed a bug where cookies set in middleware were not
reflected in the `cookies()` call in a server component from the same
request. We did this by introducing a `x-middleware-set-cookie` header,
that signaled to downstream handlers that middleware had run on the
request & set a cookie. However this handling was only applied to the
sealed/read-only cookies. Cookies accessed from a server action use
`mutableCookies`, since those aren't frozen as a server action is
allowed to modify cookies.

This pulls the cookie merge handling into a function and applies the
merge to `mutableCookies`.

Fixes #67814
Closes NDX-95
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. locked Middleware Related to Next.js Middleware.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants