feat: implement app lock feature with biometric authentication and PIN setup#23
feat: implement app lock feature with biometric authentication and PIN setup#23shreyaspapi wants to merge 3 commits into
Conversation
WalkthroughApp lock security functionality was introduced across the project, enabling PIN and biometric authentication for accessing the app. This involved adding new screens for lock and PIN setup, updating app lifecycle and navigation logic, creating an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App (UI)
participant AppLockService
participant SecureStorage
participant BiometricAPI
User->>App (UI): Launches app
App (UI)->>AppLockService: initialize()
AppLockService->>SecureStorage: Load lock settings & PIN
AppLockService-->>App (UI): Return lock state
alt App lock enabled and locked
App (UI)->>User: Show AppLockScreen
User->>App (UI): Enter PIN or tap biometric
alt Biometric auth
App (UI)->>AppLockService: authenticateWithBiometrics()
AppLockService->>BiometricAPI: Prompt biometric
BiometricAPI-->>AppLockService: Success/Failure
else PIN entry
App (UI)->>AppLockService: verifyPinCode(pin)
AppLockService->>SecureStorage: Compare PIN
SecureStorage-->>AppLockService: Result
end
AppLockService-->>App (UI): Unlock or error
App (UI)->>User: Navigate to main screen or show error
else App lock not enabled or already unlocked
App (UI)->>User: Navigate to main screen
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
android/app/src/main/AndroidManifest.xml (1)
3-4: AddmaxSdkVersionto fingerprint permission to silence deprecation lint
android.permission.USE_FINGERPRINTis deprecated from API 28 onward. Keeping it for ≤ 28 devices is fine, but addandroid:maxSdkVersion="28"so newer builds don’t flag it.- <uses-permission android:name="android.permission.USE_FINGERPRINT"/> + <uses-permission + android:name="android.permission.USE_FINGERPRINT" + android:maxSdkVersion="28"/>lib/screens/login_screen.dart (1)
50-54: Avoid hard-coded route strings – centralise to reduce drift
'/main'is now repeated four times. Declare a const (e.g.AppRoutes.main) and reuse it so future route renames touch only one place.- unawaited(Navigator.of(context).pushReplacementNamed('/main')); + unawaited(Navigator.of(context).pushReplacementNamed(AppRoutes.main));Same change applies to the other three occurrences.
Also applies to: 138-142, 156-162, 184-188
android/app/build.gradle.kts (1)
44-68: Minor duplication – lift the keystore check into a valThe identical 5-line condition appears twice. Compute it once to keep the block concise:
+val hasSigningProps = + keystorePropertiesFile.exists() && + listOf("storeFile","storePassword","keyAlias","keyPassword") + .all { keystoreProperties[it] != null } + signingConfigs { create("release") { - // Only configure signing if key.properties file exists and has required properties - if (keystorePropertiesFile.exists() && - keystoreProperties["storeFile"] != null && - keystoreProperties["storePassword"] != null && - keystoreProperties["keyAlias"] != null && - keystoreProperties["keyPassword"] != null) { + if (hasSigningProps) { … } } } … getByName("release") { - // Only set signing config if it's properly configured - if (keystorePropertiesFile.exists() && - keystoreProperties["storeFile"] != null && - keystoreProperties["storePassword"] != null && - keystoreProperties["keyAlias"] != null && - keystoreProperties["keyPassword"] != null) { + if (hasSigningProps) { signingConfig = signingConfigs.getByName("release") }Pure nit, but keeps the build script tidy.
lib/screens/splash_screen.dart (1)
40-55: Consider reusing AppLockService instance for better performanceThe current implementation creates a new AppLockService instance and initializes it on every splash screen load. Consider caching the initialized service or using a singleton pattern to avoid redundant initialization calls.
- // Check app lock status - final appLockService = AppLockService(); - await appLockService.initialize(); + // Check app lock status + final appLockService = AppLockService(); + if (!appLockService.isInitialized) { + await appLockService.initialize(); + }lib/screens/app_lock_screen.dart (2)
54-65: Consider adding haptic feedback for better UXThe PIN digit input logic is solid with proper length validation and error state clearing. Consider adding haptic feedback when digits are entered to improve the user experience.
+import 'package:flutter/services.dart'; void _onPinDigitPressed(String digit) { if (_enteredPin.length < 4) { + HapticFeedback.lightImpact(); setState(() { _enteredPin.add(digit); _showError = false; });
109-120: Consider making PIN dot size responsiveThe PIN digit visualization is clean and functional. Consider making the dot size responsive to different screen sizes for better accessibility.
Widget _buildPinDigit({bool isFilled = false}) { + final screenWidth = MediaQuery.of(context).size.width; + final dotSize = screenWidth > 400 ? 24.0 : 20.0; + return Container( - width: 20, - height: 20, + width: dotSize, + height: dotSize,lib/main.dart (1)
66-77: Consider adding error handling for app lock service failuresThe
_checkAppLockmethod assumes the AppLockService is always available and functional. Consider adding error handling for cases where the service might fail or be unavailable.Future<void> _checkAppLock() async { - if (_isAppLockEnabled) { + if (_isAppLockEnabled && _appLockService != null) { - _appLockService.onAppResumed(); - - if (_appLockService.isLocked) { - // Navigate to app lock screen - if (mounted) { - Navigator.of(context).pushReplacementNamed('/app-lock'); + try { + _appLockService.onAppResumed(); + + if (_appLockService.isLocked) { + // Navigate to app lock screen + if (mounted) { + Navigator.of(context).pushReplacementNamed('/app-lock'); + } } + } catch (e) { + // Log error or handle gracefully + debugPrint('App lock check failed: $e'); } }lib/screens/settings_screen.dart (1)
294-300: Extract timeout options to reduce duplication.The timeout options are duplicated between
_getLockTimeoutTextand_showLockTimeoutDialog. Consider extracting them to a class constant.Add this constant to the class:
static const List<Map<String, dynamic>> _timeoutOptions = [ {'value': 0, 'label': 'Immediate'}, {'value': 30, 'label': '30 seconds'}, {'value': 60, 'label': '1 minute'}, {'value': 300, 'label': '5 minutes'}, {'value': 600, 'label': '10 minutes'}, ];Then update
_getLockTimeoutTextto use this constant:String _getLockTimeoutText() { final option = _timeoutOptions.firstWhere( (opt) => opt['value'] == _lockTimeout, orElse: () => {'label': '$_lockTimeout seconds'}, ); return option['label'] as String; }lib/services/app_lock_service.dart (1)
77-89: Consider enhancing PIN security validation.The current implementation only validates PIN length. Consider adding validation for weak PINs and implementing rate limiting for verification attempts.
Consider these security enhancements:
- Validate against common weak PINs (1234, 0000, etc.)
- Add rate limiting for failed PIN attempts
- Consider hashing the PIN before storage for additional security
Example validation:
static const List<String> _weakPins = ['0000', '1111', '1234', '4321']; Future<bool> setPinCode(String pinCode) async { if (pinCode.length < 4) { return false; } if (_weakPins.contains(pinCode)) { return false; // Reject weak PINs } try { await _secureStorage.write(key: _pinCodeKey, value: pinCode); return true; } catch (e) { return false; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
README.md(3 hunks)android/app/build.gradle.kts(2 hunks)android/app/src/main/AndroidManifest.xml(1 hunks)ios/Runner/Info.plist(1 hunks)lib/main.dart(2 hunks)lib/screens/app_lock_screen.dart(1 hunks)lib/screens/login_screen.dart(4 hunks)lib/screens/pin_setup_screen.dart(1 hunks)lib/screens/settings_screen.dart(4 hunks)lib/screens/splash_screen.dart(2 hunks)lib/services/app_lock_service.dart(1 hunks)pubspec.yaml(1 hunks)
🔇 Additional comments (22)
pubspec.yaml (1)
43-43: Dependency addition LGTM
local_auth ^2.2.0is the canonical package for biometrics and is compatible with your min / target SDK versions. Nice catch including it here.ios/Runner/Info.plist (1)
53-54: Face ID usage string looks goodThe mandatory
NSFaceIDUsageDescriptionkey is present with a clear explanation — this will satisfy App Store review.lib/screens/splash_screen.dart (2)
6-6: LGTM - Clean import additionThe AppLockService import is correctly added to support the new app lock functionality.
44-58: LGTM - Proper app lock integration with navigationThe navigation logic correctly handles both reviewer mode and app lock states. The mounted checks prevent navigation issues, and the routing to
/mainand/app-lockis consistent with the new app structure.README.md (3)
32-32: LGTM - Clear feature descriptionThe app lock security feature description is concise and accurately reflects the implemented functionality.
47-55: Excellent documentation of app lock capabilitiesThe dedicated App Lock Security section provides comprehensive coverage of all features including PIN protection, biometric authentication, configurable timeouts, and secure storage. This will help users understand the full scope of the security enhancements.
111-111: LGTM - Consistent security documentationThe addition to the Security & Privacy section appropriately highlights the app lock feature alongside existing security measures.
lib/screens/app_lock_screen.dart (4)
33-52: LGTM - Secure biometric authentication flowThe biometric authentication implementation correctly checks for availability and user preferences before attempting authentication. The mounted check prevents navigation issues after async operations.
76-107: Secure PIN verification with proper error handlingThe PIN verification logic correctly handles async operations, provides user feedback for incorrect PINs, and includes proper cleanup with mounted checks. The automatic error clearing after 2 seconds provides good UX.
122-155: LGTM - Well-structured number button implementationThe number button widget provides good visual feedback with proper theming and aspect ratio handling. The tap state management correctly disables interaction during authentication.
192-302: Excellent overall UI composition and accessibilityThe main build method creates a well-structured, accessible interface with proper spacing, theming, and responsive layout. The numeric keypad layout follows standard conventions and includes proper accessibility considerations.
lib/screens/pin_setup_screen.dart (3)
27-53: LGTM - Clear two-phase PIN entry logicThe PIN entry logic correctly handles both creation and confirmation phases with proper state transitions. The length validation and error state clearing provide good user feedback.
73-123: Secure PIN verification and storage with proper error handlingThe PIN matching logic and AppLockService integration correctly handles the confirmation flow. The success feedback with SnackBar and navigation provides clear user feedback. Error handling includes proper async state management.
207-315: Well-structured setup UI with clear user guidanceThe main build method provides excellent user guidance with contextual instructions that change based on the setup phase. The layout is consistent with the app lock screen while being appropriately adapted for the setup flow.
lib/main.dart (5)
10-12: LGTM - Clean imports for app lock integrationThe new imports for AppLockScreen, PinSetupScreen, and AppLockService are properly organized and support the app lock functionality.
22-27: LGTM - Proper widget state conversionThe conversion from ConsumerWidget to ConsumerStatefulWidget is correctly implemented to support app lifecycle monitoring.
29-55: Excellent app lifecycle integration with proper cleanupThe implementation correctly adds and removes the WidgetsBindingObserver, initializes the AppLockService asynchronously, and manages the app lock state. The initialization pattern ensures the service is ready before state updates.
57-77: Secure app lifecycle state managementThe app lifecycle monitoring correctly responds to app resume events and navigates to the app lock screen when needed. The mounted check prevents navigation issues after async operations.
110-117: LGTM - Complete routing configurationThe route configuration includes all necessary screens for the app lock feature. The route naming is consistent with the navigation calls throughout the app.
lib/screens/settings_screen.dart (1)
7-14: LGTM! Widget conversion is appropriate for state management.The conversion to
ConsumerStatefulWidgetis the correct approach for managing the asynchronous loading and local state of app lock settings.lib/services/app_lock_service.dart (2)
143-167: Biometric authentication implementation looks secure.Good security practices:
- Checks availability before attempting authentication
- Uses
biometricOnly: trueto prevent fallback to device credentials- Proper error handling
- Auto-unlocks on successful authentication
186-198: Data cleanup method is comprehensive.The
clearAllDatamethod properly removes all secure storage entries and resets in-memory state, ensuring complete cleanup of app lock data.
…pp lock and pin setup screens AI code suggestion fixes
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
lib/widgets/pin_ui_components.dart (1)
77-88: Same consistency issue with PinActionButton.Apply the same fix as
PinNumberButtonto maintain consistent behavior patterns.lib/screens/settings_screen.dart (1)
231-234: Consider user experience for test functionality.Using
pushNamedinstead ofpushReplacementNamedwould allow users to return to settings after testing, improving the UX for this test feature.onTap: () { _appLockService.lock(); - Navigator.of(context).pushReplacementNamed('/app-lock'); + Navigator.of(context).pushNamed('/app-lock'); },
🧹 Nitpick comments (2)
lib/widgets/pin_ui_components.dart (1)
27-38: Consider making onPressed non-nullable for better UX.The
PinNumberButtonallowsonPressedto be null, but this creates inconsistent behavior with theisDisabledparameter. A disabled button should still have a callback but ignore it.- final VoidCallback? onPressed; + final VoidCallback onPressed;Then update the InkWell:
- onTap: isDisabled ? null : onPressed, + onTap: isDisabled ? () {} : onPressed,lib/screens/pin_setup_screen.dart (1)
23-26: Remove empty initState method.The
initStatemethod is empty and unnecessary.- @override - void initState() { - super.initState(); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/screens/app_lock_screen.dart(1 hunks)lib/screens/pin_setup_screen.dart(1 hunks)lib/screens/settings_screen.dart(4 hunks)lib/services/app_lock_service.dart(1 hunks)lib/widgets/pin_ui_components.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/services/app_lock_service.dart
🔇 Additional comments (9)
lib/widgets/pin_ui_components.dart (1)
3-10: LGTM! Well-structured PIN digit indicator widget.The
PinDigitWidgetuses proper theme colors and follows Flutter conventions with optional parameters and clear naming.lib/screens/app_lock_screen.dart (2)
82-82: Security: PIN stored in memory as plain text.The PIN is concatenated into a plain string before verification. Consider using a more secure approach where individual digits are not stored in memory longer than necessary.
This approach stores the complete PIN in memory. Verify if the
AppLockService.verifyPinCodemethod handles secure comparison to prevent timing attacks and ensure the PIN is cleared from memory promptly.
239-243: Good UX: Biometric button placement and functionality.The biometric authentication button is well-positioned and allows users to retry biometric auth if the initial attempt fails.
lib/screens/pin_setup_screen.dart (2)
105-107: Good security practice: Automatic app lock enablement.Automatically enabling app lock after PIN setup ensures the security feature is immediately active.
74-94: Well-implemented PIN validation logic.The PIN mismatch handling with error display and confirmation reset provides good user feedback.
lib/screens/settings_screen.dart (4)
31-50: Well-structured initialization with proper loading states.The initialization pattern correctly separates service setup from async operations like biometrics checking, with appropriate loading state management.
165-178: Good UX: Conditional PIN setup navigation.The logic to automatically navigate to PIN setup when enabling app lock without a PIN provides seamless user experience.
247-262: Clean timeout text formatting.The helper method provides clear, user-friendly timeout descriptions with good fallback handling.
290-330: Well-implemented timeout selection dialog.The dialog provides clear options with immediate state updates and proper navigation handling.
… and pin setup screens
| } | ||
| getByName("debug") { | ||
| signingConfig = signingConfigs.getByName("debug") | ||
| // Debug builds don't need signing config for development |
There was a problem hiding this comment.
@cogwheel0 check this if you want to keep it as is, I was unable to run as I didn't have any signing config.
|
@cogwheel0 Please take a look at this and merge for the next version |
I agree |
|
Yes, planning on it! |
3 participants
fixes #15
Summary by CodeRabbit
New Features
Improvements
Platform Support
Dependencies