From b026dfd07483d909ee27e18a4d4a38cc643422e8 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 17 Jul 2026 09:45:12 +0200 Subject: [PATCH] Recover the sync token from backup when the stored one is rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SyncSettings.load() prefers the keystore and only consults the external backup when the keystore is EMPTY. So a stale keystore entry — one left by an earlier authorization — shadows a perfectly good backup indefinitely: every sync 401s and the user is told to re-authorize for nothing. This is not hypothetical; it is the most likely explanation for the phone's 401, whose token turned out to be healthy (GET /user 200, scope repo) once read from the backup. - SyncSettings.recoverFromBackup(rejectedToken) re-seeds the keystore from the backup and returns the recovered token, or null when there is nothing to recover (no backup, empty, or the same token that was just rejected). - settings_screen retries verification once via the backup before demanding re-auth, and says so ("recovered the saved token from backup") instead of nagging. - workout_sync_service retries the read once from the backup, so history self-heals without the user having to visit Settings. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fv8ULVt3Q67GwCFbM7doHB --- .../lib/screens/settings_screen.dart | 56 +++++++++++---- .../lib/services/sync_settings.dart | 24 +++++++ .../lib/services/workout_sync_service.dart | 42 +++++++++--- .../test/screens/settings_screen_test.dart | 42 ++++++++++++ .../test/services/sync_settings_test.dart | 35 ++++++++++ .../services/workout_sync_service_test.dart | 68 +++++++++++++++++++ 6 files changed, 244 insertions(+), 23 deletions(-) diff --git a/stronglift_replacement/workout_app/lib/screens/settings_screen.dart b/stronglift_replacement/workout_app/lib/screens/settings_screen.dart index 0946f5f..efe9b13 100644 --- a/stronglift_replacement/workout_app/lib/screens/settings_screen.dart +++ b/stronglift_replacement/workout_app/lib/screens/settings_screen.dart @@ -144,7 +144,8 @@ class _SettingsScreenState extends State { } /// Confirms [token] can actually read `$syncRepoOwner/$syncRepoName`. - Future _verifyConnection(String token) async { + /// Returns null when [token] works, else the error GitHub gave. + Future _tryVerify(String token) async { final client = GitHubClient( owner: syncRepoOwner, repo: syncRepoName, @@ -153,26 +154,51 @@ class _SettingsScreenState extends State { ); try { await client.getFileText('devices/phone/log.json'); + return null; + } on GitHubSyncError catch (e) { + return e; + } finally { + client.close(); + } + } + + Future _verifyConnection(String token) async { + final error = await _tryVerify(token); + if (error == null) { if (!mounted) return; _setSyncStatus( 'Connected and verified via GitHub.', _SyncStatusKind.success, ); - } on GitHubSyncError catch (e) { - if (!mounted) return; - // Say plainly that sync is broken. "Connected, but…" reads as success - // and is how a dead token hid behind a green badge. - final rejected = e.toString().contains('401'); - _setSyncStatus( - rejected - ? 'NOT connected: GitHub rejected this token (401). Tap Connect ' - 'GitHub to re-authorize — until then nothing syncs.' - : 'NOT syncing: could not reach GitHub ($e)', - _SyncStatusKind.error, - ); - } finally { - client.close(); + return; } + + // The keystore's token may be a stale one shadowing a good backup, and + // load() only consults the backup when the keystore is EMPTY. Try the + // backup once before making the user re-authorize for nothing. + final recovered = await SyncSettings.recoverFromBackup(token); + if (recovered != null && await _tryVerify(recovered) == null) { + if (!mounted) return; + _tokenController.text = recovered; + _setSyncStatus( + 'Connected and verified via GitHub (recovered the saved token from ' + 'backup — the stored one had been rejected).', + _SyncStatusKind.success, + ); + return; + } + + if (!mounted) return; + // Say plainly that sync is broken. "Connected, but…" reads as success + // and is how a dead token hid behind a green badge. + _setSyncStatus( + error.toString().contains('401') + ? 'NOT connected: GitHub rejected this token (401) and no working ' + 'backup was found. Tap Connect GitHub to re-authorize — ' + 'until then nothing syncs.' + : 'NOT syncing: could not reach GitHub ($error)', + _SyncStatusKind.error, + ); } void _onWeightChanged(String name, double value) { diff --git a/stronglift_replacement/workout_app/lib/services/sync_settings.dart b/stronglift_replacement/workout_app/lib/services/sync_settings.dart index 4a0bbac..8f14039 100644 --- a/stronglift_replacement/workout_app/lib/services/sync_settings.dart +++ b/stronglift_replacement/workout_app/lib/services/sync_settings.dart @@ -70,6 +70,30 @@ class SyncSettings { return SyncSettings(token: token ?? ''); } + /// Re-seeds the keystore from the external backup after its token was + /// REJECTED by GitHub, returning the recovered token (or null). + /// + /// [load] prefers the keystore and only consults the backup when the + /// keystore is EMPTY. So a stale keystore entry — say, one left by an + /// earlier authorization — shadows a perfectly good backup indefinitely, + /// and the user is told to re-authorize for nothing. Callers that see a + /// token rejected call this and retry ONCE before demanding re-auth. + /// + /// Returns null when there is nothing to recover: no backup, an empty one, + /// or one holding the very token that was just rejected. + static Future recoverFromBackup(String rejectedToken) async { + final backedUp = await BackupService.instance.readSyncToken(); + if (backedUp == null || backedUp.isEmpty || backedUp == rejectedToken) { + return null; + } + try { + await _secure.write(key: _secureToken, value: backedUp); + } on PlatformException { + // Keystore unavailable — the caller can still use the returned token. + } + return backedUp; + } + /// Persists [token] to the keystore (deleting the entry when empty) and /// mirrors it to external storage so it survives an app uninstall. /// Returns false if the platform secret service is unavailable. diff --git a/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart b/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart index 818a3a8..c511cdb 100644 --- a/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart +++ b/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart @@ -148,10 +148,43 @@ class WorkoutSyncService { return const []; } + try { + return await _fetchPayloads(settings.token, kind); + } on GitHubSyncError catch (error) { + var failure = error; + // A stale keystore token shadows a good backup (SyncSettings.load only + // falls back when the keystore is EMPTY), so retry once from the backup + // rather than leaving history silently incomplete. + final recovered = await SyncSettings.recoverFromBackup(settings.token); + if (recovered != null) { + try { + final payloads = await _fetchPayloads(recovered, kind); + debugPrint( + 'WorkoutSyncService: recovered the sync token from backup after ' + 'the stored one was rejected ($failure).', + ); + return payloads; + } on GitHubSyncError catch (retryError) { + failure = retryError; + } + } + final which = kind ?? 'any'; + debugPrint( + 'WorkoutSyncService: FAILED reading synced workouts ' + '(kind=$which) from $owner/$repo: $failure — history may be incomplete.', + ); + return const []; + } + } + + Future>> _fetchPayloads( + String token, + String? kind, + ) async { final client = GitHubClient( owner: owner, repo: repo, - token: settings.token, + token: token, httpClient: _httpClient, ); try { @@ -164,13 +197,6 @@ class WorkoutSyncService { return merged.values .map((r) => (r.fields['payload']!.$1! as Map).cast()) .toList(); - } on GitHubSyncError catch (error) { - final which = kind ?? 'any'; - debugPrint( - 'WorkoutSyncService: FAILED reading synced workouts ' - '(kind=$which) from $owner/$repo: $error — history may be incomplete.', - ); - return const []; } finally { client.close(); } diff --git a/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart b/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart index 7a42867..dda29a2 100644 --- a/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart +++ b/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart @@ -1,4 +1,5 @@ import 'dart:convert'; +import 'dart:io'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -10,6 +11,8 @@ import 'package:sqflite_common_ffi/sqflite_ffi.dart'; import 'package:url_launcher_platform_interface/link.dart'; import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart'; import 'package:workout_app/models/exercise.dart'; +import 'package:workout_app/services/backup_service.dart'; +import 'package:workout_app/services/sync_settings.dart'; import 'package:workout_app/models/workout_plan.dart'; import 'package:workout_app/screens/settings_screen.dart'; import 'package:workout_app/services/storage_service.dart'; @@ -611,4 +614,43 @@ void main() { expect(find.textContaining('Connected and verified'), findsOneWidget); }); }); + + testWidgets( + 'a rejected stored token recovers from the backup instead of nagging', + (tester) async { + // load() only consults the backup when the keystore is EMPTY, so a + // stale keystore entry would otherwise shadow a good backup forever + // and demand a pointless re-authorization. + final tempDir = Directory.systemTemp.createTempSync('settings_recover_'); + BackupService.baseDirForTesting = tempDir.path; + addTearDown(() { + BackupService.baseDirForTesting = kBackupDir; + tempDir.deleteSync(recursive: true); + }); + + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + // save() writes the external-storage backup: real file I/O, which hangs + // in the widget-test fake-async zone (same reason _seed uses runAsync). + await tester.runAsync( + () async => const SyncSettings(token: 'good').save(), + ); + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + + final mock = MockClient((req) async { + if (req.headers['Authorization']?.contains('good') != true) { + return http.Response('Bad credentials', 401); + } + return http.Response( + jsonEncode({'content': base64Encode(utf8.encode('{}')), 'sha': 's'}), + 200, + ); + }); + + await _pump(tester, _wrap(httpClient: mock)); + await _scrollToGitHubSync(tester); + + expect(find.textContaining('recovered the saved token'), findsOneWidget); + expect(find.textContaining('NOT connected'), findsNothing); + }, + ); } diff --git a/stronglift_replacement/workout_app/test/services/sync_settings_test.dart b/stronglift_replacement/workout_app/test/services/sync_settings_test.dart index ac826cd..d9629d1 100644 --- a/stronglift_replacement/workout_app/test/services/sync_settings_test.dart +++ b/stronglift_replacement/workout_app/test/services/sync_settings_test.dart @@ -116,4 +116,39 @@ void main() { expect(s.token, 'gho_recovered2'); }, ); + + group('recoverFromBackup', () { + test('re-seeds the keystore when the backup holds a different token', () async { + // A stale keystore entry shadows a good backup forever: load() only + // consults the backup when the keystore is EMPTY. + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + await const SyncSettings(token: 'good-from-backup').save(); + // Put the stale token back in the keystore (save() overwrote it). + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + + final recovered = await SyncSettings.recoverFromBackup('stale'); + + expect(recovered, 'good-from-backup'); + // The keystore was re-seeded, so the next load() returns the good one. + expect((await SyncSettings.load()).token, 'good-from-backup'); + }); + + test('returns null when the backup holds the same rejected token', () async { + installFakeSecureStorage(initial: {'sync.token': 'same'}); + await const SyncSettings(token: 'same').save(); + expect(await SyncSettings.recoverFromBackup('same'), isNull); + }); + + test('returns null when there is no backup to recover from', () async { + installFakeSecureStorage(initial: {'sync.token': 'rejected'}); + expect(await SyncSettings.recoverFromBackup('rejected'), isNull); + }); + + test('still returns the token when the keystore write fails', () async { + installFakeSecureStorage(); + await const SyncSettings(token: 'good-from-backup').save(); + installFakeSecureStorage(throwing: true); + expect(await SyncSettings.recoverFromBackup('stale'), 'good-from-backup'); + }); + }); } diff --git a/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart b/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart index e8188e5..8c26d38 100644 --- a/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart +++ b/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart @@ -1,4 +1,5 @@ import 'dart:convert'; +import 'dart:io'; import 'package:crdt_sync/crdt_sync.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -8,6 +9,7 @@ import 'package:workout_app/models/exercise.dart'; import 'package:workout_app/models/exercise_result.dart'; import 'package:workout_app/models/set_result.dart'; import 'package:workout_app/models/workout_session.dart'; +import 'package:workout_app/services/backup_service.dart'; import 'package:workout_app/services/sync_settings.dart'; import 'package:workout_app/services/workout_sync_service.dart'; @@ -350,4 +352,70 @@ void main() { isEmpty, ); }); + + test('recovers from the backup when the stored token is rejected', () async { + // A stale keystore token shadows a good backup (load() only falls back + // when the keystore is EMPTY), which would leave history silently empty. + final tempDir = Directory.systemTemp.createTempSync('sync_recover_'); + BackupService.baseDirForTesting = tempDir.path; + addTearDown(() { + BackupService.baseDirForTesting = kBackupDir; + tempDir.deleteSync(recursive: true); + }); + + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + await const SyncSettings(token: 'good').save(); + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + + final pcLog = jsonEncode({ + 'manual:a': _manual('manual:a', 'NEW', 2000).toJson(), + }); + final httpClient = http_testing.MockClient((req) async { + if (req.headers['Authorization']?.contains('good') != true) { + return http.Response('Bad credentials', 401); + } + if (req.url.path.endsWith('screen-locker-sync/devices')) { + return http.Response(jsonEncode([ + {'name': 'pc', 'type': 'dir'}, + ]), 200); + } + return http.Response( + jsonEncode({ + 'content': base64Encode(utf8.encode(pcLog)), + 'sha': 'sha', + }), + 200, + ); + }); + + final payloads = await WorkoutSyncService( + httpClient: httpClient, + ).readMergedManualPayloads(); + + expect(payloads, hasLength(1)); + expect((await SyncSettings.load()).token, 'good'); + }); + + test('reports the failure when the backup token is rejected too', () async { + final tempDir = Directory.systemTemp.createTempSync('sync_recover_fail_'); + BackupService.baseDirForTesting = tempDir.path; + addTearDown(() { + BackupService.baseDirForTesting = kBackupDir; + tempDir.deleteSync(recursive: true); + }); + + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + await const SyncSettings(token: 'also-bad').save(); + installFakeSecureStorage(initial: {'sync.token': 'stale'}); + + // Every token 401s, so the recovery retry fails too. + final httpClient = http_testing.MockClient( + (req) async => http.Response('Bad credentials', 401), + ); + + expect( + await WorkoutSyncService(httpClient: httpClient).readMergedManualPayloads(), + isEmpty, + ); + }); }