Pārlūkot izejas kodu

fix: deduplicate OIDC callback sequence to prevent StrictMode race

React StrictMode's double-invocation of useEffect caused the first run to
remove oidcStateStorageKey from sessionStorage before the async code exchange
began. The second run would see the missing state, throw, and display
"Authentication failed" — while the first run completed in the background
(storing tokens and replacing the URL), making a page refresh appear to work.

Introduces a module-scope Promise (pendingCallbackSequence) that covers the
full callback sequence: token exchange, session fetch, and URL navigation. The
second effect run shares the same Promise and awaits its result, so no re-
validation or re-fetching occurs, and only the non-cancelled run sets state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pull/15/head
Daniel Covington pirms 2 dienas
vecāks
revīzija
fabed40906
2 mainītis faili ar 84 papildinājumiem un 25 dzēšanām
  1. +8
    -0
      _bmad-output/implementation-artifacts/deferred-work.md
  2. +76
    -25
      campaign-tracker-client/src/auth/useOidcSession.ts

+ 8
- 0
_bmad-output/implementation-artifacts/deferred-work.md Parādīt failu

@@ -0,0 +1,8 @@
## Deferred from: fix-strictmode-oidc-callback-race (2026-05-06)

- `pendingCallbackSequence` is not scoped to a specific callback invocation — if `useOidcSession` were ever mounted twice simultaneously, the second instance would skip CSRF validation and piggyback on the first's exchange. Pre-existing architectural assumption; low risk given single-mount usage, but worth an assertion if the hook gains wider use.
- `user.workspacePath` is used in `window.history.replaceState` without validating it is a relative path. Server currently returns only hard-coded relative paths, but an open-redirect risk exists if the return value ever comes from user-controlled input.

## Deferred from: code review of 1-4-keycloak-role-mapping-application-authorization.md (2026-05-06)

- AuthorizationProbeController ships canned operational routes in the production controller surface. Evidence: Campaign_Tracker.Server/Controllers/AuthorizationProbeController.cs:8. Reason: deferred by user choice during review.

+ 76
- 25
campaign-tracker-client/src/auth/useOidcSession.ts Parādīt failu

@@ -1,13 +1,16 @@
import { useEffect, useMemo, useState } from 'react'
import {
buildKeycloakAuthorizationUrl,
decodeAuthenticatedUser,
clearStoredAuthTokenSet,
exchangeAuthorizationCode,
fetchAuthenticatedSession,
getAuthCallbackPath,
getKeycloakClientConfig,
isAuthCallbackPath,
isTokenRefreshRequired,
mergeRefreshedTokenSet,
oidcReturnPathStorageKey,
oidcStateStorageKey,
readStoredAuthTokenSet,
refreshAccessToken,
shouldRedirectToLogin,
@@ -16,6 +19,17 @@ import {
type AuthenticatedUser,
} from './authContracts'

// React StrictMode mounts effects twice (mount → cleanup → remount). The first run
// removes the OIDC state key from sessionStorage before the async exchange completes,
// so the second run sees a missing state and throws. This module-scope promise covers
// the full callback sequence (exchange + session fetch + navigate) so the second run
// simply awaits the result and applies it, instead of re-validating or re-fetching.
let pendingCallbackSequence: Promise<{
tokens: AuthTokenSet
user: AuthenticatedUser
returnPath: string
}> | null = null

type OidcSessionState =
| { status: 'checking'; user: null; tokens: null; error: null }
| { status: 'redirecting'; user: null; tokens: null; error: null }
@@ -42,37 +56,66 @@ export function useOidcSession(): OidcSessionState {

if (isAuthCallbackPath(currentUrl.pathname, callbackPath)) {
const code = currentUrl.searchParams.get('code')

if (!code) {
throw new Error('Missing Keycloak authorization code')
}

const tokens = await exchangeAuthorizationCode(config, code)
storeAuthTokenSet(tokens)
const user = decodeAuthenticatedUser(tokens.accessToken)
const returnPath =
window.sessionStorage.getItem(oidcReturnPathStorageKey) ?? user.workspacePath
window.sessionStorage.removeItem(oidcReturnPathStorageKey)
window.history.replaceState({}, document.title, returnPath)
if (!pendingCallbackSequence) {
const returnedState = currentUrl.searchParams.get('state')
const expectedState = window.sessionStorage.getItem(oidcStateStorageKey)

if (!returnedState || returnedState !== expectedState) {
throw new Error('Invalid Keycloak authorization state')
}

window.sessionStorage.removeItem(oidcStateStorageKey)
pendingCallbackSequence = (async () => {
const tokens = await exchangeAuthorizationCode(config, code)
storeAuthTokenSet(tokens)
const user = await fetchAuthenticatedSession(tokens.accessToken)
const returnPath = user.workspacePath
window.sessionStorage.removeItem(oidcReturnPathStorageKey)
window.history.replaceState({}, document.title, returnPath)
return { tokens, user, returnPath }
})()
}

const result = await pendingCallbackSequence
pendingCallbackSequence = null

if (!cancelled) {
setState({ status: 'authenticated', user, tokens, error: null })
setState({ status: 'authenticated', user: result.user, tokens: result.tokens, error: null })
}
return
}

if (storedTokens) {
const tokens = isTokenRefreshRequired(storedTokens)
? await refreshAccessToken(config, storedTokens.refreshToken)
: storedTokens
let tokens = storedTokens

if (isTokenRefreshRequired(storedTokens)) {
try {
tokens = mergeRefreshedTokenSet(
storedTokens,
await refreshAccessToken(config, storedTokens.refreshToken),
)
} catch {
clearStoredAuthTokenSet()
redirectToLogin()
return
}
}

if (tokens !== storedTokens) {
storeAuthTokenSet(tokens)
}

const user = await fetchAuthenticatedSession(tokens.accessToken)

if (!cancelled) {
setState({
status: 'authenticated',
user: decodeAuthenticatedUser(tokens.accessToken),
user,
tokens,
error: null,
})
@@ -81,20 +124,11 @@ export function useOidcSession(): OidcSessionState {
}

if (shouldRedirectToLogin(window.location.pathname, null, config)) {
window.sessionStorage.setItem(
oidcReturnPathStorageKey,
`${window.location.pathname}${window.location.search}`,
)
setState({ status: 'redirecting', user: null, tokens: null, error: null })
window.location.assign(
buildKeycloakAuthorizationUrl(
config,
crypto.randomUUID(),
crypto.randomUUID(),
),
)
redirectToLogin()
}
} catch {
pendingCallbackSequence = null
clearStoredAuthTokenSet()
if (!cancelled) {
setState({
status: 'error',
@@ -106,6 +140,23 @@ export function useOidcSession(): OidcSessionState {
}
}

function redirectToLogin() {
const state = crypto.randomUUID()
window.sessionStorage.setItem(
oidcReturnPathStorageKey,
`${window.location.pathname}${window.location.search}`,
)
window.sessionStorage.setItem(oidcStateStorageKey, state)
setState({ status: 'redirecting', user: null, tokens: null, error: null })
window.location.assign(
buildKeycloakAuthorizationUrl(
config,
state,
crypto.randomUUID(),
),
)
}

void syncSession()

return () => {


Notiek ielāde…
Atcelt
Saglabāt

Powered by TurnKey Linux.