[SDK] Add required email verification for in-app wallet #7130
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe changes introduce a new mechanism to enforce email as a required authentication method for in-app wallets. This is achieved by updating wallet configuration types, UI components, and connection flows to check for and require email linking before completing wallet connections when specified in the wallet's configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConnectModal
participant WalletConfig
participant Profiles
participant LinkProfileScreen
User->>ConnectModal: Initiate wallet connection
ConnectModal->>WalletConfig: Retrieve wallet config
WalletConfig-->>ConnectModal: Return config (with auth.required)
alt Email required in config
ConnectModal->>Profiles: Fetch user profiles
Profiles-->>ConnectModal: Return profiles
alt No email linked
ConnectModal->>LinkProfileScreen: Show link profile UI
User->>LinkProfileScreen: Link email
LinkProfileScreen->>Profiles: Update profiles
Profiles-->>ConnectModal: Profiles updated (email linked)
ConnectModal->>WalletConfig: Finalize wallet connection
else Email already linked
ConnectModal->>WalletConfig: Finalize wallet connection
end
else Email not required
ConnectModal->>WalletConfig: Finalize wallet connection
end
Possibly related PRs
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
Playground
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
joaquim-verges
added
the
DO NOT MERGE
Graphite App
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
packages/thirdweb/src/wallets/in-app/core/wallet/types.ts (1)
72-72: Well-integrated property for required authenticationThe optional
requiredproperty fits naturally within the existing authentication configuration structure. This enables fine-grained control over which authentication methods are mandatory for wallet activation.Consider adding JSDoc comments to explain the behavior when
requiredis specified, similar to the comments on other properties in this type.+ /** + * List of authentication methods that must be completed before wallet activation + */ required?: InAppWalletRequired[];apps/playground-web/src/components/in-app-wallet/sponsored-tx.tsx (1)
46-46: Email requirement properly implementedThe
required: ["email"]configuration correctly implements the requirement for email verification in this sponsored transaction component. This ensures users must verify their email before completing wallet activation.You might want to consider adding a comment explaining the purpose of this requirement for clarity:
+ // Require email verification for security and user identification required: ["email"],packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx (1)
198-203: Consider improving type safety for wallet config access.The type casting used here indicates that the
Walletinterface might not fully expose thegetConfigmethod with the auth configuration structure.Consider updating the Wallet interface to include this configuration structure more explicitly, which would eliminate the need for type casting:
- const walletConfig = ( - activeWallet as unknown as { - getConfig?: () => { auth?: { required?: string[] } } | undefined; - } - ).getConfig?.(); + const walletConfig = activeWallet.getConfig?.();packages/thirdweb/src/react/web/wallets/shared/ConnectWalletSocialOptions.tsx (2)
142-145: Consider improving type safety for auth configuration access.The type casting suggests that the wallet's auth configuration structure might not be fully represented in the wallet's type definitions.
Consider updating the wallet interface to better represent the auth configuration structure, including the
requiredfield:- const requiresEmail = ( - (wallet.getConfig()?.auth as { required?: string[] } | undefined) - ?.required || [] - ).includes("email"); + const requiresEmail = (wallet.getConfig()?.auth?.required || []).includes("email");
582-640: Implemented VerifyEmailPrompt component for email verification.The new
VerifyEmailPromptcomponent provides a clean UI for email verification with proper validation and error handling.Consider using localized strings from the locale object for the title "Verify your email" and button text "Continue" to maintain consistency with the rest of the UI:
- <Text size="md" center weight={600} color="primaryText"> - Verify your email - </Text> + <Text size="md" center weight={600} color="primaryText"> + {props.locale.verifyEmail || "Verify your email"} + </Text> - <Button - variant="accent" - fullWidth - onClick={handleContinue} - disabled={props.disabled} - > - Continue - </Button> + <Button + variant="accent" + fullWidth + onClick={handleContinue} + disabled={props.disabled} + > + {props.locale.continue || "Continue"} + </Button>packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalContent.tsx (3)
139-142: Remove leftover debugconsole.logstatementsThese six
console.logcalls will leak internal state and spam the console in production bundles.- console.log("wallet", walletConfig); - console.log("requiresEmail", requiresEmail); … - console.log("profiles", profiles); … - console.log("hasEmail", hasEmail);Please delete or replace them with a proper logger behind a debug flag.
Also applies to: 154-161
143-171: Avoid double fetching – reuseuseProfilesinstead of directgetProfiles
handleConnectedperforms its owngetProfilesrequest although the hookuseProfilesis already instantiated (lines 95-97).
This triggers an additional network round-trip and duplicates caching logic.Consider:
- const profiles = await getProfiles({ client: props.client, ecosystem }); + const profiles = profilesQuery.data ?? + (await profilesQuery.refetch().then((r) => r.data));This re-uses the hook’s cache and avoids a second HTTP request when data is already present.
255-257: Wrap awaitinghandleConnectedin a safety net
handleConnectedis awaited but any exception will bubble up to React’s event handler and show an uncaught promise warning.- done={async (w) => { - await handleConnected(w); - }} + done={async (w) => { + try { + await handleConnected(w); + } catch (err) { + console.error("Wallet connection failed", err); + // TODO: surface error to the user + } + }}Repeating this guard for the other two
donecallbacks prevents noisy runtime errors and improves UX.Also applies to: 307-309, 329-331
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/playground-web/src/components/in-app-wallet/connect-button.tsx(2 hunks)apps/playground-web/src/components/in-app-wallet/sponsored-tx.tsx(1 hunks)apps/playground-web/src/lib/constants.ts(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalContent.tsx(9 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/constants.ts(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/LinkProfileScreen.tsx(1 hunks)packages/thirdweb/src/react/web/wallets/shared/ConnectWalletSocialOptions.tsx(4 hunks)packages/thirdweb/src/wallets/in-app/core/wallet/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/playground-web/src/components/in-app-wallet/connect-button.tsx (1)
apps/playground-web/src/components/styled-connect-button.tsx (1)
StyledConnectButton(17-38)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (13)
packages/thirdweb/src/wallets/in-app/core/wallet/types.ts (1)
63-63: Good choice for type definitionUsing a string literal type is a clean way to define the required authentication methods. This provides type safety while remaining extensible for future additional required methods.
packages/thirdweb/src/react/web/ui/ConnectWallet/constants.ts (1)
6-6: Logical addition to reserved screensAdding the
linkProfilescreen identifier follows the existing pattern and supports the email verification flow. This is a necessary addition to enable the UI flow for linking email profiles when required.apps/playground-web/src/lib/constants.ts (1)
30-30: Consistent implementation of email requirementAdding the email requirement here ensures consistent behavior across the application. This aligns with the PR objective to enforce email verification for in-app wallets.
Consider implementing a test to verify that wallets with this configuration properly enforce email verification before activation.
apps/playground-web/src/components/in-app-wallet/connect-button.tsx (3)
4-4: Component import updated to match usage pattern.This change updates the import from
StyledConnectEmbedtoStyledConnectButton, which is now used in the component rendering below.
8-8: Using StyledConnectButton instead of StyledConnectEmbed.The component has been updated to use
StyledConnectButton, which wraps theConnectButtonwith predefined chains, wallets, client, and theme.
27-27: Added email verification requirement for in-app wallet.This addition configures the in-app wallet to require email verification before activation, which aligns with the PR objective of enforcing email verification for in-app wallets.
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx (3)
22-22: Added useProfiles hook for email verification.The
useProfileshook is imported to fetch user profiles linked to the active wallet, which will be used to check if an email is linked.
188-214: Implemented email verification requirement check.This block implements the core logic for checking if the wallet requires email verification and if the user has an email linked. The code correctly:
- Fetches profiles for the connected wallet
- Checks if the wallet configuration requires email verification
- Verifies if any of the user's profiles has an email linked
- Sets the
needsEmailLinkflag if email is required but not linked
216-219: Extended UI visibility condition to include email verification requirement.The
showcondition has been extended to display the connect UI when email linking is required but missing. This ensures the wallet connection UI remains visible until the email verification is completed.packages/thirdweb/src/react/web/ui/ConnectWallet/screens/LinkProfileScreen.tsx (2)
31-31: Added optional wallet prop to support external wallet passing.Adding an optional
walletprop allows the component to accept an externally passed wallet instead of only using internal hooks, supporting the email verification flow.
38-38: Updated wallet assignment to prioritize passed wallet prop.The wallet variable assignment now prioritizes the passed wallet prop over the hooks (
adminWalletoractiveWallet), enabling better control over which wallet is being used for the linking process.packages/thirdweb/src/react/web/wallets/shared/ConnectWalletSocialOptions.tsx (2)
142-146: Added check for required email verification.This code detects if the wallet requires email verification by checking for "email" in the required array of the auth configuration. This is a crucial part of the email verification enforcement.
330-343: Added conditional rendering for email verification prompt.When in linking mode and email verification is required, the component now shows a dedicated email verification prompt instead of the regular social login options. This ensures that users must verify their email before connecting their wallet.
| // Effect to watch for email linking completion | ||
| useEffect(() => { | ||
| if (!pendingWallet) { | ||
| return; | ||
| } | ||
| const profiles = profilesQuery.data; | ||
| if (!profiles) { | ||
| return; | ||
| } | ||
| const hasEmail = profiles.some((p) => !!p.details.email); | ||
| if (hasEmail) { | ||
| // finalize connection now | ||
| if (shouldSetActive) { | ||
| setActiveWallet(pendingWallet); | ||
| } else { | ||
| connectionManager.addConnectedWallet(pendingWallet); | ||
| } | ||
| props.onConnect?.(pendingWallet); | ||
| setPendingWallet(undefined); | ||
| setScreen(initialScreen); | ||
| onClose?.(); | ||
| } | ||
| }, [ | ||
| profilesQuery.data, | ||
| pendingWallet, | ||
| shouldSetActive, | ||
| setActiveWallet, | ||
| connectionManager, | ||
| props.onConnect, | ||
| setScreen, | ||
| initialScreen, | ||
| onClose, | ||
| ]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate duplicated “finalize connection” logic
handleConnected defines finalizeConnection, but the useEffect block re-implements the same steps (setting active wallet, calling onConnect, etc.).
Duplicating critical flows increases maintenance overhead and risks divergence.
Extract finalizeConnection with useCallback at component scope and invoke it from both places:
const finalizeConnection = useCallback((wallet: Wallet) => {
if (shouldSetActive) {
setActiveWallet(wallet);
} else {
connectionManager.addConnectedWallet(wallet);
}
props.onConnect?.(wallet);
}, [shouldSetActive, setActiveWallet, connectionManager, props.onConnect]);Then call finalizeConnection(...) inside handleConnected and inside the effect.
🤖 Prompt for AI Agents
In
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalContent.tsx
around lines 199 to 231, the logic to finalize the wallet connection is
duplicated both in the handleConnected function and inside the useEffect hook.
To fix this, extract the finalize connection steps into a useCallback function
named finalizeConnection at the component scope, which takes a wallet parameter
and performs setting the active wallet or adding the connected wallet, then
calls props.onConnect. Replace the duplicated code in both handleConnected and
the useEffect hook by calling finalizeConnection with the appropriate wallet
argument.
| auth?: { | ||
| required?: string[]; | ||
| }; | ||
| partnerId?: string; | ||
| }; | ||
|
|
||
| const walletWithConfig = wallet as unknown as { | ||
| getConfig?: () => WalletConfig | undefined; | ||
| }; | ||
|
|
||
| const walletConfig = walletWithConfig.getConfig | ||
| ? walletWithConfig.getConfig() | ||
| : undefined; | ||
| const required = walletConfig?.auth?.required as string[] | undefined; | ||
| const requiresEmail = required?.includes("email"); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the required-field parsing
walletConfig?.auth?.required is cast to string[] | undefined without validation and then used directly. If a malformed wallet config (e.g. required: "email") is returned, the subsequent .includes() will throw or yield unexpected results.
- const required = walletConfig?.auth?.required as string[] | undefined;
- const requiresEmail = required?.includes("email");
+ const required = walletConfig?.auth?.required;
+ const requiresEmail =
+ Array.isArray(required) && required.includes("email");A simple guard prevents runtime errors and removes the need for a type-cast.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| auth?: { | |
| required?: string[]; | |
| }; | |
| partnerId?: string; | |
| }; | |
| const walletWithConfig = wallet as unknown as { | |
| getConfig?: () => WalletConfig | undefined; | |
| }; | |
| const walletConfig = walletWithConfig.getConfig | |
| ? walletWithConfig.getConfig() | |
| : undefined; | |
| const required = walletConfig?.auth?.required as string[] | undefined; | |
| const requiresEmail = required?.includes("email"); | |
| const walletConfig = walletWithConfig.getConfig | |
| ? walletWithConfig.getConfig() | |
| : undefined; | |
| const required = walletConfig?.auth?.required; | |
| const requiresEmail = | |
| Array.isArray(required) && required.includes("email"); |
🤖 Prompt for AI Agents
In
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModalContent.tsx
around lines 123 to 138, the code casts walletConfig?.auth?.required to string[]
without validating its type, which can cause runtime errors if the value is not
an array. Fix this by adding a type guard to check if
walletConfig?.auth?.required is an array before using it, avoiding the unsafe
type cast and preventing errors when calling includes().
size-limit report 📦
|
PR-Codex overview
This PR focuses on enhancing wallet connection functionality by adding email linking requirements for certain wallets, introducing a new
VerifyEmailPromptcomponent, and updating various components to manage email linking more effectively.Detailed summary
required: ["email"]to wallet configurations.InAppWalletRequiredtype to specify required fields.VerifyEmailPromptcomponent to handle email verification.ConnectEmbedandConnectModalContentto manage email linking.ConnectWalletSocialOptionsto restrict options based on email requirements.Summary by CodeRabbit
New Features
User Interface