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

authMiddleware() corrupts response cookies, only emits first one in local dev #1897

Closed
4 tasks done
Thinkscape opened this issue Oct 17, 2023 · 4 comments · Fixed by #2244
Closed
4 tasks done

authMiddleware() corrupts response cookies, only emits first one in local dev #1897

Thinkscape opened this issue Oct 17, 2023 · 4 comments · Fixed by #2244
Assignees
Labels
bug Something isn't working nextjs prioritized This issue has been triaged and the team is working on it

Comments

@Thinkscape
Copy link

Thinkscape commented Oct 17, 2023

Preliminary Checks

Reproduction / Replay Link

https://github.com/roev-co/clerk-corrupting-cookies

Publishable key

pk_test_c3VwZXJiLWdob3VsLTEzLmNsZXJrLmFjY291bnRzLmRldiQ

Description

Steps to reproduce

  1. Given middleware.ts with authMiddleware() using afterAuth callback
  2. Use response.cookies.set() to set 2 or more cookies
  3. Start local next dev
  4. Open the page, running the middleware.

Expected behavior:

Cookies get set.
Cookies parameters are set.

Actual behavior:

Only the first cookie gets set.
Parameters like sameSite and secure get ignored and not being sent to the browser.

image

Background

Repro app

  1. Check out https://github.com/roev-co/clerk-corrupting-cookies
  2. Put creds in .env.local
  3. npm install
  4. Run next dev
  5. Open http://localhost:3000
  6. You should be seeing cookies first second third, but it will only show first

Environment

System:
    OS: macOS 13.5.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 1.58 GB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.15.0 - ~/.nvm/versions/node/v18.15.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.5.0 - ~/.nvm/versions/node/v18.15.0/bin/npm
    pnpm: 8.3.1 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 118.0.5993.70
    Safari: 16.6
  npmPackages:
    @clerk/nextjs: ^4.21.8 => 4.21.8
    @types/node: 20.1.4 => 20.1.4
    @types/react: 18.2.6 => 18.2.6
    @types/react-dom: 18.2.4 => 18.2.4
    autoprefixer: 10.4.14 => 10.4.14
    classnames: ^2.3.2 => 2.3.2
    next: 13.4.2 => 13.4.2
    postcss: 8.4.23 => 8.4.23
    react: 18.2.0 => 18.2.0
    react-dom: 18.2.0 => 18.2.0
    tailwindcss: 3.3.2 => 3.3.2
    typescript: 5.0.4 => 5.0.4
@Thinkscape Thinkscape added the needs-triage A ticket that needs to be triaged by a team member label Oct 17, 2023
@jescalan jescalan added the linear Created by Linear-GitHub Sync label Oct 30, 2023
@jescalan jescalan added prioritized This issue has been triaged and the team is working on it and removed needs-triage A ticket that needs to be triaged by a team member linear Created by Linear-GitHub Sync labels Nov 27, 2023
@LekoArts LekoArts added bug Something isn't working nextjs labels Nov 30, 2023
@LekoArts
Copy link
Member

Hi,

thanks for the issue and the excellent reproduction. This is much appreciated! Cloning your project and running it I see the same behavior.

I'll see what I can find out what the culprit is 👍

@LekoArts LekoArts self-assigned this Nov 30, 2023
@LekoArts
Copy link
Member

I found the culprit:

const normalisedResponses = responses.filter(Boolean).map(res => new NextResponse(res!.body, res!));

In this line the cookies get removed:

CleanShot 2023-11-30 at 14 21 11

@LekoArts
Copy link
Member

LekoArts commented Dec 5, 2023

The PR fixing this issue is merged, I'll let you know once it's released.

@LekoArts
Copy link
Member

LekoArts commented Dec 8, 2023

This has been fixed in @clerk/nextjs@4.27.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working nextjs prioritized This issue has been triaged and the team is working on it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants