fix(admin-ui): top-5 critical review bugs (auth + AJV + TZ + idempotency + storage)
This commit is contained in:
Александр Зимин
@@ -26,12 +26,22 @@ export function RequireAuth({ children }: { children: ReactNode }) {
|
||||
if (oidcError) return
|
||||
if (auth.isLoading || auth.activeNavigator) return
|
||||
if (auth.isAuthenticated) return
|
||||
// Critical guard: auth.error → signinRedirect уже упал (Keycloak недоступен,
|
||||
// OIDC discovery 404 etc). Без этого guard'а useEffect зациклится: каждый
|
||||
// failed redirect ставит auth.error → re-render → guard не срабатывает →
|
||||
// снова signinRedirect. С guard'ом UI показывает "Не удалось подключиться"
|
||||
// блок ниже (использует auth.error.message).
|
||||
if (auth.error) return
|
||||
auth.signinRedirect().catch((err) => {
|
||||
// Не уйти на signinRedirect = Keycloak недоступен. Логируем, дальше
|
||||
// покажем сообщение об ошибке (auth.error будет выставлен).
|
||||
console.error('[auth] signinRedirect failed', err)
|
||||
})
|
||||
}, [auth.isAuthenticated, auth.isLoading, auth.activeNavigator, oidcError])
|
||||
}, [
|
||||
auth.isAuthenticated,
|
||||
auth.isLoading,
|
||||
auth.activeNavigator,
|
||||
auth.error,
|
||||
oidcError,
|
||||
])
|
||||
|
||||
if (oidcError) {
|
||||
return (
|
||||
|
||||
@@ -1,55 +1,73 @@
|
||||
import { useEffect } from 'react'
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { useAuth } from 'react-oidc-context'
|
||||
import { apiClient } from '@/api/client'
|
||||
import { TOKEN_STORAGE_KEY } from './oidcConfig'
|
||||
import { setAccessToken, setUnauthorizedHandler } from '@/api/client'
|
||||
import { LEGACY_TOKEN_STORAGE_KEY } from './oidcConfig'
|
||||
|
||||
/**
|
||||
* Синхронизирует {@code access_token} из react-oidc-context в localStorage и
|
||||
* axios default header. Existing API код ничего не знает про OIDC — просто
|
||||
* читает токен через axios interceptor.
|
||||
* Синхронизирует {@code access_token} из react-oidc-context в API client
|
||||
* (in-memory holder) и подключает 401 handler на silent renew / redirect.
|
||||
*
|
||||
* <p>Mount внутри {@code <AuthProvider>}, рендерит null.
|
||||
* <p>Mount внутри {@code <AuthProvider>}, рендерит null. Должен быть один
|
||||
* экземпляр на app — handler ставится глобально.
|
||||
*
|
||||
* <p><b>Single source of truth для 401:</b> раньше был дубликат interceptor'а
|
||||
* в {@code api/client.ts} (просто удалял token из localStorage) + ещё один
|
||||
* здесь (signinSilent/Redirect). Оба срабатывали на одном response — race и
|
||||
* потенциальный redirect loop. Теперь handler один, через
|
||||
* {@link setUnauthorizedHandler}.
|
||||
*
|
||||
* <p><b>Loop guard:</b> {@code redirecting} ref предотвращает burst
|
||||
* параллельных signinRedirect'ов при N concurrent 401 (типичная картина
|
||||
* после истечения токена — все pending requests возвращают 401 одновременно).
|
||||
*/
|
||||
export function TokenSync() {
|
||||
const auth = useAuth()
|
||||
const redirecting = useRef(false)
|
||||
|
||||
// 1) Token sync — single update path к accessToken holder в client.ts.
|
||||
useEffect(() => {
|
||||
const token = auth.user?.access_token
|
||||
if (token) {
|
||||
localStorage.setItem(TOKEN_STORAGE_KEY, token)
|
||||
apiClient.defaults.headers.common['Authorization'] = `Bearer ${token}`
|
||||
} else {
|
||||
localStorage.removeItem(TOKEN_STORAGE_KEY)
|
||||
delete apiClient.defaults.headers.common['Authorization']
|
||||
setAccessToken(auth.user?.access_token ?? null)
|
||||
// Cleanup legacy localStorage key (was used до fix #12 + #2/3/13 cascade).
|
||||
// Идемпотентный no-op если ключа нет.
|
||||
if (typeof window !== 'undefined') {
|
||||
window.localStorage.removeItem(LEGACY_TOKEN_STORAGE_KEY)
|
||||
}
|
||||
}, [auth.user?.access_token])
|
||||
|
||||
// 401 → автоматический редирект на Keycloak login. Покрывает два случая:
|
||||
// 1. anonymous пользователь открыл UI (нет токена вовсе) — сразу на логин
|
||||
// вместо красного "AxiosError 401".
|
||||
// 2. authenticated юзер у которого истёк access_token и silent renew не
|
||||
// успел — пробуем silent, если не вышло — full redirect.
|
||||
// Guard против infinite loop: не редиректим пока auth.isLoading (initial OIDC
|
||||
// bootstrap), пока activeNavigator (signin/signout уже идёт), и если url
|
||||
// содержит ?error= (вернулись с провалом из Keycloak — даём UI показать ошибку).
|
||||
// 2) 401 handler — registered once на app mount, делегирует silent renew /
|
||||
// redirect на Keycloak. Guard'ы:
|
||||
// - auth.isLoading / activeNavigator: OIDC bootstrap или redirect уже в
|
||||
// progress — не дублируем.
|
||||
// - URL содержит ?error=: вернулись из Keycloak с провалом — пусть UI
|
||||
// покажет RequireAuth error screen, не зацикливаемся.
|
||||
// - redirecting ref: при burst 401 (N concurrent requests) первый
|
||||
// handler инициирует redirect, остальные no-op до его завершения.
|
||||
useEffect(() => {
|
||||
const id = apiClient.interceptors.response.use(
|
||||
(r) => r,
|
||||
(err) => {
|
||||
if (err.response?.status !== 401) return Promise.reject(err)
|
||||
if (auth.isLoading || auth.activeNavigator) return Promise.reject(err)
|
||||
if (typeof window !== 'undefined' && window.location.search.includes('error=')) {
|
||||
return Promise.reject(err)
|
||||
}
|
||||
if (auth.isAuthenticated) {
|
||||
auth.signinSilent().catch(() => auth.signinRedirect())
|
||||
} else {
|
||||
auth.signinRedirect()
|
||||
}
|
||||
return Promise.reject(err)
|
||||
},
|
||||
)
|
||||
return () => apiClient.interceptors.response.eject(id)
|
||||
setUnauthorizedHandler(() => {
|
||||
if (auth.isLoading || auth.activeNavigator) return
|
||||
if (typeof window !== 'undefined' && window.location.search.includes('error=')) {
|
||||
return
|
||||
}
|
||||
if (redirecting.current) return
|
||||
redirecting.current = true
|
||||
const reset = () => {
|
||||
redirecting.current = false
|
||||
}
|
||||
if (auth.isAuthenticated) {
|
||||
auth
|
||||
.signinSilent()
|
||||
.then(reset)
|
||||
.catch(() => {
|
||||
// Silent renew не удался — full redirect. reset не нужен:
|
||||
// page reload сбросит state. Но catch всё равно нужен на случай
|
||||
// если redirect promise отвергается (Keycloak unreachable).
|
||||
auth.signinRedirect().catch(reset)
|
||||
})
|
||||
} else {
|
||||
auth.signinRedirect().catch(reset)
|
||||
}
|
||||
})
|
||||
return () => setUnauthorizedHandler(null)
|
||||
}, [auth])
|
||||
|
||||
return null
|
||||
|
||||
@@ -17,9 +17,17 @@ const baseSettings: UserManagerSettings = {
|
||||
response_type: 'code',
|
||||
automaticSilentRenew: true,
|
||||
loadUserInfo: false,
|
||||
// sessionStorage вместо localStorage: уменьшает XSS persistent reach на
|
||||
// access_token + id_token. Trade-off: токен не переживает закрытие вкладки —
|
||||
// на повторном открытии будет SSO redirect к Keycloak (но без user input
|
||||
// если SSO session в Keycloak ещё жива, т.е. UX impact минимальный).
|
||||
// Долгосрочное правильное решение — BFF + httpOnly cookie с refresh token,
|
||||
// но это отдельный epic (server-side OIDC handler).
|
||||
// См. также {@code apiClient}: access_token держится в in-memory holder'е,
|
||||
// не читается из storage на каждый request.
|
||||
userStore:
|
||||
typeof window !== 'undefined'
|
||||
? new WebStorageStateStore({ store: window.localStorage })
|
||||
? new WebStorageStateStore({ store: window.sessionStorage })
|
||||
: undefined,
|
||||
}
|
||||
|
||||
@@ -38,4 +46,11 @@ export const oidcAuthConfig: AuthProviderProps = {
|
||||
},
|
||||
}
|
||||
|
||||
export const TOKEN_STORAGE_KEY = 'ordinis.token'
|
||||
// TOKEN_STORAGE_KEY (legacy 'ordinis.token' в localStorage) удалён вместе с
|
||||
// fix дублирующих 401-interceptor'ов. Access token больше не дублируется
|
||||
// в localStorage из приложения — он живёт в memory holder'е (см. apiClient
|
||||
// setAccessToken) и oidc-client-ts userStore (sessionStorage, fix #12).
|
||||
//
|
||||
// Cleanup: на первый mount old клиенты могут иметь старый ключ — TokenSync
|
||||
// его удаляет.
|
||||
export const LEGACY_TOKEN_STORAGE_KEY = 'ordinis.token'
|
||||
|
||||
Reference in New Issue
Block a user