Skip to content

Commit bfd7e8c

Browse files
pushpak1300pushpak1300
committed
Improve two-factor authentication error handling and UI updates
1 parent 9965517 commit bfd7e8c

File tree

8 files changed

+132
-80
lines changed

8 files changed

+132
-80
lines changed

‎app/Http/Requests/Auth/LoginRequest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function validateCredentials(): User
4141
{
4242
$this->ensureIsNotRateLimited();
4343

44-
/** @var User $user */
44+
/** @var User|null $user */
4545
$user = Auth::getProvider()->retrieveByCredentials($this->only('email', 'password'));
4646

4747
if (! $user || ! Auth::getProvider()->validateCredentials($user, $this->only('password'))) {

‎resources/js/components/delete-user.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ export default function DeleteUser() {
2222

2323
<Dialog>
2424
<DialogTrigger asChild>
25-
<Button variant="destructive" data-test="delete-user-button">Delete account</Button>
25+
<Button variant="destructive" data-test="delete-user-button">
26+
Delete account
27+
</Button>
2628
</DialogTrigger>
2729
<DialogContent>
2830
<DialogTitle>Are you sure you want to delete your account?</DialogTitle>
@@ -67,7 +69,9 @@ export default function DeleteUser() {
6769
</DialogClose>
6870

6971
<Button variant="destructive" disabled={processing} asChild>
70-
<button type="submit" data-test="confirm-delete-user-button">Delete account</button>
72+
<button type="submit" data-test="confirm-delete-user-button">
73+
Delete account
74+
</button>
7175
</Button>
7276
</DialogFooter>
7377
</>

‎resources/js/components/two-factor-recovery-codes.tsx‎

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
1+
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
12
import { Button } from '@/components/ui/button';
23
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card';
34
import { regenerateRecoveryCodes } from '@/routes/two-factor';
45
import { Form } from '@inertiajs/react';
5-
import { Eye, EyeOff, LockKeyhole, RefreshCw } from 'lucide-react';
6+
import { AlertCircleIcon, Eye, EyeOff, LockKeyhole, RefreshCw } from 'lucide-react';
67
import { useCallback, useEffect, useRef, useState } from 'react';
78

89
interface TwoFactorRecoveryCodesProps {
910
recoveryCodesList: string[];
1011
fetchRecoveryCodes: () => Promise<void>;
12+
errors: string[];
1113
}
1214

13-
export default function TwoFactorRecoveryCodes({ recoveryCodesList, fetchRecoveryCodes }: TwoFactorRecoveryCodesProps) {
15+
export default function TwoFactorRecoveryCodes({ recoveryCodesList, fetchRecoveryCodes, errors }: TwoFactorRecoveryCodesProps) {
1416
const [codesAreVisible, setCodesAreVisible] = useState<boolean>(false);
1517
const codesSectionRef = useRef<HTMLDivElement | null>(null);
18+
const canRegenerateCodes = recoveryCodesList.length > 0 && codesAreVisible;
1619

1720
const toggleCodesVisibility = useCallback(async () => {
1821
if (!codesAreVisible && !recoveryCodesList.length) {
@@ -57,7 +60,7 @@ export default function TwoFactorRecoveryCodes({ recoveryCodesList, fetchRecover
5760
{codesAreVisible ? 'Hide' : 'View'} Recovery Codes
5861
</Button>
5962

60-
{codesAreVisible && (
63+
{canRegenerateCodes && (
6164
<Form {...regenerateRecoveryCodes.form()} options={{ preserveScroll: true }} onSuccess={fetchRecoveryCodes}>
6265
{({ processing }) => (
6366
<Button variant="secondary" type="submit" disabled={processing} aria-describedby="regenerate-warning">
@@ -73,32 +76,49 @@ export default function TwoFactorRecoveryCodes({ recoveryCodesList, fetchRecover
7376
aria-hidden={!codesAreVisible}
7477
>
7578
<div className="mt-3 space-y-3">
76-
<div
77-
ref={codesSectionRef}
78-
className="grid gap-1 rounded-lg bg-muted p-4 font-mono text-sm"
79-
role="list"
80-
aria-label="Recovery codes"
81-
>
82-
{recoveryCodesList.length ? (
83-
recoveryCodesList.map((code, index) => (
84-
<div key={index} role="listitem" className="select-text">
85-
{code}
86-
</div>
87-
))
88-
) : (
89-
<div className="space-y-2" aria-label="Loading recovery codes">
90-
{Array.from({ length: 8 }, (_, index) => (
91-
<div key={index} className="h-4 animate-pulse rounded bg-muted-foreground/20" aria-hidden="true" />
92-
))}
79+
{errors?.length ? (
80+
<Alert variant="destructive">
81+
<AlertCircleIcon />
82+
<AlertTitle>Something went wrong.</AlertTitle>
83+
<AlertDescription>
84+
<ul className="list-disc text-sm">
85+
{errors.map((error, index) => (
86+
<li key={index}>{error}</li>
87+
))}
88+
</ul>
89+
</AlertDescription>
90+
</Alert>
91+
) : (
92+
<>
93+
<div
94+
ref={codesSectionRef}
95+
className="grid gap-1 rounded-lg bg-muted p-4 font-mono text-sm"
96+
role="list"
97+
aria-label="Recovery codes"
98+
>
99+
{recoveryCodesList.length ? (
100+
recoveryCodesList.map((code, index) => (
101+
<div key={index} role="listitem" className="select-text">
102+
{code}
103+
</div>
104+
))
105+
) : (
106+
<div className="space-y-2" aria-label="Loading recovery codes">
107+
{Array.from({ length: 8 }, (_, index) => (
108+
<div key={index} className="h-4 animate-pulse rounded bg-muted-foreground/20" aria-hidden="true" />
109+
))}
110+
</div>
111+
)}
93112
</div>
94-
)}
95-
</div>
96-
<div className="text-xs text-muted-foreground select-none">
97-
<p id="regenerate-warning">
98-
Each recovery code can be used once to access your account and will be removed after use. If you need more, click{' '}
99-
<span className="font-bold">Regenerate Codes</span> above.
100-
</p>
101-
</div>
113+
114+
<div className="text-xs text-muted-foreground select-none">
115+
<p id="regenerate-warning">
116+
Each recovery code can be used once to access your account and will be removed after use. If you need more,
117+
click <span className="font-bold">Regenerate Codes</span> above.
118+
</p>
119+
</div>
120+
</>
121+
)}
102122
</div>
103123
</div>
104124
</CardContent>

‎resources/js/components/two-factor-setup-modal.tsx‎

Lines changed: 62 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import InputError from '@/components/input-error';
2+
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
23
import { Button } from '@/components/ui/button';
34
import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/components/ui/dialog';
45
import { InputOTP, InputOTPGroup, InputOTPSlot } from '@/components/ui/input-otp';
@@ -7,7 +8,7 @@ import { OTP_MAX_LENGTH } from '@/hooks/use-two-factor-auth';
78
import { confirm } from '@/routes/two-factor';
89
import { Form } from '@inertiajs/react';
910
import { REGEXP_ONLY_DIGITS } from 'input-otp';
10-
import { Check, Copy, Loader2, ScanLine } from 'lucide-react';
11+
import { AlertCircleIcon, Check, Copy, Loader2, ScanLine } from 'lucide-react';
1112
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
1213

1314
function GridScanIcon() {
@@ -35,63 +36,79 @@ function TwoFactorSetupStep({
3536
manualSetupKey,
3637
buttonText,
3738
onNextStep,
39+
errors,
3840
}: {
3941
qrCodeSvg: string | null;
4042
manualSetupKey: string | null;
4143
buttonText: string;
4244
onNextStep: () => void;
45+
errors: string[];
4346
}) {
4447
const [copiedText, copy] = useClipboard();
4548
const IconComponent = copiedText === manualSetupKey ? Check : Copy;
4649

4750
return (
4851
<>
49-
<div className="mx-auto flex max-w-md overflow-hidden">
50-
<div className="mx-auto aspect-square w-64 rounded-lg border border-border">
51-
{qrCodeSvg ? (
52-
<div className="z-10 p-5">
53-
<div className="flex size-full items-center justify-center" dangerouslySetInnerHTML={{ __html: qrCodeSvg }} />
54-
</div>
55-
) : (
56-
<div className="absolute inset-0 z-10 flex animate-pulse items-center justify-center bg-background">
57-
<Loader2 className="size-6 animate-spin" />
52+
{errors?.length ? (
53+
<Alert variant="destructive">
54+
<AlertCircleIcon />
55+
<AlertTitle>Something went wrong.</AlertTitle>
56+
<AlertDescription>
57+
<ul className="list-inside list-disc text-sm">
58+
{Array.from(new Set(errors)).map((error, index) => (
59+
<li key={index}>{error}</li>
60+
))}
61+
</ul>
62+
</AlertDescription>
63+
</Alert>
64+
) : (
65+
<>
66+
<div className="mx-auto flex max-w-md overflow-hidden">
67+
<div className="mx-auto aspect-square w-64 rounded-lg border border-border">
68+
<div className="z-10 flex h-full w-full items-center justify-center p-5">
69+
{qrCodeSvg ? (
70+
<div dangerouslySetInnerHTML={{ __html: qrCodeSvg }} />
71+
) : (
72+
<Loader2 className="flex size-4 animate-spin" />
73+
)}
74+
</div>
5875
</div>
59-
)}
60-
</div>
61-
</div>
76+
</div>
6277

63-
<div className="flex w-full space-x-5">
64-
<Button className="w-full" onClick={onNextStep}>
65-
{buttonText}
66-
</Button>
67-
</div>
78+
<div className="flex w-full space-x-5">
79+
<Button className="w-full" onClick={onNextStep}>
80+
{buttonText}
81+
</Button>
82+
</div>
6883

69-
<div className="relative flex w-full items-center justify-center">
70-
<div className="absolute inset-0 top-1/2 h-px w-full bg-border" />
71-
<span className="relative bg-card px-2 py-1">or, enter the code manually</span>
72-
</div>
84+
<div className="relative flex w-full items-center justify-center">
85+
<div className="absolute inset-0 top-1/2 h-px w-full bg-border" />
86+
<span className="relative bg-card px-2 py-1">or, enter the code manually</span>
87+
</div>
7388

74-
<div className="flex w-full space-x-2">
75-
<div className="flex w-full items-stretch overflow-hidden rounded-xl border border-border">
76-
{!manualSetupKey ? (
77-
<div className="flex h-full w-full items-center justify-center bg-muted p-3">
78-
<Loader2 className="size-4 animate-spin" />
89+
<div className="flex w-full space-x-2">
90+
<div className="flex w-full items-stretch overflow-hidden rounded-xl border border-border">
91+
{!manualSetupKey ? (
92+
<div className="flex h-full w-full items-center justify-center bg-muted p-3">
93+
<Loader2 className="size-4 animate-spin" />
94+
</div>
95+
) : (
96+
<>
97+
<input
98+
type="text"
99+
readOnly
100+
value={manualSetupKey}
101+
className="h-full w-full bg-background p-3 text-foreground outline-none"
102+
/>
103+
<button onClick={() => copy(manualSetupKey)} className="border-l border-border px-3 hover:bg-muted">
104+
<IconComponent className="w-4" />
105+
</button>
106+
</>
107+
)}
79108
</div>
80-
) : (
81-
<>
82-
<input
83-
type="text"
84-
readOnly
85-
value={manualSetupKey}
86-
className="h-full w-full bg-background p-3 text-foreground outline-none"
87-
/>
88-
<button onClick={() => copy(manualSetupKey)} className="border-l border-border px-3 hover:bg-muted">
89-
<IconComponent className="w-4" />
90-
</button>
91-
</>
92-
)}
93-
</div>
94-
</div>
109+
</div>
110+
</>
111+
)}
95112
</>
96113
);
97114
}
@@ -153,6 +170,7 @@ interface TwoFactorSetupModalProps {
153170
manualSetupKey: string | null;
154171
clearSetupData: () => void;
155172
fetchSetupData: () => Promise<void>;
173+
errors: string[];
156174
}
157175

158176
export default function TwoFactorSetupModal({
@@ -164,6 +182,7 @@ export default function TwoFactorSetupModal({
164182
manualSetupKey,
165183
clearSetupData,
166184
fetchSetupData,
185+
errors,
167186
}: TwoFactorSetupModalProps) {
168187
const [showVerificationStep, setShowVerificationStep] = useState<boolean>(false);
169188

@@ -239,6 +258,7 @@ export default function TwoFactorSetupModal({
239258
manualSetupKey={manualSetupKey}
240259
buttonText={modalConfig.buttonText}
241260
onNextStep={handleModalNextStep}
261+
errors={errors}
242262
/>
243263
)}
244264
</div>

‎resources/js/hooks/use-two-factor-auth.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const useTwoFactorAuth = () => {
4949

5050
setManualSetupKey(key);
5151
} catch {
52-
setErrors((prev) => [...prev, 'Failed to fetch setup key']);
52+
setErrors((prev) => [...prev, 'Failed to fetch a setup key']);
5353
setManualSetupKey(null);
5454
}
5555
}, []);
@@ -66,23 +66,25 @@ export const useTwoFactorAuth = () => {
6666

6767
const fetchRecoveryCodes = useCallback(async (): Promise<void> => {
6868
try {
69+
clearErrors();
6970
const codes = await fetchJson<string[]>(recoveryCodes.url());
7071

7172
setRecoveryCodesList(codes);
7273
} catch {
7374
setErrors((prev) => [...prev, 'Failed to fetch recovery codes']);
7475
setRecoveryCodesList([]);
7576
}
76-
}, []);
77+
}, [clearErrors]);
7778

7879
const fetchSetupData = useCallback(async (): Promise<void> => {
7980
try {
81+
clearErrors();
8082
await Promise.all([fetchQrCode(), fetchSetupKey()]);
8183
} catch {
8284
setQrCodeSvg(null);
8385
setManualSetupKey(null);
8486
}
85-
}, [fetchQrCode, fetchSetupKey]);
87+
}, [clearErrors, fetchQrCode, fetchSetupKey]);
8688

8789
return {
8890
qrCodeSvg,

‎resources/js/pages/settings/password.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ export default function Password() {
100100
</div>
101101

102102
<div className="flex items-center gap-4">
103-
<Button disabled={processing} data-test="update-password-button">Save password</Button>
103+
<Button disabled={processing} data-test="update-password-button">
104+
Save password
105+
</Button>
104106

105107
<Transition
106108
show={recentlySuccessful}

‎resources/js/pages/settings/profile.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ export default function Profile({ mustVerifyEmail, status }: { mustVerifyEmail:
9696
)}
9797

9898
<div className="flex items-center gap-4">
99-
<Button disabled={processing} data-test="update-profile-button">Save</Button>
99+
<Button disabled={processing} data-test="update-profile-button">
100+
Save
101+
</Button>
100102

101103
<Transition
102104
show={recentlySuccessful}

‎resources/js/pages/settings/two-factor.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ const breadcrumbs: BreadcrumbItem[] = [
2525
];
2626

2727
export default function TwoFactor({ requiresConfirmation = false, twoFactorEnabled = false }: TwoFactorProps) {
28-
const { qrCodeSvg, hasSetupData, manualSetupKey, clearSetupData, fetchSetupData, recoveryCodesList, fetchRecoveryCodes } = useTwoFactorAuth();
28+
const { qrCodeSvg, hasSetupData, manualSetupKey, clearSetupData, fetchSetupData, recoveryCodesList, fetchRecoveryCodes, errors } =
29+
useTwoFactorAuth();
2930
const [showSetupModal, setShowSetupModal] = useState<boolean>(false);
3031

3132
return (
@@ -42,7 +43,7 @@ export default function TwoFactor({ requiresConfirmation = false, twoFactorEnabl
4243
retrieve from the TOTP-supported application on your phone.
4344
</p>
4445

45-
<TwoFactorRecoveryCodes recoveryCodesList={recoveryCodesList} fetchRecoveryCodes={fetchRecoveryCodes} />
46+
<TwoFactorRecoveryCodes recoveryCodesList={recoveryCodesList} fetchRecoveryCodes={fetchRecoveryCodes} errors={errors} />
4647

4748
<div className="relative inline">
4849
<Form {...disable.form()}>
@@ -91,6 +92,7 @@ export default function TwoFactor({ requiresConfirmation = false, twoFactorEnabl
9192
manualSetupKey={manualSetupKey}
9293
clearSetupData={clearSetupData}
9394
fetchSetupData={fetchSetupData}
95+
errors={errors}
9496
/>
9597
</div>
9698
</SettingsLayout>

0 commit comments

Comments
 (0)