refactor: enforce passkey authentication for user switching on all platforms

[LiorAgnin]

• Make handleSelectUserById async and require passkey auth on all platforms
• Remove legacy username-based user selection (selectUser/handleSelectUser)
• Add error toast notifications when passkey authentication fails
• Simplify TurnkeyProvider to fallback to single user when none selected
• Improve error handling with user selection reversion on auth failure
• Remove SDK logout call from handleLogout to prevent race conditions

Breaking changes:

• handleSelectUserById is now async and always requires passkey authentication
• handleSelectUser function removed from useUser hook and store

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
solid-app	Ignored	Preview	Jan 12, 2026 0:03am
solid-app-staging	Ignored	Preview	Jan 12, 2026 0:03am

[claude]

Pull Request Review

Overview

This PR enforces passkey authentication for user switching across all platforms, removing the legacy username-based selection system. The refactor improves security and simplifies the codebase.

---

✅ Positive Changes

1. Security Enhancement: Requiring passkey authentication on all platforms (not just web) significantly improves security for user switching operations.

2. Code Simplification: Removing 113 lines while adding only 44 is a good indicator of effective refactoring. The removal of selectUser and handleSelectUser eliminates redundant code paths.

3. Better Error Handling: Adding error toasts in welcome.tsx:45-54 provides clear user feedback when authentication fails.

4. Smart Fallback: The TurnkeyProvider change (components/TurnkeyProvider.tsx:20) intelligently falls back to the single user when no user is selected, improving UX for single-user scenarios.

5. State Reversion: The error handling properly reverts user selection on auth failure (hooks/useUser.ts:499-505), preventing invalid states.

---

🐛 Issues & Bugs

Critical Issues

1. Debug Console.log Statements Left in Production Code (hooks/useUser.ts:489-491)

   console.log('authedUser', authedUser);
   console.log('userId', userId);
   console.log(authedUser._id === userId);

   These debug statements should be removed before merging. Consider using a proper logging library or removing them entirely.

2. Silent Error Swallowing (hooks/useUser.ts:498-506)
   The catch block doesn't log the error or re-throw it. This makes debugging auth failures very difficult:

   } catch (error) {
     // Revert to previous user or clear selection on auth failure
     if (previousUserId) {
       selectUserById(previousUserId);

   Recommendation: Add error logging before state reversion:

   } catch (error) {
     console.error('[useUser] Authentication failed during user switch:', error);
     // ... existing revert logic

3. Inconsistent Error Handling Between Components (hooks/useUser.ts:498 vs app/welcome.tsx:45)

   • The hook doesn't throw the error after catching it
   • The welcome component catches and displays the error
   • But the error is already swallowed in the hook, so the toast might not show the actual error

   Recommendation: Either:

   • Re-throw the error in the hook after state reversion: throw error;
   • Or return a result object indicating success/failure

4. Navigation Race Condition (hooks/useUser.ts:497)
   Navigation happens inside the try block before the catch. If authentication succeeds but navigation fails, the error handling will incorrectly revert the user selection.

   Recommendation: Move navigation outside the try-catch or have more specific error handling.

Moderate Issues

5. Missing User Validation (hooks/useUser.ts:493-495)
   The code only selects the user if authedUser._id === userId, but doesn't handle the case where they don't match:

   if (authedUser?._id && authedUser._id === userId) {
     selectUserById(authedUser._id);
   }
   router.replace(path.HOME); // This still executes!

   If the authenticated user doesn't match the requested userId, navigation still occurs. This is a security issue.

   Recommendation:

   if (authedUser?._id && authedUser._id === userId) {
     selectUserById(authedUser._id);
     router.replace(path.HOME);
   } else {
     throw new Error('Authentication failed: User mismatch');
   }

6. Type Safety Issue (app/welcome.tsx:45)
   Using error: any reduces type safety:

   } catch (error: any) {
     Toast.show({
       text2: error?.message || 'Please try again',

   Recommendation: Use unknown and type guard:

   } catch (error) {
     const message = error instanceof Error ? error.message : 'Please try again';

---

⚡ Performance Considerations

1. Redundant Analytics Calls (hooks/useUser.ts:461-474)
   The function calls trackIdentity and track before authentication succeeds. If auth fails, these analytics events are already sent for a failed operation.

   Recommendation: Move analytics calls after successful authentication.

2. Multiple State Updates: The TurnkeyProvider will re-mount whenever selectedUser?.credentialId changes (components/TurnkeyProvider.tsx:62). This is intentional but could be expensive if the component tree is large.

---

🔒 Security Concerns

1. Removed SDK Logout (hooks/useUser.ts:446-449)
   The PR removes the SDK logout call to "prevent race conditions." While the comment explains this is intentional, ensure the backend properly invalidates sessions. Otherwise, old sessions might remain active.

   Question: Does the backend session cleanup happen reliably without the SDK logout?

2. Passkey Authentication Error Messages: The error messages shown to users should not reveal too much about why authentication failed (e.g., don't distinguish between "user not found" vs "wrong passkey").

3. TurnkeyProvider Fallback (components/TurnkeyProvider.tsx:20)
   The automatic fallback to a single user when none is selected could be a security risk if multiple users exist. Verify this only applies when exactly one user exists.

---

🧪 Test Coverage

Missing Tests: This PR lacks test coverage for:

• User switching with passkey authentication
• Error handling when passkey auth fails
• State reversion on authentication failure
• The TurnkeyProvider fallback logic
• Analytics tracking for the refactored flows

Recommendation: Add unit tests for handleSelectUserById covering:

• Successful authentication
• Failed authentication (httpClient not initialized)
• User mismatch scenarios
• State reversion on errors

E2E Testing: Consider adding a Playwright test for the complete user switching flow.

---

📋 Additional Recommendations

1. Breaking Change Documentation: The PR description clearly documents breaking changes. Ensure migration guides are available for any external consumers.

2. Type Definitions: Update the UseUserReturn interface is correctly updated (✅ already done in hooks/useUser.ts:37).

3. Error Messages: Consider using constants for error messages to ensure consistency and facilitate i18n later.

4. Dependency Array: The handleSelectUserById dependency array (hooks/useUser.ts:508) looks correct.

---

Summary

This is a solid refactor that improves security and simplifies the codebase. However, there are several critical issues that should be addressed before merging:

Must Fix Before Merge:

• Remove debug console.log statements
• Fix silent error swallowing (add logging)
• Fix user mismatch handling (don't navigate on mismatch)
• Re-throw errors or handle the hook/component error contract properly

Should Fix:

• Move analytics calls to after successful auth
• Improve type safety (avoid any)
• Add test coverage

Nice to Have:

• Extract error messages to constants
• Add more specific error types

Overall assessment: Needs Revisions - The security improvement is excellent, but the error handling needs work before this can be safely merged.

---

Review conducted by Claude Code

[claude]

PR Review: Enforce Passkey Authentication for User Switching

Summary

This PR improves security by requiring passkey authentication when switching users across all platforms (previously only on web). It also cleans up legacy username-based selection code and improves error handling.

---

✅ Strengths

Security Improvements

1. Universal passkey enforcement - Requiring passkey authentication on all platforms (not just web) significantly improves security by ensuring user consent for every account switch
2. Proper error handling - Error reversion logic correctly restores the previous user state when authentication fails
3. Race condition fix - Removing SDK logout from handleLogout prevents potential race conditions

Code Quality

1. Simplified API - Removing the legacy handleSelectUser (username-based) in favor of handleSelectUserById reduces API surface area and potential confusion
2. Better UX - Toast notifications provide clear feedback when authentication fails
3. Smart fallback - TurnkeyProvider now gracefully handles single-user scenarios without explicit selection

---

🔍 Issues & Concerns

Critical Issues

1. Potential Breaking Change in TurnkeyProvider (components/TurnkeyProvider.tsx:20)

const selectedUser = users.find(u => u.selected) ?? (users.length === 1 ? users[0] : undefined);

• Issue: This fallback logic may cause unexpected behavior when the user hasn't explicitly selected an account yet but has exactly one user stored
• Risk: If the single user's passkey is not yet initialized or if credentialId is missing, this could cause authentication failures
• Recommendation: Add explicit validation that the user has a valid credentialId before using them as the fallback:

const selectedUser = users.find(u => u.selected) ?? 
  (users.length === 1 && users[0].credentialId ? users[0] : undefined);

2. Missing Error Type Safety (app/welcome.tsx:45)

} catch (error: any) {

• Issue: Using any for error type defeats TypeScript's type safety
• Recommendation: Define proper error types or use unknown:

} catch (error: unknown) {
  const message = error instanceof Error ? error.message : 'Please try again';
  Toast.show({
    type: 'error',
    text1: 'Authentication failed',
    text2: message,
    // ...
  });
}

3. Silent Error Swallowing in handleSelectUserById (hooks/useUser.ts:495)

} catch (error) {
  // Revert to previous user or clear selection on auth failure
  if (previousUserId) {
    selectUserById(previousUserId);
  } else {
    unselectUser();
  }
  // Don't navigate on error - stay on welcome screen
}

• Issue: The error is caught but never re-thrown, making it impossible for the caller to know authentication failed
• Current behavior: app/welcome.tsx:44 can catch the error, but only because it's awaiting the promise
• Risk: If the function behavior changes or error handling in welcome.tsx is removed, failures would be silent
• Recommendation: Re-throw the error after state cleanup:

} catch (error) {
  if (previousUserId) {
    selectUserById(previousUserId);
  } else {
    unselectUser();
  }
  throw error; // Re-throw so caller can handle
}

Medium Priority Issues

4. Inconsistent Error Handling - Missing httpClient Check (hooks/useUser.ts:477-479)

if (!httpClient) {
  throw new Error('Turnkey client is not initialized. Please wait and try again.');
}

• Good: This validation is present, but the error message could be more actionable
• Concern: What happens if this error is thrown? The user sees a toast but may not understand what "wait" means
• Recommendation: Consider adding a retry mechanism or clearer instructions

5. Potential Race Condition with selectUserById (hooks/useUser.ts:489)

if (authedUser?._id && authedUser._id === userId) {
  selectUserById(authedUser._id);
}

• Issue: selectUserById is called AFTER successful authentication, but what if authedUser._id !== userId? The user selection doesn't happen, but we still navigate to HOME
• Current behavior: Navigation only happens on success, so this is partially safe
• Wait, I see the issue: The code calls router.replace(path.HOME) inside the try block at line 492, but it should only navigate if the userId matches
• Recommendation: Move navigation inside the conditional or handle the mismatch case:

if (authedUser?._id && authedUser._id === userId) {
  selectUserById(authedUser._id);
  router.replace(path.HOME);
} else {
  throw new Error('Authentication returned a different user');
}

6. Missing Dependency in useCallback (hooks/useUser.ts:505)

[selectUserById, clearKycLinkId, router, user, unselectUser, users, httpClient, login],

• Issue: The updateUser function from line 59 is used in tracking (lines 470-473) but not included in dependencies
• Wait, checking again: Actually updateUser is referenced in the function but never called in the visible code
• Actually: Looking at line 470, updateUser is not used in the new implementation. This is fine.
• Correction: Dependencies look correct. No issue here.

Minor Issues

7. Inconsistent Property Access (hooks/useUser.ts:489)

if (authedUser?._id && authedUser._id === userId)

vs the User type which uses userId not _id

• Issue: The API returns _id but the User type defines userId
• Inconsistency: This suggests the login function returns a different shape than the User type
• Recommendation: Either update the User type or ensure consistent property naming

8. Redundant selectedUser Check (hooks/useUser.ts:461-471)

const selectedUser = users.find(u => u.userId === userId);

Then later:

track(TRACKING_EVENTS.WELCOME_USER, {
  user_id: selectedUser?._id,
  username: selectedUser?.username,
  email: selectedUser?.email,
});

• Issue: selectedUser._id is used but User type defines userId
• Inconsistency: More evidence of naming inconsistency between API response and User type

---

🧪 Testing Concerns

Missing Test Coverage

The README notes that "the project does not have a formal test suite configured." For changes this critical to authentication, testing is essential:

Recommended Tests:

1. Unit tests for handleSelectUserById:

   • Success case with valid passkey
   • Error case with authentication failure
   • Error case with missing httpClient
   • Verification that previous user is restored on failure

2. Integration tests:

   • Full user switch flow on web and mobile
   • Multiple users scenario
   • Single user scenario with TurnkeyProvider fallback

3. Edge cases:

   • User with missing credentialId
   • Authentication returning different userId
   • Concurrent selection attempts

Manual Testing Required

Since automated tests don't exist, ensure manual testing covers:

• ✅ Switching users on iOS
• ✅ Switching users on Android
• ✅ Switching users on Web
• ✅ Error handling when passkey is cancelled
• ✅ Error handling when httpClient is not ready
• ✅ Single user account fallback in TurnkeyProvider
• ✅ Multiple user accounts requiring explicit selection

---

🔒 Security Analysis

Positive Security Changes

1. ✅ All platforms now require passkey authentication (defense in depth)
2. ✅ Session management moved entirely to backend (reduced client-side complexity)
3. ✅ Removal of SDK logout prevents race conditions

Security Considerations

1. ⚠️ Error messages: Be careful not to leak sensitive information in error messages (currently looks safe)
2. ⚠️ State management: Ensure the user selection state can't be manipulated by malicious code (using Zustand persist - should be safe with MMKV)
3. ✅ Passkey enforcement: Strong security improvement

---

📊 Performance Considerations

Positive

• Removing unused selectUser reduces bundle size (minimal impact)
• Simplified logic in handleSelectUserById may improve performance slightly

Concerns

• TurnkeyProvider remounting on credential change (line 62) could cause performance issues if credentials change frequently
• The key-based remounting is necessary for SDK reinitialization but adds overhead

---

🎯 Recommendations

Required Changes

1. Fix Issue configure project for Android #1: Add credentialId validation in TurnkeyProvider fallback
2. Fix Issue Batch transaction #3: Re-throw errors in handleSelectUserById after cleanup
3. Fix Issue fix launch error on android #5: Move navigation inside conditional or handle userId mismatch

Strongly Recommended

4. Fix Issue design card screens #2: Replace any with proper error typing
5. Fix Issue Add Navbar and EAS Update #7/Update iOS bundle identifier in app.json #8: Resolve _id vs userId naming inconsistency across codebase
6. Add minimal test coverage for authentication flow before merging

Nice to Have

7. Improve error message for httpClient initialization
8. Add loading states to prevent double-clicks during authentication
9. Consider adding Sentry error tracking for authentication failures

---

📝 Code Style & Conventions

Following the README guidelines:

• ✅ TypeScript strict typing (except for error: any)
• ✅ Proper async/await usage
• ✅ React hooks best practices
• ✅ Good code organization

---

Final Verdict

This PR makes important security improvements, but has several issues that should be addressed before merging:

Merge Status: ⚠️ Requires Changes

The authentication logic improvements are solid, but the issues around error handling, property naming inconsistencies, and the TurnkeyProvider fallback logic need to be resolved. The lack of test coverage for such critical authentication code is also concerning.

Priority:

1. Fix critical issues configure project for Android #1, Batch transaction #3, fix launch error on android #5 (MUST fix before merge)
2. Address issue design card screens #2 and Add Navbar and EAS Update #7/Update iOS bundle identifier in app.json #8 (SHOULD fix before merge)
3. Consider adding basic test coverage (RECOMMENDED before merge)

Great work on improving security across platforms! The changes are well-structured and the error handling pattern is sound - just needs a few refinements.