Skip to content

Commit d8fbfde

Browse files
update location factor guards based on new proto (#1214)
* update location factor guards based on new proto * Update mfa.test.ts * review fixes * Update TauriEventProvider.tsx
1 parent d24b821 commit d8fbfde

48 files changed

Lines changed: 1145 additions & 126 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/** The session opened, but nothing it offers to verify with can run on this instance. */
2+
export class NoVerificationMethodError extends Error {
3+
constructor() {
4+
super('No MFA method available to verify the configuration session');
5+
this.name = 'NoVerificationMethodError';
6+
}
7+
}

‎new-ui/src/pages/full/ConfigureMfaPage/hooks/reportMfaConfigStartError.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,17 @@ import {
66
isMfaConfigUnsupported,
77
} from '../../../../shared/rust-api/mfaError';
88
import { showEdgeComsError } from '../components/EdgeComsError/useEdgeComsErrorStore';
9+
import { NoVerificationMethodError } from './noVerificationMethodError';
910

1011
/** How a failed `startMfaConfiguration` is reported, shared by every entry point. */
1112
export const reportMfaConfigStartError = (err: unknown): void => {
1213
void logError(`MFA configuration start failed: ${err}`);
14+
if (err instanceof NoVerificationMethodError) {
15+
Snackbar.error(
16+
'There is no method available to verify your identity on this Defguard instance. Contact your administrator.',
17+
);
18+
return;
19+
}
1320
if (isMfaConfigUnsupported(err)) {
1421
Snackbar.error(
1522
'This Defguard instance does not support configuring MFA from the client.',

‎new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import {
55
type InstanceInfo,
66
MfaMethod,
77
} from '../../../../shared/rust-api/types';
8+
import { isMfaMethodAvailable } from '../../../../shared/utils/mfa';
89
import { ConfigureMfaStep } from '../types';
10+
import { NoVerificationMethodError } from './noVerificationMethodError';
911
import {
1012
applyAuthorization,
1113
discardMfaConfiguration,
@@ -32,6 +34,8 @@ const instance: InstanceInfo = {
3234
disable_tunnels: false,
3335
openid_display_name: null,
3436
openid_provider_kind: 'custom',
37+
smtp_configured: null,
38+
openid_available: null,
3539
mfa_configured_methods: [],
3640
mfa_capabilities: {
3741
setup_methods: [MfaMethod.Totp, MfaMethod.Fido2],
@@ -153,4 +157,69 @@ describe('startMfaConfiguration', () => {
153157
expect(state.configuredMethods).toContain(MfaMethod.Totp);
154158
expect(state.initialSelection).toEqual([]);
155159
});
160+
161+
it('trusts the email fallback over a snapshot that still reports no SMTP', async () => {
162+
Object.assign(api, {
163+
mfaConfigStart: vi.fn().mockResolvedValue({
164+
...startResult,
165+
available_methods: [],
166+
configured_methods: [],
167+
email_fallback: true,
168+
}),
169+
});
170+
171+
await startMfaConfiguration({ ...instance, smtp_configured: false });
172+
173+
expect(useConfigureMfaStore.getState().verificationMethods).toEqual([
174+
MfaMethod.Email,
175+
]);
176+
});
177+
178+
it('reads email as available after the fallback, without offering it', async () => {
179+
Object.assign(api, {
180+
mfaConfigStart: vi.fn().mockResolvedValue({
181+
...startResult,
182+
available_methods: [],
183+
configured_methods: [],
184+
email_fallback: true,
185+
}),
186+
});
187+
188+
await startMfaConfiguration(
189+
{
190+
...instance,
191+
smtp_configured: false,
192+
mfa_capabilities: {
193+
setup_methods: [MfaMethod.Totp, MfaMethod.Email],
194+
authorize_methods: [MfaMethod.Totp],
195+
},
196+
},
197+
{ preselectedMethods: [MfaMethod.Email] },
198+
);
199+
200+
const state = useConfigureMfaStore.getState();
201+
expect(isMfaMethodAvailable(MfaMethod.Email, state.instance)).toBe(true);
202+
expect(state.configuredMethods).toContain(MfaMethod.Email);
203+
expect(state.initialSelection).toEqual([]);
204+
});
205+
206+
it('refuses a session nothing can verify', async () => {
207+
useConfigureMfaStore.getState().reset();
208+
const mfaConfigCancel = vi.fn().mockResolvedValue(undefined);
209+
Object.assign(api, {
210+
mfaConfigCancel,
211+
mfaConfigStart: vi.fn().mockResolvedValue({
212+
...startResult,
213+
available_methods: [],
214+
configured_methods: [],
215+
email_fallback: false,
216+
}),
217+
});
218+
219+
await expect(startMfaConfiguration(instance)).rejects.toBeInstanceOf(
220+
NoVerificationMethodError,
221+
);
222+
expect(mfaConfigCancel).toHaveBeenCalledWith('session-2');
223+
expect(useConfigureMfaStore.getState().sessionId).toBeNull();
224+
});
156225
});

‎new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ import {
2626
isMfaFactorOfferable,
2727
isMfaSetupStep,
2828
mfaFactorStep,
29-
verificationMethodsOf,
29+
sessionVerificationMethodsOf,
3030
} from '../utils';
31+
import { NoVerificationMethodError } from './noVerificationMethodError';
3132

3233
type StoreValues = {
3334
activeStep: ConfigureMfaStepValue;
@@ -151,11 +152,8 @@ export const useConfigureMfaStore = create<Store>()(
151152
(set, get) => ({
152153
...defaults,
153154
start: (instance, response, origin) => {
154-
// The fallback mails a code to the address on file, registering email along the way.
155-
const sessionMethods = response.email_fallback
156-
? [MfaMethod.Email]
157-
: response.available_methods;
158-
// a factor the instance cannot authorize with is still configured
155+
// The fallback registers email as it mails the code. A factor the instance cannot
156+
// authorize with is still configured.
159157
const sessionConfiguredMethods = response.email_fallback
160158
? [MfaMethod.Email]
161159
: response.configured_methods;
@@ -170,10 +168,13 @@ export const useConfigureMfaStore = create<Store>()(
170168
];
171169
set({
172170
...defaults,
173-
instance,
171+
// The fallback just mailed a code, so SMTP works whatever the snapshot says.
172+
instance: response.email_fallback
173+
? { ...instance, smtp_configured: true }
174+
: instance,
174175
sessionId: response.session_id,
175176
configuredMethods,
176-
verificationMethods: verificationMethodsOf(sessionMethods),
177+
verificationMethods: sessionVerificationMethodsOf(response),
177178
emailFallback: response.email_fallback,
178179
deadline: dayjs.unix(response.deadline_timestamp).toISOString(),
179180
...origin,
@@ -287,6 +288,11 @@ export const startMfaConfiguration = async (
287288
await pendingEnd;
288289
const response = await api.mfaConfigStart(instance.id);
289290
dismissEdgeComsError();
291+
if (sessionVerificationMethodsOf(response).length === 0) {
292+
pendingEnd = endSession(response.session_id);
293+
await pendingEnd;
294+
throw new NoVerificationMethodError();
295+
}
290296
useConfigureMfaStore.getState().start(instance, response, { source, location });
291297
// Only pre-ticks, the user still confirms in the selection step.
292298
const { configuredMethods } = useConfigureMfaStore.getState();

‎new-ui/src/pages/full/ConfigureMfaPage/utils.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { MfaMethod, type MfaMethodValue } from '../../../shared/rust-api/types';
1+
import {
2+
type MfaConfigStartResult,
3+
MfaMethod,
4+
type MfaMethodValue,
5+
} from '../../../shared/rust-api/types';
26
import {
37
CLIENT_CONFIGURABLE_METHODS,
48
type ClientConfigurableMethod,
@@ -62,3 +66,11 @@ export const verificationMethodsOf = (
6266
availableMethods: MfaMethodValue[],
6367
): MfaVerificationMethod[] =>
6468
mfaVerificationMethods.filter((method) => availableMethods.includes(method));
69+
70+
/** The fallback mails a code to the address on file, so email is the only way in. */
71+
export const sessionVerificationMethodsOf = (
72+
response: Pick<MfaConfigStartResult, 'email_fallback' | 'available_methods'>,
73+
): MfaVerificationMethod[] =>
74+
verificationMethodsOf(
75+
response.email_fallback ? [MfaMethod.Email] : response.available_methods,
76+
);

‎new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureMfaVerify.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export const ConfigureMfaVerify = ({ onCancel, onSessionExpired }: Props) => {
2828
return <ConfigureSelectVerificationStep />;
2929
}
3030

31-
// for the type only, a session always offers a method or falls back to email
31+
// for the type only, a session with nothing to verify with is refused at start
3232
const method: MfaVerificationMethod =
3333
verificationMethod ?? candidates[0] ?? MfaMethod.Email;
3434

‎new-ui/src/pages/full/ConfigureMfaPage/verify/ConfigureSelectMethodsStep/ConfigureSelectMethodsStep.tsx‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
CLIENT_CONFIGURABLE_METHODS,
1919
isClientConfigurableMethod,
2020
isDesktopDrivableMethod,
21+
isMfaMethodAvailable,
2122
mfaStepsOf as locationMfaSteps,
2223
mfaToText,
2324
setupMethodsOf,
@@ -63,16 +64,17 @@ export const ConfigureSelectMethodsStep = ({ onCancel }: Props) => {
6364
return configuredMethods.filter((method) => method !== MfaMethod.Email);
6465
}, [emailFallback, configuredMethods]);
6566

66-
/** A step the user is already past: it offers a factor the desktop can drive and the account
67-
* holds. Mirrors `usableMfaMethods`, on the session's fresher list of configured factors. */
67+
/** A step the user is already past: it offers a factor the desktop can drive, the account
68+
* holds and the instance runs. Mirrors usableMfaMethods, on the session's fresher list. */
6869
const isStepSatisfied = useCallback(
6970
(step: MfaStep): boolean =>
7071
step.methods.some(
7172
(entry) =>
7273
isDesktopDrivableMethod(entry.method) &&
73-
configuredMethods.includes(entry.method),
74+
configuredMethods.includes(entry.method) &&
75+
isMfaMethodAvailable(entry.method, instance),
7476
),
75-
[configuredMethods],
77+
[configuredMethods, instance],
7678
);
7779

7880
const groups = useMemo(() => {
@@ -99,10 +101,13 @@ export const ConfigureSelectMethodsStep = ({ onCancel }: Props) => {
99101
// A repeatable factor stays offerable once configured, keeping its badge and its pick.
100102
const disabled = !isMfaFactorOfferable(method, configuredMethods, setupMethods);
101103
const configured = accountConfigured.includes(method);
102-
const satisfied = configuredMethods.includes(method);
104+
const available = isMfaMethodAvailable(method, instance);
105+
const satisfied = configuredMethods.includes(method) && available;
103106

104107
let hint: string | null = null;
105-
if (disabled && !configured) {
108+
if (!available) {
109+
hint = `${mfaToText(method, instance ?? undefined)} is not available on this Defguard instance.`;
110+
} else if (disabled && !configured) {
106111
if (satisfied) {
107112
// Reached only via the email fallback, which registers the factor as it verifies.
108113
hint = `${mfaToText(method)} is required to continue.`;
@@ -125,7 +130,7 @@ export const ConfigureSelectMethodsStep = ({ onCancel }: Props) => {
125130
hint,
126131
};
127132
},
128-
[accountConfigured, configuredMethods, setupMethods, selected],
133+
[accountConfigured, configuredMethods, setupMethods, selected, instance],
129134
);
130135

131136
const { mutate: cancel, isPending: isCancelling } = useMutation({

‎new-ui/src/pages/full/OverviewPage/components/ConnectModal/hooks/useConnectModal.tsx‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import {
44
MfaMethod,
55
type MfaMethodValue,
66
} from '../../../../../../shared/rust-api/types';
7-
import { isPresent } from '../../../../../../shared/utils/isPresent';
8-
import { resolveMfaStepPlan } from '../../../../../../shared/utils/mfa';
97
import { type ConnectModalViewValue, mfaMethodToConnectModalView } from './types';
108

119
interface StoreValues {
@@ -50,9 +48,7 @@ export const useConnectModal = create<Store>((set, get) => ({
5048
set(defaults);
5149
},
5250
open: (init) => {
53-
const location = init?.location ?? null;
54-
const stepPlan = isPresent(location) ? resolveMfaStepPlan(location) : [];
55-
set({ ...defaults, ...init, stepPlan, visible: true });
51+
set({ ...defaults, ...init, visible: true });
5652
},
5753
setMfaToken: (token) => {
5854
set({ mfaToken: token });

‎new-ui/src/pages/playground/components/PlaygroundTestMfaSettingsSection/PlaygroundTestMfaSettingsSection.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ const instance: MfaSettingsInstance = {
5959
},
6060
openid_display_name: null,
6161
openid_provider_kind: 'custom',
62+
smtp_configured: null,
63+
openid_available: null,
6264
};
6365

6466
const Demo = ({

‎new-ui/src/shared/components/LocationCard/components/ConnectButton/ConnectButton.tsx‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import { TooltipContent } from '../../../../providers/tooltip/TooltipContent';
2+
import { TooltipProvider } from '../../../../providers/tooltip/TooltipContext';
3+
import { TooltipTrigger } from '../../../../providers/tooltip/TooltipTrigger';
14
import { isPresent } from '../../../../utils/isPresent';
25
import { Icon, type IconKindValue } from '../../../Icon';
36
import './style.scss';
@@ -9,30 +12,47 @@ interface Props {
912
disabled?: boolean;
1013
icon?: IconKindValue | null;
1114
text?: string | null;
15+
tooltip?: string | null;
1216
}
1317

1418
export const ConnectButton = ({
1519
active,
1620
onClick,
1721
icon,
1822
text,
23+
tooltip,
1924
disabled = false,
2025
}: Props) => {
2126
const label = isPresent(text) ? text : active ? 'Disconnect' : 'Connect VPN';
2227

23-
return (
28+
const button = (
29+
// aria-disabled instead of disabled, a disabled button gets no hover to open the tooltip
2430
<button
2531
type="button"
2632
className={clsx('connect-button', {
2733
connected: active,
2834
disconnected: !active,
2935
icon: isPresent(icon),
36+
disabled,
3037
})}
31-
disabled={disabled}
32-
onClick={onClick}
38+
aria-disabled={disabled}
39+
onClick={() => {
40+
if (!disabled) onClick();
41+
}}
3342
>
3443
{isPresent(icon) && <Icon icon={icon} size={20} />}
3544
<p>{label}</p>
3645
</button>
3746
);
47+
48+
if (!isPresent(tooltip)) return button;
49+
50+
return (
51+
<TooltipProvider placement="top">
52+
<TooltipTrigger>{button}</TooltipTrigger>
53+
<TooltipContent>
54+
<p>{tooltip}</p>
55+
</TooltipContent>
56+
</TooltipProvider>
57+
);
3858
};

0 commit comments

Comments
 (0)