[jules] enhance: Mobile biometric authentication #278
Conversation
Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
✅ Deploy Preview for split-but-wiser canceled.
|
WalkthroughThe PR introduces mobile biometric authentication features (FaceID/TouchID), including secure credential storage via expo-secure-store, automatic user-switching handling to prevent account crossover, and UI toggles in login and account screens, along with documentation and iOS configuration updates. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mobile/context/AuthContext.js (2)
46-53: 🛠️ Refactor suggestion | 🟠 MajorExtract duplicated user normalization logic into a helper.
The same
_id/idnormalization logic is repeated four times (Lines 48–53, 134–138, 259–263, 293–297). This violates DRY and increases the risk of divergence if the normalization logic needs to change.Proposed helper extraction
Add a helper at the top of the file (or in a utils module):
const normalizeUser = (userData) => { if (!userData) return userData; return userData._id ? userData : userData.id ? { ...userData, _id: userData.id } : userData; };Then replace all four occurrences, e.g.:
- const normalized = parsed?._id - ? parsed - : parsed?.id - ? { ...parsed, _id: parsed.id } - : parsed; - setUser(normalized); + setUser(normalizeUser(parsed));Also applies to: 132-138, 257-263, 291-298
177-200:⚠️ Potential issue | 🟡 MinorLogout clears in-memory state even if storage removal fails — verify intent.
setToken(null),setRefresh(null),setUser(null), andclearAuthTokens()on Lines 196–199 execute outside the try/catch block. IfAsyncStorage.removeItemthrows, the user is logged out of the UI but stale tokens may remain in AsyncStorage, causing a ghost session on next app launch.Consider moving the state-clearing calls into the
tryblock after successful storage cleanup, or adding afinallyblock that always clears in-memory state (accepting the stale-storage risk as an explicit tradeoff).
🤖 Fix all issues with AI agents
In @.Jules/todo.md:
- Around line 127-132: Add the completed "Biometric authentication option" entry
to the "✅ Completed Tasks" section so the task appears both in its original
priority section and in the completed list; locate the original checklist line
starting with "[x] **[ux]** Biometric authentication option" and copy its
summary (including Completed date, Files, Context, Impact, Size) into the
existing "✅ Completed Tasks" block at the bottom, matching the formatting used
by other duplicated entries like "Comprehensive empty states" and "Error
boundary with retry."
- Line 128: The "Completed: 2026-02-14" entry in .Jules/todo.md is a future date
relative to the PR (2026-02-09); update the "Completed: 2026-02-14" line to a
valid date on or before 2026-02-09 (e.g., "Completed: 2026-02-09") or remove the
"Completed:" line entirely to leave the task unmarked as complete so the file
accurately reflects the actual completion status.
In `@mobile/context/AuthContext.js`:
- Around line 239-273: In loginWithBiometrics, remove the unsafe fallback that
uses storedToken as a refresh token (the newRefreshToken: storedRefresh ||
storedToken pattern) and instead treat a missing storedRefresh as an explicit
error path: do not call setAuthTokens with a fake refresh token, surface the
missing refresh token (e.g., return false or trigger re-login/clear state) and
keep setToken/setRefresh only with real values; additionally, after restoring
storedToken and storedRefresh, call the auth refresh endpoint (e.g.,
authApi.refresh) to validate/refresh tokens before marking the user
authenticated so expired tokens are handled, and ensure the code references the
same SecureStore keys ("secure_auth_token", "secure_refresh_token",
"secure_user_data") and functions (loginWithBiometrics, setAuthTokens, setToken,
setRefresh, setUser) when implementing the explicit missing-refresh handling and
refresh call.
In `@mobile/package.json`:
- Around line 21-22: The new Expo dependencies expo-local-authentication and
expo-secure-store use caret ranges (^) but the project pins Expo packages with
tilde (~) for SDK stability (see expo-haptics, expo-image-picker,
expo-status-bar); update the version specifiers for expo-local-authentication
and expo-secure-store in package.json from ^17.0.8 and ^15.0.8 to ~17.0.8 and
~15.0.8 respectively so they follow the existing tilde convention and avoid
pulling incompatible minor versions.
In `@mobile/screens/AccountScreen.js`:
- Around line 77-96: The HapticListItem’s onPress and the Switch’s onValueChange
can both call toggleBiometric causing a double-toggle; remove the redundant
press handler on HapticListItem (the fragment containing HapticListItem) so only
the Switch triggers toggling, or alternatively add a short guard state (e.g.,
isToggling) around toggleBiometric to ignore concurrent calls; locate
HapticListItem, the Switch (value={isBiometricEnabled}
onValueChange={toggleBiometric}), and the toggleBiometric function to implement
the chosen fix.
In `@mobile/screens/LoginScreen.js`:
- Around line 26-34: The handleBiometricLogin function leaves isLoading true on
success due to an early return; wrap the async loginWithBiometrics call in a
try/finally so setIsLoading(false) is always called (use try { const success =
await loginWithBiometrics(); if (success) return; Alert.alert(...) } finally {
setIsLoading(false); }) to guarantee the loading state is reset even if
navigation is delayed or fails.
| - [x] **[ux]** Biometric authentication option | ||
| - Completed: 2026-02-14 | ||
| - Files: `mobile/context/AuthContext.js`, `mobile/screens/LoginScreen.js`, `mobile/screens/AccountScreen.js` | ||
| - Context: Integrated FaceID/TouchID for quick, secure login | ||
| - Impact: Faster login experience while maintaining security | ||
| - Size: ~150 lines |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding this task to the "✅ Completed Tasks" section for consistency.
Other completed tasks in this file follow a pattern of appearing both in their original priority section (marked with [x]) and duplicated in the "✅ Completed Tasks" section at the bottom (lines 144-170). For example:
- "Comprehensive empty states" appears at lines 30-35 and again at 146-149
- "Error boundary with retry" appears at lines 37-42 and again at 159-162
- "Pull-to-refresh with haptic feedback" appears at lines 53-58 and again at 163-166
The biometric authentication task should follow the same pattern to maintain documentation consistency.
🤖 Prompt for AI Agents
In @.Jules/todo.md around lines 127 - 132, Add the completed "Biometric
authentication option" entry to the "✅ Completed Tasks" section so the task
appears both in its original priority section and in the completed list; locate
the original checklist line starting with "[x] **[ux]** Biometric authentication
option" and copy its summary (including Completed date, Files, Context, Impact,
Size) into the existing "✅ Completed Tasks" block at the bottom, matching the
formatting used by other duplicated entries like "Comprehensive empty states"
and "Error boundary with retry."
| - Size: ~70 lines | ||
| - Added: 2026-01-01 | ||
| - [x] **[ux]** Biometric authentication option | ||
| - Completed: 2026-02-14 |
There was a problem hiding this comment.
Completion date is in the future relative to PR creation.
The task shows a completion date of 2026-02-14, but this PR was created on 2026-02-09. This creates a 5-day discrepancy. Either the completion date should reflect when the work was actually finished (likely on or before 2026-02-09), or the task should not yet be marked as complete.
📅 Suggested fix
- Completed: 2026-02-14
+ Completed: 2026-02-09📝 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.
| - Completed: 2026-02-14 | |
| - Completed: 2026-02-09 |
🤖 Prompt for AI Agents
In @.Jules/todo.md at line 128, The "Completed: 2026-02-14" entry in
.Jules/todo.md is a future date relative to the PR (2026-02-09); update the
"Completed: 2026-02-14" line to a valid date on or before 2026-02-09 (e.g.,
"Completed: 2026-02-09") or remove the "Completed:" line entirely to leave the
task unmarked as complete so the file accurately reflects the actual completion
status.
| const loginWithBiometrics = async () => { | ||
| try { | ||
| const result = await LocalAuthentication.authenticateAsync({ | ||
| promptMessage: "Login with biometrics", | ||
| }); | ||
|
|
||
| if (result.success) { | ||
| const storedToken = await SecureStore.getItemAsync("secure_auth_token"); | ||
| const storedRefresh = await SecureStore.getItemAsync("secure_refresh_token"); | ||
| const storedUser = await SecureStore.getItemAsync("secure_user_data"); | ||
|
|
||
| if (storedToken && storedUser) { | ||
| setToken(storedToken); | ||
| setRefresh(storedRefresh); | ||
| await setAuthTokens({ | ||
| newAccessToken: storedToken, | ||
| newRefreshToken: storedRefresh || storedToken, // Fallback if refresh missing | ||
| }); | ||
|
|
||
| const parsed = JSON.parse(storedUser); | ||
| const normalized = parsed?._id | ||
| ? parsed | ||
| : parsed?.id | ||
| ? { ...parsed, _id: parsed.id } | ||
| : parsed; | ||
| setUser(normalized); | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } catch (error) { | ||
| console.error("Biometric login failed:", error); | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Critical: access token used as refresh token fallback on Line 255.
newRefreshToken: storedRefresh || storedToken, // Fallback if refresh missingIf storedRefresh is missing, this sends the access token as a refresh token. The server will reject it (different token types), and the user will experience a silent auth breakdown shortly after a "successful" biometric login. Remove the fallback and handle the missing refresh token explicitly.
Additionally, loginWithBiometrics does not validate whether the stored tokens are still valid (e.g., expired). After biometric authentication succeeds, the user may be immediately logged out by the API client's interceptor if tokens have expired. Consider performing a token refresh call (using authApi.refresh) after restoring stored tokens.
Proposed fix
if (storedToken && storedUser) {
setToken(storedToken);
setRefresh(storedRefresh);
await setAuthTokens({
newAccessToken: storedToken,
- newRefreshToken: storedRefresh || storedToken, // Fallback if refresh missing
+ newRefreshToken: storedRefresh,
});
+ // Attempt a token refresh to ensure credentials are still valid
+ if (storedRefresh) {
+ try {
+ const refreshResponse = await authApi.refresh(storedRefresh);
+ const { access_token, refresh_token } = refreshResponse.data;
+ setToken(access_token);
+ setRefresh(refresh_token);
+ await setAuthTokens({
+ newAccessToken: access_token,
+ newRefreshToken: refresh_token,
+ });
+ } catch (refreshError) {
+ // Refresh failed — tokens are stale, force re-login
+ console.error("Stored tokens expired:", refreshError);
+ await disableBiometrics();
+ return false;
+ }
+ } else {
+ // No refresh token stored — cannot guarantee session validity
+ await disableBiometrics();
+ return false;
+ }
+
const parsed = JSON.parse(storedUser);🤖 Prompt for AI Agents
In `@mobile/context/AuthContext.js` around lines 239 - 273, In
loginWithBiometrics, remove the unsafe fallback that uses storedToken as a
refresh token (the newRefreshToken: storedRefresh || storedToken pattern) and
instead treat a missing storedRefresh as an explicit error path: do not call
setAuthTokens with a fake refresh token, surface the missing refresh token
(e.g., return false or trigger re-login/clear state) and keep
setToken/setRefresh only with real values; additionally, after restoring
storedToken and storedRefresh, call the auth refresh endpoint (e.g.,
authApi.refresh) to validate/refresh tokens before marking the user
authenticated so expired tokens are handled, and ensure the code references the
same SecureStore keys ("secure_auth_token", "secure_refresh_token",
"secure_user_data") and functions (loginWithBiometrics, setAuthTokens, setToken,
setRefresh, setUser) when implementing the explicit missing-refresh handling and
refresh call.
| "expo-local-authentication": "^17.0.8", | ||
| "expo-secure-store": "^15.0.8", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent version range operators for Expo packages.
The existing Expo packages (expo-haptics, expo-image-picker, expo-status-bar) use tilde (~) to pin to patch-level updates, but the new packages use caret (^). For Expo SDK-aligned packages, tilde is preferred to avoid pulling in minor versions that may be incompatible with the current SDK version.
Proposed fix: align with existing convention
- "expo-local-authentication": "^17.0.8",
- "expo-secure-store": "^15.0.8",
+ "expo-local-authentication": "~17.0.8",
+ "expo-secure-store": "~15.0.8",📝 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.
| "expo-local-authentication": "^17.0.8", | |
| "expo-secure-store": "^15.0.8", | |
| "expo-local-authentication": "~17.0.8", | |
| "expo-secure-store": "~15.0.8", |
🤖 Prompt for AI Agents
In `@mobile/package.json` around lines 21 - 22, The new Expo dependencies
expo-local-authentication and expo-secure-store use caret ranges (^) but the
project pins Expo packages with tilde (~) for SDK stability (see expo-haptics,
expo-image-picker, expo-status-bar); update the version specifiers for
expo-local-authentication and expo-secure-store in package.json from ^17.0.8 and
^15.0.8 to ~17.0.8 and ~15.0.8 respectively so they follow the existing tilde
convention and avoid pulling incompatible minor versions.
| {isBiometricSupported && ( | ||
| <> | ||
| <HapticListItem | ||
| title="Biometric Login" | ||
| description="Use FaceID/TouchID to login" | ||
| left={() => <List.Icon icon="face-recognition" />} | ||
| right={() => ( | ||
| <Switch | ||
| value={isBiometricEnabled} | ||
| onValueChange={toggleBiometric} | ||
| /> | ||
| )} | ||
| onPress={() => toggleBiometric(!isBiometricEnabled)} | ||
| accessibilityLabel="Enable Biometric Login" | ||
| accessibilityRole="switch" | ||
| accessibilityState={{ checked: isBiometricEnabled }} | ||
| /> | ||
| <Divider /> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Potential double-toggle: Switch.onValueChange and HapticListItem.onPress may both fire on a single tap.
When the user taps the Switch area, React Native Paper's List.Item may also propagate the press event to its onPress handler (Line 89), causing toggleBiometric to be called twice in quick succession (e.g., enable → immediately disable). This results in a no-op or flicker from the user's perspective, and two round-trips to SecureStore.
Consider removing the onPress on the HapticListItem entirely and letting the Switch be the sole toggle trigger, or add a guard (e.g., a debounce or isToggling state) to prevent concurrent calls.
Proposed fix: remove redundant onPress
<HapticListItem
title="Biometric Login"
description="Use FaceID/TouchID to login"
left={() => <List.Icon icon="face-recognition" />}
right={() => (
<Switch
value={isBiometricEnabled}
onValueChange={toggleBiometric}
/>
)}
- onPress={() => toggleBiometric(!isBiometricEnabled)}
accessibilityLabel="Enable Biometric Login"
accessibilityRole="switch"
accessibilityState={{ checked: isBiometricEnabled }}
/>📝 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.
| {isBiometricSupported && ( | |
| <> | |
| <HapticListItem | |
| title="Biometric Login" | |
| description="Use FaceID/TouchID to login" | |
| left={() => <List.Icon icon="face-recognition" />} | |
| right={() => ( | |
| <Switch | |
| value={isBiometricEnabled} | |
| onValueChange={toggleBiometric} | |
| /> | |
| )} | |
| onPress={() => toggleBiometric(!isBiometricEnabled)} | |
| accessibilityLabel="Enable Biometric Login" | |
| accessibilityRole="switch" | |
| accessibilityState={{ checked: isBiometricEnabled }} | |
| /> | |
| <Divider /> | |
| </> | |
| )} | |
| {isBiometricSupported && ( | |
| <> | |
| <HapticListItem | |
| title="Biometric Login" | |
| description="Use FaceID/TouchID to login" | |
| left={() => <List.Icon icon="face-recognition" />} | |
| right={() => ( | |
| <Switch | |
| value={isBiometricEnabled} | |
| onValueChange={toggleBiometric} | |
| /> | |
| )} | |
| accessibilityLabel="Enable Biometric Login" | |
| accessibilityRole="switch" | |
| accessibilityState={{ checked: isBiometricEnabled }} | |
| /> | |
| <Divider /> | |
| </> | |
| )} |
🤖 Prompt for AI Agents
In `@mobile/screens/AccountScreen.js` around lines 77 - 96, The HapticListItem’s
onPress and the Switch’s onValueChange can both call toggleBiometric causing a
double-toggle; remove the redundant press handler on HapticListItem (the
fragment containing HapticListItem) so only the Switch triggers toggling, or
alternatively add a short guard state (e.g., isToggling) around toggleBiometric
to ignore concurrent calls; locate HapticListItem, the Switch
(value={isBiometricEnabled} onValueChange={toggleBiometric}), and the
toggleBiometric function to implement the chosen fix.
| const handleBiometricLogin = async () => { | ||
| setIsLoading(true); | ||
| const success = await loginWithBiometrics(); | ||
| if (success) { | ||
| return; | ||
| } | ||
| setIsLoading(false); | ||
| Alert.alert('Biometric Login Failed', 'Please try again or use your password.'); | ||
| }; |
There was a problem hiding this comment.
isLoading not reset on successful biometric login — intentional but fragile.
On Line 30, early return skips setIsLoading(false). This works because a successful login triggers navigation away (unmounting this screen). However, if the navigation is delayed or doesn't occur (e.g., a race condition in AppNavigator), the buttons remain stuck in the loading state with no recovery path.
A safer pattern is to always reset loading state in a finally block:
Proposed fix
const handleBiometricLogin = async () => {
setIsLoading(true);
- const success = await loginWithBiometrics();
- if (success) {
- return;
- }
- setIsLoading(false);
- Alert.alert('Biometric Login Failed', 'Please try again or use your password.');
+ try {
+ const success = await loginWithBiometrics();
+ if (!success) {
+ Alert.alert('Biometric Login Failed', 'Please try again or use your password.');
+ }
+ } finally {
+ setIsLoading(false);
+ }
};📝 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.
| const handleBiometricLogin = async () => { | |
| setIsLoading(true); | |
| const success = await loginWithBiometrics(); | |
| if (success) { | |
| return; | |
| } | |
| setIsLoading(false); | |
| Alert.alert('Biometric Login Failed', 'Please try again or use your password.'); | |
| }; | |
| const handleBiometricLogin = async () => { | |
| setIsLoading(true); | |
| try { | |
| const success = await loginWithBiometrics(); | |
| if (!success) { | |
| Alert.alert('Biometric Login Failed', 'Please try again or use your password.'); | |
| } | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@mobile/screens/LoginScreen.js` around lines 26 - 34, The handleBiometricLogin
function leaves isLoading true on success due to an early return; wrap the async
loginWithBiometrics call in a try/finally so setIsLoading(false) is always
called (use try { const success = await loginWithBiometrics(); if (success)
return; Alert.alert(...) } finally { setIsLoading(false); }) to guarantee the
loading state is reset even if navigation is delayed or fails.
1 participant
Implemented FaceID/TouchID login support for mobile app.
expo-local-authenticationandexpo-secure-store.AuthContextto manage biometric state and secure storage.NSFaceIDUsageDescription.PR created automatically by Jules for task 2660628253277350045 started by @Devasy23
Summary by CodeRabbit
Release Notes
New Features
Chores