Skip to content

fix(settings): dont send duplicate otp codes#20250

Merged
StaberindeZA merged 1 commit intomainfrom
fxa-13276
Mar 24, 2026
Merged

fix(settings): dont send duplicate otp codes#20250
StaberindeZA merged 1 commit intomainfrom
fxa-13276

Conversation

@StaberindeZA
Copy link
Copy Markdown
Contributor

Because

  • For relying parties with configured CMS customization, users are receiving duplicate OTP codes on signin and signup.

This pull request

  • Ensure only 1 OTP code is sent, by only calling authClient.sendPasswordlessCode on component mount.

Issue that this pull request solves

Closes: #FXA-13276

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

@StaberindeZA StaberindeZA requested a review from a team as a code owner March 24, 2026 20:31
Copilot AI review requested due to automatic review settings March 24, 2026 20:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses duplicate OTP sends in the fxa-settings passwordless sign-in flow by changing when the initial passwordless code is triggered.

Changes:

  • Updated the initial “send passwordless code” useEffect to run only once on component mount (empty dependency array), aiming to prevent duplicate sends.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sendCode();
}
}, [email, service, codeSent, authClient, integration, navigateWithQuery, location.pathname, location.search, location.state, flowQueryParams]);
}, []);
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effect now has an empty dependency array but closes over email, codeSent, authClient, integration, service, flowQueryParams, navigateWithQuery, and location.*. That can lead to stale values (e.g., if route state/props change without a remount) and will also trigger react-hooks/exhaustive-deps warnings under the repo’s React ESLint config. Consider restoring an appropriate dependency list and preventing duplicate sends via an explicit in-flight guard (e.g., a ref) rather than relying on [], or add an eslint-disable with a clear rationale if running only once with initial values is truly required.

Copilot uses AI. Check for mistakes.
sendCode();
}
}, [email, service, codeSent, authClient, integration, navigateWithQuery, location.pathname, location.search, location.state, flowQueryParams]);
}, []);
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the reported production issue is duplicate OTP sends on sign-in/sign-up, it would be valuable to add a regression test that forces a re-render (e.g., by changing flowQueryParams/location state identity while passwordlessSendCode is still pending) and asserts only one send occurs. The current test does not send code multiple times doesn't actually trigger a re-render, so it may not catch the original failure mode.

Copilot uses AI. Check for mistakes.
Because:

- For relying parties with configured CMS customization, users are
  receiving duplicate OTP codes on signin and signup.

This commit:

- Ensure only 1 OTP code is sent, by only calling
  authClient.sendPasswordlessCode on component mount.

Closes #FXA-13276
@StaberindeZA StaberindeZA merged commit 85bb696 into main Mar 24, 2026
22 checks passed
@StaberindeZA StaberindeZA deleted the fxa-13276 branch March 24, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants