From 5d1a8f42987b8c242490613a5579e218ed3a607c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20=C5=9Al=C4=99zak?= Date: Fri, 30 Oct 2026 13:56:44 +0100 Subject: [PATCH] tray locationcard error view unified --- .../components/LocationCard/LocationCard.tsx | 4 +- .../LocationCard/api/cardError.test.ts | 122 ++++++++++++++++++ .../components/LocationCard/api/cardError.ts | 81 ++++++++++++ .../components/LocationCardConnectButton.tsx | 22 +++- .../LocationCard/context/context.tsx | 107 ++++++++++----- .../components/LocationCard/context/types.ts | 10 +- .../LocationCard/hooks/useMfaConnect.test.ts | 48 ++++++- .../LocationCard/hooks/useMfaConnect.ts | 19 ++- .../hooks/useMfaFido2Connect.test.ts | 54 ++++++++ .../LocationCard/hooks/useMfaFido2Connect.ts | 33 +++-- .../hooks/useMfaMobileConnect.test.ts | 58 +++++++++ .../LocationCard/hooks/useMfaMobileConnect.ts | 36 +++++- .../hooks/useMfaOidcConnect.test.ts | 19 ++- .../LocationCard/hooks/useMfaOidcConnect.ts | 48 ++----- .../LocationCardConnectionErrorView.test.tsx | 59 --------- .../LocationCardConnectionErrorView.tsx | 34 ----- .../LocationCardErrorView.test.tsx | 72 +++++++++++ .../LocationCardErrorView.tsx | 47 +++++++ .../style.scss | 13 +- .../LocationCardMfaEmailView.tsx | 18 +-- .../LocationCardMfaFido2View.tsx | 13 +- .../LocationCardMfaMobileView.tsx | 46 ++----- .../LocationCardMfaMobileView/style.scss | 6 - .../LocationCardMfaOidcView.tsx | 60 ++------- .../views/LocationCardMfaOidcView/style.scss | 6 - .../LocationCardMfaTotpView.tsx | 18 +-- new-ui/src/shared/rust-api/mfaError.ts | 37 ++++-- 27 files changed, 754 insertions(+), 336 deletions(-) create mode 100644 new-ui/src/shared/components/LocationCard/api/cardError.test.ts create mode 100644 new-ui/src/shared/components/LocationCard/api/cardError.ts delete mode 100644 new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.test.tsx delete mode 100644 new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.tsx create mode 100644 new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.test.tsx create mode 100644 new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.tsx rename new-ui/src/shared/components/LocationCard/views/{LocationCardConnectionErrorView => LocationCardErrorView}/style.scss (63%) diff --git a/new-ui/src/shared/components/LocationCard/LocationCard.tsx b/new-ui/src/shared/components/LocationCard/LocationCard.tsx index 9d5b590e5..0780b33a3 100644 --- a/new-ui/src/shared/components/LocationCard/LocationCard.tsx +++ b/new-ui/src/shared/components/LocationCard/LocationCard.tsx @@ -13,7 +13,7 @@ import { LocationCardProvider, useLocationCardContext } from './context/context' import { LocationCardViews, type LocationCardViewsValue } from './context/types'; import { ConnectedView } from './views/ConnectedView/ConnectedView'; import { DefaultView } from './views/DefaultView/DefaultView'; -import { LocationCardConnectionErrorView } from './views/LocationCardConnectionErrorView/LocationCardConnectionErrorView'; +import { LocationCardErrorView } from './views/LocationCardErrorView/LocationCardErrorView'; import { LocationCardMfaEmailView } from './views/LocationCardMfaEmailView/LocationCardMfaEmailView'; import { LocationCardMfaFido2View } from './views/LocationCardMfaFido2View/LocationCardMfaFido2View'; import { LocationCardMfaMobileView } from './views/LocationCardMfaMobileView/LocationCardMfaMobileView'; @@ -41,7 +41,7 @@ const views: Record = { [LocationCardViews.Connecting]: null, [LocationCardViews.Connected]: , [LocationCardViews.PostureCheckFail]: , - [LocationCardViews.ConnectionError]: , + [LocationCardViews.Error]: , }; interface InnerProps { diff --git a/new-ui/src/shared/components/LocationCard/api/cardError.test.ts b/new-ui/src/shared/components/LocationCard/api/cardError.test.ts new file mode 100644 index 000000000..17f5e56e0 --- /dev/null +++ b/new-ui/src/shared/components/LocationCard/api/cardError.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it, vi } from 'vitest'; +import { MfaFailureText } from '../../../rust-api/mfaError'; +import type { LocationInfo } from '../../../rust-api/types'; +import { IconKind } from '../../Icon'; +import { connectErrorToCard, MfaRetry, mfaErrorToCard } from './cardError'; +import { DEFAULT_CONNECTION_ERROR } from './connectError'; + +vi.mock('./connectError', () => ({ + DEFAULT_CONNECTION_ERROR: 'Service unavailable', +})); + +const mfaError = (type: string, message?: string) => JSON.stringify({ type, message }); + +describe('mfaErrorToCard', () => { + it.each([ + [ + 'service unavailable', + mfaError('network_error', 'unreachable'), + DEFAULT_CONNECTION_ERROR, + IconKind.ServiceUnavailable, + MfaRetry.ResumeStep, + ], + [ + 'timeout', + mfaError('timeout'), + MfaFailureText.Timeout, + IconKind.WarningOutlined, + MfaRetry.ResumeStep, + ], + [ + 'outdated mobile app', + mfaError('mobile_client_outdated'), + 'Your Defguard mobile app is out of date.\nUpgrade to approve this connection.', + IconKind.DisableMfa, + null, + ], + [ + 'attempt limit', + mfaError('attempt_limit', 'Too many failed MFA attempts'), + 'Too many failed MFA attempts', + IconKind.WarningOutlined, + null, + ], + [ + 'mobile rejection', + mfaError('mfa_rejected', 'Mobile approval rejected'), + 'Mobile approval rejected', + IconKind.WarningOutlined, + null, + ], + [ + 'blocked security key', + mfaError( + 'mfa_rejected', + 'Security key rejected the PIN: the security key is locked after too many PIN attempts', + ), + 'Security key rejected the PIN: the security key is locked after too many PIN attempts', + IconKind.WarningOutlined, + null, + ], + [ + 'stale attempt', + mfaError('mfa_rejected', 'stale MFA attempt'), + MfaFailureText.StaleAttempt, + IconKind.WarningOutlined, + MfaRetry.RestartMfa, + ], + [ + 'session expired', + mfaError('other', 'login session not found'), + MfaFailureText.SessionExpired, + IconKind.WarningOutlined, + MfaRetry.RestartMfa, + ], + [ + 'VPN failure after MFA', + 'VPN connection failed: interface error', + MfaFailureText.ConnectFailure, + IconKind.WarningOutlined, + MfaRetry.RestartMfa, + ], + [ + 'worded server error', + mfaError('mfa_rejected', 'No security key detected'), + 'No security key detected', + IconKind.WarningOutlined, + MfaRetry.RestartMfa, + ], + [ + 'unstructured error', + 'Instance not found', + MfaFailureText.Unknown, + IconKind.WarningOutlined, + MfaRetry.RestartMfa, + ], + ])('maps %s', (_, err, message, icon, retry) => { + expect(mfaErrorToCard(err)).toEqual({ message, icon, retry }); + }); +}); + +describe('connectErrorToCard', () => { + const location = { name: 'Office' } as LocationInfo; + + it('uses the service error for an unavailable service', () => { + expect( + connectErrorToCard( + { kind: 'serviceUnavailable', message: 'down', conflicts: [] }, + location, + ), + ).toEqual({ message: DEFAULT_CONNECTION_ERROR, icon: IconKind.ServiceUnavailable }); + }); + + it.each([ + ['other', { kind: 'other' as const, message: 'daemon error', conflicts: [] }], + ['unparseable', null], + ])('names the location for an %s error', (_, connectError) => { + expect(connectErrorToCard(connectError, location)).toEqual({ + message: 'Failed to connect to Office. Please try again.', + icon: IconKind.WarningOutlined, + }); + }); +}); diff --git a/new-ui/src/shared/components/LocationCard/api/cardError.ts b/new-ui/src/shared/components/LocationCard/api/cardError.ts new file mode 100644 index 000000000..c72cd237e --- /dev/null +++ b/new-ui/src/shared/components/LocationCard/api/cardError.ts @@ -0,0 +1,81 @@ +import { + isAttemptLimit, + isConnectFailure, + isMobileApprovalRejected, + isMobileClientOutdated, + isSecurityKeyPinBlocked, + isServiceUnavailable, + isSessionExpired, + isStaleAttempt, + isTimeout, + MfaFailureText, + mfaErrorMessage, + parseMfaError, +} from '../../../rust-api/mfaError'; +import type { LocationInfo } from '../../../rust-api/types'; +import { IconKind } from '../../Icon'; +import type { LocationCardError } from '../context/types'; +import { type ConnectError, DEFAULT_CONNECTION_ERROR } from './connectError'; + +export const MfaRetry = { + // the hooks keep the session token for these errors, so the step can resume + ResumeStep: 'resume-step', + RestartMfa: 'restart-mfa', +} as const; + +export type MfaRetryValue = (typeof MfaRetry)[keyof typeof MfaRetry]; + +export type MfaFailure = Omit & { + retry: MfaRetryValue | null; +}; + +export const mfaErrorToCard = (err: unknown): MfaFailure => { + if (isServiceUnavailable(err)) { + return { + message: DEFAULT_CONNECTION_ERROR, + icon: IconKind.ServiceUnavailable, + retry: MfaRetry.ResumeStep, + }; + } + if (isTimeout(err)) { + return { + message: MfaFailureText.Timeout, + icon: IconKind.WarningOutlined, + retry: MfaRetry.ResumeStep, + }; + } + const message = mfaErrorMessage(err); + if (isMobileClientOutdated(err)) { + return { message, icon: IconKind.DisableMfa, retry: null }; + } + if ( + isAttemptLimit(err) || + isMobileApprovalRejected(message) || + isSecurityKeyPinBlocked(message) + ) { + return { message, icon: IconKind.WarningOutlined, retry: null }; + } + const restart = (text: string): MfaFailure => ({ + message: text, + icon: IconKind.WarningOutlined, + retry: MfaRetry.RestartMfa, + }); + if (isStaleAttempt(message)) return restart(MfaFailureText.StaleAttempt); + if (isSessionExpired(message)) return restart(MfaFailureText.SessionExpired); + if (isConnectFailure(message)) return restart(MfaFailureText.ConnectFailure); + // only structured errors carry a message worded for the user + return restart(parseMfaError(err)?.message ?? MfaFailureText.Unknown); +}; + +export const connectErrorToCard = ( + connectError: ConnectError | null, + location: LocationInfo, +): LocationCardError => { + if (connectError?.kind === 'serviceUnavailable') { + return { message: DEFAULT_CONNECTION_ERROR, icon: IconKind.ServiceUnavailable }; + } + return { + message: `Failed to connect to ${location.name}. Please try again.`, + icon: IconKind.WarningOutlined, + }; +}; diff --git a/new-ui/src/shared/components/LocationCard/components/LocationCardConnectButton.tsx b/new-ui/src/shared/components/LocationCard/components/LocationCardConnectButton.tsx index df71004f7..b86c12960 100644 --- a/new-ui/src/shared/components/LocationCard/components/LocationCardConnectButton.tsx +++ b/new-ui/src/shared/components/LocationCard/components/LocationCardConnectButton.tsx @@ -1,9 +1,11 @@ import { useMutation } from '@tanstack/react-query'; +import { error as logError } from '@tauri-apps/plugin-log'; import { useConfigureFactorsScreen } from '../../../hooks/useConfigureFactorsScreen'; import { api } from '../../../rust-api/api'; import { connectConfigureFactorsSource } from '../../../utils/configureFactorsSource'; import { ConnectionAbility, shouldStartMfa } from '../../../utils/mfa'; import { IconKind } from '../../Icon'; +import { connectErrorToCard } from '../api/cardError'; import { openRouteConflictInFullView, parseConnectError } from '../api/connectError'; import { useLocationCardContext } from '../context/context'; import { LocationCardViews } from '../context/types'; @@ -11,7 +13,7 @@ import { ConnectButton } from './ConnectButton/ConnectButton'; import { connectBlockedTooltip } from './ConnectButton/consts'; export const LocationCardConnectButton = () => { - const { location, connectionAbility, setPostureError, setView, startMfa } = + const { location, connectionAbility, setPostureError, setView, showError, startMfa } = useLocationCardContext(); const canConfigureMfa = connectionAbility === ConnectionAbility.Configurable; @@ -23,7 +25,7 @@ export const LocationCardConnectButton = () => { onSuccess: () => { setView(LocationCardViews.Connected); }, - onError: (err) => { + onError: (err, variables) => { const connectError = parseConnectError(err); if ( @@ -34,8 +36,12 @@ export const LocationCardConnectButton = () => { setView(LocationCardViews.PostureCheckFail); } else if (connectError?.kind === 'routeConflict') { void openRouteConflictInFullView({ location, conflicts: connectError.conflicts }); - } else if (connectError?.kind === 'serviceUnavailable') { - setView(LocationCardViews.ConnectionError); + } else { + void logError(`Failed to connect to location ${location.id}: ${err}`); + showError({ + ...connectErrorToCard(connectError, location), + onRetry: () => connect(variables), + }); } }, meta: { @@ -51,6 +57,14 @@ export const LocationCardConnectButton = () => { onSuccess: () => { setView(LocationCardViews.Default); }, + onError: (err, variables) => { + void logError(`Failed to disconnect from location ${location.id}: ${err}`); + showError({ + message: `Failed to disconnect from ${location.name}. Please try again.`, + icon: IconKind.WarningOutlined, + onRetry: () => disconnect(variables), + }); + }, meta: { invalidate: ['locations'], }, diff --git a/new-ui/src/shared/components/LocationCard/context/context.tsx b/new-ui/src/shared/components/LocationCard/context/context.tsx index 7436211ca..c8be655d1 100644 --- a/new-ui/src/shared/components/LocationCard/context/context.tsx +++ b/new-ui/src/shared/components/LocationCard/context/context.tsx @@ -1,3 +1,4 @@ +import { error as logError } from '@tauri-apps/plugin-log'; import { createContext, type ReactNode, @@ -11,6 +12,7 @@ import { import { useConnectionAbility } from '../../../hooks/useConnectionAbility'; import { useAppData } from '../../../providers/AppDataContext'; import { api } from '../../../rust-api/api'; +import { isMfaPostureError, mfaErrorMessage } from '../../../rust-api/mfaError'; import type { InstanceInfo, LocationInfo, MfaStep } from '../../../rust-api/types'; import { ConnectionType, MfaMethod, type MfaMethodValue } from '../../../rust-api/types'; import { useAppStore } from '../../../store/useAppStore'; @@ -24,9 +26,10 @@ import { usableMfaMethods, } from '../../../utils/mfa'; import { IconKind } from '../../Icon'; -import type { IconKindValue } from '../../Icon/icon-types'; +import { MfaRetry, type MfaRetryValue, mfaErrorToCard } from '../api/cardError'; import { findRouteConflicts, openRouteConflictInFullView } from '../api/connectError'; import { + type LocationCardError, LocationCardViews, type LocationCardViewsValue, mfaMethodToLocationCardView, @@ -38,8 +41,7 @@ interface LocationCardContextValue { currentView: LocationCardViewsValue; previousView: LocationCardViewsValue | null; postureError: string | null; - connectionError: string | null; - connectionErrorIcon: IconKindValue | null; + cardError: LocationCardError | null; autoConnectOpenid: boolean; mfaMethod: MfaMethodValue; /** Whether the MFA flow can be passed as is, after configuring factors, or not at @@ -54,12 +56,11 @@ interface LocationCardContextValue { setMfaToken: (token: string | null) => void; goToStep: (stepIndex: number) => void; setStepPlanOnce: (plan: MfaMethodValue[]) => void; - setView: ( - view: LocationCardViewsValue, - connectionError?: string, - icon?: IconKindValue | null, - ) => void; + setView: (view: LocationCardViewsValue) => void; setPostureError: (error: string | null) => void; + showError: (error: LocationCardError) => void; + /** Routes a failed MFA attempt to the posture view or the error template. */ + showFailure: (err: unknown) => void; startMfa: () => void; } @@ -90,10 +91,7 @@ export const LocationCardProvider = ({ const [autoConnectOpenid, setAutoConnectOpenid] = useState(false); const [previousView, setPreviousView] = useState(null); const [postureError, setPostureError] = useState(null); - const [connectionError, setConnectionError] = useState(null); - const [connectionErrorIcon, setConnectionErrorIcon] = useState( - null, - ); + const [cardError, setCardError] = useState(null); const [currentView, setCurrentView] = useState( location.active ? LocationCardViews.Connected : LocationCardViews.Default, ); @@ -130,19 +128,22 @@ export const LocationCardProvider = ({ }, [location.active]); const setView = useCallback( - ( - view: LocationCardViewsValue, - connectionError?: string, - icon: IconKindValue | null = IconKind.ServiceUnavailable, - ) => { + (view: LocationCardViewsValue) => { setPreviousView(currentView); setCurrentView(view); - setConnectionError(connectionError ?? null); - setConnectionErrorIcon(view === LocationCardViews.ConnectionError ? icon : null); + if (view !== LocationCardViews.Error) setCardError(null); }, [currentView], ); + const showError = useCallback( + (error: LocationCardError) => { + setCardError(error); + setView(LocationCardViews.Error); + }, + [setView], + ); + const goToStep = useCallback( (nextStepIndex: number) => { setStepIndex(nextStepIndex); @@ -152,21 +153,56 @@ export const LocationCardProvider = ({ ); const startMfa = useCallback(async () => { - const conflicts = await findRouteConflicts(location); - if (conflicts.length > 0) { - await openRouteConflictInFullView({ location, conflicts }); - return; + try { + const conflicts = await findRouteConflicts(location); + if (conflicts.length > 0) { + await openRouteConflictInFullView({ location, conflicts }); + return; + } + mfaStarted.current = true; + const appConfig = await api.getAppConfig(); + setAutoConnectOpenid(appConfig.auto_start_openid_mfa); + const defaultPlan = resolveMfaStepPlan(location, [], instance); + setStepPlanOnce([]); + setStepIndex(0); + setMfaToken(null); + setMfaMethod(defaultPlan[0]); + setView(mfaMethodToLocationCardView(defaultPlan[0])); + } catch (err) { + void logError(`Failed to start MFA for location ${location.id}: ${err}`); + showError({ + message: 'Could not start authentication. Please try again.', + icon: IconKind.WarningOutlined, + }); } - mfaStarted.current = true; - const appConfig = await api.getAppConfig(); - setAutoConnectOpenid(appConfig.auto_start_openid_mfa); - const defaultPlan = resolveMfaStepPlan(location, [], instance); - setStepPlanOnce([]); - setStepIndex(0); - setMfaToken(null); - setMfaMethod(defaultPlan[0]); - setView(mfaMethodToLocationCardView(defaultPlan[0])); - }, [setView, location, instance]); + }, [setView, showError, location, instance]); + + const retryHandler = useCallback( + (retry: MfaRetryValue | null) => { + switch (retry) { + case MfaRetry.ResumeStep: + return () => setView(mfaMethodToLocationCardView(stepPlan[stepIndex])); + case MfaRetry.RestartMfa: + return () => void startMfa(); + default: + return undefined; + } + }, + [setView, startMfa, stepPlan, stepIndex], + ); + + const showFailure = useCallback( + (err: unknown) => { + if (isMfaPostureError(err, location)) { + setPostureError(mfaErrorMessage(err)); + setView(LocationCardViews.PostureCheckFail); + return; + } + const { retry, ...error } = mfaErrorToCard(err); + showError({ ...error, onRetry: retryHandler(retry) }); + }, + [location, setView, showError, retryHandler], + ); const mfaAutoStartRequested = useAppStore( (s) => s.mfaAutoStartLocationId === location.id, @@ -219,8 +255,7 @@ export const LocationCardProvider = ({ currentView, previousView, postureError, - connectionError, - connectionErrorIcon, + cardError, autoConnectOpenid, location, instance, @@ -233,6 +268,8 @@ export const LocationCardProvider = ({ mfaToken, setView, setPostureError, + showError, + showFailure, startMfa, setMfaMethod, setMfaToken, diff --git a/new-ui/src/shared/components/LocationCard/context/types.ts b/new-ui/src/shared/components/LocationCard/context/types.ts index 477ecf72f..212da93bf 100644 --- a/new-ui/src/shared/components/LocationCard/context/types.ts +++ b/new-ui/src/shared/components/LocationCard/context/types.ts @@ -1,4 +1,12 @@ import { MfaMethod, type MfaMethodValue } from '../../../rust-api/types'; +import type { IconKindValue } from '../../Icon/icon-types'; + +export type LocationCardError = { + message: string; + icon: IconKindValue; + // absent when retrying cannot help, the view then offers only "Got it" + onRetry?: () => void; +}; export const LocationCardViews = { Default: 'default', @@ -11,7 +19,7 @@ export const LocationCardViews = { Connecting: 'connecting', Connected: 'connected', PostureCheckFail: 'posture-check-fail', - ConnectionError: 'connection-error', + Error: 'error', } as const; export type LocationCardViewsValue = diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.test.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.test.ts index 41ce55820..cf05785b7 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.test.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.test.ts @@ -30,13 +30,17 @@ const location = { posture_check_required: false, } as LocationInfo; -const renderCodeMfa = (setMfaToken: (token: string | null) => void) => +const renderCodeMfa = ( + setMfaToken: (token: string | null) => void, + onFailure?: (err: unknown) => void, +) => renderHook(() => useMfaConnect(location, MfaMethod.Totp, { stepPlan: [MfaMethod.Totp], mfaToken: null, setMfaToken, onStepAdvanced: vi.fn(), + onFailure, }), ); @@ -74,4 +78,46 @@ describe('useMfaConnect', () => { expect(result.current.token).toBeNull(); expect(setMfaToken).toHaveBeenCalledWith(null); }); + + describe('with onFailure', () => { + it('keeps an invalid code on the input', async () => { + mocks.mfaFinishCode.mockRejectedValue( + JSON.stringify({ type: 'mfa_rejected', message: 'invalid code' }), + ); + const onFailure = vi.fn(); + const { result } = renderCodeMfa(vi.fn(), onFailure); + await waitFor(() => expect(result.current.token).toBe('session-token')); + + await act(async () => result.current.verifyCode('123456')); + + expect(result.current.verifyError).toBe('Invalid code'); + expect(onFailure).not.toHaveBeenCalled(); + }); + + it.each([ + ['attempt limit', JSON.stringify({ type: 'attempt_limit', message: 'Too many' })], + ['session expired', JSON.stringify({ type: 'other', message: 'invalid token' })], + ['VPN failure', 'VPN connection failed: interface error'], + ])('reports %s instead of showing it on the input', async (_, err) => { + mocks.mfaFinishCode.mockRejectedValue(err); + const onFailure = vi.fn(); + const { result } = renderCodeMfa(vi.fn(), onFailure); + await waitFor(() => expect(result.current.token).toBe('session-token')); + + await act(async () => result.current.verifyCode('123456')); + + expect(onFailure).toHaveBeenCalledWith(err); + expect(result.current.verifyError).toBeNull(); + }); + + it('reports a failed start instead of exposing startError', async () => { + const err = JSON.stringify({ type: 'network_error', message: 'unreachable' }); + mocks.mfaBeginStep.mockRejectedValue(err); + const onFailure = vi.fn(); + const { result } = renderCodeMfa(vi.fn(), onFailure); + + await waitFor(() => expect(onFailure).toHaveBeenCalledWith(err)); + expect(result.current.startError).toBeNull(); + }); + }); }); diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.ts index 103e8c0d7..8f8f5404d 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaConnect.ts @@ -11,6 +11,7 @@ import { isSessionExpired, isStaleAttempt, isTimeout, + MfaFailureText, mfaErrorMessage, } from '../../../rust-api/mfaError'; import { getInstancesQueryOptions } from '../../../rust-api/query'; @@ -29,6 +30,8 @@ type UseMfaConnectOptions = { onSessionExpired?: () => void; onPostureError?: (message: string) => void; onServiceUnavailable?: () => void; + // replaces the other error callbacks and all non-input error state + onFailure?: (err: unknown) => void; }; const waitForMinimumDuration = async (startedAt: number, minimumMs: number) => { @@ -51,6 +54,7 @@ export const useMfaConnect = ( onSessionExpired, onPostureError, onServiceUnavailable, + onFailure, }: UseMfaConnectOptions, ) => { const [token, setToken] = useState(mfaToken); @@ -97,6 +101,10 @@ export const useMfaConnect = ( setToken(null); setMfaToken(null); } + if (isPresent(onFailure)) { + onFailure(err); + return; + } if (isMfaPostureError(err, location)) { onPostureError?.(mfaErrorMessage(err)); return; @@ -140,14 +148,14 @@ export const useMfaConnect = ( setToken(null); setMfaToken(null); } - if (isAttemptLimit(err)) { + if (isPresent(onFailure) && !isInvalidCode(message)) { + onFailure(err); + } else if (isAttemptLimit(err)) { setVerifyError(message); } else if (isStaleAttempt(message)) { - setVerifyError( - 'Authentication request could not be started. Please try again.', - ); + setVerifyError(MfaFailureText.StaleAttempt); } else if (isConnectFailure(message)) { - setVerifyError('Failed to establish VPN connection'); + setVerifyError(MfaFailureText.ConnectFailure); } else if (isInvalidCode(message)) { setVerifyError('Invalid code'); } else if (isSessionExpired(message)) { @@ -169,6 +177,7 @@ export const useMfaConnect = ( onStepAdvanced, onConnected, onSessionExpired, + onFailure, setMfaToken, ], ); diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.test.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.test.ts index 7c34965d7..67a8d08f0 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.test.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.test.ts @@ -186,4 +186,58 @@ describe('useMfaFido2Connect', () => { expect(onConnected).not.toHaveBeenCalled(); expect(result.current.isVerifying).toBe(false); }); + + describe('with onFailure', () => { + const emitError = async (pin: string | null, err: string) => { + const onFailure = vi.fn(); + const { result } = renderHook(() => + useMfaFido2Connect(location, { + stepPlan: ['fido2'], + mfaToken: null, + setMfaToken: mocks.setMfaToken, + onFailure, + }), + ); + await act(async () => result.current.verify(pin)); + const errorListener = mocks.listen.mock.calls.find( + ([event]) => event === TauriEvent.MfaFido2Error, + )?.[1] as (event: { payload: { error: string } }) => void; + await act(async () => { + errorListener({ payload: { error: err } }); + }); + return { onFailure, result }; + }; + + const pinRejected = JSON.stringify({ + type: 'mfa_rejected', + message: 'Security key rejected the PIN: the security key rejected the PIN', + }); + + it('keeps a rejected PIN on the PIN input', async () => { + const { onFailure, result } = await emitError('1234', pinRejected); + + expect(onFailure).not.toHaveBeenCalled(); + expect(result.current.verifyError).toBe( + 'Security key rejected the PIN: the security key rejected the PIN', + ); + }); + + it('reports a rejected PIN when the platform collected it', async () => { + const { onFailure, result } = await emitError(null, pinRejected); + + expect(onFailure).toHaveBeenCalledWith(pinRejected); + expect(result.current.verifyError).toBeNull(); + }); + + it('reports a key error that the PIN input cannot fix', async () => { + const err = JSON.stringify({ + type: 'mfa_rejected', + message: 'No security key detected', + }); + const { onFailure, result } = await emitError('1234', err); + + expect(onFailure).toHaveBeenCalledWith(err); + expect(result.current.verifyError).toBeNull(); + }); + }); }); diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.ts index 225fbd068..eb4edb53b 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaFido2Connect.ts @@ -8,8 +8,10 @@ import { isCancelled, isConnectFailure, isMfaPostureError, + isSecurityKeyPinRejected, isServiceUnavailable, isTimeout, + MfaFailureText, mfaErrorMessage, } from '../../../rust-api/mfaError'; import type { @@ -19,6 +21,7 @@ import type { MfaMethodValue, } from '../../../rust-api/types'; import { TauriEvent } from '../../../rust-api/types'; +import { isPresent } from '../../../utils/isPresent'; type Options = { stepPlan: MfaMethodValue[]; @@ -28,6 +31,8 @@ type Options = { onStepAdvanced?: (nextStepIndex: number) => void; onPostureError?: (message: string) => void; onServiceUnavailable?: () => void; + // replaces the other error callbacks and all non-input error state + onFailure?: (err: unknown) => void; }; /** @@ -46,6 +51,7 @@ export const useMfaFido2Connect = ( onStepAdvanced, onPostureError, onServiceUnavailable, + onFailure, }: Options, ) => { const [isVerifying, setIsVerifying] = useState(false); @@ -75,7 +81,6 @@ export const useMfaFido2Connect = ( setIsAwaitingTouch(false); setVerifyError(null); - // Finish this attempt and update the view. const tryFinishAttempt = () => { if (!attempt.tryFinish()) return false; taskOutstandingRef.current = false; @@ -84,6 +89,14 @@ export const useMfaFido2Connect = ( return true; }; + const fail = (err: unknown) => { + if (isPresent(onFailure)) { + onFailure(err); + } else { + setVerifyError(mfaErrorMessage(err)); + } + }; + // Attach listeners before starting in case the task fails immediately. try { await Promise.all([ @@ -120,6 +133,12 @@ export const useMfaFido2Connect = ( void error( `FIDO2 MFA failed for location ${location.id}: ${event.payload.error}`, ); + const message = mfaErrorMessage(event.payload.error); + const pinRejected = pin !== null && isSecurityKeyPinRejected(message); + if (isPresent(onFailure) && !pinRejected) { + onFailure(event.payload.error); + return; + } if (isMfaPostureError(event.payload.error, location)) { onPostureError?.(mfaErrorMessage(event.payload.error)); @@ -129,12 +148,9 @@ export const useMfaFido2Connect = ( onServiceUnavailable?.(); return; } - const message = mfaErrorMessage(event.payload.error); - // Show the server error; it tells the user whether the key, PIN, or touch failed. + // the server error tells the user whether the key, PIN, or touch failed setVerifyError( - isConnectFailure(message) - ? 'Failed to establish VPN connection' - : message, + isConnectFailure(message) ? MfaFailureText.ConnectFailure : message, ); }), ), @@ -142,7 +158,7 @@ export const useMfaFido2Connect = ( } catch (err) { if (!tryFinishAttempt()) return; void error(`FIDO2 MFA listener setup failed for location ${location.id}: ${err}`); - setVerifyError(mfaErrorMessage(err)); + fail(err); return; } @@ -162,7 +178,7 @@ export const useMfaFido2Connect = ( } catch (err) { if (!tryFinishAttempt()) return; void error(`FIDO2 MFA start failed for location ${location.id}: ${err}`); - setVerifyError(mfaErrorMessage(err)); + fail(err); } }, [ @@ -175,6 +191,7 @@ export const useMfaFido2Connect = ( onStepAdvanced, onPostureError, onServiceUnavailable, + onFailure, ], ); diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.test.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.test.ts index 72f958b15..7d94f9f7e 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.test.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.test.ts @@ -455,4 +455,62 @@ describe('useMfaMobileConnect', () => { const parkedTokens = mocks.mfaConnectMobileApprove.mock.calls.map((call) => call[2]); expect(parkedTokens).toEqual(['token-1', 'token-2']); }); + + describe('with onFailure', () => { + const renderWithFailure = (onFailure: (err: unknown) => void) => + renderHook(() => + useMfaMobileConnect(location, { + stepPlan: [], + mfaToken: null, + setMfaToken: mocks.setMfaToken, + onStepAdvanced: vi.fn(), + onConnectionError: mocks.onConnectionError, + onFailure, + }), + ); + + it('reports an approval error instead of keeping it in the hook', async () => { + mocks.mfaBeginStep.mockResolvedValue({ + challenge: 'challenge-1', + token: 'token-1', + }); + mocks.mfaConnectMobileApprove.mockResolvedValue('task-1'); + const onFailure = vi.fn(); + const { result } = renderWithFailure(onFailure); + + await act(async () => { + await result.current.start(); + }); + await waitFor(() => expect(mocks.listen).toHaveBeenCalledTimes(3)); + const errorListener = mocks.listen.mock.calls.find( + ([event]) => event === TauriEvent.MfaMobileError, + )?.[1] as (event: { payload: { error: string } }) => void; + const err = JSON.stringify({ type: 'mobile_client_outdated' }); + + await act(async () => { + errorListener({ payload: { error: err } }); + }); + + expect(onFailure).toHaveBeenCalledWith(err); + expect(mocks.onConnectionError).not.toHaveBeenCalled(); + expect(result.current.connectionError).toBeNull(); + }); + + it('reports a posture rejection on start', async () => { + const err = JSON.stringify({ + type: 'posture_rejected', + message: 'no disk encryption', + }); + mocks.mfaBeginStep.mockRejectedValue(err); + const onFailure = vi.fn(); + const { result } = renderWithFailure(onFailure); + + await act(async () => { + await result.current.start(); + }); + + expect(onFailure).toHaveBeenCalledWith(err); + expect(result.current.startError).toBeNull(); + }); + }); }); diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.ts index 04555b0f1..fd0a6cf0a 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.ts @@ -8,10 +8,10 @@ import { api } from '../../../rust-api/api'; import { isConnectFailure, isMfaPostureError, + isMobileClientOutdated, isServiceUnavailable, isTimeout, mfaErrorMessage, - parseMfaError, } from '../../../rust-api/mfaError'; import { getInstancesQueryOptions } from '../../../rust-api/query'; import type { @@ -39,6 +39,8 @@ type Options = { onConnected?: () => void; onPostureError?: (message?: string) => void; onConnectionError?: (message?: string, icon?: IconKindValue) => void; + // replaces the other error callbacks and all error state + onFailure?: (err: unknown) => void; }; export const useMfaMobileConnect = ( @@ -51,6 +53,7 @@ export const useMfaMobileConnect = ( onConnected, onPostureError, onConnectionError, + onFailure, }: Options, ) => { const { data: instances } = useQuery(getInstancesQueryOptions); @@ -69,12 +72,14 @@ export const useMfaMobileConnect = ( const onStepAdvancedRef = useRef(onStepAdvanced); const setMfaTokenRef = useRef(setMfaToken); const onConnectionErrorRef = useRef(onConnectionError); + const onFailureRef = useRef(onFailure); const instanceId = instance?.id; onConnectedRef.current = onConnected; onStepAdvancedRef.current = onStepAdvanced; setMfaTokenRef.current = setMfaToken; onConnectionErrorRef.current = onConnectionError; + onFailureRef.current = onFailure; const cleanupListeners = useCallback(() => { if (unlistenRef.current !== null) { @@ -164,7 +169,11 @@ export const useMfaMobileConnect = ( isTimeout(event.payload.error) || isServiceUnavailable(event.payload.error); setTokenData(null); if (!retryable) setMfaTokenRef.current(null); - if (parseMfaError(event.payload.error)?.type === 'mobile_client_outdated') { + if (isPresent(onFailureRef.current)) { + onFailureRef.current(event.payload.error); + return; + } + if (isMobileClientOutdated(event.payload.error)) { onConnectionErrorRef.current?.(message, IconKind.DisableMfa); return; } @@ -199,8 +208,12 @@ export const useMfaMobileConnect = ( } else if (!isTimeout(e) && !isServiceUnavailable(e)) { setMfaTokenRef.current(null); } - setConnectionError('Failed to start mobile approval. Please try again.'); void error(`Mobile MFA connect failed for location ${location.id}: ${e}`); + if (isPresent(onFailureRef.current)) { + onFailureRef.current(e); + } else { + setConnectionError('Failed to start mobile approval. Please try again.'); + } } } })(); @@ -226,8 +239,16 @@ export const useMfaMobileConnect = ( }, [tokenData, instance]); const start = useCallback(async () => { + const failStart = (reason: string) => { + if (isPresent(onFailure)) { + onFailure(reason); + } else { + setStartError(reason); + } + }; + if (!instance) { - setStartError('Instance not found'); + failStart('Instance not found'); return; } @@ -258,7 +279,7 @@ export const useMfaMobileConnect = ( setMfaToken(session.token); if (!isPresent(session.challenge)) { - setStartError('Unsupported response from proxy'); + failStart('Unsupported response from proxy'); return; } @@ -272,6 +293,10 @@ export const useMfaMobileConnect = ( void error(`Mobile MFA start failed for location ${location.id}: ${e}`); const retryable = isTimeout(e) || isServiceUnavailable(e); if (!retryable) setMfaToken(null); + if (isPresent(onFailure)) { + onFailure(e); + return; + } if (isMfaPostureError(e, location)) { onPostureError?.(mfaErrorMessage(e)); return; @@ -294,6 +319,7 @@ export const useMfaMobileConnect = ( setMfaToken, onPostureError, onConnectionError, + onFailure, ]); const reset = useCallback(() => { diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.test.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.test.ts index c0696ef9b..8b90299bd 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.test.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.test.ts @@ -13,8 +13,8 @@ const mocks = vi.hoisted(() => ({ mfaPollOpenId: vi.fn(), openLink: vi.fn(), setMfaToken: vi.fn(), - setPostureError: vi.fn(), setView: vi.fn(), + showFailure: vi.fn(), useLocationCardContext: vi.fn(), })); @@ -46,8 +46,8 @@ describe('useMfaOidcConnect', () => { location: { id: 7, instance_id: 42 }, mfaToken: null, setMfaToken: mocks.setMfaToken, - setPostureError: mocks.setPostureError, setView: mocks.setView, + showFailure: mocks.showFailure, stepPlan: ['oidc'], }); mocks.cancelMfa.mockResolvedValue(undefined); @@ -102,6 +102,19 @@ describe('useMfaOidcConnect', () => { }); expect(mocks.setMfaToken).toHaveBeenLastCalledWith(null); + expect(mocks.showFailure).toHaveBeenCalledWith( + JSON.stringify({ type: 'attempt_limit', message: 'Attempt limit reached' }), + ); + }); + + it('shows a failed start through the card instead of keeping it in the hook', async () => { + const startError = JSON.stringify({ type: 'network_error', message: 'unreachable' }); + mocks.mfaBeginStep.mockRejectedValue(startError); + const { result } = renderHook(() => useMfaOidcConnect(true)); + + await waitFor(() => expect(mocks.showFailure).toHaveBeenCalledWith(startError)); + expect(result.current.isStarting).toBe(false); + expect(result.current.isPolling).toBe(false); }); it('retains and reuses the session token after a retryable poll timeout', async () => { @@ -110,8 +123,8 @@ describe('useMfaOidcConnect', () => { location: { id: 7, instance_id: 42 }, mfaToken: 'mfa-token', setMfaToken: mocks.setMfaToken, - setPostureError: mocks.setPostureError, setView: mocks.setView, + showFailure: mocks.showFailure, stepPlan: ['oidc'], }); const { result, rerender } = renderHook(() => useMfaOidcConnect()); diff --git a/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.ts b/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.ts index cfe79b48e..a6454d29b 100644 --- a/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.ts +++ b/new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.ts @@ -4,13 +4,7 @@ import { error } from '@tauri-apps/plugin-log'; import { useCallback, useEffect, useRef, useState } from 'react'; import { useMfaClientAttempt } from '../../../hooks/useMfaClientAttempt'; import { api } from '../../../rust-api/api'; -import { - classifyOidcPollFailure, - isMfaPostureError, - isServiceUnavailable, - isTimeout, - mfaErrorMessage, -} from '../../../rust-api/mfaError'; +import { isServiceUnavailable, isTimeout } from '../../../rust-api/mfaError'; import { getInstancesQueryOptions } from '../../../rust-api/query'; import type { MfaErrorPayload, MfaStepAdvancedPayload } from '../../../rust-api/types'; import { MfaMethod, TauriEvent } from '../../../rust-api/types'; @@ -19,20 +13,11 @@ import { useLocationCardContext } from '../context/context'; import { LocationCardViews } from '../context/types'; export const useMfaOidcConnect = (autoStart = false) => { - const { - location, - setPostureError, - setView, - stepPlan, - mfaToken, - setMfaToken, - goToStep, - } = useLocationCardContext(); + const { location, setView, showFailure, stepPlan, mfaToken, setMfaToken, goToStep } = + useLocationCardContext(); const [isStarting, setIsStarting] = useState(false); - const [startError, setStartError] = useState(null); const [isPolling, setIsPolling] = useState(false); - const [pollError, setPollError] = useState(null); const isPollingRef = useRef(false); const { data: instances } = useQuery(getInstancesQueryOptions); @@ -49,7 +34,7 @@ export const useMfaOidcConnect = (autoStart = false) => { const start = useCallback(async () => { if (!instance) { - setStartError('Instance not found'); + showFailure('Instance not found'); return; } @@ -61,8 +46,6 @@ export const useMfaOidcConnect = (autoStart = false) => { setMfaToken(null); } setIsStarting(true); - setStartError(null); - setPollError(null); let beginningStep = true; try { @@ -129,11 +112,13 @@ export const useMfaOidcConnect = (autoStart = false) => { void error( `OIDC MFA failed for location ${location.id}: ${event.payload.error}`, ); - const failure = classifyOidcPollFailure(event.payload.error); - if (failure.kind !== 'timeout' && !isServiceUnavailable(event.payload.error)) { + if ( + !isTimeout(event.payload.error) && + !isServiceUnavailable(event.payload.error) + ) { setMfaToken(null); } - setPollError(failure.message); + showFailure(event.payload.error); }), ); } catch (e) { @@ -147,16 +132,7 @@ export const useMfaOidcConnect = (autoStart = false) => { if (!retryable) setMfaToken(null); } void error(`OIDC MFA start failed for location ${location.id}: ${e}`); - if (isMfaPostureError(e, location)) { - setPostureError(mfaErrorMessage(e)); - setView(LocationCardViews.PostureCheckFail); - return; - } - if (isServiceUnavailable(e)) { - setView(LocationCardViews.ConnectionError); - return; - } - setStartError(mfaErrorMessage(e)); + showFailure(e); } finally { if (attempt.isLive()) { setIsStarting(false); @@ -169,8 +145,8 @@ export const useMfaOidcConnect = (autoStart = false) => { stepPlan, mfaToken, setMfaToken, - setPostureError, setView, + showFailure, goToStep, ]); @@ -187,5 +163,5 @@ export const useMfaOidcConnect = (autoStart = false) => { return () => window.clearTimeout(timer); }, [autoStart]); - return { start, isStarting, startError, isPolling, pollError }; + return { start, isStarting, isPolling }; }; diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.test.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.test.tsx deleted file mode 100644 index e37063780..000000000 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.test.tsx +++ /dev/null @@ -1,59 +0,0 @@ -import { fireEvent, render, screen } from '@testing-library/react'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { IconKind } from '../../../Icon'; -import type { IconKindValue } from '../../../Icon/icon-types'; -import { DEFAULT_CONNECTION_ERROR } from '../../api/connectError'; -import { LocationCardViews } from '../../context/types'; -import { LocationCardConnectionErrorView } from './LocationCardConnectionErrorView'; - -const mocks = vi.hoisted(() => ({ - setView: vi.fn(), - connectionError: null as string | null, - connectionErrorIcon: null as IconKindValue | null, -})); - -vi.mock('../../context/context', () => ({ - useLocationCardContext: () => mocks, -})); - -vi.mock('../../api/connectError', () => ({ - DEFAULT_CONNECTION_ERROR: 'Service unavailable', -})); - -describe('LocationCardConnectionErrorView', () => { - beforeEach(() => { - vi.clearAllMocks(); - mocks.connectionError = null; - mocks.connectionErrorIcon = null; - }); - - it('shows the mobile error icon and message', () => { - mocks.connectionError = - 'Your Defguard mobile app is out of date.\nUpgrade to approve this connection.'; - mocks.connectionErrorIcon = IconKind.DisableMfa; - const { container } = render(); - - expect(container.querySelector('[data-kind="disable-mfa"]')).not.toBeNull(); - expect(container.querySelector('.description')?.textContent).toBe( - mocks.connectionError, - ); - }); - - it('keeps the service error icon and uses the secondary Got it button', () => { - mocks.connectionErrorIcon = IconKind.ServiceUnavailable; - const { container } = render(); - - expect(container.querySelector('[data-kind="service-unavailable"]')).not.toBeNull(); - expect(screen.getByText(DEFAULT_CONNECTION_ERROR)).toBeDefined(); - const button = screen.getByRole('button', { name: 'Got it' }); - expect(button.getAttribute('data-variant')).toBe('secondary'); - fireEvent.click(button); - expect(mocks.setView).toHaveBeenCalledWith(LocationCardViews.Default); - }); - - it('omits the icon when none is supplied', () => { - const { container } = render(); - - expect(container.querySelector('.icon')).toBeNull(); - }); -}); diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.tsx deleted file mode 100644 index 9f34bea99..000000000 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/LocationCardConnectionErrorView.tsx +++ /dev/null @@ -1,34 +0,0 @@ -import './style.scss'; -import { ThemeSpacing } from '../../../../types'; -import { Button } from '../../../Button/Button'; -import { ButtonVariant } from '../../../Button/types'; -import { Divider } from '../../../Divider/Divider'; -import { Icon } from '../../../Icon'; -import { SizedBox } from '../../../SizedBox/SizedBox'; -import { DEFAULT_CONNECTION_ERROR } from '../../api/connectError'; -import { useLocationCardContext } from '../../context/context'; -import { LocationCardViews } from '../../context/types'; - -export const LocationCardConnectionErrorView = () => { - const { setView, connectionError, connectionErrorIcon } = useLocationCardContext(); - - return ( -
- - - {connectionErrorIcon && ( - <> - - - - )} -

{connectionError ?? DEFAULT_CONNECTION_ERROR}

- -
- ); -}; diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.test.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.test.tsx new file mode 100644 index 000000000..7063fde93 --- /dev/null +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.test.tsx @@ -0,0 +1,72 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { IconKind } from '../../../Icon'; +import { DEFAULT_CONNECTION_ERROR } from '../../api/connectError'; +import { type LocationCardError, LocationCardViews } from '../../context/types'; +import { LocationCardErrorView } from './LocationCardErrorView'; + +const mocks = vi.hoisted(() => ({ + setView: vi.fn(), + cardError: null as LocationCardError | null, + location: { active: false }, +})); + +vi.mock('../../context/context', () => ({ + useLocationCardContext: () => mocks, +})); + +vi.mock('../../api/connectError', () => ({ + DEFAULT_CONNECTION_ERROR: 'Service unavailable', +})); + +describe('LocationCardErrorView', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.cardError = null; + mocks.location.active = false; + }); + + it('shows the error icon and message', () => { + mocks.cardError = { + message: + 'Your Defguard mobile app is out of date.\nUpgrade to approve this connection.', + icon: IconKind.DisableMfa, + }; + const { container } = render(); + + expect(container.querySelector('[data-kind="disable-mfa"]')).not.toBeNull(); + expect(container.querySelector('.description')?.textContent).toBe( + mocks.cardError.message, + ); + }); + + it('falls back to the service error and a secondary Got it button', () => { + const { container } = render(); + + expect(container.querySelector('[data-kind="service-unavailable"]')).not.toBeNull(); + expect(screen.getByText(DEFAULT_CONNECTION_ERROR)).toBeDefined(); + expect(screen.queryByRole('button', { name: 'Retry' })).toBeNull(); + const button = screen.getByRole('button', { name: 'Got it' }); + expect(button.getAttribute('data-variant')).toBe('secondary'); + fireEvent.click(button); + expect(mocks.setView).toHaveBeenCalledWith(LocationCardViews.Default); + }); + + it('offers Retry instead of Got it when the error can be retried', () => { + const onRetry = vi.fn(); + mocks.cardError = { message: 'Timed out', icon: IconKind.WarningOutlined, onRetry }; + render(); + + expect(screen.queryByRole('button', { name: 'Got it' })).toBeNull(); + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + expect(onRetry).toHaveBeenCalledTimes(1); + }); + + it('goes back to the connected view for an active location', () => { + mocks.location.active = true; + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Got it' })); + expect(mocks.setView).toHaveBeenCalledWith(LocationCardViews.Connected); + }); +}); diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.tsx new file mode 100644 index 000000000..41c1890b2 --- /dev/null +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/LocationCardErrorView.tsx @@ -0,0 +1,47 @@ +import './style.scss'; +import { ThemeSpacing } from '../../../../types'; +import { isPresent } from '../../../../utils/isPresent'; +import { Button } from '../../../Button/Button'; +import { ButtonVariant } from '../../../Button/types'; +import { Divider } from '../../../Divider/Divider'; +import { Icon, IconKind } from '../../../Icon'; +import { IconButton } from '../../../IconButton/IconButton'; +import { IconButtonVariant } from '../../../IconButton/types'; +import { SizedBox } from '../../../SizedBox/SizedBox'; +import { DEFAULT_CONNECTION_ERROR } from '../../api/connectError'; +import { useLocationCardContext } from '../../context/context'; +import { LocationCardViews } from '../../context/types'; + +export const LocationCardErrorView = () => { + const { setView, cardError, location } = useLocationCardContext(); + const onRetry = cardError?.onRetry; + + const leave = () => { + setView(location.active ? LocationCardViews.Connected : LocationCardViews.Default); + }; + + return ( +
+ + + + +

{cardError?.message ?? DEFAULT_CONNECTION_ERROR}

+ + {isPresent(onRetry) && ( +
+ +
+ )} + {!isPresent(onRetry) && ( +
+ ); +}; diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/style.scss b/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/style.scss similarity index 63% rename from new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/style.scss rename to new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/style.scss index 988b73bfa..f292e671f 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardConnectionErrorView/style.scss +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardErrorView/style.scss @@ -1,4 +1,4 @@ -.location-card-connection-error-view { +.location-card-error-view { display: flex; flex-direction: column; align-items: center; @@ -17,4 +17,15 @@ text-align: center; white-space: pre-line; } + + .retry { + display: flex; + align-items: center; + column-gap: var(--spacing-md); + width: 100%; + + .btn { + flex: 1; + } + } } diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaEmailView/LocationCardMfaEmailView.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaEmailView/LocationCardMfaEmailView.tsx index f5d9366aa..1a3e2755f 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaEmailView/LocationCardMfaEmailView.tsx +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaEmailView/LocationCardMfaEmailView.tsx @@ -22,8 +22,8 @@ const MIN_POSTURE_LOADER_MS = 500; export const LocationCardMfaEmailView = () => { const { setView, + showFailure, location, - setPostureError, stepLabel, canPickOtherMethod, stepPlan, @@ -31,7 +31,7 @@ export const LocationCardMfaEmailView = () => { setMfaToken, goToStep, } = useLocationCardContext(); - const { verifyCode, isVerifying, verifyError, isStarting, startError } = useMfaConnect( + const { verifyCode, isVerifying, verifyError, isStarting } = useMfaConnect( location, MfaMethod.Email, { @@ -41,12 +41,7 @@ export const LocationCardMfaEmailView = () => { onStepAdvanced: goToStep, debounceMs: location.posture_check_required ? MIN_POSTURE_LOADER_MS : 0, onConnected: () => setView(LocationCardViews.Connected), - onSessionExpired: () => setView(LocationCardViews.Default), - onPostureError: (msg) => { - setPostureError(msg); - setView(LocationCardViews.PostureCheckFail); - }, - onServiceUnavailable: () => setView(LocationCardViews.ConnectionError), + onFailure: showFailure, }, ); @@ -76,14 +71,11 @@ export const LocationCardMfaEmailView = () => { if (value.length > 0) setError(null); }, []); - // Reflect server-side verify errors into the local error state useEffect(() => { if (verifyError) setError(verifyError); }, [verifyError]); - // Show loader when posture is being evaluated - const showLoader = location.posture_check_required && isStarting && !startError; - if (showLoader) { + if (location.posture_check_required && isStarting) { return ; } return ( @@ -97,7 +89,7 @@ export const LocationCardMfaEmailView = () => { length={6} value={emailCode} onChange={handleCodeChange} - error={startError ?? error} + error={error} onSubmit={handleVerify} loading={isStarting || isVerifying} /> diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaFido2View/LocationCardMfaFido2View.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaFido2View/LocationCardMfaFido2View.tsx index 2865bd3f1..6d794a465 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaFido2View/LocationCardMfaFido2View.tsx +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaFido2View/LocationCardMfaFido2View.tsx @@ -28,7 +28,7 @@ export const LocationCardMfaFido2View = () => { mfaToken, setMfaToken, goToStep, - setPostureError, + showFailure, } = useLocationCardContext(); const { verify, isVerifying, isAwaitingTouch, verifyError } = useMfaFido2Connect( location, @@ -38,11 +38,7 @@ export const LocationCardMfaFido2View = () => { setMfaToken, onConnected: () => setView(LocationCardViews.Connected), onStepAdvanced: goToStep, - onPostureError: (message) => { - setPostureError(message); - setView(LocationCardViews.PostureCheckFail); - }, - onServiceUnavailable: () => setView(LocationCardViews.ConnectionError), + onFailure: showFailure, }, ); @@ -67,7 +63,6 @@ export const LocationCardMfaFido2View = () => { setError(null); }, [pin, setError]); - // Reflect backend errors into the local error state useEffect(() => { if (verifyError) setError(verifyError); }, [verifyError]); @@ -92,7 +87,7 @@ export const LocationCardMfaFido2View = () => {

- {collectsPin ? ( + {collectsPin && ( { onChange={(value) => setPin(isPresent(value) ? String(value) : null)} error={error} /> - ) : ( - isPresent(error) &&

{error}

)} )} diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/LocationCardMfaMobileView.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/LocationCardMfaMobileView.tsx index 5632771a6..daf9eadfa 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/LocationCardMfaMobileView.tsx +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/LocationCardMfaMobileView.tsx @@ -1,6 +1,7 @@ import './style.scss'; -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useRef } from 'react'; import { ThemeSpacing } from '../../../../types'; +import { isPresent } from '../../../../utils/isPresent'; import { Button } from '../../../Button/Button'; import { ButtonVariant } from '../../../Button/types'; import { Controls } from '../../../Controls/Controls'; @@ -14,12 +15,10 @@ import { useLocationCardContext } from '../../context/context'; import { LocationCardViews } from '../../context/types'; import { useMfaMobileConnect } from '../../hooks/useMfaMobileConnect'; -type Screen = 'loading' | 'qr' | 'error'; - export const LocationCardMfaMobileView = () => { const { setView, - setPostureError, + showFailure, location, stepLabel, canPickOtherMethod, @@ -28,52 +27,32 @@ export const LocationCardMfaMobileView = () => { setMfaToken, goToStep, } = useLocationCardContext(); - const { start, startError, qrValue, connectionError } = useMfaMobileConnect(location, { + const { start, qrValue } = useMfaMobileConnect(location, { stepPlan, mfaToken, setMfaToken, onStepAdvanced: goToStep, onConnected: () => setView(LocationCardViews.Connected), - onPostureError: (message) => setPostureError(message ?? null), - onConnectionError: (message, icon) => - setView(LocationCardViews.ConnectionError, message, icon), + onFailure: showFailure, }); - const [screen, setScreen] = useState('loading'); const startedRef = useRef(false); - // Auto-start on mount useEffect(() => { if (startedRef.current) return; startedRef.current = true; void start(); }, [start]); - useEffect(() => { - if (startError ?? connectionError) { - setScreen('error'); - } else if (qrValue) { - setScreen('qr'); - } - }, [startError, connectionError, qrValue]); - - const backToLocation = () => { - setPostureError(null); - setView(LocationCardViews.Default); - }; - - const errorMessage = startError ?? connectionError; - return (
- {screen === 'loading' &&

Preparing authentication...

} - {screen === 'qr' && ( -

Open your Defguard mobile app and scan the QR code you see bellow.

+ {!isPresent(qrValue) &&

Preparing authentication...

} + {isPresent(qrValue) && ( +

Open your Defguard mobile app and scan the QR code you see below.

)} - {screen === 'error' &&

{errorMessage}

} - {screen === 'qr' && qrValue && ( + {isPresent(qrValue) && (
@@ -95,13 +74,6 @@ export const LocationCardMfaMobileView = () => { }} /> )} - {screen === 'error' && ( -
diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/style.scss b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/style.scss index 02de3b954..a53c95955 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/style.scss +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaMobileView/style.scss @@ -13,10 +13,4 @@ text-align: center; } } - - .location-card-view-header { - p.error { - color: var(--fg-critical); - } - } } diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/LocationCardMfaOidcView.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/LocationCardMfaOidcView.tsx index 6b39854e2..99f6b40be 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/LocationCardMfaOidcView.tsx +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/LocationCardMfaOidcView.tsx @@ -1,12 +1,9 @@ import './style.scss'; import { useQuery } from '@tanstack/react-query'; -import { useCallback, useEffect, useState } from 'react'; import { api } from '../../../../rust-api/api'; import { getAppConfigQueryOptions } from '../../../../rust-api/query'; import { ThemeSpacing } from '../../../../types'; import { findOpenIdProvider, openIdProviderName } from '../../../../utils/mfa'; -import { Button } from '../../../Button/Button'; -import { ButtonVariant } from '../../../Button/types'; import { Checkbox } from '../../../Checkbox/Checkbox'; import { Controls } from '../../../Controls/Controls'; import { Divider } from '../../../Divider/Divider'; @@ -20,54 +17,28 @@ import { useLocationCardContext } from '../../context/context'; import { LocationCardViews } from '../../context/types'; import { useMfaOidcConnect } from '../../hooks/useMfaOidcConnect'; -type Screen = 'idle' | 'polling' | 'error'; - export const LocationCardMfaOidcView = () => { const { data: appConfig } = useQuery(getAppConfigQueryOptions); - const { setView, setPostureError, autoConnectOpenid, stepLabel, instance } = - useLocationCardContext(); + const { setView, autoConnectOpenid, stepLabel, instance } = useLocationCardContext(); const providerName = openIdProviderName(instance); - const { start, isStarting, startError, isPolling, pollError } = - useMfaOidcConnect(autoConnectOpenid); - const [screen, setScreen] = useState('idle'); - - useEffect(() => { - if (startError ?? pollError) { - setScreen((prev) => (prev !== 'idle' ? 'error' : prev)); - } else if (isPolling) { - setScreen('polling'); - } - }, [startError, pollError, isPolling]); - - const handleStart = useCallback(async () => { - await start(); - setScreen('polling'); - }, [start]); - - const errorMessage = startError ?? pollError; - - const backToLocation = () => { - setPostureError(null); - setView(LocationCardViews.Default); - }; + const { start, isStarting, isPolling } = useMfaOidcConnect(autoConnectOpenid); return (
- {screen === 'idle' && ( + {!isPolling && (

{`To connect to the VPN, sign in with ${providerName} by clicking button below. A browser window will open and redirect you to ${providerName} for authentication.`}

)} - {screen === 'polling' && ( + {isPolling && (

{`Complete the sign-in in your browser. This page will update automatically.`}

)} - {screen === 'error' &&

{errorMessage}

} - {screen === 'idle' && !autoConnectOpenid && ( + {!isPolling && !autoConnectOpenid && (
{ onClick={() => setView(LocationCardViews.Default)} />
- {screen !== 'error' && ( - - )} - {screen === 'error' && ( -
diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/style.scss b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/style.scss index 6fbacfb6e..4f99febf6 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/style.scss +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaOidcView/style.scss @@ -1,9 +1,3 @@ -.location-card-view-header { - p.error { - color: var(--fg-critical); - } -} - .location-card-mfa-oidc { .controls { > .right { diff --git a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaTotpView/LocationCardMfaTotpView.tsx b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaTotpView/LocationCardMfaTotpView.tsx index 4f2b53579..b6c24738c 100644 --- a/new-ui/src/shared/components/LocationCard/views/LocationCardMfaTotpView/LocationCardMfaTotpView.tsx +++ b/new-ui/src/shared/components/LocationCard/views/LocationCardMfaTotpView/LocationCardMfaTotpView.tsx @@ -22,8 +22,8 @@ const MIN_POSTURE_LOADER_MS = 500; export const LocationCardMfaTotpView = () => { const { setView, + showFailure, location, - setPostureError, stepLabel, canPickOtherMethod, stepPlan, @@ -31,7 +31,7 @@ export const LocationCardMfaTotpView = () => { setMfaToken, goToStep, } = useLocationCardContext(); - const { verifyCode, isVerifying, verifyError, isStarting, startError } = useMfaConnect( + const { verifyCode, isVerifying, verifyError, isStarting } = useMfaConnect( location, MfaMethod.Totp, { @@ -41,12 +41,7 @@ export const LocationCardMfaTotpView = () => { onStepAdvanced: goToStep, debounceMs: location.posture_check_required ? MIN_POSTURE_LOADER_MS : 0, onConnected: () => setView(LocationCardViews.Connected), - onSessionExpired: () => setView(LocationCardViews.Default), - onPostureError: (msg) => { - setPostureError(msg); - setView(LocationCardViews.PostureCheckFail); - }, - onServiceUnavailable: () => setView(LocationCardViews.ConnectionError), + onFailure: showFailure, }, ); @@ -76,14 +71,11 @@ export const LocationCardMfaTotpView = () => { if (value.length > 0) setError(null); }, []); - // Reflect server-side verify errors into the local error state useEffect(() => { if (verifyError) setError(verifyError); }, [verifyError]); - // Show loader when posture is being evaluated - const showLoader = location.posture_check_required && isStarting && !startError; - if (showLoader) { + if (location.posture_check_required && isStarting) { return ; } @@ -98,7 +90,7 @@ export const LocationCardMfaTotpView = () => { length={6} value={totpCode} onChange={handleCodeChange} - error={startError ?? error} + error={error} onSubmit={handleVerify} loading={isStarting || isVerifying} /> diff --git a/new-ui/src/shared/rust-api/mfaError.ts b/new-ui/src/shared/rust-api/mfaError.ts index cdaceb0a8..4931c98f3 100644 --- a/new-ui/src/shared/rust-api/mfaError.ts +++ b/new-ui/src/shared/rust-api/mfaError.ts @@ -66,6 +66,22 @@ export const isServiceUnavailable = (err: unknown): boolean => { export const isConnectFailure = (message: string): boolean => message.includes('VPN connection failed'); +export const isMobileClientOutdated = (err: unknown): boolean => + parseMfaError(err)?.type === 'mobile_client_outdated'; + +/** The user declined the request in the mobile app. */ +export const isMobileApprovalRejected = (message: string): boolean => + message === 'Mobile approval rejected'; + +/** A mistyped PIN, which the user fixes in the PIN field. */ +export const isSecurityKeyPinRejected = (message: string): boolean => + message.startsWith('Security key rejected the PIN') && + !isSecurityKeyPinBlocked(message); + +/** Retyping does not help once the key locks itself. */ +export const isSecurityKeyPinBlocked = (message: string): boolean => + message.includes('locked after too many PIN attempts'); + /** The proxy predates the MFA configuration API (HTTP 404 on /mfa-config/start). */ export const isMfaConfigUnsupported = (err: unknown): boolean => parseMfaError(err)?.type === 'unsupported'; @@ -134,6 +150,14 @@ export type OidcPollFailure = { message: string; }; +export const MfaFailureText = { + StaleAttempt: 'Authentication request could not be started. Please try again.', + Timeout: 'Authentication timed out. Please try again.', + ConnectFailure: 'Failed to establish VPN connection', + SessionExpired: 'Session expired. Please try again.', + Unknown: 'Authentication failed. Please try again.', +} as const; + /** Maps an OIDC error event to a type and user-facing message. */ export const classifyOidcPollFailure = (rawError: string): OidcPollFailure => { const message = mfaErrorMessage(rawError); @@ -141,19 +165,16 @@ export const classifyOidcPollFailure = (rawError: string): OidcPollFailure => { return { kind: 'attemptLimit', message }; } if (isStaleAttempt(message)) { - return { - kind: 'staleAttempt', - message: 'Authentication request could not be started. Please try again.', - }; + return { kind: 'staleAttempt', message: MfaFailureText.StaleAttempt }; } if (isTimeout(rawError)) { - return { kind: 'timeout', message: 'Authentication timed out. Please try again.' }; + return { kind: 'timeout', message: MfaFailureText.Timeout }; } if (isConnectFailure(message)) { - return { kind: 'connectFailure', message: 'Failed to establish VPN connection' }; + return { kind: 'connectFailure', message: MfaFailureText.ConnectFailure }; } if (isSessionExpired(message)) { - return { kind: 'sessionExpired', message: 'Session expired. Please try again.' }; + return { kind: 'sessionExpired', message: MfaFailureText.SessionExpired }; } - return { kind: 'unknown', message: 'Authentication failed. Please try again.' }; + return { kind: 'unknown', message: MfaFailureText.Unknown }; };