Warn users before destructive password reset#568
Open
AnthonyRonning wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTwo password reset components are updated to improve user experience: PasswordResetRequestForm adds a confirmation warning dialog that gates the reset initiation and refactors reset logic into a dedicated async function, while PasswordResetConfirmForm introduces a timed alert that displays helpful guidance if the reset code fails to arrive within 30 seconds. ChangesPassword Reset Flow Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
639392f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://374dce02.maple-ca8.pages.dev |
| Branch Preview URL: | https://aead-password-reset-warning.maple-ca8.pages.dev |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/PasswordResetRequestForm.tsx (1)
30-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCapture the submitted email before starting the async reset.
Line 39 sends whatever
Suggested fix
export function PasswordResetRequestForm() { const [email, setEmail] = useState(""); + const [requestedEmail, setRequestedEmail] = useState(""); const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState<string | null>(null); const [showConfirmForm, setShowConfirmForm] = useState(false); const [showResetWarning, setShowResetWarning] = useState(false); const [secret, setSecret] = useState(""); const os = useOpenSecret(); const requestPasswordReset = async () => { + const nextEmail = email; + setIsLoading(true); setError(null); setShowResetWarning(false); + setRequestedEmail(nextEmail); try { // TODO: move this logic to the library const generatedSecret = generateSecureSecret(); const hashedSecret = await hashSecret(generatedSecret); - await os.requestPasswordReset(email, hashedSecret); + await os.requestPasswordReset(nextEmail, hashedSecret); setSecret(generatedSecret); setShowConfirmForm(true); @@ if (showConfirmForm) { - return <PasswordResetConfirmForm email={email} secret={secret} />; + return <PasswordResetConfirmForm email={requestedEmail} secret={secret} />; } @@ <Input id="email" type="email" value={email} onChange={(e) => setEmail(e.target.value)} + disabled={isLoading} required />Also applies to: 72-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/PasswordResetRequestForm.tsx` around lines 30 - 41, The requestPasswordReset flow uses the live email state which can be edited while the async call is in-flight, causing a mismatch between the address that receives the reset and the one used in the confirm UI; fix by capturing a snapshot of the email before awaiting: set a new state like submittedEmail (use a setter such as setSubmittedEmail) to the current email immediately at the start of requestPasswordReset, use that submittedEmail when rendering the confirmation flow and when calling os.requestPasswordReset, and also disable the email input while isLoading is true (or until setShowConfirmForm is set) to prevent further edits during the request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/PasswordResetConfirmForm.tsx`:
- Around line 39-47: The delayed help timer in PasswordResetConfirmForm's
useEffect currently only checks success and can show the "No code yet?" alert
after the user has entered a code; update the effect to also check that code is
empty (e.g., if (success || code) return), add code to the dependency array, and
ensure you call setShowCodeHelp(false) when code becomes non-empty and
clearTimeout(timer) in cleanup; apply the same gating and cleanup fix to the
other identical effect/logic around lines 101-111 so the help only appears while
the reset code field is empty.
---
Outside diff comments:
In `@frontend/src/components/PasswordResetRequestForm.tsx`:
- Around line 30-41: The requestPasswordReset flow uses the live email state
which can be edited while the async call is in-flight, causing a mismatch
between the address that receives the reset and the one used in the confirm UI;
fix by capturing a snapshot of the email before awaiting: set a new state like
submittedEmail (use a setter such as setSubmittedEmail) to the current email
immediately at the start of requestPasswordReset, use that submittedEmail when
rendering the confirmation flow and when calling os.requestPasswordReset, and
also disable the email input while isLoading is true (or until
setShowConfirmForm is set) to prevent further edits during the request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 616f8be2-0338-459b-954b-9ad41c2868a4
📒 Files selected for processing (2)
frontend/src/components/PasswordResetConfirmForm.tsxfrontend/src/components/PasswordResetRequestForm.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by CodeRabbit