feat: Implement email OTP authentication #2414
Conversation
ConsoleProject ID: Sites (2)
Tip Cursor pagination performs better than offset pagination when loading further pages. |
WalkthroughAdds a passwordless email OTP sign-in flow and a ResendCooldown component. The login page now conditionally toggles between password and email-sign-in (password made optional, email autofocus removed) and can request an email sign-in token. A new email-otp route/page verifies 6-digit codes, creates sessions, supports resending tokens with cooldown and localStorage persistence, tracks analytics, shows notifications, and redirects based on coupon, campaign, or explicit redirect. The sendVerificationEmailModal was refactored to use ResendCooldown, removing inline timer and lifecycle persistence. Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/routes/(public)/(guest)/login/+page.svelte(2 hunks)src/routes/(public)/(guest)/login/email-otp/+page.svelte(1 hunks)src/routes/(public)/(guest)/login/email-otp/+page.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/(public)/(guest)/login/email-otp/+page.ts (1)
src/lib/stores/sdk.ts (1)
sdk(147-170)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/components/account/sendVerificationEmailModal.svelte (1)
12-57: Drop the no-oponDestroy.We still import
onDestroybut only call an empty handler, so both lines can go. Keeps the component leaner.-import { onDestroy } from 'svelte'; ... -onDestroy(() => {});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/account/sendVerificationEmailModal.svelte(3 hunks)src/lib/components/resendCooldown.svelte(1 hunks)src/routes/(public)/(guest)/login/email-otp/+page.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
| localStorage.removeItem(EMAIL_SENT_KEY); | ||
| } | ||
| }); | ||
| onDestroy(() => {}); |
| import { Link } from '@appwrite.io/pink-svelte'; | ||
| let { | ||
| storageKey = 'resend_cooldown_default', |
There was a problem hiding this comment.
lets not have a default, always mark as required.
| try { | ||
| disabled = true; | ||
| const sessionToken = await sdk.forConsole.account.createEmailToken({ | ||
| userId: 'unique', |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/components/resendCooldown.svelte (1)
75-76: Cooldown never starts after initial send.When the component mounts with no existing timer state (e.g., after sending the initial code),
restore()finds no stored deadline and leavesremainingat0. This allows users to immediately resend, bypassing the throttle entirely.Apply this diff to start the cooldown on mount when no stored deadline exists:
- onMount(restore); + onMount(() => { + restore(); + if (remaining === 0) { + start(); + } + });
🧹 Nitpick comments (2)
src/lib/components/resendCooldown.svelte (1)
9-9: Consider one-way binding fordisabledprop.The component only reads the
disabledprop and never modifies it (line 70 checks it, never assigns). Using$bindablecreates an unnecessary two-way binding contract. A regular prop would be clearer.- disabled = $bindable(false), + disabled = false,src/lib/components/account/sendVerificationEmailModal.svelte (1)
84-84: Consider one-way binding fordisabled.ResendCooldown only reads the
disabledprop and never modifies it, so the two-way binding (bind:disabled) is unnecessary. A one-way binding would be clearer.- bind:disabled={creating} + disabled={creating}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/account/sendVerificationEmailModal.svelte(3 hunks)src/lib/components/resendCooldown.svelte(1 hunks)src/routes/(public)/(guest)/login/email-otp/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(public)/(guest)/login/email-otp/+page.svelte
🔇 Additional comments (7)
src/lib/components/resendCooldown.svelte (4)
18-28: LGTM.The
start()function correctly persists the deadline to localStorage and initiates both the immediate tick and the interval-based countdown.
30-44: LGTM.The
restore()function correctly handles SSR, expired deadlines, and active timers from localStorage.
69-73: LGTM.The function correctly starts the cooldown only after
onResendsucceeds. IfonResendthrows or rejects, the cooldown won't start, allowing the user to retry immediately.
79-83: LGTM.The conditional rendering correctly displays the countdown message when active and the resend button when the cooldown expires.
src/lib/components/account/sendVerificationEmailModal.svelte (3)
13-13: LGTM.Clean import and accurate documentation of the refactoring to use the ResendCooldown component.
Also applies to: 40-40
43-43: LGTM.Simplifying the guard to only check
creatingis correct since ResendCooldown now manages the cooldown state and prevents premature resends.
80-90: LGTM overall.The integration of ResendCooldown successfully replaces the inline timer logic. The conditional rendering, unique storage key, and callback wiring are all correct. Once the auto-start issue in ResendCooldown is fixed, this implementation will properly throttle resend attempts.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/routes/(public)/(guest)/login/+page.svelte (1)
24-46: Consider adding analytics tracking for OTP flow initiation.Unlike the
loginfunction which tracksSubmit.AccountLogin, thesendSignInCodefunction doesn't track any analytics events. Consider adding a tracking event when users initiate the email OTP flow to gain insights into adoption of this authentication method.Example implementation:
async function sendSignInCode() { try { disabled = true; // use createEmailToken for sign in with code const sessionToken = await sdk.forConsole.account.createEmailToken({ userId: 'unique', email: mail }); + + trackEvent('otp_flow_initiated', { email: mail }); const params = new URLSearchParams(window.location.search);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(public)/(guest)/login/+page.svelte(2 hunks)
🔇 Additional comments (2)
src/routes/(public)/(guest)/login/+page.svelte (2)
18-22: LGTM: Clean toggle logic for passwordless vs. password flows.The reactive statement automatically switches to password login when the user starts typing, providing a smooth UX transition between the two authentication methods.
121-140: LGTM: Clean conditional form submission and UI.The form submission logic correctly switches between password and OTP flows, and the conditional button rendering provides clear user feedback. The
required={false}on the password field appropriately supports the passwordless flow.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/routes/(public)/(guest)/login/+page.svelte (2)
18-18: Consider UX implications of dynamic mode switching.The reactive assignment causes the UI to switch between "Sign in" and "Get sign in code" modes as the user types or deletes content in the password field. While functionally correct, this dynamic behavior might be slightly confusing for users who accidentally start typing in the password field or delete their password.
Consider whether this is the intended UX or if a more explicit toggle (e.g., a "Sign in with code instead" link) would be clearer.
Also applies to: 22-22
24-45: Add error tracking for consistency.The
loginfunction tracks errors usingtrackError(error, Submit.AccountLogin)on line 88, butsendSignInCodeonly shows a notification without tracking the error. This inconsistency makes it harder to monitor and debug OTP flow failures.Apply this diff to add error tracking:
} catch (error) { disabled = false; addNotification({ type: 'error', message: error.message }); + trackError(error, Submit.AccountLogin); }Note: The query param preservation in lines 33-37 correctly addresses the past review comment about maintaining existing parameters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(public)/(guest)/login/+page.svelte(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (3)
src/routes/(public)/(guest)/login/+page.svelte (3)
120-120: LGTM!The conditional form submission logic correctly routes to the appropriate authentication flow based on whether a password is provided.
132-132: LGTM!Changing the password field to optional correctly enables the passwordless OTP flow while maintaining email validation.
135-139: LGTM!The conditional button rendering correctly displays appropriate text based on the authentication mode, maintaining consistent styling and behavior.
2 participants
What does this PR do?
Add email OTP as primary login method with separate verification page,
proper Appwrite SDK integration, and dynamic UI switching.
Test Plan
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
yes
Summary by CodeRabbit
New Features
UX Changes
Reliability
Refactor