From 954b41b55568be06c430b5768df622110fbc7546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BE=9D=E7=91=AA=E8=B2=93?= Date: Fri, 6 Mar 2026 07:51:14 +0800 Subject: [PATCH] Add Secure and SameSite=Lax flags to all cookie operations Co-Authored-By: Claude Opus 4.6 --- src/module/apiError.js | 3 +- src/stores/login.ts | 12 +++---- src/utils/cookieUtil.js | 6 ++-- tests/stores/login.test.js | 37 ++++++++++++++++++++- tests/unit/utils/cookieUtil.test.js | 50 +++++++++++++++++++++-------- 5 files changed, 84 insertions(+), 24 deletions(-) diff --git a/src/module/apiError.js b/src/module/apiError.js index b8dd8b9..869003a 100644 --- a/src/module/apiError.js +++ b/src/module/apiError.js @@ -4,6 +4,7 @@ import pinia from '@/stores/main.ts'; import {useToast} from 'vue-toast-notification'; import 'vue-toast-notification/dist/theme-sugar.css'; import axios from "axios"; +import { deleteCookie } from "@/utils/cookieUtil.js"; const loading = loadingStore(pinia); const $toast = useToast(); @@ -19,7 +20,7 @@ const delay = (s = 0) => new Promise((resolve, reject) => setTimeout(resolve, s export default async function apiError(error, toastMessage) { if(error.request?.status === 401) { delete axios.defaults.headers.common["Authorization"]; - document.cookie = 'luciaToken=; expires=Thu, 01 Jan 1970 00:00:00 UTC;'; + deleteCookie("luciaToken"); return router.push('/login'); } await delay(); diff --git a/src/stores/login.ts b/src/stores/login.ts index 1a5cef8..a72ddcf 100644 --- a/src/stores/login.ts +++ b/src/stores/login.ts @@ -1,7 +1,7 @@ import { defineStore } from "pinia"; import axios from 'axios'; import apiError from '@/module/apiError.js'; -import { deleteCookie, setCookie, getCookie } from "../utils/cookieUtil"; +import { deleteCookie, setCookie, setCookieWithoutExpiration, getCookie } from "../utils/cookieUtil"; export default defineStore('loginStore', { // data, methods, computed @@ -37,8 +37,8 @@ export default defineStore('loginStore', { const accessToken = response.data.access_token; const refresh_token = response.data.refresh_token; // 將 token 儲存在 cookie - document.cookie = `luciaToken=${accessToken}`; - document.cookie = `luciaRefreshToken=${refresh_token};expires=${new Date(this.expired)};`; + setCookieWithoutExpiration("luciaToken", accessToken); + setCookie("luciaRefreshToken", refresh_token, Math.ceil((this.expired - Date.now()) / (24 * 60 * 60 * 1000))); this.isLoggedIn = true; setCookie("isLuciaLoggedIn", "true"); @@ -76,8 +76,8 @@ export default defineStore('loginStore', { const newAccessToken = response.data.access_token; const newRefreshToken = response.data.refresh_token; - document.cookie = `luciaToken=${newAccessToken}`; - document.cookie = `luciaRefreshToken=${newRefreshToken};expires=${new Date(this.expired)}`; + setCookieWithoutExpiration("luciaToken", newAccessToken); + setCookie("luciaRefreshToken", newRefreshToken, Math.ceil((this.expired - Date.now()) / (24 * 60 * 60 * 1000))); axios.defaults.headers.common['Authorization'] = `Bearer ${newAccessToken}`; } @@ -92,7 +92,7 @@ export default defineStore('loginStore', { */ logOut() { delete axios.defaults.headers.common["Authorization"]; - document.cookie = 'luciaToken=; expires=Thu, 01 Jan 1970 00:00:00 UTC;'; + deleteCookie("luciaToken"); this.isLoggedIn = false; deleteCookie("isLuciaLoggedIn"); diff --git a/src/utils/cookieUtil.js b/src/utils/cookieUtil.js index e768f76..0dc2a55 100644 --- a/src/utils/cookieUtil.js +++ b/src/utils/cookieUtil.js @@ -19,14 +19,14 @@ export function setCookie(name, value, days=1) { date.setTime(date.getTime() + (days * 24 * 60 * 60 * 1000)); expires = "; expires=" + date.toUTCString(); } - document.cookie = name + "=" + (value || "") + expires + "; path=/"; + document.cookie = name + "=" + (value || "") + expires + "; path=/; Secure; SameSite=Lax"; } export function setCookieWithoutExpiration(name, value) { - document.cookie = name + "=" + (value || ""); + document.cookie = name + "=" + (value || "") + "; Secure; SameSite=Lax"; } export function deleteCookie(name, path = '/') { - document.cookie = name + '=; Max-Age=-99999999; path=' + path; + document.cookie = name + '=; Max-Age=-99999999; path=' + path + '; Secure; SameSite=Lax'; } \ No newline at end of file diff --git a/tests/stores/login.test.js b/tests/stores/login.test.js index e806dc3..40a5316 100644 --- a/tests/stores/login.test.js +++ b/tests/stores/login.test.js @@ -56,7 +56,23 @@ describe('loginStore', () => { }), ); expect(store.isLoggedIn).toBe(true); - expect(document.cookie).toContain('luciaToken=test-access-token'); + // Verify token cookie was set with Secure flag + // (jsdom drops Secure cookies, so spy on setter) + const cookieSetter = vi.spyOn(document, 'cookie', 'set'); + vi.clearAllMocks(); + axios.post.mockResolvedValue({ + data: { + access_token: 'test-access-token', + refresh_token: 'test-refresh-token', + }, + }); + await store.signIn(); + const tokenCall = cookieSetter.mock.calls.find( + (c) => c[0].includes('luciaToken='), + ); + expect(tokenCall).toBeDefined(); + expect(tokenCall[0]).toContain('Secure'); + cookieSetter.mockRestore(); expect(store.$router.push).toHaveBeenCalledWith('/files'); }); @@ -173,6 +189,25 @@ describe('loginStore', () => { // Should update axios default Authorization header expect(axios.defaults.headers.common['Authorization']) .toBe('Bearer new-access-token'); + + // Verify cookies were set with Secure flag + const cookieSetter = vi.spyOn(document, 'cookie', 'set'); + vi.clearAllMocks(); + document.cookie = 'luciaRefreshToken=old-refresh-token'; + axios.post.mockResolvedValue({ + status: 200, + data: { + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + }, + }); + await store.refreshToken(); + const tokenCall = cookieSetter.mock.calls.find( + (c) => c[0].includes('luciaToken='), + ); + expect(tokenCall).toBeDefined(); + expect(tokenCall[0]).toContain('Secure'); + cookieSetter.mockRestore(); }); it('redirects to login and re-throws on failure', async () => { diff --git a/tests/unit/utils/cookieUtil.test.js b/tests/unit/utils/cookieUtil.test.js index 4642f3a..44f9b0b 100644 --- a/tests/unit/utils/cookieUtil.test.js +++ b/tests/unit/utils/cookieUtil.test.js @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { getCookie, setCookie, @@ -7,6 +7,8 @@ import { } from '@/utils/cookieUtil.js'; describe('cookieUtil', () => { + let cookieSetter; + beforeEach(() => { // Clear all cookies before each test document.cookie.split(';').forEach((c) => { @@ -16,6 +18,9 @@ describe('cookieUtil', () => { name + '=; Max-Age=-99999999; path=/'; } }); + // Spy on document.cookie setter to capture the raw string + // (jsdom silently drops Secure cookies on http://) + cookieSetter = vi.spyOn(document, 'cookie', 'set'); }); describe('getCookie', () => { @@ -43,30 +48,49 @@ describe('cookieUtil', () => { }); describe('setCookie', () => { - it('sets a cookie with default 1-day expiration', () => { + it('sets cookie with Secure and SameSite=Lax flags', () => { setCookie('myKey', 'myValue'); - expect(getCookie('myKey')).toBe('myValue'); - }); - it('sets a cookie with empty value', () => { - setCookie('emptyKey', ''); - expect(getCookie('emptyKey')).toBe(''); + const written = cookieSetter.mock.calls.find( + (c) => c[0].startsWith('myKey='), + ); + expect(written).toBeDefined(); + const str = written[0]; + expect(str).toContain('myKey=myValue'); + expect(str).toContain('expires='); + expect(str).toContain('path=/'); + expect(str).toContain('Secure'); + expect(str).toContain('SameSite=Lax'); }); }); describe('setCookieWithoutExpiration', () => { - it('sets a session cookie', () => { + it('sets session cookie with Secure and SameSite=Lax flags', () => { setCookieWithoutExpiration('sessionKey', 'sessionVal'); - expect(getCookie('sessionKey')).toBe('sessionVal'); + + const written = cookieSetter.mock.calls.find( + (c) => c[0].startsWith('sessionKey='), + ); + expect(written).toBeDefined(); + const str = written[0]; + expect(str).toContain('sessionKey=sessionVal'); + expect(str).toContain('Secure'); + expect(str).toContain('SameSite=Lax'); }); }); describe('deleteCookie', () => { - it('removes an existing cookie', () => { - setCookie('toDelete', 'value'); - expect(getCookie('toDelete')).toBe('value'); + it('sets Max-Age=-99999999 with Secure and SameSite=Lax flags', () => { deleteCookie('toDelete'); - expect(getCookie('toDelete')).toBeNull(); + + const written = cookieSetter.mock.calls.find( + (c) => c[0].startsWith('toDelete='), + ); + expect(written).toBeDefined(); + const str = written[0]; + expect(str).toContain('Max-Age=-99999999'); + expect(str).toContain('Secure'); + expect(str).toContain('SameSite=Lax'); }); }); });