github-starred/better-auth
2
0
Fork 0
mirror of https://github.com/better-auth/better-auth.git synced 2026-05-24 16:11:53 -05:00
Code Issues 2.5k Packages Projects Releases 112 Wiki Activity

[PR #7133] [MERGED] fix(last-login-method): correctly handle multiple Set-Cookie headers #23994

New Issue
Closed
opened 2026-04-15 22:06:50 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-15 22:06:50 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/better-auth/better-auth/pull/7133
Author: @dngpng
Created: 1/5/2026
Status: ✅ Merged
Merged: 2/28/2026
Merged by: @Bekacru

Base: canary ← Head: canary

📝 Commits (4)

  • 733a8b3 fix(last-login-method): enhance handling of multiple Set-Cookie headers
  • b4cd963 Merge branch 'better-auth:canary' into canary
  • 335dc43 fix: remove lint error
  • fd1eb14 Merge branch 'canary' into canary

📊 Changes

2 files changed (+78 additions, -3 deletions)

View changed files

📝 packages/better-auth/src/plugins/last-login-method/index.ts (+5 -3)
📝 packages/better-auth/src/plugins/last-login-method/last-login-method.test.ts (+73 -0)

📄 Description

Problem

The last-login-method plugin may fail to set the login method cookie when multiple Set-Cookie headers were present in the response.

The issue occurred in the after hook handler:

0827fc3f49/packages/better-auth/src/plugins/last-login-method/index.ts (L139-L143)

Why this may fail:

  • HTTP responses can have multiple Set-Cookie headers (one per cookie)
  • The .get("set-cookie") method only returns the first Set-Cookie header value
  • If the session token cookie was not in the first header, the check would fail
  • This caused the last-login-method cookie to not be set

Real-world scenario:

When Better Auth sets multiple cookies (session token, CSRF token, additional plugin cookies, etc.), they each get their own Set-Cookie header. If the session token wasn't first, the last-login-method cookie would be silently skipped. (See screenshot below)

SCR-20260105-hvoi

Solution

Replace .get("set-cookie") with .getSetCookie() which returns an array of all Set-Cookie headers:

  const setCookieHeaders = ctx.context.responseHeaders?.getSetCookie?.() || [];
  const hasSessionToken = setCookieHeaders.some((cookie) =>
    cookie.includes(sessionTokenName),
  );

This approach:

  • ✅ Checks all Set-Cookie headers, not just the first one
  • ✅ Uses the standard Headers API .getSetCookie() method designed for this purpose
  • ✅ Includes optional chaining and fallback for compatibility
  • ✅ Matches the pattern used elsewhere in the codebase (e.g., mcp.test.ts:577)

Testing

Added a comprehensive test case that:

  1. Creates a custom plugin that sets an additional cookie (simulating multiple Set-Cookie headers)
  2. Verifies that multiple cookies are present in the response using getSetCookie()
  3. Confirms the last-login-method cookie is still correctly set
  4. Validates both cookies are present in the final response

Test results:

✓ src/plugins/last-login-method/last-login-method.test.ts (10 tests)
✓ should handle multiple set-cookie headers correctly

All existing tests continue to pass.

Files Changed

  • packages/better-auth/src/plugins/last-login-method/index.ts - Fixed Set-Cookie header handling
  • packages/better-auth/src/plugins/last-login-method/last-login-method.test.ts - Added test for multiple Set-Cookie headers

Related

This fix aligns with the Headers API standard and follows the same pattern used in other parts of the codebase for handling multiple Set-Cookie headers.

Summary by cubic

Fixes the last-login-method plugin so it correctly sets the last_used_login_method cookie when responses include multiple Set-Cookie headers. Prevents skips when the session cookie isn’t the first header.

  • Bug Fixes
    • Replaced get("set-cookie") with getSetCookie() to read all Set-Cookie headers.
    • Checked for the session token across headers before setting the cookie.
    • Added a test that simulates multiple cookies and verifies both cookies are set.

Written for commit fd1eb14a2c. Summary will update on new commits.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/better-auth/better-auth/pull/7133 **Author:** [@dngpng](https://github.com/dngpng) **Created:** 1/5/2026 **Status:** ✅ Merged **Merged:** 2/28/2026 **Merged by:** [@Bekacru](https://github.com/Bekacru) **Base:** `canary` ← **Head:** `canary` --- ### 📝 Commits (4) - [`733a8b3`](https://github.com/better-auth/better-auth/commit/733a8b36c9d30952be8af7db1f91b3e9118bc4d4) fix(last-login-method): enhance handling of multiple Set-Cookie headers - [`b4cd963`](https://github.com/better-auth/better-auth/commit/b4cd963a2590e0f6963cd7a53656890d90075767) Merge branch 'better-auth:canary' into canary - [`335dc43`](https://github.com/better-auth/better-auth/commit/335dc43094ba86bc363e7a6a0a2cf1ff5650148c) fix: remove lint error - [`fd1eb14`](https://github.com/better-auth/better-auth/commit/fd1eb14a2cce498d6853a042c10a112192d02641) Merge branch 'canary' into canary ### 📊 Changes **2 files changed** (+78 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `packages/better-auth/src/plugins/last-login-method/index.ts` (+5 -3) 📝 `packages/better-auth/src/plugins/last-login-method/last-login-method.test.ts` (+73 -0) </details> ### 📄 Description ## Problem The last-login-method plugin may fail to set the login method cookie when multiple `Set-Cookie` headers were present in the response. The issue occurred in the after hook handler: https://github.com/better-auth/better-auth/blob/0827fc3f49c4968ea37f4a42a26a309edf75bea1/packages/better-auth/src/plugins/last-login-method/index.ts#L139-L143 ### Why this may fail: - HTTP responses can have multiple Set-Cookie headers (one per cookie) - The `.get("set-cookie")` method only returns the first `Set-Cookie` header value - If the session token cookie was not in the first header, the check would fail - This caused the last-login-method cookie to not be set ### Real-world scenario: When Better Auth sets multiple cookies (session token, CSRF token, additional plugin cookies, etc.), they each get their own Set-Cookie header. If the session token wasn't first, the last-login-method cookie would be silently skipped. (See screenshot below) <img width="1653" height="143" alt="SCR-20260105-hvoi" src="https://github.com/user-attachments/assets/27970e28-6f5a-410a-8e9c-968d1e7b5a2e" /> ## Solution Replace `.get("set-cookie")` with `.getSetCookie()` which returns an array of all `Set-Cookie` headers: ``` const setCookieHeaders = ctx.context.responseHeaders?.getSetCookie?.() || []; const hasSessionToken = setCookieHeaders.some((cookie) => cookie.includes(sessionTokenName), ); ``` This approach: - ✅ Checks all Set-Cookie headers, not just the first one - ✅ Uses the standard Headers API .getSetCookie() method designed for this purpose - ✅ Includes optional chaining and fallback for compatibility - ✅ Matches the pattern used elsewhere in the codebase (e.g., mcp.test.ts:577) ## Testing Added a comprehensive test case that: 1. Creates a custom plugin that sets an additional cookie (simulating multiple Set-Cookie headers) 2. Verifies that multiple cookies are present in the response using getSetCookie() 3. Confirms the last-login-method cookie is still correctly set 4. Validates both cookies are present in the final response ### Test results: ✓ src/plugins/last-login-method/last-login-method.test.ts (10 tests) ✓ should handle multiple set-cookie headers correctly All existing tests continue to pass. ## Files Changed - `packages/better-auth/src/plugins/last-login-method/index.ts` - Fixed Set-Cookie header handling - `packages/better-auth/src/plugins/last-login-method/last-login-method.test.ts` - Added test for multiple Set-Cookie headers ## Related This fix aligns with the Headers API standard and follows the same pattern used in other parts of the codebase for handling multiple Set-Cookie headers. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Fixes the last-login-method plugin so it correctly sets the last_used_login_method cookie when responses include multiple Set-Cookie headers. Prevents skips when the session cookie isn’t the first header. - **Bug Fixes** - Replaced get("set-cookie") with getSetCookie() to read all Set-Cookie headers. - Checked for the session token across headers before setting the cookie. - Added a test that simulates multiple cookies and verifies both cookies are set. <sup>Written for commit fd1eb14a2cce498d6853a042c10a112192d02641. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-15 22:06:50 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
adapter
astro
awaiting external contributor
blocked
breaking
breaking change
bug
c-devops
core
credentials
database
dependencies
devops
devtools
docs
documentation
duplicate
elysia
enhancement
enterprise
expo
express
fastify
good first issue
help wanted
hono
identity
infra
integration
invalid
javascript
locked
maintenance
need-more-information
needs: info
needs: repro
nextjs
nuxt
oauth
organization
P0
payments
perf
platform
plugin
pull-request
Mirrored from GitHub Pull Request
 question
ready
regression
remix
security
social-provider
solid
stale
svelte
tanstack-start
tracking
version-bump
vue
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/better-auth#23994
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?