feat: add encrypted token storage and proper logout handling#492
feat: add encrypted token storage and proper logout handling#492spc-28 wants to merge 3 commits intoCircuitVerse:masterfrom
Conversation
✅ Deploy Preview for cv-mobile-app-web ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new SecureStorageService (singleton wrapper around FlutterSecureStorage) and integrates it into LocalStorageService with in-memory caching, initializeSecureData(), and clearSecureData(). Locator initialization now awaits secure data initialization. Introduces UsersApi.logout() and calls it from CVLandingViewModel before clearing secure data. Adds flutter_secure_storage and raises the Dart SDK lower bound. Updates Android toolchain and Gradle wrapper versions, adjusts README header and mobile service note, removes synthetic-package from l10n.yaml, registers quill_native_bridge_windows for Windows, changes several DropdownButtonFormField usages from Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@hardik17771 @JatsuAkaYashvant please let me know, any further changes are required. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/viewmodels/cv_landing_viewmodel.dart (2)
42-55:⚠️ Potential issue | 🔴 CriticalNetwork failure in
logout()blocks the entire local cleanup — user can't log out offline.If
_usersApi.logout()throws (e.g., no network), the exception propagates and lines 45–54 never execute, leaving the user stuck in a logged-in state. Backend revocation should be best-effort; local cleanup must always proceed.🐛 Proposed fix
void onLogout() async { - await _usersApi.logout(); - + try { + await _usersApi.logout(); + } catch (_) { + // Best-effort: proceed with local cleanup even if server revocation fails + } + _storage.isLoggedIn = false; _storage.currentUser = null; _storage.token = null; await _storage.clearSecureData();
97-99:⚠️ Potential issue | 🟠 Major
onLogout()is not awaited — logout may be incomplete when the snackbar shows.
onLogout()is declared asvoid(notFuture<void>), so the call at line 98 is fire-and-forget. TheselectedIndex = 0andSnackBarUtils.showDark(...)on lines 99–103 execute immediately, before the async logout work finishes.♻️ Suggested fix
- void onLogout() async { + Future<void> onLogout() async {if (_dialogResponse?.confirmed ?? false) { - onLogout(); + await onLogout(); selectedIndex = 0;
🤖 Fix all issues with AI agents
In `@android/settings.gradle`:
- Around line 21-22: Kotlin was bumped to 2.1.0; inspect MainActivity.kt and any
Kotlin-coded dependencies for 2.1.0 breaking changes: replace any usages of the
locale-sensitive toLowerCase()/toUpperCase() with lowercase()/uppercase(), fix
nullability annotations/usages to satisfy JSpecify stricter nullability (add
explicit ? or !! or safe calls as appropriate), and verify code relying on K2
smart-cast propagation (e.g., long-lived mutable vars or casts in functions) is
made explicit with local val copies or explicit casts; update any affected
functions, properties, or call sites in MainActivity.kt and Kotlin-based libs to
compile cleanly under Kotlin 2.1.0.
In `@lib/services/API/users_api.dart`:
- Around line 224-233: The logout() method currently swallows all exceptions in
its catch block; update the catch to capture the error (and optionally stack)
like catch (e, s) and record it instead of ignoring—use the project's logger (or
at minimum print) to log a clear message including the endpoint
(EnvironmentConfig.CV_API_BASE_URL + '/auth/logout'), the headers context, and
the exception and stack; keep the method non-throwing so UX isn't blocked but
ensure ApiUtils.delete failures are observable for debugging and monitoring.
In `@lib/services/secure_storage_service.dart`:
- Around line 6-22: The public default constructor of SecureStorageService
allows callers to instantiate it directly and bypass getInstance(), which can
leave _secureStorage null; make the class constructor private (e.g.,
SecureStorageService._()) so external code cannot call SecureStorageService()
and must use SecureStorageService.getInstance(), and verify that getInstance
initializes _secureStorage and returns the singleton _instance; update any
tests/usages to call getInstance() if necessary.
- Around line 52-54: The current deleteAll() implementation calls
_secureStorage!.deleteAll(), which will erase every key in FlutterSecureStorage;
change it to only remove keys owned by this service by iterating over the
service's known keys (or by reading all keys and filtering by your service
prefix) and calling _secureStorage!.delete(key: key) for each; update
SecureStorageService (or the class containing deleteAll) to store a list of
owned keys or a key prefix (e.g., a private List<String> _ownedKeys or String
_keyPrefix) and use that list/prefix in deleteAll() instead of deleteAll().
In `@windows/flutter/generated_plugin_registrant.cc`:
- Line 14: The generated registrant incorrectly includes a placeholder header
and registration for quill_native_bridge_windows (the include
<quill_native_bridge_windows/none.h> and the noneRegisterWithRegistrar call);
remove the bogus include and registration invocation from the
generated_plugin_registrant.cc so the plugin is not registered on the C++ side,
and instead ensure the package only exposes Dart-only registration
(QuillNativeBridgeWindows.registerWith) upstream; search for the symbols
"none.h" and "noneRegisterWithRegistrar" and delete those lines or update the
plugin's Windows metadata so the generated C++ does not emit the placeholder.
🧹 Nitpick comments (6)
android/app/build.gradle (1)
76-77:targetSdkVersion 33is quite stale relative tocompileSdk 36.Google Play requires
targetSdkVersion 34for new app updates (since August 2024) and will require 35 soon. Consider bumpingtargetSdkVersionas well to avoid Play Store rejection.lib/services/API/users_api.dart (1)
36-36: Parameter name mismatch between abstract class and implementation.The abstract class declares
bool subscribed(line 36) while the implementation usesbool isSubscribed(line 167). While Dart allows positional parameter names to differ, this inconsistency can confuse callers and future maintainers.Align the abstract declaration
Future<User>? updateProfile( String name, String? educationalInstitute, String? country, - bool subscribed, + bool isSubscribed, File? image, bool removePicture, );Also applies to: 167-167
lib/services/secure_storage_service.dart (1)
36-42: Consider handling JSON decode errors for corrupted user data.If the stored JSON is malformed (e.g., due to a schema change or corruption),
json.decodeorUser.fromJsonwill throw an unhandled exception. A try-catch that deletes the corrupt entry and returnsnullwould be more resilient.Proposed improvement
Future<User?> get currentUser async { final userJson = await _secureStorage!.read(key: _userKey); if (userJson == null || userJson == 'null') { return null; } - return User.fromJson(json.decode(userJson)); + try { + return User.fromJson(json.decode(userJson)); + } catch (_) { + await _secureStorage!.delete(key: _userKey); + return null; + } }lib/services/local_storage_service.dart (2)
57-60: Fire-and-forget writes to secure storage lose errors and persistence guarantees.The
tokenandcurrentUsersetters call async secure-storage methods withoutawait, so write failures are silently swallowed and data may not persist if the app is killed immediately after. Since Dart setters can't beasync, consider converting these to explicit async methods (e.g.,Future<void> setToken(...)) so callers can await them.♻️ Suggested approach
- String? get token => _cachedToken; - - set token(String? _token) { - _cachedToken = _token; - _secureStorage!.setToken(_token); - } + String? get token => _cachedToken; + + Future<void> setToken(String? _token) async { + _cachedToken = _token; + await _secureStorage!.setToken(_token); + }Apply the same pattern to
currentUser. All call-sites that currently use the setter syntax (_storage.token = value) would need to be updated toawait _storage.setToken(value).
47-48: Instance fields for cache in a singleton with static factory — works but fragile.
_cachedTokenand_cachedUserare instance fields, while_instanceis static and reused. This works because only one instance exists, but ifgetInstance()were ever called after the singleton was created (returning the existing_instance), a newLocalStorageService()would not be constructed, so caches are safe. Just noting this coupling is implicit.lib/viewmodels/cv_landing_viewmodel.dart (1)
50-52:authTypecheck occurs after token is already nullified.Line 47 sets
_storage.token = nulland line 48 clears secure data, but line 50 reads_storage.authType(stored in SharedPreferences, so still available). This happens to work becauseauthTypeis in SharedPreferences, not secure storage. However, ifauthTypeis ever moved to secure storage, this will break. Consider readingauthTypebefore clearing state, or explicitly clearing it afterward.
| id "com.android.application" version "8.9.1" apply false | ||
| id "org.jetbrains.kotlin.android" version "2.1.0" apply false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find all Kotlin files in the repo
fd -e kt -e ktsRepository: CircuitVerse/mobile-app
Length of output: 138
🏁 Script executed:
# Check the full android/settings.gradle file for context
cat -n android/settings.gradleRepository: CircuitVerse/mobile-app
Length of output: 967
🏁 Script executed:
# Search for Kotlin-related configurations in gradle files
rg -l "kotlin" --type gradleRepository: CircuitVerse/mobile-app
Length of output: 115
🏁 Script executed:
# Look for any kotlinOptions or compiler settings
rg "kotlinOptions|compileOptions|jvmTarget" --type gradle -A 3Repository: CircuitVerse/mobile-app
Length of output: 492
🏁 Script executed:
# Check build.gradle files for kotlin dependencies
rg "kotlin\(" --type gradleRepository: CircuitVerse/mobile-app
Length of output: 49
🏁 Script executed:
# Check the android/app/build.gradle for more context on Kotlin configuration
cat -n android/app/build.gradle | head -100Repository: CircuitVerse/mobile-app
Length of output: 3823
🌐 Web query:
Kotlin 2.1.0 K2 compiler default breaking changes migration guide
💡 Result:
Kotlin 2.1.0 + “K2 is the default”: what to migrate, what breaks, and how to de-risk
1) Reality check: K2 became the default before 2.1.0
Kotlin’s K2 compiler frontend is enabled by default starting with Kotlin 2.0.0, so moving to 2.1.0 mainly means you’re picking up additional K2 behavior changes + 2.1.x compatibility changes, not “turning K2 on for the first time.” [1]
2) The official migration docs you should follow (in this order)
- K2 compiler migration guide (core “what changed in the compiler frontend” + rollback options) [1]
- Compatibility guide for Kotlin 2.1.x (source/binary/behavioral breaks from 2.0 → 2.1) [2]
- What’s new in Kotlin 2.1.0 (high-level release notes + notable deprecations) [3]
3) The K2-related breaking/behavior changes that most often bite during 2.1.0 upgrades
A) K2 frontend semantic/typing differences (may change inference / behavior)
The 2.1.x compatibility guide calls out behavior changes in K2 smart-cast propagation (bidirectional propagation for inferred variables; explicitly typed variables stop propagating type info). This can change which branches compile or which overload is chosen. [2]
B) Java nullability becomes stricter (common “suddenly red” upgrade pain)
In 2.1.x, JSpecify nullability mismatches are treated as errors by default (“strict” mode) unless you override diagnostics with -Xnullability-annotations. [2]
C) Standard library deprecations that become errors in 2.1.0
A common one: locale-sensitive case conversion APIs (String.toLowerCase(), toUpperCase(), etc.) are error-level in 2.1.0; replace with lowercase()/uppercase() (optionally with an explicit locale). [2] (also noted in the 2.1.0 release notes) [3]
4) K2 migration-guide “compiler changed” items to scan for in your codebase
The K2 compiler migration guide highlights several categories that can break builds or change resolution, including:
- Immediate initialization of open properties with backing fields
- Deprecated synthetic setters on a projected receiver
- Forbidden use of inaccessible generic types
- More consistent resolution between Kotlin properties and Java fields with the same name
- Improved null safety for Java primitive arrays
- Stricter rules for abstract members in
expectclasses [1]
(If you hit odd compilation errors after upgrading, these bullets are usually where the explanation is.)
5) Practical migration steps (safe, incremental)
- Upgrade Kotlin to 2.1.0 and run CI with warnings-as-errors off first (to see the delta). [3]
- Verify compiler plugins are K2-compatible (K2 support is not universal across all plugins). The K2 migration guide lists supported plugins and notes that custom frontend plugins may need a rewrite. [1]
- If you’re on Compose Multiplatform: note that from Compose Multiplatform 1.8.0, some klibs can be consumed only with Kotlin 2.1.0+ (so this upgrade may be mandatory depending on your dependency graph). [4]
6) Rollback / “stabilize first” options if you need time
The K2 migration guide documents how to roll back behavior by setting an older language version (for example, -language-version 1.9). This can be useful to unblock a release while you fix K2-related issues properly. [1]
Key sources
- K2 compiler migration guide (official): [1]
- Kotlin 2.1.x compatibility guide (official): [2]
- Kotlin 2.1.0 release notes (official): [3]
- Compose Multiplatform K2 migration note / Kotlin 2.1.0 requirement: [4]
🌐 Web query:
Kotlin 2.1.0 android plugin compatibility requirements
💡 Result:
Kotlin 2.1.0 + Android (Kotlin Gradle plugin 2.1.0) compatibility requirements
- Minimum Android Gradle Plugin (AGP) for the Kotlin Gradle plugin: AGP 7.3.1+ (this is the Kotlin plugin’s own minimum supported AGP). [1]
- Minimum Gradle version for Kotlin 2.1.0: Gradle 7.6.3+. [1]
- Recommended / “fully compatible” Gradle range (per Kotlin 2.1.0 release notes): 7.6.3–8.6; 8.7–8.10 are supported with a noted exception (KMP
withJava()deprecation warnings). [1]
Important Android build-tools constraint (D8/R8 via AGP)
Even though the Kotlin plugin supports AGP 7.3.1+, Google’s Android toolchain compatibility table lists Kotlin 2.1 as requiring at least:
- AGP 8.6+ (bundles matching D8/R8), and
- R8 8.6.17+. [2]
Practical takeaway: use AGP 8.6+ (and its matching Gradle version) if you want to stay within the officially required Android toolchain versions for Kotlin 2.1; otherwise, you may be in a “Kotlin plugin supports it, but Android shrinker/tooling may not” zone. [1][2]
Sources: Kotlin 2.1.0 release notes / Gradle & AGP minimums [1]; Android Developers Kotlin↔AGP/R8 requirements table [2].
Review Kotlin 2.1.0 compatibility—verify codebase handles breaking changes.
Kotlin 2.1.0 introduces breaking changes beyond K2 (which became default in 2.0.0): JSpecify nullability is stricter by default, locale-sensitive case conversion APIs (toLowerCase(), toUpperCase()) are now errors (use lowercase()/uppercase()), and K2 smart-cast propagation behavior has changed. Your Android Gradle Plugin 8.9.1 is compatible with Kotlin 2.1.0, but verify the single Kotlin file (MainActivity.kt) and any Kotlin-based dependencies handle these 2.1.0 changes.
🤖 Prompt for AI Agents
In `@android/settings.gradle` around lines 21 - 22, Kotlin was bumped to 2.1.0;
inspect MainActivity.kt and any Kotlin-coded dependencies for 2.1.0 breaking
changes: replace any usages of the locale-sensitive toLowerCase()/toUpperCase()
with lowercase()/uppercase(), fix nullability annotations/usages to satisfy
JSpecify stricter nullability (add explicit ? or !! or safe calls as
appropriate), and verify code relying on K2 smart-cast propagation (e.g.,
long-lived mutable vars or casts in functions) is made explicit with local val
copies or explicit casts; update any affected functions, properties, or call
sites in MainActivity.kt and Kotlin-based libs to compile cleanly under Kotlin
2.1.0.
| #include <flutter_inappwebview_windows/flutter_inappwebview_windows_plugin_c_api.h> | ||
| #include <flutter_secure_storage_windows/flutter_secure_storage_windows_plugin.h> | ||
| #include <permission_handler_windows/permission_handler_windows_plugin.h> | ||
| #include <quill_native_bridge_windows/none.h> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file in question
head -40 windows/flutter/generated_plugin_registrant.ccRepository: CircuitVerse/mobile-app
Length of output: 1845
🏁 Script executed:
# Search for quill_native_bridge_windows references in the codebase
rg "quill_native_bridge_windows" --type yaml --type lock --type txt -iRepository: CircuitVerse/mobile-app
Length of output: 163
🏁 Script executed:
# Check for pubspec files that might reference this plugin
fd "pubspec.yaml" | head -20Repository: CircuitVerse/mobile-app
Length of output: 79
🏁 Script executed:
# Look for the plugin registration metadata
find . -name "*.yaml" -o -name "*.yml" | xargs grep -l "quill_native_bridge_windows" 2>/dev/null | head -10Repository: CircuitVerse/mobile-app
Length of output: 49
🌐 Web query:
quill_native_bridge_windows flutter plugin windows registration
💡 Result:
quill_native_bridge_windows uses Dart-side registration (it doesn’t require a Windows C++ RegisterWithRegistrar step).
- If you depend on
quill_native_bridge(the app-facing federated plugin), the Windows implementation is auto-included on Windows builds; you normally don’t add or registerquill_native_bridge_windowsyourself. (pub.dev) - The Windows “registration” is just a static Dart method that sets the platform-interface singleton:
QuillNativeBridgeWindows.registerWith()→QuillNativeBridgePlatform.instance = QuillNativeBridgeWindows();(pub.dev)
When would you manually register?
Only if you’re doing something non-standard (custom plugin registrant / unusual bootstrap). Then call it early (before using QuillNativeBridge), e.g.:
import 'package:quill_native_bridge_windows/quill_native_bridge_windows.dart';
void main() {
QuillNativeBridgeWindows.registerWith();
// runApp(...)
}Citations:
- 1: https://pub.dev/documentation/quill_native_bridge_windows/latest/
- 2: https://pub.dev/documentation/quill_native_bridge_windows/latest/quill_native_bridge_windows/QuillNativeBridgeWindows/registerWith.html
Suspicious none.h header and noneRegisterWithRegistrar — upstream plugin misconfiguration.
The quill_native_bridge_windows plugin appears in the C++ registrant as #include <quill_native_bridge_windows/none.h> (line 14) and noneRegisterWithRegistrar(...) (line 30-31). This is incorrect: quill_native_bridge_windows uses Dart-side registration only (QuillNativeBridgeWindows.registerWith()) and should not require C++ registration. The "none.h" header and "none" plugin name are placeholder values indicating a misconfiguration in the plugin's Windows registration metadata. This is an upstream issue in the quill_native_bridge_windows package; verify this doesn't cause build or runtime issues on Windows.
🤖 Prompt for AI Agents
In `@windows/flutter/generated_plugin_registrant.cc` at line 14, The generated
registrant incorrectly includes a placeholder header and registration for
quill_native_bridge_windows (the include <quill_native_bridge_windows/none.h>
and the noneRegisterWithRegistrar call); remove the bogus include and
registration invocation from the generated_plugin_registrant.cc so the plugin is
not registered on the C++ side, and instead ensure the package only exposes
Dart-only registration (QuillNativeBridgeWindows.registerWith) upstream; search
for the symbols "none.h" and "noneRegisterWithRegistrar" and delete those lines
or update the plugin's Windows metadata so the generated C++ does not emit the
placeholder.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@lib/services/API/users_api.dart`:
- Around line 224-226: The logout() override is currently empty; implement it to
call the backend logout endpoint (POST/DELETE to /auth/logout per API spec) to
revoke the server-side token before clearing local state. In the UsersApi
class/function logout(), perform the network request using the existing HTTP
client (reuse the same client used by other methods like
login()/refreshToken()), handle non-2xx responses by logging or throwing an
error, and only then clear local credentials/session cache so the token is
revoked server-side as required by the PR.
In `@lib/services/local_storage_service.dart`:
- Around line 47-55: The synchronous getters token and currentUser can return
null before initializeSecureData() populates _cachedToken/_cachedUser; add an
initialization guard (e.g., a private bool _initialized set true at the end of
initializeSecureData()) and assert or throw a clear StateError when token or
currentUser is accessed while !_initialized (or use assert(_initialized) in
debug builds) so callers get a loud failure during development; update
getToken() to still populate lazily but keep the synchronous getters protected
using the _initialized flag and reference _cachedToken, _cachedUser,
initializeSecureData(), token, currentUser, and getToken() in the change.
- Around line 57-60: The token setter currently updates _cachedToken and calls
_secureStorage!.setToken(_token) fire-and-forget; if the async write fails the
error is lost and in-memory state is inconsistent—update the setter for token
(and mirror the same change for the currentUser setter) to attach a completion
handler to the Future returned by _secureStorage.setToken (e.g., using
.catchError or .then(..., onError: ...)) and log or surface failures via the
existing logger so persistence errors are observable; ensure you handle nullable
_secureStorage safely and include context in the log message (e.g., "Failed to
persist token" / "Failed to persist currentUser") and do the same for the
currentUser setter.
In `@lib/services/secure_storage_service.dart`:
- Around line 36-42: The currentUser getter can throw during json.decode or
User.fromJson causing startup crashes; wrap the deserialization in a try-catch
inside the currentUser getter, and on exception delete the corrupt entry via
_secureStorage!.delete(key: _userKey) and return null so initializeSecureData
can continue; optionally log the error (e.g., using your logger) for
diagnostics.
🧹 Nitpick comments (1)
lib/services/secure_storage_service.dart (1)
13-15: Consider specifyingAndroidOptionsfor completeness.You've configured
IOSOptionsfor Keychain accessibility but haven't setAndroidOptions. While the defaults useEncryptedSharedPreferences, explicitly passingaOptions: AndroidOptions(encryptedSharedPreferences: true)documents the intent and guards against future library default changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pubspec.yaml`:
- Line 55: Update the pubspec to flutter_secure_storage: ^10.0.0 and ensure the
package constraints in the environment section require Dart ≥3.3 (e.g., sdk:
">=3.3.0 <4.0.0") and your project uses Flutter ≥3.19; verify Android
configuration by checking android/app/build.gradle for minSdkVersion and follow
the flutter_secure_storage changelog instructions for any Jetpack Security /
cipher or migration setup; audit usages of shared_preferences (the symbol
shared_preferences) and remove or restrict it to non-sensitive data, or replace
sensitive storage with flutter_secure_storage and run the provided migration
tooling for Android if you keep existing encryptedSharedPreferences data.
- Line 18: Check whether any package in pubspec.yaml actually requires Dart
>=3.9.0 by running `dart pub deps --json` or inspecting dependency
pages/CHANGELOGs (notably flutter_secure_storage requires >=3.3), then if no
dependency needs 3.9.0, change the SDK constraint in pubspec.yaml from ">=3.9.0
<4.0.0" to the lowest required version (e.g., ">=3.3.0 <4.0.0") to maximize
compatibility; if a specific dependency does require 3.9.0, document that
dependency name and version in the pubspec.yaml comment and keep the bumped
constraint.
|
|
||
| environment: | ||
| sdk: ">=3.7.2 <4.0.0" | ||
| sdk: ">=3.9.0 <4.0.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Dart SDK 3.9.0 release date stable
💡 Result:
Dart SDK 3.9.0 was released to the stable channel on August 13, 2025. [1] [2]
🌐 Web query:
flutter_secure_storage 10.0.0 version pub.dev
💡 Result:
flutter_secure_storage v10.0.0 is published on pub.dev (publisher: steenbakker.dev) and is the current stable 10.x release. It requires Dart SDK >= 3.3 (and the v10 beta notes indicate Flutter >= 3.19). [1][2]
Add it to pubspec.yaml:
dependencies:
flutter_secure_storage: ^10.0.0Key v10 highlights (from the changelog): major security/platform modernization, with a largely rewritten Android implementation due to Jetpack Security deprecation, new/updated default ciphers (RSA-OAEP + AES-GCM), improved biometrics support, and migration tools (with some breaking changes around encryptedSharedPreferences behavior/options). [3]
🏁 Script executed:
# Check the pubspec.yaml file to understand the actual changes
find . -name "pubspec.yaml" -type f | head -5Repository: CircuitVerse/mobile-app
Length of output: 81
🏁 Script executed:
cat -n ./pubspec.yamlRepository: CircuitVerse/mobile-app
Length of output: 3812
Verify that the Dart SDK 3.9.0 bump is actually required by dependencies.
Dart 3.9.0 is a confirmed stable release (August 13, 2025). However, flutter_secure_storage v10.0.0 only requires Dart >= 3.3, not 3.9.0. Verify whether other dependencies in this project mandate the 3.9.0 minimum. If not, prefer keeping the minimum SDK constraint as low as possible for broader compatibility with contributor environments and CI pipelines.
🤖 Prompt for AI Agents
In `@pubspec.yaml` at line 18, Check whether any package in pubspec.yaml actually
requires Dart >=3.9.0 by running `dart pub deps --json` or inspecting dependency
pages/CHANGELOGs (notably flutter_secure_storage requires >=3.3), then if no
dependency needs 3.9.0, change the SDK constraint in pubspec.yaml from ">=3.9.0
<4.0.0" to the lowest required version (e.g., ">=3.3.0 <4.0.0") to maximize
compatibility; if a specific dependency does require 3.9.0, document that
dependency name and version in the pubspec.yaml comment and keep the bumped
constraint.
| scroll_to_index: ^3.0.1 | ||
| share_plus: ^11.1.0 | ||
| shared_preferences: ^2.5.4 | ||
| flutter_secure_storage: ^10.0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
flutter_secure_storage 10.0.0 pub.dev
💡 Result:
flutter_secure_storage v10.0.0 on pub.dev is the latest stable major release (published ~2 months ago) and requires Dart SDK ≥ 3.3 (Flutter ≥ 3.19). [1][2]
What changed in 10.0.0 (high level):
- Major Android rewrite due to Jetpack Security deprecation: new secure cipher implementations, improved biometrics support, updated default ciphers, and migration tooling (including migration away from deprecated
encryptedSharedPreferences). [3] - Platform modernization updates across supported platforms. [3]
Add to pubspec.yaml:
dependencies:
flutter_secure_storage: ^10.0.0[1]
Sources: pub.dev package page [1], versions [2], changelog [3].
flutter_secure_storage v10.0.0 is published and compatible.
Version 10.0.0 is the latest stable release and requires Dart SDK ≥ 3.3 (Flutter ≥ 3.19). The version includes a major Android rewrite due to Jetpack Security deprecation with improved cipher implementations and migration tooling to move away from deprecated encryptedSharedPreferences.
Ensure your project targets Dart 3.3+ and Flutter 3.19+. Check the changelog for any specific Android configuration requirements for your minSdkVersion.
Regarding shared_preferences (line 54): the migration away from encryptedSharedPreferences reinforces using secure storage for sensitive data. Verify whether shared_preferences is still needed for non-sensitive data or can be removed.
🤖 Prompt for AI Agents
In `@pubspec.yaml` at line 55, Update the pubspec to flutter_secure_storage:
^10.0.0 and ensure the package constraints in the environment section require
Dart ≥3.3 (e.g., sdk: ">=3.3.0 <4.0.0") and your project uses Flutter ≥3.19;
verify Android configuration by checking android/app/build.gradle for
minSdkVersion and follow the flutter_secure_storage changelog instructions for
any Jetpack Security / cipher or migration setup; audit usages of
shared_preferences (the symbol shared_preferences) and remove or restrict it to
non-sensitive data, or replace sensitive storage with flutter_secure_storage and
run the provided migration tooling for Android if you keep existing
encryptedSharedPreferences data.
|
@spc-28 Please address the coderabbit's comment |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30d4cb0f-e78d-4bec-a8b0-ee3ff19ceb21
📒 Files selected for processing (3)
lib/services/API/users_api.dartlib/services/local_storage_service.dartlib/services/secure_storage_service.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/services/API/users_api.dart
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/services/local_storage_service.dart (1)
134-138: Consider settingisLoggedIn = falseinclearSecureData()for defensive consistency.While the current logout flow correctly sets
isLoggedIn = falsebefore callingclearSecureData(), the method would be more self-contained and resilient to misuse if it also cleared this flag. TheinitializeSecureData()method similarly setsisLoggedIn = falseas a defensive measure when detecting inconsistent state (line 129), establishing a precedent for defensive state management in this class.♻️ Proposed fix
Future<void> clearSecureData() async { _cachedToken = null; _cachedUser = null; await _secureStorage!.deleteAll(); + isLoggedIn = false; }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ac8e8af-abea-4ad5-958a-45e34af21fac
📒 Files selected for processing (3)
lib/services/API/users_api.dartlib/services/local_storage_service.dartlib/services/secure_storage_service.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/services/secure_storage_service.dart
- lib/services/API/users_api.dart
|
@JatsuAkaYashvant I have solved the issues addressed by coderabbit's review, the remaining reviews are version related Please let me know if anymore changes needed |
2 participants
Fixes #488
Describe the changes you have made in this PR:
-Add SecureStorageService using flutter_secure_storage for encrypted token and user data storage (iOS Keychain, Android KeyStore)
-Remove debug print statements that logged sensitive data
-Improve Google OAuth error handling with null checks
-Update LocalStorageService to use encrypted storage with caching
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores