diff --git a/lib/repository/database.dart b/lib/repository/database.dart index 6c6e6fef..3a97a7f0 100644 --- a/lib/repository/database.dart +++ b/lib/repository/database.dart @@ -54,6 +54,8 @@ class DatabaseRepository { _servers = piHoleClientData.servers; _appConfig = piHoleClientData.appConfig; _dbInstance = piHoleClientData.dbInstance; + await cleanUpSecureStorage(); + logger.d('Secure storage: ${await _secureStorage.readAll()}'); } Map toDict() { @@ -149,9 +151,6 @@ class DatabaseRepository { final passCode = await _secureStorage.getValue('passCode'); appConfig = AppDbData.withSecrets(appConfig!, passCode); - // _secureStorage.readAll() - logger.d((await _secureStorage.readAll()).toString()); - if (servers != null && servers!.isNotEmpty) { for (var i = 0; i < servers!.length; i++) { final server = servers![i]; @@ -512,6 +511,38 @@ class DatabaseRepository { } } + /// Delete and clean up unused secure storage data for servers not present in the database. + /// This method is intended to be called during application startup. + Future cleanUpSecureStorage() async { + final serverAddresses = servers.map((e) => e.address).toList(); + final keys = await _secureStorage.readAll(); + + // Collect keys to be deleted + final keysToDelete = []; + + for (final key in keys.keys) { + if (key.endsWith('_token') || + key.endsWith('_basicAuthUser') || + key.endsWith('_basicAuthPassword') || + key.endsWith('_password') || + key.endsWith('_sid')) { + final address = key.split('_').first; + if (!serverAddresses.contains(address)) { + keysToDelete.add(key); + } + } + } + + // Delete collected keys separately + for (final key in keysToDelete) { + await _secureStorage.deleteValue(key); + } + } + + // ========================================================================== + // MIGRATION METHODS + // ========================================================================== + // Migration methods Future _upgradeToV2(Database db) async { // await db.execute('ALTER TABLE appConfig DROP COLUMN oneColumnLegend'); diff --git a/lib/screens/servers/add_server_fullscreen.dart b/lib/screens/servers/add_server_fullscreen.dart index 0533108e..b5fcc496 100644 --- a/lib/screens/servers/add_server_fullscreen.dart +++ b/lib/screens/servers/add_server_fullscreen.dart @@ -278,6 +278,10 @@ class _AddServerFullscreenState extends State { isConnecting = false; _restoreSecrets(); }); + + await serverObj.sm.deletePassword(); + await serverObj.sm.deleteToken(); + if (result?.result == APiResponseType.socket) { showErrorSnackBar( context: context, diff --git a/lib/services/secret_manager.dart b/lib/services/secret_manager.dart index b4125c95..a2525a86 100644 --- a/lib/services/secret_manager.dart +++ b/lib/services/secret_manager.dart @@ -71,4 +71,22 @@ class SecretManager { return false; } } + + Future deletePassword() async { + try { + await _storage.deleteValue('${_address}_password'); + return true; + } catch (e) { + return false; + } + } + + Future deleteToken() async { + try { + await _storage.deleteValue('${_address}_token'); + return true; + } catch (e) { + return false; + } + } } diff --git a/test/full_coverage_test.dart b/test/full_coverage_test.dart index 1aabe45a..2e6142ea 100644 --- a/test/full_coverage_test.dart +++ b/test/full_coverage_test.dart @@ -31,6 +31,12 @@ import 'package:pi_hole_client/gateways/api_gateway_factory.dart'; import 'package:pi_hole_client/gateways/api_gateway_interface.dart'; import 'package:pi_hole_client/gateways/v5/api_gateway_v5.dart'; import 'package:pi_hole_client/gateways/v6/api_gateway_v6.dart'; +import 'package:pi_hole_client/l10n/generated/app_localizations.dart'; +import 'package:pi_hole_client/l10n/generated/app_localizations_de.dart'; +import 'package:pi_hole_client/l10n/generated/app_localizations_en.dart'; +import 'package:pi_hole_client/l10n/generated/app_localizations_es.dart'; +import 'package:pi_hole_client/l10n/generated/app_localizations_ja.dart'; +import 'package:pi_hole_client/l10n/generated/app_localizations_pl.dart'; import 'package:pi_hole_client/main.dart'; import 'package:pi_hole_client/models/api/v6/auth/auth.dart'; import 'package:pi_hole_client/models/api/v6/dns/dns.dart'; diff --git a/test/ut/gateways/v6/api_gateway_v6_test.dart b/test/ut/gateways/v6/api_gateway_v6_test.dart index b8091976..8c9c9d0b 100644 --- a/test/ut/gateways/v6/api_gateway_v6_test.dart +++ b/test/ut/gateways/v6/api_gateway_v6_test.dart @@ -67,6 +67,18 @@ class SecretManagerMock implements SecretManager { _sid = token; return true; } + + @override + Future deletePassword() async { + _password = null; + return true; + } + + @override + Future deleteToken() async { + _sid = null; + return true; + } } @GenerateMocks([http.Client]) diff --git a/test/ut/repository/database_test.dart b/test/ut/repository/database_test.dart index 8a57a6b6..f3cbf999 100644 --- a/test/ut/repository/database_test.dart +++ b/test/ut/repository/database_test.dart @@ -58,7 +58,6 @@ void main() async { expect(appConfig.passCode, isNull); expect(appConfig.useBiometricAuth, 0); expect(appConfig.sendCrashReports, 0); - // TODO: check logger messages }); test('should open database', () async { @@ -83,7 +82,6 @@ void main() async { expect(appConfig.passCode, isNull); expect(appConfig.useBiometricAuth, 0); expect(appConfig.sendCrashReports, 0); - // TODO: check logger messages }); test('should return one registered server without passcode', () async { @@ -822,4 +820,75 @@ void main() async { }, ); }); + + group('DatabaseRepository.cleanUpSecureStorage', () { + late DbHelper dbHelper; + + setUp(() async { + await deleteDatabase(testDb); + FlutterSecureStorage.setMockInitialValues({ + 'http://localhost_password': 'test123', + 'http://localhost_token': '', + 'http://localhost_sid': 'XXXXXxxx1234Q=', + }); + }); + + tearDown(() async { + await deleteDatabase(testDb); + }); + + test('should not cleanup secure storage', () async { + final secureStorageRepository = SecureStorageRepository(); + final server = Server( + address: 'http://localhost', + alias: 'test v6', + defaultServer: false, + apiVersion: 'v6', + sm: SecretManager( + secureStorageRepository, + 'http://localhost', + ), + ); + + dbHelper = DbHelper(testDb); + await dbHelper.loadDb(); + await dbHelper.saveDb(server); + await dbHelper.closeDb(); + + final databaseRepository = DatabaseRepository(secureStorageRepository); + await databaseRepository.initialize(path: testDb); + + final actual = await secureStorageRepository.readAll(); + expect(actual['http://localhost_password'], 'test123'); + expect(actual['http://localhost_token'], ''); + expect(actual['http://localhost_sid'], 'XXXXXxxx1234Q='); + }); + + test('should cleanup secure storage', () async { + final secureStorageRepository = SecureStorageRepository(); + final server = Server( + address: 'http://localhost:8080', + alias: 'test v6', + defaultServer: false, + apiVersion: 'v6', + sm: SecretManager( + secureStorageRepository, + 'http://localhost:8080', + ), + ); + + dbHelper = DbHelper(testDb); + await dbHelper.loadDb(); + await dbHelper.saveDb(server); + await dbHelper.closeDb(); + + final databaseRepository = DatabaseRepository(secureStorageRepository); + await databaseRepository.initialize(path: testDb); + + final actual = await secureStorageRepository.readAll(); + expect(actual['http://localhost_password'], null); + expect(actual['http://localhost_token'], null); + expect(actual['http://localhost_sid'], null); + }); + }); } diff --git a/test/widgets/screens/statistics/statistics_test.dart b/test/widgets/screens/statistics/statistics_test.dart index d68a4d68..1ff6b328 100644 --- a/test/widgets/screens/statistics/statistics_test.dart +++ b/test/widgets/screens/statistics/statistics_test.dart @@ -9,7 +9,6 @@ import 'package:pi_hole_client/screens/statistics/statistics_triple_column.dart' import 'package:pie_chart/pie_chart.dart'; import '../../helpers.dart'; -import '../utils.dart'; void main() async { await initializeApp(); @@ -113,7 +112,6 @@ void main() async { // Switch to Domains tab await tester.tap(find.text('Domains')); await tester.pump(); - showText(); expect(find.text('Loading stats...'), findsOneWidget); expect(find.byType(StatisticsListContent), findsNothing); }, @@ -147,7 +145,6 @@ void main() async { // Switch to Domains tab await tester.tap(find.text('Domains')); await tester.pump(); - showText(); expect(find.text('Stats could not be loaded'), findsOneWidget); expect(find.byType(StatisticsListContent), findsNothing); },