fix(auth): mark store authenticated after OAuth setTokens
setTokens() previously only set { token } without flipping
isAuthenticated, so after the OAuth callback exchange the store had
fresh tokens but ProtectedRoute still saw isAuthenticated === false and
bounced the user to /landing before fetchUser() could complete.
Storing tokens implies an active session, so set isAuthenticated: true
inside setTokens. The other caller (refresh interceptor in api/client.ts)
runs from an already-authenticated session, so the flag flip is a no-op
there.
Tests:
- new src/store/authStore.test.ts covers the setTokens contract
- src/pages/__tests__/OAuthCallbackPage.test.tsx adds a successful-
callback case asserting setTokens + fetchUser are invoked with the
exchanged tokens
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -13,10 +13,13 @@ vi.mock('@/api/auth', () => ({
|
|||||||
},
|
},
|
||||||
}))
|
}))
|
||||||
|
|
||||||
|
const mockSetTokens = vi.fn()
|
||||||
|
const mockFetchUser = vi.fn().mockResolvedValue(undefined)
|
||||||
|
|
||||||
vi.mock('@/store/authStore', () => ({
|
vi.mock('@/store/authStore', () => ({
|
||||||
useAuthStore: () => ({
|
useAuthStore: () => ({
|
||||||
setTokens: vi.fn(),
|
setTokens: mockSetTokens,
|
||||||
fetchUser: vi.fn().mockResolvedValue(undefined),
|
fetchUser: mockFetchUser,
|
||||||
}),
|
}),
|
||||||
}))
|
}))
|
||||||
|
|
||||||
@@ -79,3 +82,40 @@ describe('OAuthCallbackPage CSRF state validation', () => {
|
|||||||
expect(authApi.googleCallback).not.toHaveBeenCalled()
|
expect(authApi.googleCallback).not.toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('OAuthCallbackPage successful callback', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
sessionStorage.clear()
|
||||||
|
vi.clearAllMocks()
|
||||||
|
localStorage.clear()
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
sessionStorage.clear()
|
||||||
|
localStorage.clear()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('persists tokens via setTokens (which marks the store authenticated) and fetches the user', async () => {
|
||||||
|
sessionStorage.setItem('rf-oauth-state', 'csrf-value')
|
||||||
|
;(authApi.googleCallback as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
|
access_token: 'access-123',
|
||||||
|
refresh_token: 'refresh-456',
|
||||||
|
token_type: 'bearer',
|
||||||
|
is_new_user: false,
|
||||||
|
})
|
||||||
|
|
||||||
|
renderAt('/auth/google/callback?code=auth-code-123&state=csrf-value')
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(mockSetTokens).toHaveBeenCalledWith({
|
||||||
|
access_token: 'access-123',
|
||||||
|
refresh_token: 'refresh-456',
|
||||||
|
token_type: 'bearer',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
expect(mockFetchUser).toHaveBeenCalled()
|
||||||
|
// Tokens are also persisted for the apiClient interceptor.
|
||||||
|
expect(localStorage.getItem('access_token')).toBe('access-123')
|
||||||
|
expect(localStorage.getItem('refresh_token')).toBe('refresh-456')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
68
frontend/src/store/authStore.test.ts
Normal file
68
frontend/src/store/authStore.test.ts
Normal file
@@ -0,0 +1,68 @@
|
|||||||
|
import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||||
|
import { useAuthStore } from './authStore'
|
||||||
|
import type { Token } from '@/types'
|
||||||
|
|
||||||
|
// Avoid pulling in real analytics / Sentry side effects during tests.
|
||||||
|
vi.mock('@/lib/analytics', () => ({
|
||||||
|
identifyUser: vi.fn(),
|
||||||
|
resetAnalytics: vi.fn(),
|
||||||
|
analytics: {
|
||||||
|
loginSuccess: vi.fn(),
|
||||||
|
accountCreated: vi.fn(),
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('@sentry/react', () => ({
|
||||||
|
setUser: vi.fn(),
|
||||||
|
}))
|
||||||
|
|
||||||
|
describe('authStore.setTokens', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
// Reset store to initial state between tests.
|
||||||
|
useAuthStore.setState({
|
||||||
|
user: null,
|
||||||
|
token: null,
|
||||||
|
account: null,
|
||||||
|
subscription: null,
|
||||||
|
isAuthenticated: false,
|
||||||
|
isLoading: false,
|
||||||
|
error: null,
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
it('marks the store as authenticated and persists the token', () => {
|
||||||
|
const fakeToken: Token = {
|
||||||
|
access_token: 'access-abc',
|
||||||
|
refresh_token: 'refresh-xyz',
|
||||||
|
token_type: 'bearer',
|
||||||
|
}
|
||||||
|
|
||||||
|
useAuthStore.getState().setTokens(fakeToken)
|
||||||
|
|
||||||
|
const state = useAuthStore.getState()
|
||||||
|
expect(state.token).toEqual(fakeToken)
|
||||||
|
expect(state.isAuthenticated).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('keeps isAuthenticated true when called again (refresh-token path)', () => {
|
||||||
|
// Simulate an already-authenticated session (refresh interceptor case).
|
||||||
|
useAuthStore.setState({
|
||||||
|
token: {
|
||||||
|
access_token: 'old',
|
||||||
|
refresh_token: 'old-r',
|
||||||
|
token_type: 'bearer',
|
||||||
|
},
|
||||||
|
isAuthenticated: true,
|
||||||
|
})
|
||||||
|
|
||||||
|
useAuthStore.getState().setTokens({
|
||||||
|
access_token: 'new',
|
||||||
|
refresh_token: 'new-r',
|
||||||
|
token_type: 'bearer',
|
||||||
|
})
|
||||||
|
|
||||||
|
const state = useAuthStore.getState()
|
||||||
|
expect(state.token?.access_token).toBe('new')
|
||||||
|
expect(state.isAuthenticated).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -131,7 +131,13 @@ export const useAuthStore = create<AuthState>()(
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
setTokens: (token: Token) => set({ token }),
|
// Storing tokens implies an active session — mark the store as
|
||||||
|
// authenticated so <ProtectedRoute> doesn't bounce the user back to
|
||||||
|
// /landing while fetchUser() is still inflight (e.g. immediately after
|
||||||
|
// the OAuth callback exchange). The refresh interceptor in api/client.ts
|
||||||
|
// also calls this; that path is already authenticated, so flipping the
|
||||||
|
// flag has no effect there.
|
||||||
|
setTokens: (token: Token) => set({ token, isAuthenticated: true }),
|
||||||
clearError: () => set({ error: null }),
|
clearError: () => set({ error: null }),
|
||||||
setLoading: (loading: boolean) => set({ isLoading: loading }),
|
setLoading: (loading: boolean) => set({ isLoading: loading }),
|
||||||
}),
|
}),
|
||||||
|
|||||||
Reference in New Issue
Block a user