diff --git a/new-ui/src/pages/compact/CompactLocationsPage/CompactLocationsPage.tsx b/new-ui/src/pages/compact/CompactLocationsPage/CompactLocationsPage.tsx index 1a9e864b8..6ac58d30f 100644 --- a/new-ui/src/pages/compact/CompactLocationsPage/CompactLocationsPage.tsx +++ b/new-ui/src/pages/compact/CompactLocationsPage/CompactLocationsPage.tsx @@ -76,31 +76,29 @@ export const CompactLocationsPage = () => { }} > + - - - - {displayedLocations.map((location) => { - const isOpen = - location.id === openLocation || displayedLocations.length === 1; - return ( - { - if (isOpen) { - useAppStore.setState({ expandedLocation: null }); - } else { - useAppStore.setState({ expandedLocation: location.id }); - } - }} - /> - ); - })} - + + {displayedLocations.map((location) => { + const isOpen = + location.id === openLocation || displayedLocations.length === 1; + return ( + { + if (isOpen) { + useAppStore.setState({ expandedLocation: null }); + } else { + useAppStore.setState({ expandedLocation: location.id }); + } + }} + /> + ); + })} diff --git a/new-ui/src/pages/compact/CompactLocationsPage/components/InstanceSwitcher.tsx b/new-ui/src/pages/compact/CompactLocationsPage/components/InstanceSwitcher.tsx index c182bced3..ce3a9999e 100644 --- a/new-ui/src/pages/compact/CompactLocationsPage/components/InstanceSwitcher.tsx +++ b/new-ui/src/pages/compact/CompactLocationsPage/components/InstanceSwitcher.tsx @@ -1,4 +1,6 @@ import { useQuery } from '@tanstack/react-query'; +import { platform } from '@tauri-apps/plugin-os'; +import clsx from 'clsx'; import { useMemo } from 'react'; import { Select } from '../../../../shared/components/Select/Select'; import type { @@ -14,6 +16,8 @@ import { import type { OverviewViewSelection } from '../../../../shared/rust-api/types'; import { isPresent } from '../../../../shared/utils/isPresent'; +const isWindows = platform() === 'windows'; + export const InstanceSwitcher = () => { const { viewSelection: selectedInstance, setViewSelection } = useAppData(); @@ -76,12 +80,18 @@ export const InstanceSwitcher = () => { if (totalOptions <= 1) return null; return ( - { - setViewSelection(option.value); - }} - /> + + { + setViewSelection(option.value); + }} + /> + ); }; diff --git a/new-ui/src/pages/compact/CompactLocationsPage/style.scss b/new-ui/src/pages/compact/CompactLocationsPage/style.scss index ede1152da..71ce18d87 100644 --- a/new-ui/src/pages/compact/CompactLocationsPage/style.scss +++ b/new-ui/src/pages/compact/CompactLocationsPage/style.scss @@ -3,20 +3,27 @@ flex-flow: column; height: 100dvh; - > .compact-footer { + & > .compact-footer { display: flex; flex-flow: column; } - > .window-header, - > .compact-footer { + & > .window-header, + & > .compact-footer { flex: 0 0 auto; } - .main-content { - display: flex; - flex-flow: column; - row-gap: var(--spacing-sm); + & > .instance-switcher { + flex: 0 0 auto; + margin-bottom: var(--spacing-sm); + box-sizing: border-box; + + // lines the select up with the cards inside .scroll-container.windows + &.windows { + scrollbar-gutter: stable; + overflow-y: hidden; + padding-right: var(--scroll-container-gutter); + } } .locations { diff --git a/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx b/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx index 5ac9b0dcb..697870a1a 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/ConfigureMfaPage.tsx @@ -1,5 +1,6 @@ import { useNavigate } from '@tanstack/react-router'; import { useCallback } from 'react'; +import { isPresent } from '../../../shared/utils/isPresent'; import { ConfigureMfaTimeoutProvider, useConfigureMfaSessionExpired, @@ -18,16 +19,24 @@ export const ConfigureMfaPage = () => { const ConfigureMfaContent = () => { const navigate = useNavigate(); - const authorized = useConfigureMfaStore((s) => s.authorized); + // a late authorization on the selection screen waits for the picks + const inWizard = useConfigureMfaStore( + (s) => s.authorized && isPresent(s.selectedMethods), + ); const handleSessionExpired = useConfigureMfaSessionExpired(); const leave = useCallback(() => { navigate({ to: '/full/add' }); }, [navigate]); - return authorized ? ( - - ) : ( - + return ( + <> + {inWizard && ( + + )} + {!inWizard && ( + + )} + > ); }; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector.tsx b/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector.tsx index 3a640cdd2..1b44494a4 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector.tsx @@ -23,13 +23,19 @@ const factorText = (method: MfaMethodValue): FactorText => { description: `We'll send a temporary security code to your email. Enter the code to confirm it's you and continue.`, }; case MfaMethod.Oidc: - return { title: '', description: '' }; + return { + title: 'OpenID sign-in', + description: `Sign in with your identity provider in the browser to confirm it's you.`, + }; case MfaMethod.Biometric: return { title: '', description: '' }; case MfaMethod.MobileApprove: return { title: '', description: '' }; case MfaMethod.Fido2: - return { title: '', description: '' }; + return { + title: 'Security key', + description: `Use a security key registered to your account, such as a YubiKey, to confirm it's you.`, + }; } }; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts new file mode 100644 index 000000000..9ec75d0fd --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts @@ -0,0 +1,69 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { MfaMethod } from '../../../../shared/rust-api/types'; +import { ConfigureMfaStep } from '../types'; +import { applyAuthorization, useConfigureMfaStore } from './useConfigureMfaStore'; + +vi.mock('@tauri-apps/plugin-log', () => ({ error: vi.fn() })); +vi.mock('../../../../shared/rust-api/api', () => ({ api: {} })); + +const sessionId = 'session-1'; +const authorizeResult = { deadline_timestamp: 1_900_000_000, recovery_codes: [] }; + +describe('applyAuthorization', () => { + beforeEach(() => { + useConfigureMfaStore.getState().reset(); + useConfigureMfaStore.setState({ + sessionId, + configuredMethods: [MfaMethod.Email], + verificationMethods: [MfaMethod.Email], + }); + }); + + it('waits on the selection when the answer lands after Back', () => { + useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); + useConfigureMfaStore.getState().backFromVerification(); + applyAuthorization(sessionId, authorizeResult); + + const state = useConfigureMfaStore.getState(); + expect(state.authorized).toBe(true); + expect(state.selectedMethods).toBeNull(); + expect(state.activeStep).toBe(ConfigureMfaStep.Configuration); + expect(state.deadline).not.toBeNull(); + }); + + it('sets up the picks confirmed after a late answer', () => { + useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); + useConfigureMfaStore.getState().backFromVerification(); + applyAuthorization(sessionId, authorizeResult); + useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); + + const state = useConfigureMfaStore.getState(); + expect(state.activeStep).toBe(ConfigureMfaStep.Fido2); + expect(state.deadline).not.toBeNull(); + }); + + it('finishes when the pick confirmed after a late answer is empty', () => { + useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); + useConfigureMfaStore.getState().backFromVerification(); + applyAuthorization(sessionId, authorizeResult); + useConfigureMfaStore.getState().selectMethods([]); + + const state = useConfigureMfaStore.getState(); + expect(state.activeStep).toBe(ConfigureMfaStep.Finish); + expect(state.deadline).toBeNull(); + }); + + it('keeps the deadline of an unauthorized session with an empty pick', () => { + useConfigureMfaStore.setState({ deadline: '2030-01-01T00:00:00.000Z' }); + useConfigureMfaStore.getState().selectMethods([]); + + expect(useConfigureMfaStore.getState().deadline).toBe('2030-01-01T00:00:00.000Z'); + }); + + it('drops an answer for another session', () => { + useConfigureMfaStore.getState().selectMethods([MfaMethod.Fido2]); + applyAuthorization('session-2', authorizeResult); + + expect(useConfigureMfaStore.getState().authorized).toBe(false); + }); +}); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx index ab4d7411f..a71e4b63e 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx @@ -25,6 +25,7 @@ import { isMfaFactorOfferable, isMfaSetupStep, mfaFactorStep, + verificationMethodsOf, } from '../utils'; type StoreValues = { @@ -41,7 +42,9 @@ type StoreValues = { selectedMethods: MfaMethodValue[] | null; /** Pre-ticked in the selection step, from the entry point or an earlier pass. */ initialSelection: MfaMethodValue[]; - /** Null until picked. Asked only when more than one code factor is configured. */ + /** most preferred first, from the session since an older Core rejects some factors */ + verificationMethods: MfaVerificationMethod[]; + /** null until picked, asked only when the session offers more than one method */ verificationMethod: MfaVerificationMethod | null; /** No factor was configured, so an emailed code was the only way in. */ emailFallback: boolean; @@ -103,6 +106,7 @@ const defaults: StoreValues = { completedMethods: [], selectedMethods: null, initialSelection: [], + verificationMethods: [], verificationMethod: null, emailFallback: false, deadline: null, @@ -125,11 +129,11 @@ interface Store extends StoreValues { selectVerificationMethod: (method: MfaVerificationMethod) => void; /** Keeps the current picks ticked and the session alive. */ backToSelection: () => void; + backFromVerification: () => void; /** The fresh deadline bounds every setup still to come, not just the next one. */ authorize: (response: MfaConfigAuthorizeResult) => void; factorConfigured: (method: MfaMethodValue, recoveryCodes: string[]) => void; next: () => void; - back: () => void; reset: () => void; } @@ -139,30 +143,41 @@ export const useConfigureMfaStore = create()( ...defaults, start: (instance, response, origin) => { // The fallback mails a code to the address on file, registering email along the way. - const codeFactors = response.email_fallback + const sessionMethods = response.email_fallback ? [MfaMethod.Email] : response.available_methods; + // the session and the snapshot may both list FIDO2 const configuredMethods = [ - ...codeFactors, - ...(instance.mfa_configured_methods ?? []).filter( - (method) => !isCodeMfaMethod(method), - ), + ...new Set([ + ...sessionMethods, + ...(instance.mfa_configured_methods ?? []).filter( + (method) => !isCodeMfaMethod(method), + ), + ]), ]; set({ ...defaults, instance, sessionId: response.session_id, configuredMethods, + verificationMethods: verificationMethodsOf(sessionMethods), emailFallback: response.email_fallback, deadline: dayjs.unix(response.deadline_timestamp).toISOString(), ...origin, }); }, selectMethods: (methods) => { - set((current) => ({ - selectedMethods: methods, - activeStep: firstStep({ ...current, selectedMethods: methods }), - })); + set((current) => { + const next = { ...current, selectedMethods: methods }; + return { + selectedMethods: methods, + activeStep: firstStep(next), + // picked after a late authorization, so the deadline was kept for it + ...(current.authorized && { + deadline: sessionDeadline(next, current.deadline), + }), + }; + }); }, selectVerificationMethod: (method) => { set({ verificationMethod: method }); @@ -175,17 +190,26 @@ export const useConfigureMfaStore = create()( activeStep: defaults.activeStep, })); }, + backFromVerification: () => { + if (isPresent(get().verificationMethod)) { + set({ verificationMethod: null }); + return; + } + get().backToSelection(); + }, authorize: (response) => { set((current) => { + const deadline = dayjs.unix(response.deadline_timestamp).toISOString(); + // a late answer can land after Back, the steps wait for its Continue + if (!isPresent(current.selectedMethods)) { + return { authorized: true, recoveryCodes: response.recovery_codes, deadline }; + } // The fallback enables email as it verifies, so only this authorization issues codes. const next = { ...current, recoveryCodes: response.recovery_codes }; return { authorized: true, recoveryCodes: response.recovery_codes, - deadline: sessionDeadline( - next, - dayjs.unix(response.deadline_timestamp).toISOString(), - ), + deadline: sessionDeadline(next, deadline), activeStep: firstStep(next), }; }); @@ -217,15 +241,6 @@ export const useConfigureMfaStore = create()( ); if (isPresent(next)) set({ activeStep: next }); }, - back: () => { - const current = get(); - const from = MFA_WIZARD_STEPS.indexOf(current.activeStep); - // A step with nothing left to do is not one to go back to. - const previous = remainingSteps(current) - .filter((step) => MFA_WIZARD_STEPS.indexOf(step) < from) - .at(-1); - if (isPresent(previous)) set({ activeStep: previous }); - }, reset: () => { set({ ...defaults }); }, @@ -234,7 +249,7 @@ export const useConfigureMfaStore = create()( name: 'configure-mfa-store', storage: createJSONStorage(() => sessionStorage), // Bumped on every shape change: a stored session is never resumable across one. - version: 11, + version: 12, }, ), ); @@ -263,6 +278,17 @@ export const startMfaConfiguration = async ( useConfigureMfaStore.setState({ initialSelection }); }; +/** applied even after the asking step unmounts, Core has authorized the session either way. + * a cancel resets sessionId, so a late answer for a discarded session is dropped */ +export const applyAuthorization = ( + sessionId: string, + result: MfaConfigAuthorizeResult, +): void => { + const store = useConfigureMfaStore.getState(); + if (store.sessionId !== sessionId) return; + store.authorize(result); +}; + /** A copy the proxy still holds expires on its own, so a failed cancel is not worth raising. */ export const discardMfaConfiguration = async (): Promise => { const { sessionId } = useConfigureMfaStore.getState(); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useMfaConfigErrorHandler.ts b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useMfaConfigErrorHandler.ts index 6fed37832..ad95430c9 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/hooks/useMfaConfigErrorHandler.ts +++ b/new-ui/src/pages/full/ConfigureMfaPage/hooks/useMfaConfigErrorHandler.ts @@ -1,12 +1,17 @@ import { error as logError } from '@tauri-apps/plugin-log'; import { useCallback } from 'react'; import { + isMfaConfigAlreadyAuthorized, isMfaConfigCancelled, + isMfaConfigFailedPrecondition, + isMfaConfigForbidden, isMfaConfigInvalidCode, + isMfaConfigMethodNotConfigured, isMfaConfigNetworkError, isMfaConfigProxyError, isMfaConfigSecurityKeyError, isMfaConfigSessionExpired, + isMfaConfigTimeout, mfaErrorMessage, } from '../../../../shared/rust-api/mfaError'; import { showEdgeComsError } from '../components/EdgeComsError/useEdgeComsErrorStore'; @@ -41,11 +46,34 @@ export const useMfaConfigErrorHandler = ({ setError(hasCodeInput ? 'Invalid code' : mfaErrorMessage(err)); return; } + // an OpenID login as another user lands here too, Core ends the session for it if (isMfaConfigSessionExpired(err)) { setError('Configuration session expired, start again.'); onSessionExpired(); return; } + if (isMfaConfigAlreadyAuthorized(err)) { + setError('This session was already verified, start again.'); + onSessionExpired(); + return; + } + if (isMfaConfigMethodNotConfigured(err)) { + setError('This method is no longer set up for your account.'); + return; + } + if (isMfaConfigForbidden(err)) { + setError(mfaErrorMessage(err)); + return; + } + // a consumed or replaced challenge, the next attempt fetches a fresh one + if (isMfaConfigFailedPrecondition(err)) { + setError('Verification expired, try again.'); + return; + } + if (isMfaConfigTimeout(err)) { + setError('Sign-in timed out, try again.'); + return; + } // The backend writes these for the user (no key, wrong PIN, no touch), so show as is. if (isMfaConfigSecurityKeyError(err)) { setError(mfaErrorMessage(err)); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/types.ts b/new-ui/src/pages/full/ConfigureMfaPage/types.ts index 4880a77f2..3be9ef2ba 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/types.ts +++ b/new-ui/src/pages/full/ConfigureMfaPage/types.ts @@ -26,7 +26,12 @@ export type MfaFactor = { repeatable: boolean; }; -/** Factors that can authorize a session, most preferred first. Core only accepts code factors. */ -export const MFA_VERIFICATION_METHODS = [MfaMethod.Totp, MfaMethod.Email] as const; +/** most preferred first */ +export const mfaVerificationMethods = [ + MfaMethod.Totp, + MfaMethod.Email, + MfaMethod.Fido2, + MfaMethod.Oidc, +] as const; -export type MfaVerificationMethod = (typeof MFA_VERIFICATION_METHODS)[number]; +export type MfaVerificationMethod = (typeof mfaVerificationMethods)[number]; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/utils.ts b/new-ui/src/pages/full/ConfigureMfaPage/utils.ts index cb6cee57f..a2c3c2f8f 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/utils.ts +++ b/new-ui/src/pages/full/ConfigureMfaPage/utils.ts @@ -6,10 +6,10 @@ import { import { ConfigureMfaStep, type ConfigureMfaStepValue, - MFA_VERIFICATION_METHODS, MFA_WIZARD_STEPS, type MfaFactor, type MfaVerificationMethod, + mfaVerificationMethods, } from './types'; /** Keyed on the shared list, so a new factor fails to compile until mapped here. */ @@ -48,12 +48,15 @@ export const isMfaFactorOfferable = ( return factor.repeatable || !configuredMethods.includes(method); }; -/** A session reports only code factors, others come from the instance snapshot. */ +const codeMfaMethods: MfaMethodValue[] = [MfaMethod.Totp, MfaMethod.Email]; + +/** every Core reports code factors in a session but older ones leave out FIDO2, + * so the instance snapshot stays the source for the rest */ export const isCodeMfaMethod = (method: MfaMethodValue): boolean => - MFA_VERIFICATION_METHODS.some((code) => code === method); + codeMfaMethods.includes(method); -/** Most preferred first. */ +/** ordered by preference, not by the input */ export const verificationMethodsOf = ( - configuredMethods: MfaMethodValue[], + availableMethods: MfaMethodValue[], ): MfaVerificationMethod[] => - MFA_VERIFICATION_METHODS.filter((method) => configuredMethods.includes(method)); + mfaVerificationMethods.filter((method) => availableMethods.includes(method)); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx index 8ddb79d79..3548ed070 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx @@ -1,12 +1,12 @@ -import { useMemo } from 'react'; import { MfaMethod } from '../../../../shared/rust-api/types'; import { isPresent } from '../../../../shared/utils/isPresent'; import { useConfigureMfaStore } from '../hooks/useConfigureMfaStore'; import type { MfaVerificationMethod } from '../types'; -import { verificationMethodsOf } from '../utils'; import { ConfigureSelectMethodsStep } from './ConfigureSelectMethodsStep/ConfigureSelectMethodsStep'; import { ConfigureSelectVerificationStep } from './ConfigureSelectVerificationStep/ConfigureSelectVerificationStep'; import { ConfigureVerifyEmailStep } from './ConfigureVerifyEmailStep/ConfigureVerifyEmailStep'; +import { ConfigureVerifyFido2Step } from './ConfigureVerifyFido2Step/ConfigureVerifyFido2Step'; +import { ConfigureVerifyOidcStep } from './ConfigureVerifyOidcStep/ConfigureVerifyOidcStep'; import { ConfigureVerifyTotpStep } from './ConfigureVerifyTotpStep/ConfigureVerifyTotpStep'; type Props = { @@ -16,15 +16,10 @@ type Props = { /** Picks what the wizard sets up, then verifies the session with an existing factor. */ export const ConfigureMfaVerify = ({ onCancel, onSessionExpired }: Props) => { - const configuredMethods = useConfigureMfaStore((s) => s.configuredMethods); + const candidates = useConfigureMfaStore((s) => s.verificationMethods); const methodsSelected = useConfigureMfaStore((s) => isPresent(s.selectedMethods)); const verificationMethod = useConfigureMfaStore((s) => s.verificationMethod); - const candidates = useMemo( - () => verificationMethodsOf(configuredMethods), - [configuredMethods], - ); - if (!methodsSelected) { return ; } @@ -33,23 +28,18 @@ export const ConfigureMfaVerify = ({ onCancel, onSessionExpired }: Props) => { return ; } + // for the type only, a session always offers a method or falls back to email const method: MfaVerificationMethod = verificationMethod ?? candidates[0] ?? MfaMethod.Email; switch (method) { case MfaMethod.Totp: - return ( - - ); + return ; case MfaMethod.Email: - return ( - - ); + return ; + case MfaMethod.Fido2: + return ; + case MfaMethod.Oidc: + return ; } }; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx index 9d1834469..891111769 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectVerificationStep/ConfigureSelectVerificationStep.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react'; +import { useState } from 'react'; import { Button } from '../../../../../shared/components/Button/Button'; import { ButtonVariant } from '../../../../../shared/components/Button/types'; import { Controls } from '../../../../../shared/components/Controls/Controls'; @@ -7,18 +7,12 @@ import { FullPage } from '../../../../../shared/layouts/FullPage/FullPage'; import { ConfigureMfaVerificatorFactorSelector } from '../../components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector'; import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; import type { MfaVerificationMethod } from '../../types'; -import { verificationMethodsOf } from '../../utils'; import '../style.scss'; import './style.scss'; -/** Shown only when more than one code factor is configured. */ +/** shown only when the session offers more than one method */ export const ConfigureSelectVerificationStep = () => { - const configuredMethods = useConfigureMfaStore((s) => s.configuredMethods); - - const methods = useMemo( - () => verificationMethodsOf(configuredMethods), - [configuredMethods], - ); + const methods = useConfigureMfaStore((s) => s.verificationMethods); const [selected, setSelected] = useState(methods[0]); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyEmailStep/ConfigureVerifyEmailStep.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyEmailStep/ConfigureVerifyEmailStep.tsx index a68937e71..6b4ae0d3b 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyEmailStep/ConfigureVerifyEmailStep.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyEmailStep/ConfigureVerifyEmailStep.tsx @@ -9,23 +9,19 @@ import { FullPage } from '../../../../../shared/layouts/FullPage/FullPage'; import { api } from '../../../../../shared/rust-api/api'; import { MfaMethod } from '../../../../../shared/rust-api/types'; import { isPresent } from '../../../../../shared/utils/isPresent'; -import { - discardMfaConfiguration, - useConfigureMfaStore, -} from '../../hooks/useConfigureMfaStore'; +import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; import { useMfaConfigErrorHandler } from '../../hooks/useMfaConfigErrorHandler'; import '../style.scss'; const CODE_LENGTH = 6; interface Props { - onCancel: () => void; /** The session outlived its deadline, so the whole flow has to start over. */ onSessionExpired: () => void; } /** The fallback when no factor exists, or email picked from the configured code factors. */ -export const ConfigureVerifyEmailStep = ({ onCancel, onSessionExpired }: Props) => { +export const ConfigureVerifyEmailStep = ({ onSessionExpired }: Props) => { const sessionId = useConfigureMfaStore((s) => s.sessionId); const [code, setCode] = useState(null); @@ -75,12 +71,7 @@ export const ConfigureVerifyEmailStep = ({ onCancel, onSessionExpired }: Props) onError: (err) => handleApiError(err), }); - const { mutate: cancel, isPending: isCancelling } = useMutation({ - mutationFn: discardMfaConfiguration, - onSettled: onCancel, - }); - - const isBusy = isRequestingCode || isSubmitting || isCancelling; + const isBusy = isRequestingCode || isSubmitting; const handleSubmit = useCallback( (pastedCode?: string) => { @@ -133,11 +124,11 @@ export const ConfigureVerifyEmailStep = ({ onCancel, onSessionExpired }: Props) { - cancel(); + useConfigureMfaStore.getState().backFromVerification(); }} /> @@ -145,14 +136,14 @@ export const ConfigureVerifyEmailStep = ({ onCancel, onSessionExpired }: Props) text="Resend code" variant={ButtonVariant.Secondary} loading={isRequestingCode} - disabled={isSubmitting || isCancelling} + disabled={isSubmitting} onClick={handleResend} /> { handleSubmit(); }} diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/ConfigureVerifyFido2Step.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/ConfigureVerifyFido2Step.tsx new file mode 100644 index 000000000..cc3fce410 --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/ConfigureVerifyFido2Step.tsx @@ -0,0 +1,104 @@ +import { Enter } from '@fluentui/keyboard-keys'; +import { useState } from 'react'; +import { Button } from '../../../../../shared/components/Button/Button'; +import { ButtonVariant } from '../../../../../shared/components/Button/types'; +import { Controls } from '../../../../../shared/components/Controls/Controls'; +import { FullPageTitle } from '../../../../../shared/components/FullPageTitle/FullPageTitle'; +import { Input } from '../../../../../shared/components/Input/Input'; +import { Fido2TouchPrompt } from '../../../../../shared/components/LocationCard/components/Fido2TouchPrompt/Fido2TouchPrompt'; +import { FullPage } from '../../../../../shared/layouts/FullPage/FullPage'; +import { fido2CollectsPinInApp } from '../../../../../shared/rust-api/fido2'; +import { isPresent } from '../../../../../shared/utils/isPresent'; +import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; +import { useConfigureVerifyFido2 } from './useConfigureVerifyFido2'; +import '../style.scss'; + +interface Props { + onSessionExpired: () => void; +} + +export const ConfigureVerifyFido2Step = ({ onSessionExpired }: Props) => { + const collectsPin = fido2CollectsPinInApp(); + const [pin, setPin] = useState(null); + const { verify, abort, isVerifying, isAwaitingTouch, error, setError } = + useConfigureVerifyFido2({ onSessionExpired, autoStart: !collectsPin }); + + const handleVerify = () => { + if (isVerifying) return; + if (!collectsPin) { + void verify(null); + return; + } + if (!isPresent(pin) || pin.length === 0) { + setError('Enter PIN'); + return; + } + void verify(pin); + }; + + const handleBack = async () => { + // Core holds one pending attempt per session, so abort it before another method starts + await abort(); + useConfigureMfaStore.getState().backFromVerification(); + }; + + return ( + + + {isAwaitingTouch && } + {!isAwaitingTouch && ( + <> + + + {collectsPin + ? 'Insert your security key and enter its PIN to continue.' + : 'Insert your security key and continue in the prompt your system shows.'} + + + {collectsPin && ( + { + if (e.key === Enter) handleVerify(); + }} + > + { + setPin(isPresent(value) ? String(value) : null); + setError(null); + }} + error={error} + /> + + )} + {!collectsPin && isPresent(error) && {error}} + > + )} + + { + void handleBack(); + }} + /> + + + + + + ); +}; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/useConfigureVerifyFido2.test.ts b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/useConfigureVerifyFido2.test.ts new file mode 100644 index 000000000..cddd8667a --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/useConfigureVerifyFido2.test.ts @@ -0,0 +1,120 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; +import { useConfigureVerifyFido2 } from './useConfigureVerifyFido2'; + +const mocks = vi.hoisted(() => ({ + error: vi.fn(), + listen: vi.fn(), + mfaConfigAbortAttempt: vi.fn(), + mfaConfigAuthorizeFido2: vi.fn(), +})); + +vi.mock('@tauri-apps/api/event', () => ({ listen: mocks.listen })); +vi.mock('@tauri-apps/plugin-log', () => ({ error: mocks.error })); +vi.mock('../../../../../shared/rust-api/api', () => ({ + api: { + mfaConfigAbortAttempt: mocks.mfaConfigAbortAttempt, + mfaConfigAuthorizeFido2: mocks.mfaConfigAuthorizeFido2, + }, +})); + +const sessionId = 'session-1'; +const authorizeResult = { deadline_timestamp: 1_900_000_000, recovery_codes: [] }; + +const deferred = () => { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +const renderFido2 = () => + renderHook(() => + useConfigureVerifyFido2({ onSessionExpired: vi.fn(), autoStart: false }), + ); + +describe('useConfigureVerifyFido2', () => { + beforeEach(() => { + vi.clearAllMocks(); + useConfigureMfaStore.getState().reset(); + useConfigureMfaStore.setState({ sessionId }); + mocks.listen.mockResolvedValue(vi.fn()); + mocks.mfaConfigAbortAttempt.mockResolvedValue(undefined); + mocks.mfaConfigAuthorizeFido2.mockResolvedValue(authorizeResult); + }); + + it('does not start a ceremony when unmounted while listen is pending', async () => { + const listening = deferred<() => void>(); + const unlisten = vi.fn(); + mocks.listen.mockReturnValue(listening.promise); + const { result, unmount } = renderFido2(); + + let verifying: Promise | undefined; + act(() => { + verifying = result.current.verify(null); + }); + unmount(); + listening.resolve(unlisten); + await verifying; + + expect(mocks.mfaConfigAuthorizeFido2).not.toHaveBeenCalled(); + expect(unlisten).toHaveBeenCalledTimes(1); + }); + + it('recovers when listen rejects', async () => { + mocks.listen.mockRejectedValueOnce(new Error('no event bridge')); + const { result } = renderFido2(); + + await act(async () => { + await result.current.verify(null); + }); + expect(result.current.isVerifying).toBe(false); + + await act(async () => { + await result.current.verify(null); + }); + expect(mocks.mfaConfigAuthorizeFido2).toHaveBeenCalledTimes(1); + }); + + it('keeps an authorization that lands after unmount', async () => { + const authorizing = deferred(); + mocks.mfaConfigAuthorizeFido2.mockReturnValue(authorizing.promise); + const { result, unmount } = renderFido2(); + + let verifying: Promise | undefined; + await act(async () => { + verifying = result.current.verify(null); + await Promise.resolve(); + }); + unmount(); + authorizing.resolve(authorizeResult); + await verifying; + + expect(useConfigureMfaStore.getState().authorized).toBe(true); + }); + + it('drops an authorization for a discarded session', async () => { + const authorizing = deferred(); + mocks.mfaConfigAuthorizeFido2.mockReturnValue(authorizing.promise); + const { result } = renderFido2(); + + let verifying: Promise | undefined; + await act(async () => { + verifying = result.current.verify(null); + await Promise.resolve(); + }); + act(() => { + useConfigureMfaStore.getState().reset(); + }); + await act(async () => { + authorizing.resolve(authorizeResult); + await verifying; + }); + + expect(useConfigureMfaStore.getState().authorized).toBe(false); + }); +}); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/useConfigureVerifyFido2.ts b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/useConfigureVerifyFido2.ts new file mode 100644 index 000000000..51479fefe --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyFido2Step/useConfigureVerifyFido2.ts @@ -0,0 +1,107 @@ +import { listen, type UnlistenFn } from '@tauri-apps/api/event'; +import { error as logError } from '@tauri-apps/plugin-log'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import { api } from '../../../../../shared/rust-api/api'; +import { fido2ShowsTouchPrompt } from '../../../../../shared/rust-api/fido2'; +import { isMfaConfigInvalidCode } from '../../../../../shared/rust-api/mfaError'; +import { TauriEvent } from '../../../../../shared/rust-api/types'; +import { isPresent } from '../../../../../shared/utils/isPresent'; +import { + applyAuthorization, + useConfigureMfaStore, +} from '../../hooks/useConfigureMfaStore'; +import { useMfaConfigErrorHandler } from '../../hooks/useMfaConfigErrorHandler'; + +type Options = { + onSessionExpired: () => void; + autoStart: boolean; +}; + +/** each verify fetches a fresh single-use challenge, so a retry is just another call */ +export const useConfigureVerifyFido2 = ({ onSessionExpired, autoStart }: Options) => { + const [isVerifying, setIsVerifying] = useState(false); + const [isAwaitingTouch, setIsAwaitingTouch] = useState(false); + const [error, setError] = useState(null); + const runningRef = useRef(false); + const mountedRef = useRef(true); + + const handleApiError = useMfaConfigErrorHandler({ + context: 'Security key MFA configuration verification failed', + setError, + onSessionExpired, + fallback: 'Verification failed', + hasCodeInput: false, + }); + + const verify = useCallback( + async (pin: string | null) => { + const { sessionId } = useConfigureMfaStore.getState(); + if (runningRef.current || !isPresent(sessionId)) return; + runningRef.current = true; + setIsVerifying(true); + setError(null); + let unlisten: UnlistenFn | undefined; + try { + // listen before invoking, the touch event fires while the call is still running + unlisten = await listen(TauriEvent.MfaConfigFido2Touch, () => { + // a platform that runs the ceremony shows its own prompt, ours would sit behind it + if (mountedRef.current) setIsAwaitingTouch(fido2ShowsTouchPrompt()); + }); + // an abort sent while listen was pending found no ceremony to stop + if (!mountedRef.current) return; + const result = await api.mfaConfigAuthorizeFido2(sessionId, pin); + applyAuthorization(sessionId, result); + } catch (err) { + if (!mountedRef.current) return; + // Core says "invalid code", which means nothing next to a security key + if (isMfaConfigInvalidCode(err)) { + void logError(`Security key MFA configuration verification rejected: ${err}`); + setError('Security key verification failed, try again.'); + return; + } + handleApiError(err); + } finally { + unlisten?.(); + runningRef.current = false; + if (mountedRef.current) { + setIsAwaitingTouch(false); + setIsVerifying(false); + } + } + }, + [handleApiError], + ); + + const abort = useCallback(async () => { + const { sessionId } = useConfigureMfaStore.getState(); + if (!runningRef.current || !isPresent(sessionId)) return; + try { + await api.mfaConfigAbortAttempt(sessionId); + } catch (err) { + void logError(`Failed to abort security key verification: ${err}`); + } + }, []); + + // biome-ignore lint/correctness/useExhaustiveDependencies: aborts on unmount only + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + void abort(); + }; + }, []); + + // deferred a tick so a StrictMode replay or an instant unmount clears it before it runs + // biome-ignore lint/correctness/useExhaustiveDependencies: auto-start only on mount + useEffect(() => { + if (!autoStart) return; + const timer = window.setTimeout(() => { + void verify(null); + }, 0); + return () => { + window.clearTimeout(timer); + }; + }, [autoStart]); + + return { verify, abort, isVerifying, isAwaitingTouch, error, setError }; +}; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/ConfigureVerifyOidcStep.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/ConfigureVerifyOidcStep.tsx new file mode 100644 index 000000000..5d8029196 --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/ConfigureVerifyOidcStep.tsx @@ -0,0 +1,63 @@ +import { Button } from '../../../../../shared/components/Button/Button'; +import { ButtonVariant } from '../../../../../shared/components/Button/types'; +import { Controls } from '../../../../../shared/components/Controls/Controls'; +import { FullPageTitle } from '../../../../../shared/components/FullPageTitle/FullPageTitle'; +import { FullPage } from '../../../../../shared/layouts/FullPage/FullPage'; +import { isPresent } from '../../../../../shared/utils/isPresent'; +import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; +import { useConfigureVerifyOidc } from './useConfigureVerifyOidc'; +import '../style.scss'; + +interface Props { + onSessionExpired: () => void; +} + +export const ConfigureVerifyOidcStep = ({ onSessionExpired }: Props) => { + const { start, abort, isOpening, isPolling, error } = useConfigureVerifyOidc({ + onSessionExpired, + }); + + const handleBack = async () => { + // Core holds one pending attempt per session, so abort it before another method starts + await abort(); + useConfigureMfaStore.getState().backFromVerification(); + }; + + return ( + + + + + {isPolling + ? 'Complete the sign-in in your browser. This page will update automatically.' + : 'Authenticate via your OpenID provider. A browser window will open for you to sign in.'} + + + {isPresent(error) && {error}} + + { + void start(); + }} + /> + + + { + void handleBack(); + }} + /> + + + ); +}; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/useConfigureVerifyOidc.test.ts b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/useConfigureVerifyOidc.test.ts new file mode 100644 index 000000000..688226fc6 --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/useConfigureVerifyOidc.test.ts @@ -0,0 +1,77 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; +import { useConfigureVerifyOidc } from './useConfigureVerifyOidc'; + +const mocks = vi.hoisted(() => ({ + error: vi.fn(), + mfaConfigAbortAttempt: vi.fn(), + mfaConfigAuthorizeOidc: vi.fn(), + mfaConfigOidcUrl: vi.fn(), + openLink: vi.fn(), +})); + +vi.mock('@tauri-apps/plugin-log', () => ({ error: mocks.error })); +vi.mock('../../../../../shared/rust-api/api', () => ({ + api: { + mfaConfigAbortAttempt: mocks.mfaConfigAbortAttempt, + mfaConfigAuthorizeOidc: mocks.mfaConfigAuthorizeOidc, + mfaConfigOidcUrl: mocks.mfaConfigOidcUrl, + openLink: mocks.openLink, + }, +})); + +const sessionId = 'session-1'; +const oidcUrl = 'https://idp.test/auth'; +const authorizeResult = { deadline_timestamp: 1_900_000_000, recovery_codes: [] }; + +const deferred = () => { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +}; + +const renderOidc = () => + renderHook(() => useConfigureVerifyOidc({ onSessionExpired: vi.fn() })); + +describe('useConfigureVerifyOidc', () => { + beforeEach(() => { + vi.clearAllMocks(); + useConfigureMfaStore.getState().reset(); + useConfigureMfaStore.setState({ sessionId }); + mocks.mfaConfigAbortAttempt.mockResolvedValue(undefined); + mocks.mfaConfigAuthorizeOidc.mockResolvedValue(authorizeResult); + mocks.mfaConfigOidcUrl.mockResolvedValue(oidcUrl); + mocks.openLink.mockResolvedValue(undefined); + }); + + it('opens the browser and polls', async () => { + const { result } = renderOidc(); + + await act(async () => { + await result.current.start(); + }); + + expect(mocks.openLink).toHaveBeenCalledWith(oidcUrl); + expect(mocks.mfaConfigAuthorizeOidc).toHaveBeenCalledWith(sessionId); + }); + + it('does not open the browser when unmounted while the URL is pending', async () => { + const fetching = deferred(); + mocks.mfaConfigOidcUrl.mockReturnValue(fetching.promise); + const { result, unmount } = renderOidc(); + + let starting: Promise | undefined; + act(() => { + starting = result.current.start(); + }); + unmount(); + fetching.resolve(oidcUrl); + await starting; + + expect(mocks.openLink).not.toHaveBeenCalled(); + expect(mocks.mfaConfigAuthorizeOidc).not.toHaveBeenCalled(); + }); +}); diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/useConfigureVerifyOidc.ts b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/useConfigureVerifyOidc.ts new file mode 100644 index 000000000..48ca1495a --- /dev/null +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyOidcStep/useConfigureVerifyOidc.ts @@ -0,0 +1,86 @@ +import { error as logError } from '@tauri-apps/plugin-log'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import { api } from '../../../../../shared/rust-api/api'; +import { isPresent } from '../../../../../shared/utils/isPresent'; +import { + applyAuthorization, + useConfigureMfaStore, +} from '../../hooks/useConfigureMfaStore'; +import { useMfaConfigErrorHandler } from '../../hooks/useMfaConfigErrorHandler'; + +type Options = { + onSessionExpired: () => void; +}; + +/** reopening the page starts a new attempt on Core, and the running poll picks it up */ +export const useConfigureVerifyOidc = ({ onSessionExpired }: Options) => { + const [isOpening, setIsOpening] = useState(false); + const [isPolling, setIsPolling] = useState(false); + const [error, setError] = useState(null); + const pollingRef = useRef(false); + const mountedRef = useRef(true); + + const handleApiError = useMfaConfigErrorHandler({ + context: 'OpenID MFA configuration verification failed', + setError, + onSessionExpired, + fallback: 'Verification failed', + hasCodeInput: false, + }); + + const poll = useCallback( + async (sessionId: string) => { + pollingRef.current = true; + setIsPolling(true); + try { + const result = await api.mfaConfigAuthorizeOidc(sessionId); + applyAuthorization(sessionId, result); + } catch (err) { + if (mountedRef.current) handleApiError(err); + } finally { + pollingRef.current = false; + if (mountedRef.current) setIsPolling(false); + } + }, + [handleApiError], + ); + + const start = useCallback(async () => { + const { sessionId } = useConfigureMfaStore.getState(); + if (!isPresent(sessionId)) return; + setIsOpening(true); + setError(null); + try { + const url = await api.mfaConfigOidcUrl(sessionId); + if (!mountedRef.current) return; + await api.openLink(url); + } catch (err) { + if (mountedRef.current) handleApiError(err); + return; + } finally { + if (mountedRef.current) setIsOpening(false); + } + if (mountedRef.current && !pollingRef.current) void poll(sessionId); + }, [handleApiError, poll]); + + const abort = useCallback(async () => { + const { sessionId } = useConfigureMfaStore.getState(); + if (!pollingRef.current || !isPresent(sessionId)) return; + try { + await api.mfaConfigAbortAttempt(sessionId); + } catch (err) { + void logError(`Failed to abort OpenID verification: ${err}`); + } + }, []); + + // biome-ignore lint/correctness/useExhaustiveDependencies: aborts on unmount only + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + void abort(); + }; + }, []); + + return { start, abort, isOpening, isPolling, error }; +}; diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyTotpStep/ConfigureVerifyTotpStep.tsx b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyTotpStep/ConfigureVerifyTotpStep.tsx index 45c866abc..eb3b2d3d6 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyTotpStep/ConfigureVerifyTotpStep.tsx +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureVerifyTotpStep/ConfigureVerifyTotpStep.tsx @@ -9,23 +9,19 @@ import { FullPage } from '../../../../../shared/layouts/FullPage/FullPage'; import { api } from '../../../../../shared/rust-api/api'; import { MfaMethod } from '../../../../../shared/rust-api/types'; import { isPresent } from '../../../../../shared/utils/isPresent'; -import { - discardMfaConfiguration, - useConfigureMfaStore, -} from '../../hooks/useConfigureMfaStore'; +import { useConfigureMfaStore } from '../../hooks/useConfigureMfaStore'; import { useMfaConfigErrorHandler } from '../../hooks/useMfaConfigErrorHandler'; import '../style.scss'; const CODE_LENGTH = 6; interface Props { - onCancel: () => void; /** The session outlived its deadline, so the whole flow has to start over. */ onSessionExpired: () => void; } /** The code comes from an app the user already has, so there is nothing to request first. */ -export const ConfigureVerifyTotpStep = ({ onCancel, onSessionExpired }: Props) => { +export const ConfigureVerifyTotpStep = ({ onSessionExpired }: Props) => { const sessionId = useConfigureMfaStore((s) => s.sessionId); const [code, setCode] = useState(null); @@ -49,16 +45,9 @@ export const ConfigureVerifyTotpStep = ({ onCancel, onSessionExpired }: Props) = onError: (err) => handleApiError(err), }); - const { mutate: cancel, isPending: isCancelling } = useMutation({ - mutationFn: discardMfaConfiguration, - onSettled: onCancel, - }); - - const isBusy = isSubmitting || isCancelling; - const handleSubmit = useCallback( (pastedCode?: string) => { - if (isBusy) return; + if (isSubmitting) return; const toSubmit = (pastedCode ?? code)?.trim(); if (toSubmit?.length !== CODE_LENGTH) { setError('Enter a valid code'); @@ -66,7 +55,7 @@ export const ConfigureVerifyTotpStep = ({ onCancel, onSessionExpired }: Props) = } submitCode(toSubmit); }, - [code, isBusy, submitCode], + [code, isSubmitting, submitCode], ); // Only real input clears the error, CodeInput's own reset passes ''. @@ -94,16 +83,16 @@ export const ConfigureVerifyTotpStep = ({ onCancel, onSessionExpired }: Props) = onChange={handleCodeChange} error={error} onSubmit={handleSubmit} - loading={isBusy} + loading={isSubmitting} /> { - cancel(); + useConfigureMfaStore.getState().backFromVerification(); }} /> @@ -111,7 +100,6 @@ export const ConfigureVerifyTotpStep = ({ onCancel, onSessionExpired }: Props) = text="Verify" variant={ButtonVariant.Primary} loading={isSubmitting} - disabled={isCancelling} onClick={() => { handleSubmit(); }} diff --git a/new-ui/src/pages/full/ConfigureMfaPage/verify/style.scss b/new-ui/src/pages/full/ConfigureMfaPage/verify/style.scss index dd4c1598a..2edba5094 100644 --- a/new-ui/src/pages/full/ConfigureMfaPage/verify/style.scss +++ b/new-ui/src/pages/full/ConfigureMfaPage/verify/style.scss @@ -1,7 +1,7 @@ .configure-mfa-verify-page { padding: var(--spacing-md) var(--spacing-lg); - > .description { + & > .description { display: flex; flex-flow: column; row-gap: var(--spacing-xl); @@ -15,6 +15,33 @@ } } + & > .error { + padding-bottom: var(--spacing-xl); + box-sizing: border-box; + font: var(--t-body-sm-400); + color: var(--fg-critical); + } + + & > .pin-track { + padding-bottom: var(--spacing-3xl); + box-sizing: border-box; + } + + & > .fido2-touch-prompt { + width: 100%; + padding-top: var(--spacing-xl); + box-sizing: border-box; + } + + & > .oidc-action { + display: flex; + flex-flow: row nowrap; + align-items: center; + justify-content: center; + padding: var(--spacing-2xl) 0 var(--spacing-3xl); + box-sizing: border-box; + } + .code-track { padding-bottom: var(--spacing-3xl); diff --git a/new-ui/src/pages/playground/components/PlaygroundTestMfaVerificatorFactorSelector/PlaygroundTestMfaVerificatorFactorSelector.tsx b/new-ui/src/pages/playground/components/PlaygroundTestMfaVerificatorFactorSelector/PlaygroundTestMfaVerificatorFactorSelector.tsx index 92f8a65f5..37f50b470 100644 --- a/new-ui/src/pages/playground/components/PlaygroundTestMfaVerificatorFactorSelector/PlaygroundTestMfaVerificatorFactorSelector.tsx +++ b/new-ui/src/pages/playground/components/PlaygroundTestMfaVerificatorFactorSelector/PlaygroundTestMfaVerificatorFactorSelector.tsx @@ -4,7 +4,12 @@ import { MfaMethod, type MfaMethodValue } from '../../../../shared/rust-api/type import { ConfigureMfaVerificatorFactorSelector } from '../../../full/ConfigureMfaPage/components/ConfigureMfaVerificatorFactorSelector/ConfigureMfaVerificatorFactorSelector'; import { PlaygroundCard } from '../PlaygroundCard/PlaygroundCard'; -const factors: MfaMethodValue[] = [MfaMethod.Totp, MfaMethod.Email]; +const factors: MfaMethodValue[] = [ + MfaMethod.Totp, + MfaMethod.Email, + MfaMethod.Fido2, + MfaMethod.Oidc, +]; export const PlaygroundTestMfaVerificatorFactorSelector = () => { const [selected, setSelected] = useState(); diff --git a/new-ui/src/shared/components/LocationCard/components/LocationCardMfaEdit/LocationCardMfaEdit.tsx b/new-ui/src/shared/components/LocationCard/components/LocationCardMfaEdit/LocationCardMfaEdit.tsx index 02540c854..2d9783309 100644 --- a/new-ui/src/shared/components/LocationCard/components/LocationCardMfaEdit/LocationCardMfaEdit.tsx +++ b/new-ui/src/shared/components/LocationCard/components/LocationCardMfaEdit/LocationCardMfaEdit.tsx @@ -7,6 +7,7 @@ import type { InstanceInfo, LocationInfo } from '../../../../rust-api/types'; import { ConnectionAbility, type ConnectionAbilityValue, + hasMfaMethodChoice, mfaStepCount, mfaStepsToText, mfaToText, @@ -42,8 +43,8 @@ export const LocationCardMfaEdit = ({ ? mfaStepsToText(stepCount) : mfaToText(resolveMfaStepPlan(location)[0], instance); - // `Configurable` stays editable, configuring a factor unblocks it. - const canEdit = connectionAbility !== ConnectionAbility.Unavailable; + const canEdit = + connectionAbility === ConnectionAbility.Available && hasMfaMethodChoice(location); const canConfigure = connectionAbility === ConnectionAbility.Configurable; diff --git a/new-ui/src/shared/components/ScrollContainer/style.scss b/new-ui/src/shared/components/ScrollContainer/style.scss index 6a8248041..0a79a9c32 100644 --- a/new-ui/src/shared/components/ScrollContainer/style.scss +++ b/new-ui/src/shared/components/ScrollContainer/style.scss @@ -8,6 +8,6 @@ &.windows { scrollbar-gutter: stable; overflow-y: scroll; - padding-right: 6px; + padding-right: var(--scroll-container-gutter); } } diff --git a/new-ui/src/shared/rust-api/api.ts b/new-ui/src/shared/rust-api/api.ts index 75fe80268..cfdec48a8 100644 --- a/new-ui/src/shared/rust-api/api.ts +++ b/new-ui/src/shared/rust-api/api.ts @@ -239,7 +239,7 @@ const mfaFido2Pin = ( locationId: number, methods: MfaMethodValue[], token: string | null, - // Null where the platform collects the PIN itself - see `fido2CollectsPinInApp`. + // null where the platform collects the PIN itself, see fido2CollectsPinInApp pin: string | null, ): Promise => invoke(TauriCommand.MfaFido2Pin, { instanceId, locationId, methods, token, pin }); @@ -277,6 +277,27 @@ const mfaConfigAuthorize = ( ): Promise => invoke(TauriCommand.MfaConfigAuthorize, { sessionId, method, code }); +// runs challenge, ceremony and assertion in one call, so a retry is just another call. +// emits mfa-config-fido2-touch while the key waits +const mfaConfigAuthorizeFido2 = ( + sessionId: string, + // null where the platform collects the PIN itself, see fido2CollectsPinInApp + pin: string | null, +): Promise => + invoke(TauriCommand.MfaConfigAuthorizeFido2, { sessionId, pin }); + +// each open of the returned page supersedes the previous OpenID attempt +const mfaConfigOidcUrl = (sessionId: string): Promise => + invoke(TauriCommand.MfaConfigOidcUrl, { sessionId }); + +// long-running, resolves once the login opened from mfaConfigOidcUrl completes +const mfaConfigAuthorizeOidc = (sessionId: string): Promise => + invoke(TauriCommand.MfaConfigAuthorizeOidc, { sessionId }); + +// unlike mfaConfigCancel, the session stays usable for another method +const mfaConfigAbortAttempt = (sessionId: string): Promise => + invoke(TauriCommand.MfaConfigAbortAttempt, { sessionId }); + const mfaConfigSetupStart = ( sessionId: string, method: MfaMethodValue, @@ -295,7 +316,7 @@ const mfaConfigSetupFinish = ( const mfaConfigSetupFido2 = ( sessionId: string, name: string, - // Null where the platform collects the PIN itself - see `fido2CollectsPinInApp`. + // null where the platform collects the PIN itself, see fido2CollectsPinInApp pin: string | null, ): Promise => invoke(TauriCommand.MfaConfigSetupFido2, { sessionId, name, pin }); @@ -370,6 +391,10 @@ export const api = { mfaConfigStart, mfaConfigSendCode, mfaConfigAuthorize, + mfaConfigAuthorizeFido2, + mfaConfigOidcUrl, + mfaConfigAuthorizeOidc, + mfaConfigAbortAttempt, mfaConfigSetupStart, mfaConfigSetupFinish, mfaConfigSetupFido2, diff --git a/new-ui/src/shared/rust-api/mfaError.ts b/new-ui/src/shared/rust-api/mfaError.ts index ab73a940f..12a66550b 100644 --- a/new-ui/src/shared/rust-api/mfaError.ts +++ b/new-ui/src/shared/rust-api/mfaError.ts @@ -77,11 +77,31 @@ export const isMfaConfigSecurityKeyError = (err: unknown): boolean => export const isMfaConfigCancelled = (err: unknown): boolean => parseMfaError(err)?.type === 'cancelled'; +/** e.g. the last security key was removed while the session was open */ +export const isMfaConfigMethodNotConfigured = (err: unknown): boolean => + parseMfaError(err)?.type === 'method_not_configured'; + +/** e.g. an inactive user or too many attempts, Core words these for the user */ +export const isMfaConfigForbidden = (err: unknown): boolean => + parseMfaError(err)?.type === 'forbidden'; + +/** the response that authorized it was lost, so only a new session gets past it */ +export const isMfaConfigAlreadyAuthorized = (err: unknown): boolean => + parseMfaError(err)?.type === 'already_authorized'; + +/** e.g. a consumed or replaced FIDO2 challenge, a fresh attempt fixes it */ +export const isMfaConfigFailedPrecondition = (err: unknown): boolean => + parseMfaError(err)?.type === 'failed_precondition'; + +/** only the OpenID wait times out */ +export const isMfaConfigTimeout = (err: unknown): boolean => + parseMfaError(err)?.type === 'timeout'; + /** The request never reached the proxy, unlike `proxy_error` where it answered. */ export const isMfaConfigNetworkError = (err: unknown): boolean => parseMfaError(err)?.type === 'network_error'; -/** A status with no specific handling, 403 / 429 / 5xx all land here. */ +/** a status with no specific handling, such as 429 or 5xx */ export const isMfaConfigProxyError = (err: unknown): boolean => parseMfaError(err)?.type === 'proxy_error'; diff --git a/new-ui/src/shared/rust-api/types.ts b/new-ui/src/shared/rust-api/types.ts index 5d108509c..0c58f2f32 100644 --- a/new-ui/src/shared/rust-api/types.ts +++ b/new-ui/src/shared/rust-api/types.ts @@ -101,6 +101,10 @@ export const TauriCommand = { MfaConfigStart: 'mfa_config_start', MfaConfigSendCode: 'mfa_config_send_code', MfaConfigAuthorize: 'mfa_config_authorize', + MfaConfigAuthorizeFido2: 'mfa_config_authorize_fido2', + MfaConfigOidcUrl: 'mfa_config_oidc_url', + MfaConfigAuthorizeOidc: 'mfa_config_authorize_oidc', + MfaConfigAbortAttempt: 'mfa_config_abort_attempt', MfaConfigSetupStart: 'mfa_config_setup_start', MfaConfigSetupFinish: 'mfa_config_setup_finish', MfaConfigSetupFido2: 'mfa_config_setup_fido2', diff --git a/new-ui/src/shared/scss/_shared_tokens.scss b/new-ui/src/shared/scss/_shared_tokens.scss index 4d368f784..8a2d5165c 100644 --- a/new-ui/src/shared/scss/_shared_tokens.scss +++ b/new-ui/src/shared/scss/_shared_tokens.scss @@ -253,6 +253,9 @@ $jetbrains: --tooltip-letter-spacing: 0.3; --tooltip-spacing: var(--spacing-sm) var(--spacing-md); + // Scroll container + --scroll-container-gutter: 6px; + // custom // how much space does error message in all takes diff --git a/new-ui/src/shared/utils/mfa.test.ts b/new-ui/src/shared/utils/mfa.test.ts new file mode 100644 index 000000000..7d84e41df --- /dev/null +++ b/new-ui/src/shared/utils/mfa.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest'; +import { + ConnectionType, + MfaMethod, + type MfaMethodValue, + type MfaStep, +} from '../rust-api/types'; +import { hasMfaMethodChoice } from './mfa'; + +const step = (...methods: MfaMethodValue[]): MfaStep => ({ + methods: methods.map((method) => ({ method, configured: true })), +}); + +const locationOf = ( + mfa_steps: MfaStep[], + connection_type: ConnectionType = ConnectionType.Location, +) => ({ connection_type, mfa_steps }); + +describe('hasMfaMethodChoice', () => { + it('is false for a single step with a single factor', () => { + expect(hasMfaMethodChoice(locationOf([step(MfaMethod.Email)]))).toBe(false); + }); + + it('is true when a step offers two factors', () => { + expect(hasMfaMethodChoice(locationOf([step(MfaMethod.Email, MfaMethod.Totp)]))).toBe( + true, + ); + }); + + it('is false for several single-factor steps', () => { + expect( + hasMfaMethodChoice(locationOf([step(MfaMethod.Email), step(MfaMethod.Oidc)])), + ).toBe(false); + }); + + it('does not count biometric as a desktop choice', () => { + expect( + hasMfaMethodChoice(locationOf([step(MfaMethod.Totp, MfaMethod.Biometric)])), + ).toBe(false); + }); + + it('is false for a tunnel', () => { + expect( + hasMfaMethodChoice( + locationOf([step(MfaMethod.Email, MfaMethod.Totp)], ConnectionType.Tunnel), + ), + ).toBe(false); + }); +}); diff --git a/new-ui/src/shared/utils/mfa.ts b/new-ui/src/shared/utils/mfa.ts index 4e0bc41d3..86941b11b 100644 --- a/new-ui/src/shared/utils/mfa.ts +++ b/new-ui/src/shared/utils/mfa.ts @@ -97,6 +97,11 @@ export const pickableMfaMethods = (step: MfaStep): MfaStepMethod[] => { return drivable.length > 0 ? drivable : step.methods; }; +/** gates the MFA settings view, which has nothing to pick otherwise */ +export const hasMfaMethodChoice = ( + location: Pick, +): boolean => mfaStepsOf(location).some((step) => pickableMfaMethods(step).length > 1); + export const resolveMfaStepPlan = ( location: Pick, oneOffPlan: MfaMethodValue[] = [], diff --git a/src-tauri/client-proto/build.rs b/src-tauri/client-proto/build.rs index 50ea47adf..58a7bb1a7 100644 --- a/src-tauri/client-proto/build.rs +++ b/src-tauri/client-proto/build.rs @@ -47,6 +47,10 @@ fn main() -> Result<(), Box> { ".defguard.client_types.MfaConfigStartResponse.available_methods", "#[serde(default)]", ) + .field_attribute( + ".defguard.client_types.MfaConfigFido2ChallengeResponse.credential_ids", + "#[serde(default)]", + ) // [2.2] FIDO2 setup fields and the email fallback's recovery codes, absent on older edges. .field_attribute( ".defguard.client_types.MfaConfigAuthorizeResponse.recovery_codes", diff --git a/src-tauri/core/src/mfa.rs b/src-tauri/core/src/mfa.rs index df95dd44f..3e59f3fb0 100644 --- a/src-tauri/core/src/mfa.rs +++ b/src-tauri/core/src/mfa.rs @@ -313,14 +313,14 @@ pub async fn mfa_finish_code( } #[cfg(not(test))] -const OIDC_POLL_INTERVAL: Duration = Duration::from_secs(5); +pub(crate) const OIDC_POLL_INTERVAL: Duration = Duration::from_secs(5); #[cfg(test)] -const OIDC_POLL_INTERVAL: Duration = Duration::from_millis(5); +pub(crate) const OIDC_POLL_INTERVAL: Duration = Duration::from_millis(5); #[cfg(not(test))] -const OIDC_POLL_TIMEOUT: Duration = Duration::from_mins(5); +pub(crate) const OIDC_POLL_TIMEOUT: Duration = Duration::from_mins(5); #[cfg(test)] -const OIDC_POLL_TIMEOUT: Duration = Duration::from_millis(200); +pub(crate) const OIDC_POLL_TIMEOUT: Duration = Duration::from_millis(200); #[cfg(not(test))] const MOBILE_APPROVE_TIMEOUT: Duration = Duration::from_mins(2); diff --git a/src-tauri/core/src/mfa_config.rs b/src-tauri/core/src/mfa_config.rs index 3c5a6232c..eb9b36306 100644 --- a/src-tauri/core/src/mfa_config.rs +++ b/src-tauri/core/src/mfa_config.rs @@ -1,19 +1,27 @@ //! Post-enrollment MFA factor configuration over HTTP. The proxy mints a short-lived session //! from the device's polling token, which stands in for the enrollment cookie. -use std::fmt; +use std::{fmt, time::Duration}; +use chrono::Utc; use defguard_client_proto::defguard::client_types::{ CodeMfaSetupFinishRequest, CodeMfaSetupFinishResponse, CodeMfaSetupStartRequest, CodeMfaSetupStartResponse, MfaConfigAuthorizeRequest, MfaConfigAuthorizeResponse, - MfaConfigSendCodeRequest, MfaConfigStartRequest, MfaConfigStartResponse, MfaMethod, + MfaConfigFido2ChallengeRequest, MfaConfigFido2ChallengeResponse, MfaConfigSendCodeRequest, + MfaConfigStartRequest, MfaConfigStartResponse, MfaMethod, }; use reqwest::{Response, StatusCode, Url}; use serde::{de::DeserializeOwned, Serialize}; use thiserror::Error; +use tokio::{ + select, + time::{sleep, Instant}, +}; +use tokio_util::sync::CancellationToken; use crate::{ database::models::Id, + mfa::{OIDC_POLL_INTERVAL, OIDC_POLL_TIMEOUT}, proxy::{post_with_headers, read_error_message}, }; @@ -21,11 +29,27 @@ use crate::{ const START: &str = "api/v1/mfa-config/start"; const SEND_CODE: &str = "api/v1/mfa-config/send-code"; const AUTHORIZE: &str = "api/v1/mfa-config/authorize"; +const FIDO2_CHALLENGE: &str = "api/v1/mfa-config/fido2-challenge"; const SETUP_START: &str = "api/v1/mfa-config/setup/start"; const SETUP_FINISH: &str = "api/v1/mfa-config/setup/finish"; -// `MfaConfigAuthorizeRequest` carries only a `code`, so a non-code factor cannot authorize here. -pub const AUTHORIZING_METHODS: &[MfaMethod] = &[MfaMethod::Totp, MfaMethod::Email]; +// mirrors the methods Core accepts in mfa_config_authorize, keep in step +pub const AUTHORIZING_METHODS: &[MfaMethod] = &[ + MfaMethod::Totp, + MfaMethod::Email, + MfaMethod::Fido2, + MfaMethod::Oidc, +]; + +// FIDO2 sends an assertion and OIDC completes in the browser, so only these send a code +const CODE_METHODS: &[MfaMethod] = &[MfaMethod::Totp, MfaMethod::Email]; + +// Core reuses 401, 403 and 428 for several errors, only the message tells these apart +const INVALID_CODE_MESSAGE: &str = "invalid code"; +const METHOD_NOT_CONFIGURED_MESSAGE: &str = "method not configured"; +const ALREADY_AUTHORIZED_MESSAGE: &str = "session already authorized"; +// a prefix, Core's login flow words it "OIDC authentication not completed yet" +const OIDC_PENDING_MESSAGE: &str = "OIDC authentication not completed"; // Mirrors the methods Core accepts in `mfa_setup_start` / `mfa_setup_finish`, keep in step. pub const CONFIGURABLE_METHODS: &[MfaMethod] = @@ -38,6 +62,30 @@ pub enum SetupProof { Fido2 { name: String, attestation: String }, } +pub enum AuthorizeProof { + Code { + method: MfaMethod, + code: String, + }, + Fido2 { + signature: Vec, + auth_data: Vec, + credential_id: Vec, + }, + /// Core has already seen the browser login, so there is nothing to send + Oidc, +} + +impl AuthorizeProof { + fn method(&self) -> MfaMethod { + match self { + Self::Code { method, .. } => *method, + Self::Fido2 { .. } => MfaMethod::Fido2, + Self::Oidc => MfaMethod::Oidc, + } + } +} + /// One authorized session configures several factors, so it outlives a single setup. #[derive(Clone)] pub struct MfaConfigSession { @@ -92,6 +140,29 @@ pub enum MfaConfigError { #[error("MFA configuration was cancelled")] Cancelled, + /// e.g. a FIDO2 challenge for a user with no security key + #[error("{message}")] + MethodNotConfigured { message: String }, + + /// e.g. an inactive user or too many attempts, Core words these for the user + #[error("{message}")] + Forbidden { message: String }, + + /// never leaves the OIDC poll loop, which retries until the browser login completes + #[error("OpenID authentication is not completed yet")] + OidcPending, + + /// the response that authorized it was lost, so the session cannot be resumed + #[error("MFA configuration session is already authorized")] + AlreadyAuthorized, + + /// e.g. no FIDO2 challenge pending + #[error("{message}")] + FailedPrecondition { message: String }, + + #[error("Timed out waiting for authentication")] + Timeout, + #[error("{message}")] NetworkError { message: String }, @@ -113,17 +184,25 @@ fn method_name(method: MfaMethod) -> &'static str { } } -fn ensure_can_authorize(method: MfaMethod) -> Result<(), MfaConfigError> { - if AUTHORIZING_METHODS.contains(&method) { - Ok(()) - } else { - Err(MfaConfigError::UnsupportedMethod { +fn ensure_can_authorize(proof: &AuthorizeProof) -> Result<(), MfaConfigError> { + let method = proof.method(); + if !AUTHORIZING_METHODS.contains(&method) { + return Err(MfaConfigError::UnsupportedMethod { message: format!( - "A {} cannot authorize MFA configuration; use a one-time code instead.", + "A {} cannot authorize MFA configuration.", method_name(method) ), - }) + }); + } + if matches!(proof, AuthorizeProof::Code { .. }) && !CODE_METHODS.contains(&method) { + return Err(MfaConfigError::UnsupportedMethod { + message: format!( + "A {} does not authorize with a one-time code.", + method_name(method) + ), + }); } + Ok(()) } fn ensure_can_configure(method: MfaMethod) -> Result<(), MfaConfigError> { @@ -171,8 +250,24 @@ async fn check_response(response: Response, endpoint: &str) -> Result { + Err(MfaConfigError::InvalidCode { message }) + } StatusCode::UNAUTHORIZED => Err(MfaConfigError::SessionExpired), StatusCode::BAD_REQUEST => Err(MfaConfigError::InvalidCode { message }), + StatusCode::FORBIDDEN if message == METHOD_NOT_CONFIGURED_MESSAGE => { + Err(MfaConfigError::MethodNotConfigured { message }) + } + StatusCode::FORBIDDEN => Err(MfaConfigError::Forbidden { message }), + // other 428s end a poll, so the status alone cannot mean keep polling + StatusCode::PRECONDITION_REQUIRED if message.starts_with(OIDC_PENDING_MESSAGE) => { + Err(MfaConfigError::OidcPending) + } + StatusCode::PRECONDITION_REQUIRED if message == ALREADY_AUTHORIZED_MESSAGE => { + Err(MfaConfigError::AlreadyAuthorized) + } + StatusCode::PRECONDITION_REQUIRED => Err(MfaConfigError::FailedPrecondition { message }), _ => Err(MfaConfigError::ProxyError { status: status.as_u16(), message, @@ -222,22 +317,90 @@ pub async fn mfa_config_send_code( Ok(()) } +/// single-use, every authorize call that reaches verification consumes it +pub async fn mfa_config_fido2_challenge( + proxy_url: Url, + session_token: String, +) -> Result { + debug!("Requesting MFA configuration FIDO2 challenge"); + let request = MfaConfigFido2ChallengeRequest { session_token }; + parse(post(&proxy_url, FIDO2_CHALLENGE, &request).await?).await +} + pub async fn mfa_config_authorize( proxy_url: Url, session_token: String, - method: MfaMethod, - code: String, + proof: AuthorizeProof, ) -> Result { - ensure_can_authorize(method)?; + ensure_can_authorize(&proof)?; debug!("Authorizing MFA configuration session"); + let method = proof.method() as i32; + let (code, signature, auth_data, credential_id) = match proof { + AuthorizeProof::Code { code, .. } => (code, None, None, None), + AuthorizeProof::Fido2 { + signature, + auth_data, + credential_id, + } => ( + String::new(), + Some(signature), + Some(auth_data), + Some(credential_id), + ), + AuthorizeProof::Oidc => (String::new(), None, None, None), + }; let request = MfaConfigAuthorizeRequest { session_token, - method: method as i32, + method, code, + signature, + auth_data, + credential_id, }; parse(post(&proxy_url, AUTHORIZE, &request).await?).await } +/// the browser login must already be open, this only polls for its result. +/// the first poll waits an interval, Core knows of the attempt only once the browser loads it +pub async fn mfa_config_poll_oidc( + proxy_url: Url, + session_token: String, + deadline_timestamp: i64, + cancel: CancellationToken, +) -> Result { + let session_left = u64::try_from(deadline_timestamp - Utc::now().timestamp()).unwrap_or(0); + let started = Instant::now(); + let session_deadline = started + Duration::from_secs(session_left); + let poll_deadline = started + OIDC_POLL_TIMEOUT; + + loop { + select! { + () = cancel.cancelled() => return Err(MfaConfigError::Cancelled), + () = sleep(OIDC_POLL_INTERVAL) => {} + } + + let now = Instant::now(); + if now >= session_deadline { + return Err(MfaConfigError::SessionExpired); + } + if now >= poll_deadline { + return Err(MfaConfigError::Timeout); + } + + // not raced against the cancel, Core may authorize even if the answer is dropped + match mfa_config_authorize( + proxy_url.clone(), + session_token.clone(), + AuthorizeProof::Oidc, + ) + .await + { + Err(MfaConfigError::OidcPending) => {} + other => return other, + } + } +} + pub async fn mfa_config_setup_start( proxy_url: Url, session_token: String, diff --git a/src-tauri/core/src/mfa_config/tests.rs b/src-tauri/core/src/mfa_config/tests.rs index 079db544d..a76e333c2 100644 --- a/src-tauri/core/src/mfa_config/tests.rs +++ b/src-tauri/core/src/mfa_config/tests.rs @@ -1,5 +1,6 @@ use reqwest::Url; use serde_json::json; +use tokio_util::sync::CancellationToken; use wiremock::{ matchers::{body_partial_json, method, path}, Mock, MockServer, ResponseTemplate, @@ -12,6 +13,9 @@ const SESSION_TOKEN: &str = "mfa-config-session"; const CHALLENGE: &str = r#"{"publicKey":{}}"#; const ATTESTATION: &str = r#"{"id":"cred"}"#; +/// far enough out that only OIDC_POLL_TIMEOUT bounds a poll +const LIVE_DEADLINE: i64 = 4_000_000_000; + fn mock_url(server: &MockServer) -> Url { Url::parse(&server.uri()).expect("MockServer URI should be valid") } @@ -25,6 +29,13 @@ fn start_response_json() -> serde_json::Value { }) } +fn code(method: MfaMethod, code: &str) -> AuthorizeProof { + AuthorizeProof::Code { + method, + code: code.into(), + } +} + async fn mount(server: &MockServer, endpoint: &str, template: ResponseTemplate) { Mock::given(method("POST")) .and(path(format!("/{endpoint}"))) @@ -120,8 +131,7 @@ async fn test_method_is_sent_as_a_number() { mfa_config_authorize( url.clone(), SESSION_TOKEN.into(), - MfaMethod::Email, - "123456".into(), + code(MfaMethod::Email, "123456"), ) .await .unwrap(); @@ -304,8 +314,7 @@ async fn test_unauthorized_means_session_expired() { let err = mfa_config_authorize( mock_url(&server), "stale".into(), - MfaMethod::Email, - "000000".into(), + code(MfaMethod::Email, "000000"), ) .await .unwrap_err(); @@ -313,6 +322,83 @@ async fn test_unauthorized_means_session_expired() { assert!(matches!(err, MfaConfigError::SessionExpired)); } +/// Core answers a wrong code or FIDO2 assertion with 401 too, but the session is still alive +#[tokio::test] +async fn test_unauthorized_invalid_code_is_an_invalid_code() { + let server = MockServer::start().await; + mount( + &server, + AUTHORIZE, + ResponseTemplate::new(401).set_body_json(json!({ "error": "invalid code" })), + ) + .await; + + let err = mfa_config_authorize(mock_url(&server), SESSION_TOKEN.into(), fido2_proof()) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::InvalidCode { .. })); +} + +#[tokio::test] +async fn test_forbidden_means_method_not_configured() { + let server = MockServer::start().await; + mount( + &server, + FIDO2_CHALLENGE, + ResponseTemplate::new(403).set_body_json(json!({ "error": "method not configured" })), + ) + .await; + + let err = mfa_config_fido2_challenge(mock_url(&server), SESSION_TOKEN.into()) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::MethodNotConfigured { .. })); +} + +#[tokio::test] +async fn test_other_forbidden_carries_the_core_message() { + let server = MockServer::start().await; + mount( + &server, + AUTHORIZE, + ResponseTemplate::new(403).set_body_json(json!({ "error": "user is inactive" })), + ) + .await; + + let err = mfa_config_authorize(mock_url(&server), SESSION_TOKEN.into(), fido2_proof()) + .await + .unwrap_err(); + + match err { + MfaConfigError::Forbidden { message } => assert_eq!(message, "user is inactive"), + other => panic!("expected Forbidden, got {other:?}"), + } +} + +#[tokio::test] +async fn test_no_fido2_challenge_is_a_failed_precondition() { + let server = MockServer::start().await; + mount( + &server, + AUTHORIZE, + ResponseTemplate::new(428).set_body_json(json!({ "error": "no FIDO2 challenge" })), + ) + .await; + + let err = mfa_config_authorize(mock_url(&server), SESSION_TOKEN.into(), fido2_proof()) + .await + .unwrap_err(); + + match err { + MfaConfigError::FailedPrecondition { message } => { + assert_eq!(message, "no FIDO2 challenge"); + } + other => panic!("expected FailedPrecondition, got {other:?}"), + } +} + #[tokio::test] async fn test_bad_request_carries_the_proxy_message() { let server = MockServer::start().await; @@ -380,7 +466,7 @@ async fn test_unsupported_methods_are_rejected_before_the_request() { MfaMethod::MobileApprove, ] { assert!(matches!( - mfa_config_authorize(url.clone(), "t".into(), unsupported, "1".into()) + mfa_config_authorize(url.clone(), "t".into(), code(unsupported, "1")) .await .unwrap_err(), MfaConfigError::UnsupportedMethod { .. } @@ -405,9 +491,9 @@ async fn test_unsupported_methods_are_rejected_before_the_request() { } } -/// FIDO2 can be configured, but it cannot authorize the session: it has no code to submit. +/// FIDO2 both configures and authorizes, but it authorizes with an assertion, never a code #[tokio::test] -async fn test_fido2_can_be_configured_but_not_authorize() { +async fn test_fido2_cannot_authorize_with_a_code() { let server = MockServer::start().await; Mock::given(method("POST")) .respond_with(ResponseTemplate::new(200)) @@ -416,12 +502,301 @@ async fn test_fido2_can_be_configured_but_not_authorize() { .await; assert!(matches!( - mfa_config_authorize(mock_url(&server), "t".into(), MfaMethod::Fido2, "1".into()) + mfa_config_authorize(mock_url(&server), "t".into(), code(MfaMethod::Fido2, "1")) .await .unwrap_err(), MfaConfigError::UnsupportedMethod { .. } )); assert!(CONFIGURABLE_METHODS.contains(&MfaMethod::Fido2)); + assert!(AUTHORIZING_METHODS.contains(&MfaMethod::Fido2)); +} + +fn fido2_proof() -> AuthorizeProof { + AuthorizeProof::Fido2 { + signature: vec![6, 7], + auth_data: vec![1, 2, 3], + credential_id: vec![4, 5], + } +} + +#[tokio::test] +async fn test_fido2_challenge_sends_the_session_token() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/{FIDO2_CHALLENGE}"))) + .and(body_partial_json(json!({ "session_token": SESSION_TOKEN }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "challenge": "abc123", + "credential_ids": ["Y3JlZA"], + }))) + .expect(1) + .mount(&server) + .await; + + let response = mfa_config_fido2_challenge(mock_url(&server), SESSION_TOKEN.into()) + .await + .unwrap(); + + assert_eq!(response.challenge, "abc123"); + assert_eq!(response.credential_ids, ["Y3JlZA"]); +} + +#[tokio::test] +async fn test_fido2_challenge_tolerates_missing_credential_ids() { + let server = MockServer::start().await; + mount( + &server, + FIDO2_CHALLENGE, + ResponseTemplate::new(200).set_body_json(json!({ "challenge": "abc123" })), + ) + .await; + + let response = mfa_config_fido2_challenge(mock_url(&server), SESSION_TOKEN.into()) + .await + .unwrap(); + + assert!(response.credential_ids.is_empty()); +} + +/// the assertion fields go out as serde byte arrays, with the code left empty +#[tokio::test] +async fn test_fido2_authorize_sends_the_assertion() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .and(body_partial_json(json!({ + "session_token": SESSION_TOKEN, + "method": MfaMethod::Fido2 as i32, + "code": "", + "signature": [6, 7], + "auth_data": [1, 2, 3], + "credential_id": [4, 5], + }))) + .respond_with( + ResponseTemplate::new(200).set_body_json(json!({ "deadline_timestamp": 7i64 })), + ) + .expect(1) + .mount(&server) + .await; + + let response = mfa_config_authorize(mock_url(&server), SESSION_TOKEN.into(), fido2_proof()) + .await + .unwrap(); + + assert_eq!(response.deadline_timestamp, 7); + assert!(response.recovery_codes.is_empty()); +} + +fn oidc_pending() -> ResponseTemplate { + ResponseTemplate::new(428) + .set_body_json(json!({ "error": "OIDC authentication not completed yet" })) +} + +#[tokio::test] +async fn test_oidc_pending_matches_both_wordings() { + for message in [ + "OIDC authentication not completed", + "OIDC authentication not completed yet", + ] { + let server = MockServer::start().await; + mount( + &server, + AUTHORIZE, + ResponseTemplate::new(428).set_body_json(json!({ "error": message })), + ) + .await; + + let err = mfa_config_authorize( + mock_url(&server), + SESSION_TOKEN.into(), + AuthorizeProof::Oidc, + ) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::OidcPending), "{message}"); + } +} + +#[tokio::test] +async fn test_oidc_poll_keeps_going_until_the_login_completes() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .and(body_partial_json(json!({ + "session_token": SESSION_TOKEN, + "method": MfaMethod::Oidc as i32, + "code": "", + }))) + .respond_with(oidc_pending()) + .up_to_n_times(2) + .expect(2) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .respond_with( + ResponseTemplate::new(200).set_body_json(json!({ "deadline_timestamp": 9i64 })), + ) + .expect(1) + .mount(&server) + .await; + + let response = mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + LIVE_DEADLINE, + CancellationToken::new(), + ) + .await + .unwrap(); + + assert_eq!(response.deadline_timestamp, 9); +} + +/// the other 428s end the poll, keying on the status alone would spin until the deadline +#[tokio::test] +async fn test_oidc_poll_stops_on_session_already_authorized() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .respond_with( + ResponseTemplate::new(428) + .set_body_json(json!({ "error": "session already authorized" })), + ) + .expect(1) + .mount(&server) + .await; + + let err = mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + LIVE_DEADLINE, + CancellationToken::new(), + ) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::AlreadyAuthorized)); +} + +/// Core may authorize the session even if the answer is dropped, so a cancel waits for it +#[tokio::test] +async fn test_oidc_poll_keeps_an_answer_that_lands_after_cancel() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ "deadline_timestamp": 9i64 })) + .set_delay(Duration::from_millis(100)), + ) + .expect(1) + .mount(&server) + .await; + let cancel = CancellationToken::new(); + let poll = tokio::spawn(mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + LIVE_DEADLINE, + cancel.clone(), + )); + + tokio::time::sleep(Duration::from_millis(50)).await; + cancel.cancel(); + + let response = poll.await.unwrap().unwrap(); + assert_eq!(response.deadline_timestamp, 9); +} + +/// a foreign identity in the browser ends the session on Core +#[tokio::test] +async fn test_oidc_poll_stops_when_the_session_ends() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .respond_with(oidc_pending()) + .up_to_n_times(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path(format!("/{AUTHORIZE}"))) + .respond_with(ResponseTemplate::new(401).set_body_json(json!({ "error": "invalid token" }))) + .expect(1) + .mount(&server) + .await; + + let err = mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + LIVE_DEADLINE, + CancellationToken::new(), + ) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::SessionExpired)); +} + +#[tokio::test] +async fn test_oidc_poll_times_out() { + let server = MockServer::start().await; + mount(&server, AUTHORIZE, oidc_pending()).await; + + let err = mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + LIVE_DEADLINE, + CancellationToken::new(), + ) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::Timeout)); +} + +#[tokio::test] +async fn test_oidc_poll_is_bounded_by_the_session_deadline() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(oidc_pending()) + .expect(0) + .mount(&server) + .await; + + let err = mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + 0, + CancellationToken::new(), + ) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::SessionExpired)); +} + +#[tokio::test] +async fn test_oidc_poll_stops_on_cancel() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(oidc_pending()) + .expect(0) + .mount(&server) + .await; + let cancel = CancellationToken::new(); + cancel.cancel(); + + let err = mfa_config_poll_oidc( + mock_url(&server), + SESSION_TOKEN.into(), + LIVE_DEADLINE, + cancel, + ) + .await + .unwrap_err(); + + assert!(matches!(err, MfaConfigError::Cancelled)); } /// FIDO2 proves itself with an attestation, so the code field goes out empty and the security @@ -480,13 +855,15 @@ async fn test_setup_start_returns_the_fido2_creation_challenge() { } #[test] -fn test_authorizing_methods_drops_unknown_and_non_code_entries() { +fn test_authorizing_methods_drops_unknown_and_non_authorizing_entries() { let response = MfaConfigStartResponse { session_token: SESSION_TOKEN.into(), available_methods: vec![ MfaMethod::Totp as i32, MfaMethod::Fido2 as i32, + MfaMethod::Biometric as i32, MfaMethod::Email as i32, + MfaMethod::Oidc as i32, 99, ], email_fallback: false, @@ -495,7 +872,12 @@ fn test_authorizing_methods_drops_unknown_and_non_code_entries() { assert_eq!( authorizing_methods(&response), - vec![MfaMethod::Totp, MfaMethod::Email] + vec![ + MfaMethod::Totp, + MfaMethod::Fido2, + MfaMethod::Email, + MfaMethod::Oidc + ] ); } diff --git a/src-tauri/fido2/src/windows/mod.rs b/src-tauri/fido2/src/windows/mod.rs index a598a3724..f57ba94b3 100644 --- a/src-tauri/fido2/src/windows/mod.rs +++ b/src-tauri/fido2/src/windows/mod.rs @@ -7,6 +7,7 @@ mod api; mod convert; +mod options; use std::{ sync::{Arc, Mutex}, @@ -27,9 +28,7 @@ use windows::{ WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE, WEBAUTHN_AUTHENTICATOR_ATTACHMENT_ANY, WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM, WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS, - WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS_VERSION_4, - WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS, - WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS_VERSION_4, WEBAUTHN_CLIENT_DATA, + WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS, WEBAUTHN_CLIENT_DATA, WEBAUTHN_CLIENT_DATA_CURRENT_VERSION, WEBAUTHN_CREDENTIAL_ATTESTATION, WEBAUTHN_CREDENTIAL_ATTESTATION_VERSION_3, WEBAUTHN_CTAP_TRANSPORT_BLE, WEBAUTHN_CTAP_TRANSPORT_FLAGS_MASK, WEBAUTHN_CTAP_TRANSPORT_NFC, @@ -46,6 +45,10 @@ use windows::{ use self::{ api::{api, error_name, Api}, convert::{copy_out, Buffer, CoseParameters, CredentialList, WideString}, + options::{ + get_assertion_options_version, make_credential_options_version, CredentialHints, + GetAssertionOptions, MakeCredentialOptions, + }, }; use crate::{ protocol::{ @@ -391,32 +394,43 @@ pub(crate) async fn register( pbClientDataJSON: client_data.as_mut_ptr(), pwszHashAlgId: WEBAUTHN_HASH_ALGORITHM_SHA_256, }; + let hints = CredentialHints::security_key(); + let (credential_hints_len, credential_hints) = hints.for_api(api.version); let mut cancellation_id = cancellation_id; - let options = WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS { - dwVersion: WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS_VERSION_4, - dwTimeoutMilliseconds: u32::try_from(request.timeout.as_millis()).unwrap_or(u32::MAX), - dwAuthenticatorAttachment: attachment(request.attachment), - bRequireResidentKey: BOOL::from(request.resident_key == ResidentKey::Required), - bPreferResidentKey: BOOL::from(request.resident_key == ResidentKey::Preferred), - dwUserVerificationRequirement: user_verification(request.user_verification), - // The statement is discarded anyway, see `protocol::attestation_object`. - dwAttestationConveyancePreference: WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE, - pCancellationId: &raw mut cancellation_id, - pExcludeCredentialList: exclude.as_mut_ptr(), - ..Default::default() + let options = MakeCredentialOptions { + base: WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS { + dwVersion: make_credential_options_version(api.version), + dwTimeoutMilliseconds: u32::try_from(request.timeout.as_millis()) + .unwrap_or(u32::MAX), + dwAuthenticatorAttachment: attachment(request.attachment), + bRequireResidentKey: BOOL::from(request.resident_key == ResidentKey::Required), + bPreferResidentKey: BOOL::from(request.resident_key == ResidentKey::Preferred), + dwUserVerificationRequirement: user_verification(request.user_verification), + // the statement is discarded anyway, see protocol::attestation_object + dwAttestationConveyancePreference: WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE, + pCancellationId: &raw mut cancellation_id, + pExcludeCredentialList: exclude.as_mut_ptr(), + ..Default::default() + }, + prf_global_eval: std::ptr::null_mut(), + credential_hints_len, + credential_hints, + third_party_payment: BOOL::from(false), }; begin(progress, cancellation_id)?; // What a failed assertion gets checked against, and neither field names the user. tracing::debug!( "Windows WebAuthn make credential: rp_id={}, algorithms={}, exclude={}, \ - resident_key={:?}, user_verification={:?}, api={}", + resident_key={:?}, user_verification={:?}, api={}, options_version={}, hints={}", request.rp_id, request.algorithms.len(), request.exclude_credentials.len(), request.resident_key, request.user_verification, api.version, + options.base.dwVersion, + credential_hints_len, ); let started = Instant::now(); let mut raw = std::ptr::null_mut(); @@ -429,7 +443,8 @@ pub(crate) async fn register( &raw const user, algorithms.as_ptr(), &raw const client_data_raw, - &raw const options, + // cast from the whole struct, the dll reads the v8 tail past base + (&raw const options).cast(), &raw mut raw, ) }; @@ -508,26 +523,35 @@ pub(crate) async fn assert( pbClientDataJSON: client_data.as_mut_ptr(), pwszHashAlgId: WEBAUTHN_HASH_ALGORITHM_SHA_256, }; + let hints = CredentialHints::security_key(); + let (credential_hints_len, credential_hints) = hints.for_api(api.version); let mut cancellation_id = cancellation_id; - let options = WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS { - dwVersion: WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS_VERSION_4, - dwTimeoutMilliseconds: u32::try_from(request.timeout.as_millis()).unwrap_or(u32::MAX), - dwAuthenticatorAttachment: attachment(request.attachment), - dwUserVerificationRequirement: user_verification(request.user_verification), - pCancellationId: &raw mut cancellation_id, - pAllowCredentialList: allow.as_mut_ptr(), - ..Default::default() + let options = GetAssertionOptions { + base: WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS { + dwVersion: get_assertion_options_version(api.version), + dwTimeoutMilliseconds: u32::try_from(request.timeout.as_millis()) + .unwrap_or(u32::MAX), + dwAuthenticatorAttachment: attachment(request.attachment), + dwUserVerificationRequirement: user_verification(request.user_verification), + pCancellationId: &raw mut cancellation_id, + pAllowCredentialList: allow.as_mut_ptr(), + ..Default::default() + }, + credential_hints_len, + credential_hints, }; begin(progress, cancellation_id)?; // An empty allow list and a wrong rp id both surface as the same opaque refusal. tracing::debug!( "Windows WebAuthn get assertion: rp_id={}, allow_credentials={}, \ - user_verification={:?}, api={}", + user_verification={:?}, api={}, options_version={}, hints={}", request.rp_id, request.allow_credentials.len(), request.user_verification, api.version, + options.base.dwVersion, + credential_hints_len, ); let started = Instant::now(); let mut raw = std::ptr::null_mut(); @@ -538,7 +562,7 @@ pub(crate) async fn assert( hwnd, rp_id.as_pcwstr(), &raw const client_data_raw, - &raw const options, + (&raw const options).cast(), &raw mut raw, ) }; diff --git a/src-tauri/fido2/src/windows/options.rs b/src-tauri/fido2/src/windows/options.rs new file mode 100644 index 000000000..dc0818b75 --- /dev/null +++ b/src-tauri/fido2/src/windows/options.rs @@ -0,0 +1,111 @@ +//! v8 option structs, which the windows bindings stop short of. each is the bound v7 struct +//! followed by the fields webauthn.h appends at v8, in its order + +use std::ffi::c_void; + +use windows::{ + core::{BOOL, PCWSTR}, + Win32::Networking::WindowsWebServices::{ + WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS, + WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS_VERSION_4, + WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS, + WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS_VERSION_4, + }, +}; + +/// first api with credential hints +const API_VERSION_8: u32 = 8; +const MAKE_CREDENTIAL_OPTIONS_VERSION_8: u32 = 8; +const GET_ASSERTION_OPTIONS_VERSION_8: u32 = 8; + +/// the dll reads only up to dwVersion, so the v8 struct is safe to hand any platform +pub(super) fn make_credential_options_version(api_version: u32) -> u32 { + if api_version >= API_VERSION_8 { + MAKE_CREDENTIAL_OPTIONS_VERSION_8 + } else { + WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS_VERSION_4 + } +} + +pub(super) fn get_assertion_options_version(api_version: u32) -> u32 { + if api_version >= API_VERSION_8 { + GET_ASSERTION_OPTIONS_VERSION_8 + } else { + WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS_VERSION_4 + } +} + +#[repr(C)] +pub(super) struct MakeCredentialOptions { + pub(super) base: WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL_OPTIONS, + pub(super) prf_global_eval: *mut c_void, + pub(super) credential_hints_len: u32, + pub(super) credential_hints: *const PCWSTR, + pub(super) third_party_payment: BOOL, +} + +#[repr(C)] +pub(super) struct GetAssertionOptions { + pub(super) base: WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS, + pub(super) credential_hints_len: u32, + pub(super) credential_hints: *const PCWSTR, +} + +/// steers the platform dialog to its security key flow. advisory only, the transport check +/// on the way out is what actually refuses a phone +pub(super) struct CredentialHints([PCWSTR; 1]); + +impl CredentialHints { + pub(super) fn security_key() -> Self { + Self([windows::core::w!("security-key")]) + } + + /// nothing is sent on a platform that predates hints + pub(super) fn for_api(&self, api_version: u32) -> (u32, *const PCWSTR) { + if api_version >= API_VERSION_8 { + (self.0.len() as u32, self.0.as_ptr()) + } else { + (0, std::ptr::null()) + } + } +} + +#[cfg(test)] +mod tests { + use std::mem::{offset_of, size_of}; + + use super::*; + + /// offsets worked out by hand from webauthn.h for x64. a bindings bump that grows the v7 + /// structs would shift every appended field, and this is where it shows + #[cfg(target_pointer_width = "64")] + #[test] + fn test_v8_fields_sit_where_webauthn_h_puts_them() { + assert_eq!( + size_of::(), + 128 + ); + assert_eq!(offset_of!(MakeCredentialOptions, prf_global_eval), 128); + assert_eq!(offset_of!(MakeCredentialOptions, credential_hints_len), 136); + assert_eq!(offset_of!(MakeCredentialOptions, credential_hints), 144); + assert_eq!(offset_of!(MakeCredentialOptions, third_party_payment), 152); + + assert_eq!( + size_of::(), + 144 + ); + assert_eq!(offset_of!(GetAssertionOptions, credential_hints_len), 144); + assert_eq!(offset_of!(GetAssertionOptions, credential_hints), 152); + } + + #[test] + fn test_hints_are_withheld_below_v8() { + let hints = CredentialHints::security_key(); + + assert_eq!(hints.for_api(API_VERSION_8 - 1), (0, std::ptr::null())); + let (len, pointer) = hints.for_api(API_VERSION_8); + assert_eq!(len, 1); + // SAFETY: one entry, and hints outlives the read + assert_eq!(unsafe { (*pointer).to_string() }.unwrap(), "security-key"); + } +} diff --git a/src-tauri/permissions/default.toml b/src-tauri/permissions/default.toml index 46117e68e..1455e1746 100644 --- a/src-tauri/permissions/default.toml +++ b/src-tauri/permissions/default.toml @@ -18,6 +18,10 @@ commands.allow = [ "mfa_config_start", "mfa_config_send_code", "mfa_config_authorize", + "mfa_config_authorize_fido2", + "mfa_config_oidc_url", + "mfa_config_authorize_oidc", + "mfa_config_abort_attempt", "mfa_config_setup_start", "mfa_config_setup_finish", "mfa_config_setup_fido2", diff --git a/src-tauri/proto b/src-tauri/proto index c7bda9d94..1fa1c70be 160000 --- a/src-tauri/proto +++ b/src-tauri/proto @@ -1 +1 @@ -Subproject commit c7bda9d94d4f1906331e0d1d21af197a40ecc160 +Subproject commit 1fa1c70be598c3987ba62391a9a7b44dd1017c69 diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 1e78fe676..15a241999 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -12,7 +12,7 @@ use defguard_client_core::{ }, enrollment::{self}, mfa, - mfa_config::{self, MfaConfigError, MfaConfigSession, SetupProof}, + mfa_config::{self, AuthorizeProof, MfaConfigError, MfaConfigSession, SetupProof}, }; use defguard_client_fido2::{pin_policy, Assertion, Fido2Error, PinPolicy, PlatformContext}; use defguard_client_posture::authorize_posture_session; @@ -2038,8 +2038,8 @@ fn fido2_pin(pin: Option) -> Result, &'static str> { Ok(Some(pin)) } -/// Registers a running security key ceremony for the life of the call. Claimed before the first -/// await, so a cancel racing the setup never finds the slot empty. +/// one attempt per session since FIDO2 and OIDC share state on Core. claimed before the first +/// await, so a cancel racing the attempt never finds the slot empty struct CeremonyGuard<'a> { state: &'a AppState, session: Uuid, @@ -2057,10 +2057,11 @@ impl<'a> CeremonyGuard<'a> { .mfa_config_ceremonies .lock() .expect("mfa_config_ceremonies mutex poisoned"); - // There is one key, and a second claim would leave the first ceremony's cancel unreachable. + // a second claim would leave the first attempt's cancel unreachable if ceremonies.contains_key(&session) { return Err(MfaConfigError::SecurityKey { - message: "A security key registration is already in progress".to_string(), + message: "Another verification or security key registration is already in progress" + .to_string(), }); } ceremonies.insert(session, ceremony.clone()); @@ -2141,11 +2142,11 @@ fn assertion_error(err: &Fido2Error) -> mfa::MfaError { /// As on the assertion side, but a cancellation keeps its own variant rather than arriving as /// a security key error, since the user is the one who asked for it. -fn registration_error(err: &Fido2Error) -> MfaConfigError { +fn mfa_config_fido2_error(err: &Fido2Error, ceremony: &str) -> MfaConfigError { match err { Fido2Error::Cancelled => MfaConfigError::Cancelled, err => MfaConfigError::SecurityKey { - message: fido2_message(err, "complete the registration"), + message: fido2_message(err, ceremony), }, } } @@ -2323,7 +2324,7 @@ pub async fn mfa_fido2_pin( handle: AppHandle, ) -> Result { debug!("Starting FIDO2 MFA for location {location_id} of instance {instance_id}"); - // The PIN is never logged, here or anywhere below. + // never log the PIN, here or below let pin = fido2_pin(pin).map_err(ToString::to_string)?; let step_methods = methods @@ -2482,12 +2483,21 @@ pub async fn mfa_config_authorize( let method = parse_mfa_method(&method)?; let uid = parse_mfa_config_session_id(&session_id)?; let session = get_mfa_config_session(&state, &session_id)?; - let response = - mfa_config::mfa_config_authorize(session.proxy_url, session.session_token, method, code) - .await - .map_err(err_to_json)?; + let response = mfa_config::mfa_config_authorize( + session.proxy_url, + session.session_token, + AuthorizeProof::Code { method, code }, + ) + .await + .map_err(err_to_json)?; + + extend_mfa_config_session(&state, uid, &response); + info!("Authorized MFA configuration session"); + Ok(response) +} - // The new deadline bounds every setup in this session, not just the next one. +/// the new deadline bounds every setup in this session, not just the next one +fn extend_mfa_config_session(state: &AppState, uid: Uuid, response: &MfaConfigAuthorizeResponse) { if let Some(session) = state .mfa_config_sessions .lock() @@ -2496,8 +2506,132 @@ pub async fn mfa_config_authorize( { session.deadline_timestamp = response.deadline_timestamp; } +} - info!("Authorized MFA configuration session"); +/// challenge, ceremony and assertion run in one call since the challenge is single-use. +/// abort or cancel takes a platform prompt down +#[tauri::command(async)] +pub async fn mfa_config_authorize_fido2( + session_id: String, + pin: Option, + window: WebviewWindow, + state: State<'_, AppState>, + handle: AppHandle, +) -> Result { + debug!("Authorizing MFA configuration session with a security key"); + // never log the PIN, here or below + let pin = fido2_pin(pin).map_err(|message| { + err_to_json(MfaConfigError::SecurityKey { + message: message.to_string(), + }) + })?; + + let uid = parse_mfa_config_session_id(&session_id)?; + let ceremony = CeremonyGuard::register(&state, uid).map_err(err_to_json)?; + let session = get_mfa_config_session(&state, &session_id)?; + let instance = Instance::find_by_id(&*DB_POOL, session.instance_id) + .await + .map_err(|err| mfa_config_other(err.to_string()))? + .ok_or_else(|| mfa_config_other("Instance not found"))?; + let rp_id = fido2_rp_id(&instance).map_err(mfa_config_other)?; + + let challenge = mfa_config::mfa_config_fido2_challenge( + session.proxy_url.clone(), + session.session_token.clone(), + ) + .await + .map_err(err_to_json)?; + + // the cancel may have landed while the challenge was in flight + if ceremony.is_cancelled() { + debug!("Security key verification was cancelled before the prompt opened"); + return Err(err_to_json(MfaConfigError::Cancelled)); + } + + // the key blinks from here on and gives up without a touch + let _ = handle.emit(EventKey::MfaConfigFido2Touch.into(), ()); + // the prompt must not open behind the window that asked for it + let level = WindowLevelGuard::lower(&window); + let assertion = defguard_client_fido2::assert_for_mfa( + &rp_id, + &challenge.challenge, + &challenge.credential_ids, + pin, + platform_context(&window), + ceremony.token(), + ) + .await + .map_err(|err| match err { + // Core minted the challenge, so a bad one is not the user's problem + Fido2Error::MalformedChallenge(detail) => mfa_config_other(format!( + "Defguard sent a malformed security key challenge: {detail}" + )), + err => err_to_json(mfa_config_fido2_error(&err, "authorize the request")), + })?; + drop(level); + + // a platform that cannot abort a waiting key reports the cancel only once the ceremony ends + if ceremony.is_cancelled() { + debug!("Security key verification was cancelled, discarding the assertion"); + return Err(err_to_json(MfaConfigError::Cancelled)); + } + + let response = mfa_config::mfa_config_authorize( + session.proxy_url, + session.session_token, + AuthorizeProof::Fido2 { + signature: assertion.signature, + auth_data: assertion.authenticator_data, + credential_id: assertion.credential_id, + }, + ) + .await + .map_err(err_to_json)?; + + extend_mfa_config_session(&state, uid, &response); + info!("Authorized MFA configuration session with a security key"); + Ok(response) +} + +/// each open supersedes the previous attempt on Core, and a running poll picks it up. +/// the token stays out of the persisted frontend store but does land in browser history +#[tauri::command(async)] +pub async fn mfa_config_oidc_url( + session_id: String, + state: State<'_, AppState>, +) -> Result { + let session = get_mfa_config_session(&state, &session_id)?; + let mut url = session + .proxy_url + .join("openid/mfa") + .map_err(|err| mfa_config_other(format!("Failed to build OpenID URL: {err}")))?; + url.query_pairs_mut() + .append_pair("token", &session.session_token); + Ok(url.to_string()) +} + +/// the browser must already be open on mfa_config_oidc_url, this only waits for the login +#[tauri::command(async)] +pub async fn mfa_config_authorize_oidc( + session_id: String, + state: State<'_, AppState>, +) -> Result { + debug!("Waiting for OpenID to authorize the MFA configuration session"); + let uid = parse_mfa_config_session_id(&session_id)?; + let attempt = CeremonyGuard::register(&state, uid).map_err(err_to_json)?; + let session = get_mfa_config_session(&state, &session_id)?; + + let response = mfa_config::mfa_config_poll_oidc( + session.proxy_url, + session.session_token, + session.deadline_timestamp, + attempt.token(), + ) + .await + .map_err(err_to_json)?; + + extend_mfa_config_session(&state, uid, &response); + info!("Authorized MFA configuration session with OpenID"); Ok(response) } @@ -2562,7 +2696,7 @@ pub async fn mfa_config_setup_fido2( handle: AppHandle, ) -> Result { debug!("Starting FIDO2 MFA factor setup"); - // The PIN is never logged, here or anywhere below. + // never log the PIN, here or below let pin = fido2_pin(pin).map_err(|message| { err_to_json(MfaConfigError::SecurityKey { message: message.to_string(), @@ -2597,15 +2731,15 @@ pub async fn mfa_config_setup_fido2( .fido2_creation_challenge .ok_or_else(|| mfa_config_other("Defguard did not return a security key challenge"))?; - // The cancel may have landed while the challenge was in flight. + // the cancel may have landed while the challenge was in flight if ceremony.is_cancelled() { debug!("Security key registration was cancelled before the prompt opened"); return Err(err_to_json(MfaConfigError::Cancelled)); } - // From here the key blinks and waits for a touch, and gives up if none comes. + // the key blinks from here on and gives up without a touch let _ = handle.emit(EventKey::MfaConfigFido2Touch.into(), ()); - // The prompt must not open behind the window that asked for it. + // the prompt must not open behind the window that asked for it let _level = WindowLevelGuard::lower(&window); let attestation = defguard_client_fido2::register_security_key( &challenge, @@ -2616,15 +2750,15 @@ pub async fn mfa_config_setup_fido2( ) .await .map_err(|err| match err { - // Core minted the challenge, so a bad one is not the user's problem. + // Core minted the challenge, so a bad one is not the user's problem Fido2Error::MalformedChallenge(detail) => mfa_config_other(format!( "Defguard sent a malformed security key challenge: {detail}" )), - err => err_to_json(registration_error(&err)), + err => err_to_json(mfa_config_fido2_error(&err, "complete the registration")), })?; - // A platform that cannot abort a waiting key reports the cancel only once the ceremony is - // over, and this is the last point one can be caught before the factor is submitted. + // a platform that cannot abort a waiting key reports the cancel only once the ceremony is + // over, the last point one can be caught before the factor is submitted if ceremony.is_cancelled() { debug!("Security key registration was cancelled, discarding the attestation"); return Err(err_to_json(MfaConfigError::Cancelled)); @@ -2656,13 +2790,8 @@ pub async fn mfa_config_setup_fido2( Ok(response) } -fn cancel_mfa_config_session(state: &AppState, uid: Uuid) { - state - .mfa_config_sessions - .lock() - .expect("mfa_config_sessions mutex poisoned") - .remove(&uid); - // A ceremony may still be waiting for a touch, behind a prompt only the platform can close. +/// a ceremony may still be waiting for a touch, behind a prompt only the platform can close +fn abort_mfa_config_attempt(state: &AppState, uid: Uuid) { if let Some(ceremony) = state .mfa_config_ceremonies .lock() @@ -2673,6 +2802,27 @@ fn cancel_mfa_config_session(state: &AppState, uid: Uuid) { } } +fn cancel_mfa_config_session(state: &AppState, uid: Uuid) { + state + .mfa_config_sessions + .lock() + .expect("mfa_config_sessions mutex poisoned") + .remove(&uid); + abort_mfa_config_attempt(state, uid); +} + +/// unlike mfa_config_cancel, the session survives for another method +#[tauri::command(async)] +pub async fn mfa_config_abort_attempt( + session_id: String, + state: State<'_, AppState>, +) -> Result<(), String> { + debug!("Aborting the running MFA configuration attempt"); + let uid = parse_mfa_config_session_id(&session_id)?; + abort_mfa_config_attempt(&state, uid); + Ok(()) +} + #[tauri::command(async)] pub async fn mfa_config_cancel( session_id: String, @@ -3147,6 +3297,20 @@ mod tests { assert!(ceremonies_are_empty(&state)); } + #[test] + fn test_mfa_config_abort_attempt_cancels_the_token_and_keeps_the_session() { + let state = AppState::new(AppConfig::default(), None); + let uid = live_session(&state); + let attempt = CeremonyGuard::register(&state, uid).expect("attempt registers"); + + abort_mfa_config_attempt(&state, uid); + + assert!(attempt.is_cancelled()); + assert!(ceremonies_are_empty(&state)); + assert!(get_mfa_config_session(&state, &uid.to_string()).is_ok()); + drop(CeremonyGuard::register(&state, uid).expect("the slot is free again")); + } + #[test] fn test_ceremony_guard_rejects_a_second_claim() { let state = AppState::new(AppConfig::default(), None); diff --git a/src-tauri/src/gui.rs b/src-tauri/src/gui.rs index c1366b9dd..3bba07a1d 100644 --- a/src-tauri/src/gui.rs +++ b/src-tauri/src/gui.rs @@ -219,6 +219,10 @@ pub fn run_app() { mfa_config_start, mfa_config_send_code, mfa_config_authorize, + mfa_config_authorize_fido2, + mfa_config_oidc_url, + mfa_config_authorize_oidc, + mfa_config_abort_attempt, mfa_config_setup_start, mfa_config_setup_finish, mfa_config_setup_fido2,
+ + {collectsPin + ? 'Insert your security key and enter its PIN to continue.' + : 'Insert your security key and continue in the prompt your system shows.'} + +
{error}
+ + {isPolling + ? 'Complete the sign-in in your browser. This page will update automatically.' + : 'Authenticate via your OpenID provider. A browser window will open for you to sign in.'} + +