Skip to content

Commit 1603fcb

Browse files
Fix mobile approve factor flickering issue (#1143)
1 parent 08fe4ab commit 1603fcb

5 files changed

Lines changed: 68 additions & 40 deletions

File tree

‎new-ui/src/pages/compact/CompactEmptyPage/CompactEmptyPage.tsx‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,32 @@
11
import './style.scss';
2+
import { useQuery } from '@tanstack/react-query';
3+
import { useNavigate } from '@tanstack/react-router';
4+
import { useEffect } from 'react';
25
import { Button } from '../../../shared/components/Button/Button';
36
import { ButtonSize, ButtonVariant } from '../../../shared/components/Button/types';
47
import { Icon, IconKind } from '../../../shared/components/Icon';
58
import { WindowHeader } from '../../../shared/components/WindowHeader/WindowHeader';
69
import { api } from '../../../shared/rust-api/api';
10+
import {
11+
getInstancesQueryOptions,
12+
getTunnelsQueryOptions,
13+
} from '../../../shared/rust-api/query';
714
import { CompactPage } from '../CompactPage/CompactPage';
815

916
export const CompactEmptyPage = () => {
17+
const navigate = useNavigate();
18+
19+
const { data: instances } = useQuery(getInstancesQueryOptions);
20+
const { data: tunnels } = useQuery(getTunnelsQueryOptions);
21+
22+
const hasAny = (instances?.length ?? 0) > 0 || (tunnels?.length ?? 0) > 0;
23+
24+
useEffect(() => {
25+
if (hasAny) {
26+
void navigate({ to: '/compact', replace: true });
27+
}
28+
}, [hasAny, navigate]);
29+
1030
return (
1131
<CompactPage containerProps={{ id: 'compact-empty-page' }}>
1232
<WindowHeader variant="compact" />

‎new-ui/src/routes/empty.tsx‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,6 @@
1-
import { useQuery } from '@tanstack/react-query';
2-
import { createFileRoute, useNavigate } from '@tanstack/react-router';
3-
import { useEffect } from 'react';
4-
5-
import { hasAnyVisibleLocationsQueryOptions } from '../shared/rust-api/query';
1+
import { createFileRoute } from '@tanstack/react-router';
2+
import { CompactEmptyPage } from '../pages/compact/CompactEmptyPage/CompactEmptyPage';
63

74
export const Route = createFileRoute('/empty')({
8-
component: RouteComponent,
5+
component: CompactEmptyPage,
96
});
10-
11-
function RouteComponent() {
12-
const navigate = useNavigate();
13-
const { data: hasLocations } = useQuery(hasAnyVisibleLocationsQueryOptions);
14-
15-
useEffect(() => {
16-
if (hasLocations === true) {
17-
void navigate({ to: '/' });
18-
}
19-
}, [hasLocations, navigate]);
20-
21-
return <div></div>;
22-
}

‎new-ui/src/shared/components/LocationCard/hooks/useMfaMobileConnect.ts‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { error } from '@tauri-apps/plugin-log';
66
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
77
import { api } from '../../../rust-api/api';
88
import {
9+
isCancelled,
910
isConnectFailure,
1011
isMfaPostureError,
1112
isServiceUnavailable,
@@ -14,6 +15,7 @@ import {
1415
import { getInstancesQueryOptions } from '../../../rust-api/query';
1516
import type { LocationInfo, MfaErrorPayload } from '../../../rust-api/types';
1617
import { MfaMethod, TauriEvent } from '../../../rust-api/types';
18+
import { isPresent } from '../../../utils/isPresent';
1719

1820
type TokenData = {
1921
token: string;
@@ -27,10 +29,18 @@ type Options = {
2729
};
2830

2931
export const useMfaMobileConnect = (location: LocationInfo, options?: Options) => {
30-
const { onConnected, onPostureError, onServiceUnavailable } = options ?? {};
31-
3232
const { data: instances } = useQuery(getInstancesQueryOptions);
3333
const instance = instances?.find((i) => i.id === location.instance_id);
34+
const instanceId = instance?.id;
35+
const instanceUuid = instance?.uuid;
36+
const locationId = location.id;
37+
38+
// Read through refs so callers passing inline callbacks or a re-fetched
39+
// `location` object don't restart the mobile-approve task on every render.
40+
const optionsRef = useRef(options);
41+
optionsRef.current = options;
42+
const locationRef = useRef(location);
43+
locationRef.current = location;
3444

3545
const [isStarting, setIsStarting] = useState(false);
3646
const [startError, setStartError] = useState<string | null>(null);
@@ -61,7 +71,7 @@ export const useMfaMobileConnect = (location: LocationInfo, options?: Options) =
6171

6272
// Connect WebSocket via Rust when tokenData is available
6373
useEffect(() => {
64-
if (!tokenData || !instance) return;
74+
if (!tokenData || !isPresent(instanceId)) return;
6575

6676
let cancelled = false;
6777
cleanupListeners();
@@ -71,8 +81,8 @@ export const useMfaMobileConnect = (location: LocationInfo, options?: Options) =
7181
(async () => {
7282
try {
7383
const taskId = await api.mfaConnectMobileApprove(
74-
instance.id,
75-
location.id,
84+
instanceId,
85+
locationId,
7686
tokenData.token,
7787
);
7888
if (cancelled) {
@@ -84,18 +94,20 @@ export const useMfaMobileConnect = (location: LocationInfo, options?: Options) =
8494
// The backend brings up the connection itself; completion means connected.
8595
const completeUnlisten = await listen(TauriEvent.MfaMobileComplete, () => {
8696
cleanupListeners();
97+
taskIdRef.current = null;
8798
setIsConnecting(false);
88-
onConnected?.();
99+
optionsRef.current?.onConnected?.();
89100
});
90101

91102
const errorUnlisten = await listen<MfaErrorPayload>(
92103
TauriEvent.MfaMobileError,
93104
(event) => {
105+
// Emitted app-wide without a task id, so our own cancels land here.
106+
if (isCancelled(event.payload.error)) return;
94107
cleanupListeners();
108+
taskIdRef.current = null;
95109
setIsConnecting(false);
96-
error(
97-
`Mobile MFA failed for location ${location.id}: ${event.payload.error}`,
98-
);
110+
error(`Mobile MFA failed for location ${locationId}: ${event.payload.error}`);
99111
const message = mfaErrorMessage(event.payload.error);
100112
setConnectionError(
101113
isConnectFailure(message)
@@ -113,30 +125,35 @@ export const useMfaMobileConnect = (location: LocationInfo, options?: Options) =
113125
if (!cancelled) {
114126
setIsConnecting(false);
115127
setConnectionError('Failed to start mobile approval. Please try again.');
116-
error(`Mobile MFA connect failed for location ${location.id}: ${e}`);
128+
error(`Mobile MFA connect failed for location ${locationId}: ${e}`);
117129
}
118130
}
119131
})();
120132

121133
return () => {
122134
cancelled = true;
123135
cleanupListeners();
136+
const taskId = taskIdRef.current;
137+
if (taskId) {
138+
taskIdRef.current = null;
139+
void api.cancelMfa(taskId).catch(() => {});
140+
}
124141
setIsConnecting(false);
125142
};
126-
}, [tokenData, instance, location, onConnected, cleanupListeners]);
143+
}, [tokenData, instanceId, locationId, cleanupListeners]);
127144

128145
const qrValue = useMemo(() => {
129-
if (!tokenData || !instance) return null;
146+
if (!tokenData || !isPresent(instanceUuid)) return null;
130147
const json = JSON.stringify({
131148
token: tokenData.token,
132149
challenge: tokenData.challenge,
133-
instance_id: instance.uuid,
150+
instance_id: instanceUuid,
134151
});
135152
return encode(new TextEncoder().encode(json));
136-
}, [tokenData, instance]);
153+
}, [tokenData, instanceUuid]);
137154

138155
const start = useCallback(async () => {
139-
if (!instance) {
156+
if (!isPresent(instanceId)) {
140157
setStartError('Instance not found');
141158
return;
142159
}
@@ -148,28 +165,28 @@ export const useMfaMobileConnect = (location: LocationInfo, options?: Options) =
148165
setTokenData(null);
149166

150167
try {
151-
const info = await api.mfaStart(instance.id, location.id, MfaMethod.MobileApprove);
168+
const info = await api.mfaStart(instanceId, locationId, MfaMethod.MobileApprove);
152169
if (!info.challenge) {
153170
setStartError('Unsupported response from proxy');
154171
return;
155172
}
156173

157174
setTokenData({ token: info.token, challenge: info.challenge });
158175
} catch (e) {
159-
void error(`Mobile MFA start failed for location ${location.id}: ${e}`);
160-
if (isMfaPostureError(e, location)) {
161-
onPostureError?.(mfaErrorMessage(e));
176+
void error(`Mobile MFA start failed for location ${locationId}: ${e}`);
177+
if (isMfaPostureError(e, locationRef.current)) {
178+
optionsRef.current?.onPostureError?.(mfaErrorMessage(e));
162179
return;
163180
}
164181
if (isServiceUnavailable(e)) {
165-
onServiceUnavailable?.();
182+
optionsRef.current?.onServiceUnavailable?.();
166183
return;
167184
}
168185
setStartError(mfaErrorMessage(e));
169186
} finally {
170187
setIsStarting(false);
171188
}
172-
}, [instance, location, onPostureError, onServiceUnavailable]);
189+
}, [instanceId, locationId]);
173190

174191
const reset = useCallback(() => {
175192
cleanupListeners();

‎new-ui/src/shared/components/LocationCard/hooks/useMfaOidcConnect.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { error } from '@tauri-apps/plugin-log';
55
import { useCallback, useEffect, useRef, useState } from 'react';
66
import { api } from '../../../rust-api/api';
77
import {
8+
isCancelled,
89
isConnectFailure,
910
isMfaPostureError,
1011
isServiceUnavailable,
@@ -81,6 +82,8 @@ export const useMfaOidcConnect = () => {
8182
const errorUnlisten = await listen<MfaErrorPayload>(
8283
TauriEvent.MfaOpenIdError,
8384
(event) => {
85+
// Emitted app-wide without a task id, so our own cancels land here.
86+
if (isCancelled(event.payload.error)) return;
8487
cleanup();
8588
setIsPolling(false);
8689
error(`OIDC MFA failed for location ${location.id}: ${event.payload.error}`);

‎new-ui/src/shared/rust-api/mfaError.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ export const isSessionExpired = (message: string): boolean =>
3737
export const isTimeout = (err: unknown): boolean =>
3838
parseMfaError(err)?.type === 'timeout';
3939

40+
/** The task was cancelled via `cancel_mfa`, not a user-facing failure. */
41+
export const isCancelled = (err: unknown): boolean =>
42+
parseMfaError(err)?.type === 'cancelled';
43+
4044
/** A submitted one-time code was rejected. */
4145
export const isInvalidCode = (message: string): boolean =>
4246
message.includes('Unauthorized');

0 commit comments

Comments
 (0)