Forward OAuth state to the backend callback endpoint during sign in

Author: jake-low

src/api/user/auth.test.ts

@@ -8,12 +8,6 @@ import { stubFetch } from '@/test/stubFetch'
 import { waitFor } from '@/test/waitFor'
 import { userAuth } from './auth'
 
-// window.env's fields are declared read-only (set once at app boot from
-// env.json), but tests need to simulate a deployment with no configured app
-// URL, so we write through a narrow mutable view instead of widening with
-// `as any`.
-type MutableEnv = { VITE_APP_URL?: string }
-
 afterEach(() => {
   vi.unstubAllGlobals()
 })
@@ -30,34 +24,18 @@ describe('userAuth.signOut', () => {
 })
 
 describe('userAuth.callback', () => {
-  it('uses window.env.VITE_APP_URL as the redirect_uri when configured', async () => {
-    const fetchMock = stubFetch(new Response(JSON.stringify({ apiKey: 'abc' }), { status: 200 }))
+  it('sends the code and state, and no redirect_uri', async () => {
+    const fetchMock = stubFetch(new Response(JSON.stringify({ token: 'abc' }), { status: 200 }))
 
-    const result = await userAuth.callback('the-code')
+    const result = await userAuth.callback('the/code', 'the-state')
 
-    expect(result).toEqual({ apiKey: 'abc' })
+    expect(result).toEqual({ token: 'abc' })
     const [request] = fetchMock.mock.calls[0]
     const url = new URL(request.url)
     expect(url.pathname).toBe('/auth/callback')
-    expect(url.searchParams.get('code')).toBe('the-code')
-    expect(url.searchParams.get('redirect_uri')).toBe(window.env.VITE_APP_URL)
-  })
-
-  it('falls back to window.location.origin when VITE_APP_URL is not configured', async () => {
-    const mutableEnv = window.env as MutableEnv
-    const originalAppUrl = mutableEnv.VITE_APP_URL
-    mutableEnv.VITE_APP_URL = undefined
-    const fetchMock = stubFetch(new Response(JSON.stringify({}), { status: 200 }))
-
-    try {
-      await userAuth.callback('another-code')
-
-      const [request] = fetchMock.mock.calls[0]
-      const url = new URL(request.url)
-      expect(url.searchParams.get('redirect_uri')).toBe(window.location.origin)
-    } finally {
-      mutableEnv.VITE_APP_URL = originalAppUrl
-    }
+    expect(url.searchParams.get('code')).toBe('the/code')
+    expect(url.searchParams.get('state')).toBe('the-state')
+    expect(url.searchParams.has('redirect_uri')).toBe(false)
   })
 })
 

src/api/user/auth.ts

@@ -7,12 +7,12 @@ import { apiRequest } from '../client'
 export const userAuth = {
   signOut: async () => await apiRequest.get('auth/signout').json<void>(),
 
-  callback: async (code: string) => {
-    const frontendOrigin = window.env.VITE_APP_URL || window.location.origin
-    return await apiRequest
-      .get(`auth/callback?code=${code}&redirect_uri=${encodeURIComponent(frontendOrigin)}`)
-      .json<OAuthCallbackResponse>()
-  },
+  // The backend checks state against the one it stored in the session at
+  // auth/authenticate, and reuses the redirect_uri it stored there too.
+  callback: async (code: string, state: string) =>
+    await apiRequest
+      .get('auth/callback', { searchParams: { code, state } })
+      .json<OAuthCallbackResponse>(),
 
   whoAmI: (isLoggedOut: boolean) =>
     useQuery(

src/contexts/AuthContext.test.ts

@@ -2,6 +2,7 @@
 import { type QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import { useLocation, useSearch } from '@tanstack/react-router'
 import { act, createElement, type ReactNode } from 'react'
+import { toast } from 'sonner'
 import { afterEach, describe, expect, it, vi } from 'vitest'
 import { api, apiRequest } from '@/api'
 import { Loader } from '@/components/ui/Loader'
@@ -43,6 +44,10 @@ vi.mock('@/components/ui/Loader', () => ({ Loader: vi.fn(() => null) }))
 
 vi.mock('@/lib/logger', () => ({ logger: { warn: vi.fn(), error: vi.fn() } }))
 
+vi.mock('@/i18n', () => ({ useIntl: () => ({ t: (_k: string, _v: unknown, d: string) => d }) }))
+
+vi.mock('sonner', () => ({ toast: { error: vi.fn() } }))
+
 describe('validateOAuthState', () => {
   afterEach(() => {
     clearOAuthState()
@@ -395,7 +400,7 @@ describe('useAuthContext', () => {
       await waitFor(() => {
         expect(localStorage.getItem('osm_token')).toBe('jwt-token')
       })
-      expect(api.user.callback).toHaveBeenCalledWith('the-code')
+      expect(api.user.callback).toHaveBeenCalledWith('the-code', 'matching-state')
       expect(api.user.refreshAuth).toHaveBeenCalled()
       expect(window.location.href).toContain('/dashboard')
       expect(getStoredRedirectUrl()).toBeNull()
@@ -476,9 +481,10 @@ describe('useAuthContext', () => {
       })
       expect(api.user.refreshAuth).toHaveBeenCalled()
       expect(logger.error).not.toHaveBeenCalledWith('OAuth callback error', expect.anything())
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it('logs the error and does not refresh auth when the callback fails for a non-security reason', async () => {
+    it('logs the error, tells the user, and does not refresh auth when the callback fails for a non-security reason', async () => {
       window.history.pushState({}, '', '/?code=the-code&state=matching-state')
       setOAuthState('matching-state')
       mockSearch({ code: 'the-code', state: 'matching-state' })
@@ -495,6 +501,11 @@ describe('useAuthContext', () => {
         error: expect.any(Error),
       })
       expect(api.user.refreshAuth).not.toHaveBeenCalled()
+      await waitFor(() => {
+        expect(toast.error).toHaveBeenCalledWith(
+          'Sign-in failed or expired. Please try signing in again.'
+        )
+      })
     })
   })
 })

src/contexts/AuthContext.tsx

@@ -1,8 +1,10 @@
 import { useQueryClient } from '@tanstack/react-query'
 import { useLocation, useSearch } from '@tanstack/react-router'
 import { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'
+import { toast } from 'sonner'
 import { api, apiRequest } from '@/api'
 import { Loader } from '@/components/ui/Loader'
+import { useIntl } from '@/i18n'
 import { logger } from '@/lib/logger'
 import type { OAuthLoginResponse } from '@/types/Oauth'
 import type { User } from '@/types/User'
@@ -78,6 +80,8 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => {
   const queryClient = useQueryClient()
   const { data: user, isLoading, error } = api.user.whoAmI(isLoggedOut)
   const [codeUsed, setCodeUsed] = useState<boolean>(false)
+  const [loginFailed, setLoginFailed] = useState(false)
+  const { t } = useIntl()
 
   const stripOAuthParamsFromUrl = (): void => {
     const url = new URL(window.location.href)
@@ -100,7 +104,7 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => {
     setIsVerifying(true)
 
     try {
-      const { token } = await api.user.callback(code)
+      const { token } = await api.user.callback(code, state)
 
       if (token) {
         localStorage.setItem('osm_token', token)
@@ -121,6 +125,7 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => {
         api.user.refreshAuth(queryClient)
       } else {
         logger.error('OAuth callback error', { error })
+        setLoginFailed(true)
       }
     } finally {
       stripOAuthParamsFromUrl()
@@ -145,6 +150,17 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => {
     }
   }, [codeUsed, search.code])
 
+  // The Toaster is rendered inside this provider, which shows only a Loader
+  // while loading or verifying, so the toast is deferred until it has mounted.
+  useEffect(() => {
+    if (loginFailed && !isLoading && !isVerifying) {
+      toast.error(
+        t('auth.loginFailed', {}, 'Sign-in failed or expired. Please try signing in again.')
+      )
+      setLoginFailed(false)
+    }
+  }, [loginFailed, isLoading, isVerifying, t])
+
   useEffect(() => {
     if (user && isLoggedOut) {
       setIsLoggedOut(false)

src/i18n/messages/en-US.json

@@ -70,6 +70,7 @@
   "appLayout.header.userMenu.theme": "Theme",
   "appLayout.header.userMenu.triggerSrOnly": "User menu",
   "appLayout.header.userMenu.viewPublicProfile": "View public profile",
+  "auth.loginFailed": "Sign-in failed or expired. Please try signing in again.",
   "browsedChallengePage.actionButtons.alreadyReported": "You have already reported this challenge; it is awaiting review",
   "browsedChallengePage.actionButtons.challengeActions": "Challenge actions",
   "browsedChallengePage.actionButtons.challengeReported": "Challenge Reported",