From 07b9e44350ab8fae33c7f5990470f4f219bba924 Mon Sep 17 00:00:00 2001 From: gasaichandesu Date: Fri, 19 Jun 2026 22:58:27 +0400 Subject: [PATCH] refactor(auth): Migrate session storage to flutter_secure_storage --- AGENTS.md | 8 +- README.md | 5 +- TODO.md | 6 ++ docs/project_review.md | 34 +++---- .../launcher/data/launcher_log_service.dart | 2 + ...dart => local_preferences_repository.dart} | 44 +++++---- lib/service_container.config.dart | 36 +++++--- lib/third_party/flutter_secure_storage.dart | 8 ++ .../local_preferences_repository_test.dart | 89 +++++++++++++++++++ ...red_prefs_preferences_repository_test.dart | 52 ----------- 10 files changed, 181 insertions(+), 103 deletions(-) create mode 100644 TODO.md rename lib/features/preferences/data/repositories/{shared_prefs_preferences_repository.dart => local_preferences_repository.dart} (50%) create mode 100644 lib/third_party/flutter_secure_storage.dart create mode 100644 test/features/preferences/data/repositories/local_preferences_repository_test.dart delete mode 100644 test/features/preferences/data/repositories/shared_prefs_preferences_repository_test.dart diff --git a/AGENTS.md b/AGENTS.md index 7db9200..72a0a1f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,7 +23,8 @@ Important areas: and client synchronization. - `lib/features/launcher/data` contains API, installation, and log services. - `lib/features/preferences` stores selected install directory and launcher - session data. + session data. Launcher sessions must use secure storage; non-sensitive + preferences may use `shared_preferences`. - `lib/service_container.dart` and `lib/service_container.config.dart` provide `get_it`/`injectable` dependency wiring. - `docs/launcher_web_api_spec.md` documents the current launcher Web API and @@ -52,9 +53,6 @@ flutter build windows --dart-define=MOONWELL_API_BASE_URL=https://host See `docs/linting.md` for the required format, analyze, test, and Lefthook pre-commit workflow. -In the current shell on 2026-06-18, `flutter` was not recognized by -`Get-Command flutter` or `where.exe flutter`. - ## Configuration The launcher API base URL is compile-time configuration: @@ -69,6 +67,8 @@ or local-only API URLs. ## Architecture Notes - All backend calls should go through `LauncherApiClient`. +- Launcher bearer tokens must be persisted through `flutter_secure_storage`. + Do not store sessions or other secrets in `shared_preferences`. - File sync behavior should stay in `ClientSyncUseCase` and `GameInstallationService`. - The server manifest is the source of truth for client files. diff --git a/README.md b/README.md index a0ab40a..082dfbc 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,8 @@ The launcher is responsible for: - `flutter_bloc` for launcher UI state - `get_it` and `injectable` for dependency injection - `dio` for Web API and file download requests -- `shared_preferences` for launcher preferences +- `flutter_secure_storage` for launcher sessions +- `shared_preferences` for non-sensitive launcher preferences ## Prerequisites @@ -93,7 +94,7 @@ See `docs/launcher_web_api_spec.md` for the API and sync contract. ## Documentation - `AGENTS.md`: architecture, commands, and repository rules for contributors -- `TODO.md`: refactoring, verification, and Flutter upgrade tasks +- `TODO.md`: remaining verification and maintenance tasks - `docs/linting.md`: format, analyze, test, and Lefthook workflow - `docs/project_review.md`: project review with strengths, weaknesses, and upgrade feasibility notes diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..353d311 --- /dev/null +++ b/TODO.md @@ -0,0 +1,6 @@ +# TODO + +## Verification + +- [ ] Run a Windows build smoke test: + `flutter build windows --dart-define=MOONWELL_API_BASE_URL=https://host`. diff --git a/docs/project_review.md b/docs/project_review.md index 608d927..b747eb3 100644 --- a/docs/project_review.md +++ b/docs/project_review.md @@ -51,8 +51,9 @@ most important file verification behavior. normalized to UTF-8. - At initial review, custom title-bar widgets were duplicated between login and home screens. They have since been extracted into a shared widget. -- Session tokens are persisted through `shared_preferences`. That is simple and - testable, but it is not hardened storage for bearer tokens. +- Session tokens were originally persisted through `shared_preferences`. They + now use `flutter_secure_storage`, while `shared_preferences` remains limited + to non-sensitive launcher preferences. - The dependency injection output file `lib/service_container.config.dart` is committed, but the regeneration command was not previously documented. - UI/widget/integration coverage is still light compared with the risk in login, @@ -69,37 +70,30 @@ Assumption: `3.44.2` means Flutter SDK `3.44.2`. Current local indicators: -- `.fvmrc` pins Flutter `3.35.2`. +- `.fvmrc` is absent. - `pubspec.yaml` declares Dart SDK `^3.9.0`. - `pubspec.lock` declares Flutter SDK `>=3.38.0`. - The project policy is latest stable Flutter SDK only; FVM-pinned workflows are not supported. -- The local shell cannot find `flutter`: both `Get-Command flutter` and - `where.exe flutter` fail. +- The local shell now finds Flutter `3.44.2` on the stable channel. -Because `flutter` is not available in this shell, the upgrade cannot be fully -validated here. If Flutter is installed, restart the shell or fix PATH so -`flutter --version` works. The code and lockfile already suggest that the -project has moved beyond the stale `.fvmrc` pin, so the upgrade is plausible, -but it remains a tooling validation task. +Flutter `3.44.2` has been validated for dependency resolution, formatting, +analysis, and tests. A production-like Windows build smoke test remains open. Recommended upgrade path: -1. Install or expose the latest stable Flutter SDK on PATH. -2. Confirm whether the latest stable Flutter SDK is `3.44.2` or newer. -3. Remove or ignore stale FVM pinning if it conflicts with the latest-SDK - policy. -4. Run `flutter pub get`. -5. Regenerate dependency injection only if dependency resolution or generator +1. Keep the latest stable Flutter SDK on PATH. +2. Run `flutter pub get`. +3. Regenerate dependency injection only if dependency resolution or generator output requires it: `dart run build_runner build --delete-conflicting-outputs`. -6. Run `flutter analyze`. -7. Run a Windows build smoke test: +4. Run `flutter analyze`. +5. Run a Windows build smoke test: `flutter build windows --dart-define=MOONWELL_API_BASE_URL=https://host`. ## Recommended Refactoring Direction -- Decide whether bearer tokens can remain in `shared_preferences`; if not, - migrate session storage to platform-secure storage. +- Keep secure session persistence covered by tests when login or session + restore behavior changes. - Add CI or a documented local verification workflow that runs analysis and a Windows build smoke test with the latest stable Flutter SDK. diff --git a/lib/features/launcher/data/launcher_log_service.dart b/lib/features/launcher/data/launcher_log_service.dart index 21f7817..dbcb554 100644 --- a/lib/features/launcher/data/launcher_log_service.dart +++ b/lib/features/launcher/data/launcher_log_service.dart @@ -2,9 +2,11 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:injectable/injectable.dart'; import 'package:moonwell_launcher/config.dart'; import 'package:path/path.dart' as p; +@lazySingleton class LauncherLogService { static Future _pendingWrite = Future.value(); diff --git a/lib/features/preferences/data/repositories/shared_prefs_preferences_repository.dart b/lib/features/preferences/data/repositories/local_preferences_repository.dart similarity index 50% rename from lib/features/preferences/data/repositories/shared_prefs_preferences_repository.dart rename to lib/features/preferences/data/repositories/local_preferences_repository.dart index dff448a..2ee49b7 100644 --- a/lib/features/preferences/data/repositories/shared_prefs_preferences_repository.dart +++ b/lib/features/preferences/data/repositories/local_preferences_repository.dart @@ -1,5 +1,6 @@ import 'dart:convert'; +import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:injectable/injectable.dart'; import 'package:moonwell_launcher/features/launcher/domain/entities/launcher_session.dart'; import 'package:moonwell_launcher/features/preferences/domain/repositories/preferences_repository.dart'; @@ -9,12 +10,15 @@ const _outputDirKey = 'output_dir'; const _launcherSessionKey = 'launcher_session'; @LazySingleton(as: PreferencesRepository, env: ['flutter']) -class SharedPrefsPreferencesRepository implements PreferencesRepository { +class LocalPreferencesRepository implements PreferencesRepository { final SharedPreferences _preferences; + final FlutterSecureStorage _secureStorage; - const SharedPrefsPreferencesRepository({ + const LocalPreferencesRepository({ required SharedPreferences preferences, - }) : _preferences = preferences; + required FlutterSecureStorage secureStorage, + }) : _preferences = preferences, + _secureStorage = secureStorage; @override Future getOutputDir() { @@ -33,35 +37,45 @@ class SharedPrefsPreferencesRepository implements PreferencesRepository { } @override - Future getLauncherSession() { - final rawSession = _preferences.getString(_launcherSessionKey); + Future getLauncherSession() async { + await _maybeClearLegacyLauncherSession(); + + final rawSession = await _secureStorage.read(key: _launcherSessionKey); if (rawSession == null || rawSession.isEmpty) { - return Future.value(null); + return null; } try { final decoded = jsonDecode(rawSession); if (decoded is! Map) { - return Future.value(null); + return null; } final json = decoded.map((key, value) => MapEntry(key.toString(), value)); - return Future.value(LauncherSession.fromJson(json)); + return LauncherSession.fromJson(json); } catch (_) { - return Future.value(null); + return null; } } @override - Future setLauncherSession(LauncherSession session) { - return _preferences.setString( - _launcherSessionKey, - jsonEncode(session.toJson()), + Future setLauncherSession(LauncherSession session) async { + await _secureStorage.write( + key: _launcherSessionKey, + value: jsonEncode(session.toJson()), ); + await _maybeClearLegacyLauncherSession(); } @override - Future clearLauncherSession() { - return _preferences.remove(_launcherSessionKey); + Future clearLauncherSession() async { + await _secureStorage.delete(key: _launcherSessionKey); + await _maybeClearLegacyLauncherSession(); + } + + Future _maybeClearLegacyLauncherSession() async { + if (_preferences.containsKey(_launcherSessionKey)) { + await _preferences.remove(_launcherSessionKey); + } } } diff --git a/lib/service_container.config.dart b/lib/service_container.config.dart index 770be03..8a52867 100644 --- a/lib/service_container.config.dart +++ b/lib/service_container.config.dart @@ -1,5 +1,5 @@ -// dart format width=80 // GENERATED CODE - DO NOT MODIFY BY HAND +// dart format width=80 // ************************************************************************** // InjectableConfigGenerator @@ -9,6 +9,7 @@ // coverage:ignore-file // ignore_for_file: no_leading_underscores_for_library_prefixes +import 'package:flutter_secure_storage/flutter_secure_storage.dart' as _i558; import 'package:get_it/get_it.dart' as _i174; import 'package:injectable/injectable.dart' as _i526; import 'package:shared_preferences/shared_preferences.dart' as _i460; @@ -16,10 +17,12 @@ import 'package:shared_preferences/shared_preferences.dart' as _i460; import 'features/launcher/application/client_sync_use_case.dart' as _i757; import 'features/launcher/data/game_installation_service.dart' as _i315; import 'features/launcher/data/launcher_api_client.dart' as _i703; -import 'features/preferences/data/repositories/shared_prefs_preferences_repository.dart' - as _i662; +import 'features/launcher/data/launcher_log_service.dart' as _i43; +import 'features/preferences/data/repositories/local_preferences_repository.dart' + as _i658; import 'features/preferences/domain/repositories/preferences_repository.dart' as _i44; +import 'third_party/flutter_secure_storage.dart' as _i782; import 'third_party/shared_preferences.dart' as _i1006; const String _flutter = 'flutter'; @@ -32,6 +35,7 @@ extension GetItInjectableX on _i174.GetIt { }) async { final gh = _i526.GetItHelper(this, environment, environmentFilter); final sharedPreferencesModule = _$SharedPreferencesModule(); + final flutterSecureStorageModule = _$FlutterSecureStorageModule(); await gh.factoryAsync<_i460.SharedPreferences>( () => sharedPreferencesModule.prefs, preResolve: true, @@ -39,21 +43,33 @@ extension GetItInjectableX on _i174.GetIt { gh.lazySingleton<_i315.GameInstallationService>( () => _i315.GameInstallationService(), ); - gh.lazySingleton<_i703.LauncherApiClient>(() => _i703.LauncherApiClient()); + gh.lazySingleton<_i43.LauncherLogService>(() => _i43.LauncherLogService()); + gh.lazySingleton<_i558.FlutterSecureStorage>( + () => flutterSecureStorageModule.storage, + ); + gh.lazySingleton<_i703.LauncherApiClient>( + () => _i703.LauncherApiClient( + launcherLogService: gh<_i43.LauncherLogService>(), + ), + ); + gh.lazySingleton<_i44.PreferencesRepository>( + () => _i658.LocalPreferencesRepository( + preferences: gh<_i460.SharedPreferences>(), + secureStorage: gh<_i558.FlutterSecureStorage>(), + ), + registerFor: {_flutter}, + ); gh.lazySingleton<_i757.ClientSyncUseCase>( () => _i757.ClientSyncUseCase( launcherApiClient: gh<_i703.LauncherApiClient>(), installationService: gh<_i315.GameInstallationService>(), + launcherLogService: gh<_i43.LauncherLogService>(), ), ); - gh.lazySingleton<_i44.PreferencesRepository>( - () => _i662.SharedPrefsPreferencesRepository( - preferences: gh<_i460.SharedPreferences>(), - ), - registerFor: {_flutter}, - ); return this; } } class _$SharedPreferencesModule extends _i1006.SharedPreferencesModule {} + +class _$FlutterSecureStorageModule extends _i782.FlutterSecureStorageModule {} diff --git a/lib/third_party/flutter_secure_storage.dart b/lib/third_party/flutter_secure_storage.dart new file mode 100644 index 0000000..1f90129 --- /dev/null +++ b/lib/third_party/flutter_secure_storage.dart @@ -0,0 +1,8 @@ +import 'package:flutter_secure_storage/flutter_secure_storage.dart'; +import 'package:injectable/injectable.dart'; + +@module +abstract class FlutterSecureStorageModule { + @lazySingleton + FlutterSecureStorage get storage => const FlutterSecureStorage(); +} diff --git a/test/features/preferences/data/repositories/local_preferences_repository_test.dart b/test/features/preferences/data/repositories/local_preferences_repository_test.dart new file mode 100644 index 0000000..d44e6c5 --- /dev/null +++ b/test/features/preferences/data/repositories/local_preferences_repository_test.dart @@ -0,0 +1,89 @@ +import 'dart:convert'; + +import 'package:flutter_secure_storage/flutter_secure_storage.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:moonwell_launcher/features/launcher/domain/entities/launcher_session.dart'; +import 'package:moonwell_launcher/features/preferences/data/repositories/local_preferences_repository.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +void main() { + group('LocalPreferencesRepository', () { + setUp(() { + SharedPreferences.setMockInitialValues({}); + FlutterSecureStorage.setMockInitialValues({}); + }); + + Future createRepository() async { + final preferences = await SharedPreferences.getInstance(); + return LocalPreferencesRepository( + preferences: preferences, + secureStorage: const FlutterSecureStorage(), + ); + } + + test('persists output directory through shared preferences', () async { + final repository = await createRepository(); + final uri = Uri.file(r'C:\Games\MoonWell'); + + await repository.setOutputDir(uri); + + expect(await repository.getOutputDir(), uri); + }); + + test('persists launcher session through secure storage', () async { + final repository = await createRepository(); + final session = LauncherSession( + accessToken: 'token', + tokenType: 'Bearer', + expiresAt: DateTime.parse('2030-01-01T00:00:00Z'), + ); + + await repository.setLauncherSession(session); + final restoredSession = await repository.getLauncherSession(); + + expect(restoredSession?.accessToken, 'token'); + expect(restoredSession?.tokenType, 'Bearer'); + expect( + restoredSession?.expiresAt?.toUtc(), + DateTime.parse('2030-01-01T00:00:00Z'), + ); + }); + + test('clears persisted launcher session', () async { + final repository = await createRepository(); + + await repository.setLauncherSession( + const LauncherSession( + accessToken: 'token', + tokenType: 'Bearer', + expiresAt: null, + ), + ); + await repository.clearLauncherSession(); + + expect(await repository.getLauncherSession(), isNull); + }); + + test( + 'removes legacy shared preferences session without restoring it', + () async { + final legacySession = LauncherSession( + accessToken: 'legacy-token', + tokenType: 'Bearer', + expiresAt: DateTime.parse('2030-01-01T00:00:00Z'), + ); + SharedPreferences.setMockInitialValues({ + 'launcher_session': jsonEncode(legacySession.toJson()), + }); + final preferences = await SharedPreferences.getInstance(); + final repository = LocalPreferencesRepository( + preferences: preferences, + secureStorage: const FlutterSecureStorage(), + ); + + expect(await repository.getLauncherSession(), isNull); + expect(preferences.containsKey('launcher_session'), isFalse); + }, + ); + }); +} diff --git a/test/features/preferences/data/repositories/shared_prefs_preferences_repository_test.dart b/test/features/preferences/data/repositories/shared_prefs_preferences_repository_test.dart deleted file mode 100644 index de549d7..0000000 --- a/test/features/preferences/data/repositories/shared_prefs_preferences_repository_test.dart +++ /dev/null @@ -1,52 +0,0 @@ -import 'package:flutter_test/flutter_test.dart'; -import 'package:moonwell_launcher/features/launcher/domain/entities/launcher_session.dart'; -import 'package:moonwell_launcher/features/preferences/data/repositories/shared_prefs_preferences_repository.dart'; -import 'package:shared_preferences/shared_preferences.dart'; - -void main() { - group('SharedPrefsPreferencesRepository', () { - setUp(() { - SharedPreferences.setMockInitialValues({}); - }); - - test('persists launcher session between reads', () async { - final preferences = await SharedPreferences.getInstance(); - final repository = SharedPrefsPreferencesRepository( - preferences: preferences, - ); - final session = LauncherSession( - accessToken: 'token', - tokenType: 'Bearer', - expiresAt: DateTime.parse('2030-01-01T00:00:00Z'), - ); - - await repository.setLauncherSession(session); - final restoredSession = await repository.getLauncherSession(); - - expect(restoredSession?.accessToken, 'token'); - expect(restoredSession?.tokenType, 'Bearer'); - expect( - restoredSession?.expiresAt?.toUtc(), - DateTime.parse('2030-01-01T00:00:00Z'), - ); - }); - - test('clears persisted launcher session', () async { - final preferences = await SharedPreferences.getInstance(); - final repository = SharedPrefsPreferencesRepository( - preferences: preferences, - ); - - await repository.setLauncherSession( - LauncherSession( - accessToken: 'token', - tokenType: 'Bearer', - expiresAt: null, - ), - ); - await repository.clearLauncherSession(); - - expect(await repository.getLauncherSession(), isNull); - }); - }); -}