mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-02 08:33:36 +00:00
refactor: update session cookie options and improve login view authentication flow
- Revised SameSite attribute for session cookies to clarify its behavior in documentation. - Streamlined cookie clearing logic in the authentication route by utilizing `getSessionCookieOptions()`. - Enhanced the login view to support aborting server checks, improving responsiveness during component unmounting. - Ensured proper handling of server check retries with abort signal integration for better user experience.
This commit is contained in:
@@ -262,7 +262,7 @@ export function getSessionCookieOptions(): {
|
|||||||
return {
|
return {
|
||||||
httpOnly: true, // JavaScript cannot access this cookie
|
httpOnly: true, // JavaScript cannot access this cookie
|
||||||
secure: process.env.NODE_ENV === 'production', // HTTPS only in production
|
secure: process.env.NODE_ENV === 'production', // HTTPS only in production
|
||||||
sameSite: 'lax', // Sent on same-site requests including cross-origin fetches
|
sameSite: 'lax', // Sent for same-site requests and top-level navigations, but not cross-origin fetch/XHR
|
||||||
maxAge: SESSION_MAX_AGE_MS,
|
maxAge: SESSION_MAX_AGE_MS,
|
||||||
path: '/',
|
path: '/',
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -233,10 +233,7 @@ export function createAuthRoutes(): Router {
|
|||||||
// Using res.cookie() with maxAge: 0 is more reliable than clearCookie()
|
// Using res.cookie() with maxAge: 0 is more reliable than clearCookie()
|
||||||
// in cross-origin development environments
|
// in cross-origin development environments
|
||||||
res.cookie(cookieName, '', {
|
res.cookie(cookieName, '', {
|
||||||
httpOnly: true,
|
...getSessionCookieOptions(),
|
||||||
secure: process.env.NODE_ENV === 'production',
|
|
||||||
sameSite: 'lax',
|
|
||||||
path: '/',
|
|
||||||
maxAge: 0,
|
maxAge: 0,
|
||||||
expires: new Date(0),
|
expires: new Date(0),
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -277,7 +277,7 @@ describe('auth.ts', () => {
|
|||||||
const options = getSessionCookieOptions();
|
const options = getSessionCookieOptions();
|
||||||
|
|
||||||
expect(options.httpOnly).toBe(true);
|
expect(options.httpOnly).toBe(true);
|
||||||
expect(options.sameSite).toBe('strict');
|
expect(options.sameSite).toBe('lax');
|
||||||
expect(options.path).toBe('/');
|
expect(options.path).toBe('/');
|
||||||
expect(options.maxAge).toBeGreaterThan(0);
|
expect(options.maxAge).toBeGreaterThan(0);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ import { RouterProvider } from '@tanstack/react-router';
|
|||||||
import { createLogger } from '@automaker/utils/logger';
|
import { createLogger } from '@automaker/utils/logger';
|
||||||
import { router } from './utils/router';
|
import { router } from './utils/router';
|
||||||
import { SplashScreen } from './components/splash-screen';
|
import { SplashScreen } from './components/splash-screen';
|
||||||
import { LoadingState } from './components/ui/loading-state';
|
|
||||||
import { useSettingsSync } from './hooks/use-settings-sync';
|
import { useSettingsSync } from './hooks/use-settings-sync';
|
||||||
import { useCursorStatusInit } from './hooks/use-cursor-status-init';
|
import { useCursorStatusInit } from './hooks/use-cursor-status-init';
|
||||||
import './styles/global.css';
|
import './styles/global.css';
|
||||||
|
|||||||
@@ -125,14 +125,25 @@ async function checkAuthStatusSafe(): Promise<{ authenticated: boolean }> {
|
|||||||
*/
|
*/
|
||||||
async function checkServerAndSession(
|
async function checkServerAndSession(
|
||||||
dispatch: React.Dispatch<Action>,
|
dispatch: React.Dispatch<Action>,
|
||||||
setAuthState: (state: { isAuthenticated: boolean; authChecked: boolean }) => void
|
setAuthState: (state: { isAuthenticated: boolean; authChecked: boolean }) => void,
|
||||||
|
signal?: AbortSignal
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
|
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
|
||||||
|
// Return early if the component has unmounted
|
||||||
|
if (signal?.aborted) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
dispatch({ type: 'SERVER_CHECK_RETRY', attempt });
|
dispatch({ type: 'SERVER_CHECK_RETRY', attempt });
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const result = await checkAuthStatusSafe();
|
const result = await checkAuthStatusSafe();
|
||||||
|
|
||||||
|
// Return early if the component has unmounted
|
||||||
|
if (signal?.aborted) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (result.authenticated) {
|
if (result.authenticated) {
|
||||||
// Server is reachable and we're authenticated
|
// Server is reachable and we're authenticated
|
||||||
setAuthState({ isAuthenticated: true, authChecked: true });
|
setAuthState({ isAuthenticated: true, authChecked: true });
|
||||||
@@ -148,10 +159,13 @@ async function checkServerAndSession(
|
|||||||
console.debug(`Server check attempt ${attempt}/${MAX_RETRIES} failed:`, error);
|
console.debug(`Server check attempt ${attempt}/${MAX_RETRIES} failed:`, error);
|
||||||
|
|
||||||
if (attempt === MAX_RETRIES) {
|
if (attempt === MAX_RETRIES) {
|
||||||
dispatch({
|
// Return early if the component has unmounted
|
||||||
type: 'SERVER_ERROR',
|
if (!signal?.aborted) {
|
||||||
message: 'Unable to connect to server. Please check that the server is running.',
|
dispatch({
|
||||||
});
|
type: 'SERVER_ERROR',
|
||||||
|
message: 'Unable to connect to server. Please check that the server is running.',
|
||||||
|
});
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -225,7 +239,12 @@ export function LoginView() {
|
|||||||
if (initialCheckDone.current) return;
|
if (initialCheckDone.current) return;
|
||||||
initialCheckDone.current = true;
|
initialCheckDone.current = true;
|
||||||
|
|
||||||
checkServerAndSession(dispatch, setAuthState);
|
const controller = new AbortController();
|
||||||
|
checkServerAndSession(dispatch, setAuthState, controller.signal);
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
controller.abort();
|
||||||
|
};
|
||||||
}, [setAuthState]);
|
}, [setAuthState]);
|
||||||
|
|
||||||
// When we enter checking_setup phase, check setup status
|
// When we enter checking_setup phase, check setup status
|
||||||
@@ -255,7 +274,8 @@ export function LoginView() {
|
|||||||
const handleRetry = () => {
|
const handleRetry = () => {
|
||||||
initialCheckDone.current = false;
|
initialCheckDone.current = false;
|
||||||
dispatch({ type: 'RETRY_SERVER_CHECK' });
|
dispatch({ type: 'RETRY_SERVER_CHECK' });
|
||||||
checkServerAndSession(dispatch, setAuthState);
|
const controller = new AbortController();
|
||||||
|
checkServerAndSession(dispatch, setAuthState, controller.signal);
|
||||||
};
|
};
|
||||||
|
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
|
|||||||
Reference in New Issue
Block a user