From f687909f913d593c20099e53392296dd0b8da3c8 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Fri, 30 Jan 2026 12:39:18 -0600 Subject: [PATCH] Implement OAuth callback with token handling and profile fetch (F1-002) Complete the AuthCallbackPage to handle OAuth redirects by parsing tokens from URL fragment, fetching user profile, and redirecting based on starter deck status. Includes open-redirect protection and comprehensive tests. Co-Authored-By: Claude Opus 4.5 --- frontend/PROJECT_PLAN_FRONTEND.json | 9 +- .../PHASE_F1_authentication.json | 12 +- frontend/src/pages/AuthCallbackPage.spec.ts | 437 ++++++++++++++++++ frontend/src/pages/AuthCallbackPage.vue | 234 +++++++++- 4 files changed, 665 insertions(+), 27 deletions(-) create mode 100644 frontend/src/pages/AuthCallbackPage.spec.ts diff --git a/frontend/PROJECT_PLAN_FRONTEND.json b/frontend/PROJECT_PLAN_FRONTEND.json index 44cb3e9..6add2b1 100644 --- a/frontend/PROJECT_PLAN_FRONTEND.json +++ b/frontend/PROJECT_PLAN_FRONTEND.json @@ -7,8 +7,8 @@ "projectName": "Mantimon TCG - Frontend", "description": "Vue 3 + Phaser 3 frontend for pocket.manticorum.com - real-time multiplayer TCG with campaign mode", "totalPhases": 8, - "completedPhases": 0, - "status": "Phase F0 in progress" + "completedPhases": 1, + "status": "Phase F1 in progress" }, "techStack": { @@ -124,7 +124,8 @@ { "id": "PHASE_F0", "name": "Project Foundation", - "status": "in_progress", + "status": "COMPLETE", + "completedDate": "2026-01-30", "description": "Scaffolding, tooling, core infrastructure, API client setup", "estimatedDays": "3-5", "dependencies": [], @@ -240,7 +241,7 @@ { "id": "PHASE_F1", "name": "Authentication", - "status": "NOT_STARTED", + "status": "in_progress", "description": "OAuth login flow, token management, protected routes", "estimatedDays": "2-3", "dependencies": ["PHASE_F0"], diff --git a/frontend/project_plans/PHASE_F1_authentication.json b/frontend/project_plans/PHASE_F1_authentication.json index 6cbcfa2..cafd520 100644 --- a/frontend/project_plans/PHASE_F1_authentication.json +++ b/frontend/project_plans/PHASE_F1_authentication.json @@ -6,8 +6,8 @@ "created": "2026-01-30", "lastUpdated": "2026-01-30", "totalTasks": 10, - "completedTasks": 0, - "status": "not_started", + "completedTasks": 2, + "status": "in_progress", "description": "Complete OAuth authentication flow including login, callback handling, starter deck selection, profile management, and app initialization." }, "dependencies": { @@ -36,8 +36,8 @@ "description": "Replace username/password form with OAuth provider buttons", "category": "pages", "priority": 1, - "completed": false, - "tested": false, + "completed": true, + "tested": true, "dependencies": [], "files": [ {"path": "src/pages/LoginPage.vue", "status": "modify"} @@ -64,8 +64,8 @@ "description": "Handle OAuth callback and extract tokens from URL fragment", "category": "pages", "priority": 2, - "completed": false, - "tested": false, + "completed": true, + "tested": true, "dependencies": ["F1-001"], "files": [ {"path": "src/pages/AuthCallbackPage.vue", "status": "modify"} diff --git a/frontend/src/pages/AuthCallbackPage.spec.ts b/frontend/src/pages/AuthCallbackPage.spec.ts new file mode 100644 index 0000000..8b66b91 --- /dev/null +++ b/frontend/src/pages/AuthCallbackPage.spec.ts @@ -0,0 +1,437 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest' +import { mount, flushPromises } from '@vue/test-utils' +import { createRouter, createWebHistory } from 'vue-router' +import { setActivePinia, createPinia } from 'pinia' + +import AuthCallbackPage from './AuthCallbackPage.vue' +import { useAuthStore } from '@/stores/auth' + +// Mock the API client +vi.mock('@/api/client', () => ({ + apiClient: { + get: vi.fn(), + }, +})) + +import { apiClient } from '@/api/client' + +describe('AuthCallbackPage', () => { + let router: ReturnType + let mockLocation: { hash: string; pathname: string } + + beforeEach(() => { + setActivePinia(createPinia()) + + // Mock window.location + mockLocation = { hash: '', pathname: '/auth/callback' } + Object.defineProperty(window, 'location', { + value: mockLocation, + writable: true, + }) + + // Mock history.replaceState + vi.spyOn(window.history, 'replaceState').mockImplementation(() => {}) + + // Reset mocks + vi.mocked(apiClient.get).mockReset() + + router = createRouter({ + history: createWebHistory(), + routes: [ + { path: '/auth/callback', name: 'AuthCallback', component: AuthCallbackPage }, + { path: '/login', name: 'Login', component: { template: '
Login
' } }, + { path: '/', name: 'Dashboard', component: { template: '
Dashboard
' } }, + { path: '/starter', name: 'StarterSelection', component: { template: '
Starter
' } }, + ], + }) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe('token parsing', () => { + it('extracts tokens from URL hash fragment', async () => { + /** + * Test that tokens are correctly parsed from the URL hash. + * + * The OAuth provider redirects back with tokens in the fragment + * (after #) for security. We must parse access_token, refresh_token, + * and expires_in from this fragment. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'Test User', + avatar_url: null, + has_starter_deck: true, + }) + + await router.push('/auth/callback') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + const auth = useAuthStore() + expect(auth.accessToken).toBe('abc123') + expect(auth.refreshToken).toBe('xyz789') + expect(auth.expiresAt).toBeGreaterThan(Date.now()) + }) + + it('shows error when tokens are missing', async () => { + /** + * Test error handling for malformed OAuth response. + * + * If the URL fragment is missing required tokens, we should + * show an error rather than crash or behave unexpectedly. + */ + mockLocation.hash = '#access_token=abc123' // Missing refresh_token and expires_in + + await router.push('/auth/callback') + await router.isReady() + + const wrapper = mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(wrapper.text()).toContain('Authentication Failed') + expect(wrapper.text()).toContain('Missing tokens') + }) + + it('shows error when hash is empty', async () => { + /** + * Test error handling when no hash fragment is present. + * + * If the OAuth provider redirects without tokens (empty hash), + * we should show an appropriate error. + */ + mockLocation.hash = '' + + await router.push('/auth/callback') + await router.isReady() + + const wrapper = mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(wrapper.text()).toContain('Authentication Failed') + }) + }) + + describe('error handling from OAuth', () => { + it('redirects to login with error from query params', async () => { + /** + * Test that OAuth errors are forwarded to the login page. + * + * When OAuth fails, the backend redirects to callback with + * error details in query params. We should forward these + * to the login page for display. + */ + await router.push('/auth/callback?error=access_denied&message=User%20cancelled') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(router.currentRoute.value.name).toBe('Login') + expect(router.currentRoute.value.query.error).toBe('access_denied') + expect(router.currentRoute.value.query.message).toBe('User cancelled') + }) + + it('uses default error message when message param is missing', async () => { + /** + * Test fallback error message. + * + * If the backend only provides an error code without a message, + * we should use a sensible default message. + */ + await router.push('/auth/callback?error=unknown_error') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(router.currentRoute.value.name).toBe('Login') + expect(router.currentRoute.value.query.message).toBe('Authentication failed. Please try again.') + }) + }) + + describe('user profile handling', () => { + it('fetches user profile after storing tokens', async () => { + /** + * Test that user profile is fetched after successful token storage. + * + * After storing tokens, we need to fetch the user's profile + * to know their display name, avatar, and starter deck status. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'Test User', + avatar_url: 'https://example.com/avatar.png', + has_starter_deck: true, + }) + + await router.push('/auth/callback') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(apiClient.get).toHaveBeenCalledWith('/api/users/me') + + const auth = useAuthStore() + expect(auth.user).toEqual({ + id: 'user-1', + displayName: 'Test User', + avatarUrl: 'https://example.com/avatar.png', + hasStarterDeck: true, + }) + }) + + it('redirects to starter selection if user has no starter deck', async () => { + /** + * Test redirect flow for new users without starter decks. + * + * New users must select a starter deck before accessing the main app. + * If has_starter_deck is false, redirect to the selection page. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'New User', + avatar_url: null, + has_starter_deck: false, + }) + + await router.push('/auth/callback') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(router.currentRoute.value.name).toBe('StarterSelection') + }) + + it('redirects to dashboard if user has starter deck', async () => { + /** + * Test redirect flow for existing users with starter decks. + * + * Users who already have a starter deck should go directly + * to the dashboard after successful login. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'Existing User', + avatar_url: null, + has_starter_deck: true, + }) + + await router.push('/auth/callback') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(router.currentRoute.value.name).toBe('Dashboard') + }) + + it('redirects to intended destination after login', async () => { + /** + * Test redirect to originally requested page. + * + * If a user was redirected to login while trying to access + * a protected route, they should return there after auth. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'Test User', + avatar_url: null, + has_starter_deck: true, + }) + + await router.push('/auth/callback?redirect=/decks') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(router.currentRoute.value.path).toBe('/decks') + }) + }) + + describe('API errors', () => { + it('shows error when profile fetch fails', async () => { + /** + * Test error handling for profile fetch failures. + * + * If we successfully get tokens but fail to fetch the profile + * (network error, server error, etc.), show an error state. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockRejectedValue(new Error('Network error')) + + await router.push('/auth/callback') + await router.isReady() + + const wrapper = mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(wrapper.text()).toContain('Authentication Failed') + expect(wrapper.text()).toContain('Network error') + }) + }) + + describe('security', () => { + it('rejects protocol-relative redirect URLs', async () => { + /** + * Test that protocol-relative URLs are rejected as redirects. + * + * An attacker could try to use //evil.com as a redirect, which + * passes a naive startsWith('/') check but redirects to another domain. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'Test User', + avatar_url: null, + has_starter_deck: true, + }) + + await router.push('/auth/callback?redirect=//evil.com') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + // Should go to dashboard, not the malicious redirect + expect(router.currentRoute.value.name).toBe('Dashboard') + }) + + it('clears tokens from URL after parsing', async () => { + /** + * Test that tokens are removed from browser history. + * + * Tokens in the URL should be cleared after parsing to prevent + * them from appearing in browser history or being logged. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + vi.mocked(apiClient.get).mockResolvedValue({ + id: 'user-1', + display_name: 'Test User', + avatar_url: null, + has_starter_deck: true, + }) + + await router.push('/auth/callback') + await router.isReady() + + mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + expect(window.history.replaceState).toHaveBeenCalledWith( + null, + '', + '/auth/callback' + ) + }) + }) + + describe('UI states', () => { + it('shows processing state while handling callback', async () => { + /** + * Test loading state during authentication. + * + * Users should see a loading indicator while tokens are being + * processed and profile is being fetched. + */ + mockLocation.hash = '#access_token=abc123&refresh_token=xyz789&expires_in=3600' + + // Don't resolve the promise immediately + vi.mocked(apiClient.get).mockImplementation(() => new Promise(() => {})) + + await router.push('/auth/callback') + await router.isReady() + + const wrapper = mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + expect(wrapper.text()).toContain('Completing login') + expect(wrapper.text()).toContain('Please wait') + }) + + it('shows back to login button on error', async () => { + /** + * Test error recovery UI. + * + * When authentication fails, users should have a clear + * way to return to the login page and try again. + */ + mockLocation.hash = '' // No tokens = error + + await router.push('/auth/callback') + await router.isReady() + + const wrapper = mount(AuthCallbackPage, { + global: { plugins: [router] }, + }) + + await flushPromises() + + const button = wrapper.find('button') + expect(button.exists()).toBe(true) + expect(button.text()).toContain('Back to Login') + + await button.trigger('click') + await flushPromises() + + expect(router.currentRoute.value.name).toBe('Login') + }) + }) +}) diff --git a/frontend/src/pages/AuthCallbackPage.vue b/frontend/src/pages/AuthCallbackPage.vue index 83e82fd..651ba53 100644 --- a/frontend/src/pages/AuthCallbackPage.vue +++ b/frontend/src/pages/AuthCallbackPage.vue @@ -3,35 +3,235 @@ * OAuth callback page. * * Handles the redirect from OAuth providers (Google, Discord). - * Extracts tokens from URL fragment and stores them in auth store. + * Extracts tokens from URL fragment, stores them, fetches user + * profile, and redirects to the appropriate destination. + * + * Success URL format: /auth/callback#access_token={token}&refresh_token={token}&expires_in={seconds} + * Error URL format: /auth/callback?error={code}&message={message} */ -import { onMounted } from 'vue' -import { useRouter } from 'vue-router' +import { ref, onMounted } from 'vue' +import { useRouter, useRoute } from 'vue-router' + +import { useAuthStore } from '@/stores/auth' +import { apiClient } from '@/api/client' +import type { User } from '@/stores/auth' + +interface UserResponse { + id: string + display_name: string + avatar_url: string | null + has_starter_deck: boolean +} const router = useRouter() +const route = useRoute() +const auth = useAuthStore() + +const status = ref<'processing' | 'error'>('processing') +const errorMessage = ref(null) + +/** + * Parse tokens from URL hash fragment. + * + * The backend redirects with tokens in the fragment (after #) for security, + * since fragments are not sent to servers in HTTP requests. + */ +function parseTokensFromHash(): { accessToken: string; refreshToken: string; expiresIn: number } | null { + const hash = window.location.hash.substring(1) // Remove leading # + if (!hash) return null + + const params = new URLSearchParams(hash) + const accessToken = params.get('access_token') + const refreshToken = params.get('refresh_token') + const expiresIn = params.get('expires_in') + + if (!accessToken || !refreshToken || !expiresIn) { + return null + } + + return { + accessToken, + refreshToken, + expiresIn: parseInt(expiresIn, 10), + } +} + +/** + * Check for error in URL query params. + */ +function parseErrorFromQuery(): { error: string; message: string } | null { + const error = route.query.error as string | undefined + const message = route.query.message as string | undefined + + if (error) { + return { + error, + message: message || 'Authentication failed. Please try again.', + } + } + + return null +} + +/** + * Fetch user profile from API. + */ +async function fetchUserProfile(): Promise { + return apiClient.get('/api/users/me') +} + +/** + * Transform API response to auth store User type. + */ +function transformUser(response: UserResponse): User { + return { + id: response.id, + displayName: response.display_name, + avatarUrl: response.avatar_url, + hasStarterDeck: response.has_starter_deck, + } +} + +/** + * Handle the OAuth callback process. + */ +async function handleCallback(): Promise { + // First, check for errors from failed OAuth + const errorInfo = parseErrorFromQuery() + if (errorInfo) { + // Redirect back to login with error message + router.push({ + name: 'Login', + query: { error: errorInfo.error, message: errorInfo.message }, + }) + return + } + + // Parse tokens from URL fragment + const tokens = parseTokensFromHash() + if (!tokens) { + status.value = 'error' + errorMessage.value = 'Invalid authentication response. Missing tokens.' + return + } + + try { + // Store tokens in auth store + auth.setTokens({ + accessToken: tokens.accessToken, + refreshToken: tokens.refreshToken, + expiresAt: Date.now() + tokens.expiresIn * 1000, + }) + + // Clear the hash from URL for security (tokens shouldn't linger in browser history) + window.history.replaceState(null, '', window.location.pathname) + + // Fetch user profile + const userResponse = await fetchUserProfile() + const user = transformUser(userResponse) + auth.setUser(user) + + // Redirect based on starter deck status + if (!user.hasStarterDeck) { + router.push({ name: 'StarterSelection' }) + } else { + // Check if there was an intended destination before login + const redirect = route.query.redirect as string | undefined + if (redirect && redirect.startsWith('/') && !redirect.startsWith('//')) { + router.push(redirect) + } else { + router.push({ name: 'Dashboard' }) + } + } + } catch (error) { + status.value = 'error' + if (error instanceof Error) { + errorMessage.value = error.message + } else { + errorMessage.value = 'Failed to complete authentication. Please try again.' + } + } +} onMounted(() => { - // TODO: Extract tokens from URL hash fragment - // TODO: Store tokens in auth store - // TODO: Fetch user profile - // TODO: Redirect to intended destination or home - - // Placeholder: redirect to home after brief delay - setTimeout(() => { - router.push('/') - }, 1000) + handleCallback() }) + +function goToLogin(): void { + router.push({ name: 'Login' }) +}