Skip to content

Conversation

@nikospapcom
Copy link
Contributor

@nikospapcom nikospapcom commented Mar 14, 2025

Description

In this pr we're handling the redirect to two-factor when user try to login with web3 and has enabled the Multi-factor authentication

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

BEFORE

Screen.Recording.2025-03-18.at.11.19.54.AM.mov

AFTER

Screen.Recording.2025-03-18.at.11.18.31.AM.mov

@nikospapcom nikospapcom self-assigned this Mar 14, 2025
@vercel
Copy link

vercel bot commented Mar 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 8, 2025 0:27am

@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 3902d48 to 091af83 Compare March 18, 2025 07:47
@changeset-bot
Copy link

changeset-bot bot commented Mar 18, 2025

🦋 Changeset detected

Latest commit: 23e05da

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 091af83 to 054d34e Compare March 18, 2025 07:50
@nikospapcom nikospapcom changed the title fix(clerk-js): Navigate to factor-two when authenticate with web3 and response status is needs_second_factor fix(clerk-js): Handle two factor redirect when authenticate with web3 Mar 18, 2025
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 054d34e to f57c9d1 Compare March 18, 2025 09:23
@nikospapcom nikospapcom marked this pull request as ready for review March 18, 2025 09:23
Copy link
Member

@anagstef anagstef left a comment

Choose a reason for hiding this comment

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

Is it possible to add an integration test for this? 🤔

});
switch (signInOrSignUp.status) {
case 'needs_second_factor':
await navigate('factor-two');
Copy link
Member

Choose a reason for hiding this comment

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

This assumes usage is always with in our components which is not the case.

Sounds like we need to borrow a few things from handleRedirectCallback.

@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from f57c9d1 to 540e1b5 Compare April 1, 2025 08:20
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 540e1b5 to 7df6eba Compare April 1, 2025 08:21
@nikospapcom nikospapcom requested a review from panteliselef April 1, 2025 08:21
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 7df6eba to 290aaf3 Compare April 1, 2025 08:28
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 290aaf3 to f2f804c Compare April 1, 2025 08:29
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from f2f804c to eabb329 Compare April 1, 2025 08:34
@nikospapcom nikospapcom requested review from Nikpolik and kostaspt April 1, 2025 08:38
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from eabb329 to 09f38bd Compare April 8, 2025 07:04
@nikospapcom nikospapcom force-pushed the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch from 09f38bd to 23e05da Compare April 8, 2025 12:27
@nikospapcom
Copy link
Contributor Author

Is it possible to add an integration test for this? 🤔

@anagstef we don't have e2e for web3 apps right, that's the reason I didn't add tests

@nikospapcom nikospapcom merged commit c4836f9 into main Apr 8, 2025
30 checks passed
@nikospapcom nikospapcom deleted the nikospap/user-1350-we-fail-to-navigate-to-mfa-page-using-web3-wallet branch April 8, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants