From 1b9c35f322941c49c8e933d2c776cfd2eb3adc43 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:07:49 +0700 Subject: [PATCH] fix(downloads): preserve browser verification and queues across restarts --- .../spotiflac/ExtensionAuthCallbacksTest.kt | 71 ++++++ .../zarz/spotiflac/ExtensionAuthCallbacks.kt | 35 +++ .../kotlin/com/zarz/spotiflac/MainActivity.kt | 68 +++--- lib/main.dart | 15 ++ lib/providers/download_queue_provider.dart | 13 +- ...download_queue_provider_native_worker.dart | 8 + .../download_queue_provider_verification.dart | 13 +- lib/providers/extension_provider.dart | 16 +- lib/services/download_queue_codec.dart | 27 ++- lib/services/download_queue_persistence.dart | 138 ++++++------ lib/services/platform_bridge.dart | 13 ++ rust_backend/crates/extensions/src/auth.rs | 203 +++++++++++++++++- .../crates/extensions/src/auth/persistence.rs | 74 +++++++ .../crates/extensions/src/environment.rs | 2 +- test/download_queue_persistence_test.dart | 71 ++++++ test/download_queue_recovery_test.dart | 54 +++++ test/extension_initialization_test.dart | 24 +++ 17 files changed, 710 insertions(+), 135 deletions(-) create mode 100644 android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacksTest.kt create mode 100644 android/app/src/main/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacks.kt create mode 100644 rust_backend/crates/extensions/src/auth/persistence.rs diff --git a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacksTest.kt b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacksTest.kt new file mode 100644 index 00000000..f0c8542f --- /dev/null +++ b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacksTest.kt @@ -0,0 +1,71 @@ +package com.zarz.spotiflac + +import android.net.Uri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.async +import kotlinx.coroutines.runBlocking +import org.junit.Assert.* +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ExtensionAuthCallbacksTest { + private val uri = Uri.parse("spotiflac://session-grant?state=nonce&grant=grant") + + @Test + fun coldCallbackWaitsForReadyBackendAndDrainsOnlyOnce() = runBlocking { + val callbacks = ExtensionAuthCallbacks() + assertTrue(callbacks.offer(uri)) + assertTrue(callbacks.offer(uri)) + val started = CompletableDeferred() + val ready = CompletableDeferred() + val handled = mutableListOf() + val first = async { + callbacks.drain { + started.complete(Unit) + ready.await() + handled.add(it) + } + } + started.await() + val second = async { callbacks.drain { handled.add(it) } } + assertTrue(handled.isEmpty()) + ready.complete(Unit) + first.await() + second.await() + assertEquals(listOf(ExtensionAuthCallbacks.Callback("nonce", "grant", true)), handled) + } + + @Test + fun interruptedHandlingLeavesTheResultAvailable() = runBlocking { + val callbacks = ExtensionAuthCallbacks() + callbacks.offer(uri) + try { + callbacks.drain { throw IllegalStateException("backend not ready") } + fail("Expected an interrupted callback") + } catch (_: IllegalStateException) { + } + var handled = 0 + callbacks.drain { handled++ } + callbacks.drain { handled++ } + assertEquals(1, handled) + } + + @Test + fun onlySupportedCallbacksWithStateAndCodeAreAccepted() = runBlocking { + val callbacks = ExtensionAuthCallbacks() + for (invalid in listOf( + "https://session-grant?state=nonce&grant=grant", + "spotiflac://unknown?state=nonce&code=code", + "spotiflac://session-grant?grant=grant", + "spotiflac://callback?state=nonce", + )) assertFalse(callbacks.offer(Uri.parse(invalid))) + assertTrue(callbacks.offer(Uri.parse("spotiflac://callback?state=oauth&code=code"))) + callbacks.drain { + assertEquals("oauth", it.state) + assertEquals("code", it.code) + assertFalse(it.sessionGrant) + } + } +} diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacks.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacks.kt new file mode 100644 index 00000000..8f3e8cbc --- /dev/null +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/ExtensionAuthCallbacks.kt @@ -0,0 +1,35 @@ +package com.zarz.spotiflac + +import android.net.Uri +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock + +/** Process-owned inbox: Activity recreation must not discard browser results. */ +internal class ExtensionAuthCallbacks { + data class Callback(val state: String, val code: String, val sessionGrant: Boolean) + + private val pending = linkedSetOf() + private val processing = Mutex() + + fun offer(uri: Uri): Boolean { + if (!uri.scheme.equals("spotiflac", ignoreCase = true)) return false + val host = uri.host.orEmpty().lowercase(java.util.Locale.US) + val grant = host == "session-grant" + if (!grant && host != "callback" && host != "spotify-callback") return false + val state = uri.getQueryParameter("state")?.trim().orEmpty() + val code = (if (grant) uri.getQueryParameter("grant") ?: uri.getQueryParameter("code") + else uri.getQueryParameter("code"))?.trim().orEmpty() + if (state.isEmpty() || code.isEmpty() || state.length > 4096 || code.length > 16384) return false + synchronized(pending) { pending.add(Callback(state, code, grant)) } + return true + } + + suspend fun drain(complete: suspend (Callback) -> Unit) = processing.withLock { + while (true) { + val callback = synchronized(pending) { pending.firstOrNull() } ?: break + // If initialization fails, leave the callback available for retry. + complete(callback) + synchronized(pending) { pending.remove(callback) } + } + } +} diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt index 34d85d2a..2b4991f4 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -105,6 +105,7 @@ class MainActivity: FlutterFragmentActivity() { "pickSafTree", "readSetupFolderSelection", "acknowledgeSetupFolderSelection", + "processPendingExtensionCallbacks", "safExists", "safExistsBatch", "isSafTreeAccessible", @@ -294,6 +295,7 @@ class MainActivity: FlutterFragmentActivity() { ) companion object { + private val extensionAuthCallbacks = ExtensionAuthCallbacks() // audio_service keeps the engine across Activity recreation. Retain // its outstanding picker reply for the same lifetime, not the Activity. private var pendingSafTreeResult: MethodChannel.Result? = null @@ -839,56 +841,34 @@ class MainActivity: FlutterFragmentActivity() { */ private fun handleExtensionOAuthIntent(intent: Intent?) { val uri = intent?.data ?: return - if (!uri.scheme.equals("spotiflac", ignoreCase = true)) { - return - } - val host = (uri.host ?: "").lowercase(Locale.US) - val path = (uri.path ?: "").lowercase(Locale.US) - val isSessionGrant = host == "session-grant" - val isCallback = - isSessionGrant || - host == "callback" || - host == "spotify-callback" || - path.contains("callback") - if (!isCallback) { - return - } - val code = ( - if (isSessionGrant) { - uri.getQueryParameter("grant") ?: uri.getQueryParameter("code") - } else { - uri.getQueryParameter("code") - } - )?.trim().orEmpty() - if (code.isEmpty()) { - return - } - val callbackState = uri.getQueryParameter("state")?.trim().orEmpty() - if (callbackState.isEmpty()) { - android.util.Log.w("SpotiFLAC", "Extension callback missing state") - return - } - intent.data = null - var callbackExtensionId = "" - scope.launch(Dispatchers.IO) { + if (!extensionAuthCallbacks.offer(uri)) return + // Keep the Intent intact until the result is handled. A cold start has + // no extension owner yet; Dart drains this inbox after initialization. + backendChannel?.invokeMethod("extensionAuthCallbackReceived", null) + } + + private suspend fun processPendingExtensionCallbacks() { + extensionAuthCallbacks.drain { callback -> + var callbackExtensionId = "" try { - coreBackend.completeAuthCallback(callbackState, code, isSessionGrant) { - callbackExtensionId = it + withContext(Dispatchers.IO) { + coreBackend.completeAuthCallback(callback.state, callback.code, callback.sessionGrant) { + callbackExtensionId = it + } } android.util.Log.i("SpotiFLAC", "Extension callback completed") - if (isSessionGrant) { - withContext(Dispatchers.Main) { - notifySessionGrantCompleted(callbackExtensionId, true) - } + if (callback.sessionGrant) { + notifySessionGrantCompleted(callbackExtensionId, true) } } catch (e: Exception) { + if (e is kotlinx.coroutines.CancellationException) throw e android.util.Log.w("SpotiFLAC", "Extension callback failed (${e.javaClass.simpleName})") - if (isSessionGrant && callbackExtensionId.isNotEmpty()) { - withContext(Dispatchers.Main) { - notifySessionGrantCompleted(callbackExtensionId, false) - } + if (callback.sessionGrant && callbackExtensionId.isNotEmpty()) { + notifySessionGrantCompleted(callbackExtensionId, false) } } + val current = intent?.data + if (current?.getQueryParameter("state") == callback.state) intent.data = null } } @@ -1126,6 +1106,10 @@ class MainActivity: FlutterFragmentActivity() { return@launch } when (call.method) { + "processPendingExtensionCallbacks" -> { + processPendingExtensionCallbacks() + result.success(null) + } "setScreenAwake" -> { val flag = android.view.WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON if (call.argument("enabled") == true) { diff --git a/lib/main.dart b/lib/main.dart index 44d09bd3..768c8d8f 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -381,6 +381,7 @@ class _EagerInitializationState extends ConsumerState bool _localLibraryWarmupScheduled = false; bool _autoScanTriggeredOnLaunch = false; StreamSubscription? _verificationNotificationSubscription; + StreamSubscription? _extensionAuthCallbackSubscription; @override void initState() { @@ -392,6 +393,10 @@ class _EagerInitializationState extends ConsumerState PlatformBridge.verificationNotificationEvents().listen( (_) => unawaited(_consumeVerificationNotification()), ); + _extensionAuthCallbackSubscription = + PlatformBridge.extensionAuthCallbackEvents().listen( + (_) => unawaited(_completeExtensionCallback()), + ); _initializeAppServices(); _initializeExtensions(); _initializeDeferredProviders(); @@ -406,6 +411,7 @@ class _EagerInitializationState extends ConsumerState _downloadHistoryWarmupTimer?.cancel(); _localLibraryWarmupTimer?.cancel(); _verificationNotificationSubscription?.cancel(); + _extensionAuthCallbackSubscription?.cancel(); NotificationService().verificationNotifications.setHandler(null); super.dispose(); } @@ -610,6 +616,15 @@ class _EagerInitializationState extends ConsumerState } } + Future _completeExtensionCallback() async { + try { + await ref.read(extensionProvider.notifier).ensureInitialized(); + await PlatformBridge.processPendingExtensionCallbacks(); + } catch (error) { + _log.w('Could not complete extension verification: $error'); + } + } + @override Widget build(BuildContext context) { return widget.child; diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 6d3b652a..2061fa3f 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -486,7 +486,7 @@ class DownloadQueueNotifier extends Notifier { /// the app is backgrounded: the OS may kill the process without a graceful /// teardown, and a pending debounce timer would silently drop the most /// recent queue mutations. - Future flushQueuePersistence() async { + Future flushQueuePersistence({bool requireDurable = false}) async { // Startup publishes restored items before completing this gate. Capturing // the initial empty state while hydration is in flight would delete the // restored rows when the persistence writer reaches that empty snapshot. @@ -496,7 +496,7 @@ class DownloadQueueNotifier extends Notifier { if (!_queueStorageReady) { throw StateError('Download queue restoration failed; retry the flush'); } - await _queuePersistence.flush(); + await _queuePersistence.flush(requireDurable: requireDurable); } void _persistUserPausedQueue(bool paused) { @@ -1731,6 +1731,15 @@ class DownloadQueueNotifier extends Notifier { final networkOnlyQueue = state.items .where((item) => item.status == DownloadStatus.queued) .every((item) => item.networkDownloadFolder.isNotEmpty); + if (!networkOnlyQueue && settings.useExtensionProviders) { + // Cold browser callbacks and restored downloads share initialization. + // An empty list while loading must not be treated as removed providers. + await ref.read(extensionProvider.notifier).ensureInitialized(); + if (!ref.mounted || state.isPaused) return; + settings = ref.read(settingsProvider); + updateSettings(settings); + isSafMode = _isSafMode(settings); + } IosSecurityScopedAccess? iosDownloadBookmarkAccess; // Validate SAF before handing the batch to either queue implementation. diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index 373306c3..f62c93f3 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -1202,6 +1202,14 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { reconciledIds.add(pendingId); updateItemStatus(pendingId, DownloadStatus.queued, progress: 0.0); } + if (_queueStartupPending) { + // This snapshot predates the browser return. Let startup finish + // exchanging its callback before retrying through the regular + // queue; opening another challenge here would race that grant + // and wait on our own queue-restoration barrier. + updateItemStatus(itemId, DownloadStatus.queued, progress: 0.0); + continue; + } await _handleVerificationRequiredDownload( current, errorMsg, diff --git a/lib/providers/download_queue_provider_verification.dart b/lib/providers/download_queue_provider_verification.dart index 9f7e01f3..1784761f 100644 --- a/lib/providers/download_queue_provider_verification.dart +++ b/lib/providers/download_queue_provider_verification.dart @@ -21,8 +21,8 @@ extension _DownloadQueueVerificationGate on DownloadQueueNotifier { _verificationWaitCoordinator.hasActiveFlow(target.extensionId)) { return; } - // Failed items may not survive queue restoration. The notification still - // identifies the owning extension, but cannot restart a missing item. + // Older builds did not retain verification failures. The notification + // still identifies the extension if its queue item is already missing. final verified = await _openVerificationAndWait( item?.id ?? 'notification:${target.tapId}', target.extensionId, @@ -93,6 +93,15 @@ extension _DownloadQueueVerificationGate on DownloadQueueNotifier { } } + // Finish the durable write before handing control to another app; a + // background lifecycle callback may not finish before Android kills us. + try { + await flushQueuePersistence(requireDurable: true); + } catch (error) { + _log.e('Could not save downloads before verification: $error'); + return false; + } + if (!ref.mounted) return false; return openVerificationAndAwaitGrant( extensionId, browserMode: ref.read(settingsProvider).extensionVerificationBrowserMode, diff --git a/lib/providers/extension_provider.dart b/lib/providers/extension_provider.dart index c5176107..89f37945 100644 --- a/lib/providers/extension_provider.dart +++ b/lib/providers/extension_provider.dart @@ -93,14 +93,11 @@ class ExtensionNotifier extends Notifier { } Future _cleanupExtensions({required String reason}) async { - if (!PlatformBridge.supportsExtensionSystem) { - _cleanupInFlight = false; - return; - } - try { - await PlatformBridge.cleanupExtensions(); - _log.d('Extensions cleaned up ($reason)'); + // Android may detach/recreate its Activity while the cached engine and + // browser verification remain alive. Keep the process-owned backend. + await PlatformBridge.releaseNativeMemory(); + _log.d('Released idle extension resources ($reason)'); } catch (e) { _log.w('Extension cleanup failed ($reason): $e'); } finally { @@ -208,6 +205,11 @@ class ExtensionNotifier extends Notifier { } await loadProviderPriority(); await loadMetadataProviderPriority(); + if (Platform.isAndroid) { + // Exchange a cold-start browser callback before restored downloads can + // bootstrap a replacement challenge and invalidate its original state. + await PlatformBridge.processPendingExtensionCallbacks(); + } state = state.copyWith( isInitialized: true, isLoading: false, diff --git a/lib/services/download_queue_codec.dart b/lib/services/download_queue_codec.dart index a89ac7a8..a76fe11a 100644 --- a/lib/services/download_queue_codec.dart +++ b/lib/services/download_queue_codec.dart @@ -11,17 +11,25 @@ const _workerItemThreshold = 64; const _workerPayloadThreshold = 64 * 1024; const _workerChunkSize = 1024; -DownloadStatus downloadQueuePersistenceStatus(DownloadStatus status) => - switch (status) { - DownloadStatus.downloading || - DownloadStatus.finalizing => DownloadStatus.queued, - _ => status, - }; +DownloadStatus downloadQueuePersistenceStatus( + DownloadStatus status, { + DownloadErrorType? errorType, +}) => switch (status) { + DownloadStatus.failed + when errorType == DownloadErrorType.verificationRequired => + DownloadStatus.queued, + DownloadStatus.downloading || + DownloadStatus.finalizing => DownloadStatus.queued, + _ => status, +}; /// Transfer samples are transient; only restart-relevant state reaches SQLite. String encodeDownloadQueueItemForPersistence(DownloadItem item) => jsonEncode( item.toJson() - ..['status'] = downloadQueuePersistenceStatus(item.status).name + ..['status'] = downloadQueuePersistenceStatus( + item.status, + errorType: item.errorType, + ).name ..['progress'] = 0.0 ..['speedMBps'] = 0.0 ..['bytesReceived'] = 0 @@ -49,7 +57,10 @@ RestoredDownloadQueue _decodeRows(List> rows) { final decoded = jsonDecode(payload); if (decoded is! Map) continue; var item = DownloadItem.fromJson(Map.from(decoded)); - final status = downloadQueuePersistenceStatus(item.status); + final status = downloadQueuePersistenceStatus( + item.status, + errorType: item.errorType, + ); saved[id] = ( item: item, payload: diff --git a/lib/services/download_queue_persistence.dart b/lib/services/download_queue_persistence.dart index 7216bdcd..d91d5ef2 100644 --- a/lib/services/download_queue_persistence.dart +++ b/lib/services/download_queue_persistence.dart @@ -84,11 +84,11 @@ class DownloadQueuePersistence { _debounce = Timer(const Duration(milliseconds: 350), flush); } - Future flush() async { + Future flush({bool requireDurable = false}) async { _debounce?.cancel(); _debounce = null; _pendingItems = null; - await _enqueue(currentItems()); + await _enqueue(currentItems(), requireDurable: requireDurable); await _pauseWrite; } @@ -100,68 +100,76 @@ class DownloadQueuePersistence { _pendingItems = null; } - Future _enqueue(List items) { - return _write = _write - .then((_) async { - // Queue states use immutable storage. Repeated lifecycle/debounce - // flushes of an already committed snapshot need no diff or encoding. - if (identical(items, _savedSnapshot)) return; - final seenIds = {}; - final savedUpdates = {}; - final upserts = >[]; - final now = DateTime.now().toIso8601String(); - final changed = []; - for (final item in items) { - if (item.status == DownloadStatus.completed || - item.status == DownloadStatus.failed) { - continue; - } - final saved = _saved[item.id]; - final cached = - identical(saved?.item, item) && saved?.payload != null; - seenIds.add(item.id); - if (!cached) { - savedUpdates[item.id] = (item: item, payload: null); - changed.add(item); - } else if (savedUpdates.containsKey(item.id)) { - // Preserve the last non-terminal occurrence for malformed input - // with repeated request IDs, just as the full snapshot did. - savedUpdates[item.id] = saved!; - } - } - final encoded = await encodeDownloadQueueItems(changed); - for (var index = 0; index < changed.length; index++) { - final item = changed[index]; - final saved = _saved[item.id]; - final payload = encoded[index]; - if (identical(savedUpdates[item.id]?.item, item)) { - savedUpdates[item.id] = (item: item, payload: payload); - } - if (saved?.payload == payload) continue; - upserts.add({ - 'id': item.id, - 'item_json': payload, - 'status': downloadQueuePersistenceStatus(item.status).name, - 'created_at': item.createdAt.toIso8601String(), - 'updated_at': now, - }); - } - final deletedIds = _saved.keys - .where((id) => !seenIds.contains(id)) - .toList(growable: false); - if (upserts.isNotEmpty || deletedIds.isNotEmpty) { - await applyChanges(upserts: upserts, deletedIds: deletedIds); - } - // Keep the last durable cache intact until the complete write has - // succeeded, so failed changes are still retried by the next flush. - for (final id in deletedIds) { - _saved.remove(id); - } - _saved.addAll(savedUpdates); - // Callers outside the notifier may supply a mutable List. Its - // identity alone never proves that its contents stayed unchanged. - _savedSnapshot = items is ChunkedList ? items : null; - }) - .catchError(onError); + Future _enqueue( + List items, { + bool requireDurable = false, + }) { + final write = _write.then((_) async { + // Queue states use immutable storage. Repeated lifecycle/debounce + // flushes of an already committed snapshot need no diff or encoding. + if (identical(items, _savedSnapshot)) return; + final seenIds = {}; + final savedUpdates = {}; + final upserts = >[]; + final now = DateTime.now().toIso8601String(); + final changed = []; + for (final item in items) { + if (item.status == DownloadStatus.completed || + (item.status == DownloadStatus.failed && + item.errorType != DownloadErrorType.verificationRequired)) { + continue; + } + final saved = _saved[item.id]; + final cached = identical(saved?.item, item) && saved?.payload != null; + seenIds.add(item.id); + if (!cached) { + savedUpdates[item.id] = (item: item, payload: null); + changed.add(item); + } else if (savedUpdates.containsKey(item.id)) { + // Preserve the last non-terminal occurrence for malformed input + // with repeated request IDs, just as the full snapshot did. + savedUpdates[item.id] = saved!; + } + } + final encoded = await encodeDownloadQueueItems(changed); + for (var index = 0; index < changed.length; index++) { + final item = changed[index]; + final saved = _saved[item.id]; + final payload = encoded[index]; + if (identical(savedUpdates[item.id]?.item, item)) { + savedUpdates[item.id] = (item: item, payload: payload); + } + if (saved?.payload == payload) continue; + upserts.add({ + 'id': item.id, + 'item_json': payload, + 'status': downloadQueuePersistenceStatus( + item.status, + errorType: item.errorType, + ).name, + 'created_at': item.createdAt.toIso8601String(), + 'updated_at': now, + }); + } + final deletedIds = _saved.keys + .where((id) => !seenIds.contains(id)) + .toList(growable: false); + if (upserts.isNotEmpty || deletedIds.isNotEmpty) { + await applyChanges(upserts: upserts, deletedIds: deletedIds); + } + // Keep the last durable cache intact until the complete write has + // succeeded, so failed changes are still retried by the next flush. + for (final id in deletedIds) { + _saved.remove(id); + } + _saved.addAll(savedUpdates); + // Callers outside the notifier may supply a mutable List. Its + // identity alone never proves that its contents stayed unchanged. + _savedSnapshot = items is ChunkedList ? items : null; + }); + // The writer stays usable after failure, but external-app handoffs must + // know whether their snapshot actually reached disk before opening a URL. + _write = write.catchError(onError); + return requireDurable ? write : _write; } } diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 4af420a5..7d8bcffc 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -322,6 +322,8 @@ class PlatformBridge { StreamController.broadcast(); static final StreamController _setupFolderSelectionEvents = StreamController.broadcast(); + static final StreamController _extensionAuthCallbackEvents = + StreamController.broadcast(); static final StreamController> _iosBackgroundDownloadExpirationEvents = StreamController>.broadcast(); @@ -352,6 +354,14 @@ class PlatformBridge { return _setupFolderSelectionEvents.stream; } + static Stream extensionAuthCallbackEvents() { + _ensureBackendEventHandler(); + return _extensionAuthCallbackEvents.stream; + } + + static Future processPendingExtensionCallbacks() => + _channel.invokeMethod('processPendingExtensionCallbacks'); + static Future consumeVerificationNotification() => _channel.invokeMethod('consumeVerificationNotification'); @@ -365,6 +375,9 @@ class PlatformBridge { _backendEventHandlerInstalled = true; _channel.setMethodCallHandler((call) async { switch (call.method) { + case 'extensionAuthCallbackReceived': + _extensionAuthCallbackEvents.add(null); + return null; case 'setupFolderSelectionChanged': _setupFolderSelectionEvents.add(null); return null; diff --git a/rust_backend/crates/extensions/src/auth.rs b/rust_backend/crates/extensions/src/auth.rs index fc5a184e..1cb2c7e6 100644 --- a/rust_backend/crates/extensions/src/auth.rs +++ b/rust_backend/crates/extensions/src/auth.rs @@ -1,7 +1,7 @@ //! Shared OAuth state and one-time callback ownership for managed runtimes. use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use sha2::{Digest, Sha256}; use std::collections::BTreeMap; @@ -10,6 +10,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use zeroize::Zeroizing; mod client; +mod persistence; pub use client::ExtensionAuth; const SECOND: i128 = 1_000_000_000; @@ -29,7 +30,7 @@ impl AuthClock for SystemClock { } } -#[derive(Clone, Serialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct PendingAuthRequest { pub extension_id: String, pub auth_url: String, @@ -63,6 +64,7 @@ struct State { pub struct AuthRegistry { state: Mutex, clock: Arc, + persistence: Option, } impl Default for AuthRegistry { @@ -76,9 +78,45 @@ impl AuthRegistry { Self { state: Mutex::default(), clock, + persistence: None, } } + pub fn persistent(root: &std::path::Path, key: &crate::storage::StorageMasterKey) -> Self { + Self::persistent_with_clock(root, key, Arc::new(SystemClock)) + } + + fn persistent_with_clock( + root: &std::path::Path, + key: &crate::storage::StorageMasterKey, + clock: Arc, + ) -> Self { + let mut registry = Self::with_clock(clock); + let store = persistence::PendingStore::new(root, key); + // A corrupt/old-key journal invalidates only pending challenges, not + // installed extensions. Callback ownership is never inferred from URLs. + for saved in store.load(registry.now()).unwrap_or_default() { + let mut request = saved.request; + request.state = saved.nonce; + request.created_at = saved.created_at; + let id = request.extension_id.clone(); + if registry.register_pending(request).is_ok() && !saved.verifier.is_empty() { + let _ = registry.edit(&id, |record| { + record.challenge = pkce_challenge(&saved.verifier); + record.verifier = Zeroizing::new(saved.verifier); + }); + } + } + registry.persistence = Some(store); + registry + } + + fn persist(&self, state: &State) -> Result<(), String> { + self.persistence + .as_ref() + .map_or(Ok(()), |store| store.save(state, self.now())) + } + pub fn now(&self) -> i128 { self.clock.now_nanos() } @@ -141,6 +179,7 @@ impl AuthRegistry { let generation = state.generations.entry(id.to_owned()).or_default(); *generation = generation.wrapping_add(1); Self::remove_pending(&mut state, id, self.now()); + let _ = self.persist(&state); } pub fn register_pending(&self, mut request: PendingAuthRequest) -> Result<(), String> { @@ -157,6 +196,8 @@ impl AuthRegistry { if state.closed { return Err("extension environment closed".into()); } + let previous = state.pending.clone(); + let previous_owners = state.owners.clone(); if let Some(owner) = state .owners .get(&request.state) @@ -184,6 +225,11 @@ impl AuthRegistry { .entry(request.state.clone()) .or_insert_with(|| request.extension_id.clone()); state.pending.insert(request.extension_id.clone(), request); + if let Err(error) = self.persist(&state) { + state.pending = previous; + state.owners = previous_owners; + return Err(error); + } Ok(()) } @@ -207,11 +253,9 @@ impl AuthRegistry { } pub fn clear_pending(&self, id: &str) { - Self::remove_pending( - &mut self.state.lock().expect("auth registry lock"), - id, - self.now(), - ); + let mut state = self.state.lock().expect("auth registry lock"); + Self::remove_pending(&mut state, id, self.now()); + let _ = self.persist(&state); } pub fn pending(&self, id: &str) -> Option { @@ -245,11 +289,13 @@ impl AuthRegistry { .as_ref() .and_then(|owner| state.pending.get(owner)) .is_some_and(|request| { - request.state == nonce && self.now() - request.created_at < PENDING_TTL + request.state == nonce + && (0..PENDING_TTL).contains(&(self.now() - request.created_at)) }); if !valid || consume { state.owners.remove(nonce); state.pending.retain(|_, request| request.state != nonce); + self.persist(&state)?; } if valid { Ok(owner.expect("validated callback owner")) @@ -291,6 +337,147 @@ pub fn pkce_challenge(verifier: &str) -> String { mod tests { use super::*; + fn key(byte: u8) -> crate::storage::StorageMasterKey { + crate::storage::StorageMasterKey::from_base64( + &base64::engine::general_purpose::STANDARD.encode([byte; 32]), + ) + .unwrap() + } + + fn challenge(nonce: &str, age_seconds: i128) -> PendingAuthRequest { + PendingAuthRequest { + extension_id: "example.auth".into(), + auth_url: "https://example.test/verify".into(), + callback_url: "spotiflac://session-grant".into(), + state: nonce.into(), + created_at: (1000 - age_seconds) * SECOND, + } + } + + #[test] + fn encrypted_challenge_and_pkce_survive_restart_but_consumption_does_not_replay() { + let root = tempfile::tempdir().unwrap(); + let key = key(7); + let auth = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + auth.edit("example.auth", |record| { + record.verifier = Zeroizing::new("private-pkce-verifier".into()); + }) + .unwrap(); + auth.register_pending(challenge("private-browser-nonce", 0)) + .unwrap(); + let encrypted = std::fs::read(root.path().join(".pending-auth.enc")).unwrap(); + for secret in ["private-pkce-verifier", "private-browser-nonce"] { + assert!( + !encrypted + .windows(secret.len()) + .any(|window| window == secret.as_bytes()) + ); + } + auth.shutdown(); + drop(auth); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert_eq!( + restored + .resolve_callback("private-browser-nonce", false) + .unwrap(), + "example.auth" + ); + assert_eq!( + restored.state.lock().unwrap().records["example.auth"] + .verifier + .as_str(), + "private-pkce-verifier" + ); + assert!(restored.resolve_callback("unregistered", false).is_err()); + let public = serde_json::to_value(restored.pending("example.auth").unwrap()).unwrap(); + assert!(public.get("state").is_none()); + assert!(public.get("created_at").is_none()); + restored + .resolve_callback("private-browser-nonce", true) + .unwrap(); + drop(restored); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert!( + restored + .resolve_callback("private-browser-nonce", false) + .is_err() + ); + } + + #[test] + fn restart_rejects_expired_future_replaced_and_cleared_challenges() { + let root = tempfile::tempdir().unwrap(); + let key = key(7); + for age in [-1, 180] { + let auth = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + auth.register_pending(challenge("stale", age)).unwrap(); + drop(auth); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert!(restored.resolve_callback("stale", false).is_err()); + } + let auth = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + auth.register_pending(challenge("old", 0)).unwrap(); + auth.register_pending(challenge("new", 0)).unwrap(); + drop(auth); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert!(restored.resolve_callback("old", false).is_err()); + assert!(restored.resolve_callback("new", false).is_ok()); + restored.clear_pending("example.auth"); + drop(restored); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert!(restored.resolve_callback("new", false).is_err()); + } + + #[test] + fn shared_scope_callbacks_remain_one_time_after_restart() { + let root = tempfile::tempdir().unwrap(); + let key = key(7); + let auth = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + auth.register_pending(challenge("shared", 0)).unwrap(); + let mut alias = challenge("shared", 0); + alias.extension_id = "example.other".into(); + auth.register_pending(alias).unwrap(); + drop(auth); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + restored.clear_pending("example.auth"); + assert_eq!( + restored.resolve_callback("shared", true).unwrap(), + "example.other" + ); + drop(restored); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert!(restored.resolve_callback("shared", false).is_err()); + } + + #[test] + fn corrupt_or_changed_key_journal_never_restores_callback_ownership() { + let root = tempfile::tempdir().unwrap(); + let key = key(7); + let auth = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + auth.register_pending(challenge("nonce", 0)).unwrap(); + drop(auth); + let other_key = super::tests::key(8); + let restored = + AuthRegistry::persistent_with_clock(root.path(), &other_key, Arc::new(Clock)); + assert!(restored.pending("example.auth").is_none()); + std::fs::write(root.path().join(".pending-auth.enc"), b"corrupt").unwrap(); + let restored = AuthRegistry::persistent_with_clock(root.path(), &key, Arc::new(Clock)); + assert!(restored.pending("example.auth").is_none()); + restored + .register_pending(challenge("replacement", 0)) + .unwrap(); + assert!(restored.resolve_callback("replacement", false).is_ok()); + } + + #[test] + fn failed_journal_write_does_not_publish_an_unsaved_challenge() { + let root = tempfile::tempdir().unwrap(); + let auth = AuthRegistry::persistent_with_clock(root.path(), &key(7), Arc::new(Clock)); + std::fs::create_dir(root.path().join(".pending-auth.enc")).unwrap(); + assert!(auth.register_pending(challenge("unsaved", 0)).is_err()); + assert!(auth.pending("example.auth").is_none()); + } + struct Clock; impl AuthClock for Clock { fn now_nanos(&self) -> i128 { diff --git a/rust_backend/crates/extensions/src/auth/persistence.rs b/rust_backend/crates/extensions/src/auth/persistence.rs new file mode 100644 index 00000000..0878b8f9 --- /dev/null +++ b/rust_backend/crates/extensions/src/auth/persistence.rs @@ -0,0 +1,74 @@ +use super::{PENDING_TTL, PendingAuthRequest, State}; +use crate::storage::{StorageMasterKey, atomic_write, decrypt, encrypt, valid_extension_id}; +use serde::{Deserialize, Serialize}; +use std::fs; +use std::path::{Path, PathBuf}; +use zeroize::Zeroizing; + +#[derive(Serialize, Deserialize)] +pub(super) struct SavedPending { + pub request: PendingAuthRequest, + pub nonce: String, + pub created_at: i128, + pub verifier: String, +} + +pub(super) struct PendingStore { + path: PathBuf, + key: Zeroizing<[u8; 32]>, +} + +impl PendingStore { + pub fn new(root: &Path, key: &StorageMasterKey) -> Self { + Self { + path: root.join(".pending-auth.enc"), + key: key.derive("host", "pending-auth"), + } + } + + pub fn load(&self, now: i128) -> Result, String> { + let metadata = match fs::symlink_metadata(&self.path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(_) => return Err("cannot read pending verification".into()), + }; + if !metadata.is_file() || metadata.len() > 2 * 1024 * 1024 { + return Err("invalid pending verification file".into()); + } + let encrypted = fs::read(&self.path).map_err(|_| "cannot read pending verification")?; + let plaintext = + decrypt(&encrypted, &self.key).map_err(|_| "invalid pending verification")?; + let mut saved: Vec = + serde_json::from_slice(&plaintext).map_err(|_| "invalid pending verification")?; + saved.retain(|entry| { + valid_extension_id(&entry.request.extension_id) + && !entry.nonce.is_empty() + && (0..PENDING_TTL).contains(&(now - entry.created_at)) + }); + Ok(saved) + } + + pub fn save(&self, state: &State, now: i128) -> Result<(), String> { + let saved: Vec<_> = state + .pending + .values() + .filter(|request| (0..PENDING_TTL).contains(&(now - request.created_at))) + .map(|request| SavedPending { + request: request.clone(), + nonce: request.state.clone(), + created_at: request.created_at, + verifier: state + .records + .get(&request.extension_id) + .map(|record| record.verifier.to_string()) + .unwrap_or_default(), + }) + .collect(); + let bytes = Zeroizing::new( + serde_json::to_vec(&saved).map_err(|_| "cannot encode pending verification")?, + ); + let encrypted = + encrypt(&bytes, &self.key).map_err(|_| "cannot encrypt pending verification")?; + atomic_write(&self.path, &encrypted).map_err(|_| "cannot save pending verification".into()) + } +} diff --git a/rust_backend/crates/extensions/src/environment.rs b/rust_backend/crates/extensions/src/environment.rs index 74878f4d..45f2aa9a 100644 --- a/rust_backend/crates/extensions/src/environment.rs +++ b/rust_backend/crates/extensions/src/environment.rs @@ -135,7 +135,7 @@ impl ExtensionEnvironment { let data_directory_alias = crate::files::clean(&std::path::absolute(data_directory).map_err(StorageError::from)?); let data_directory = fs::canonicalize(data_directory).map_err(StorageError::from)?; - let auth = Arc::new(AuthRegistry::default()); + let auth = Arc::new(AuthRegistry::persistent(&data_directory, &master_key)); let sessions = SignedRegistry::new(&data_directory, Arc::clone(&auth)); Ok(Self { logs: Arc::new(crate::logging::LogBuffer::default()), diff --git a/test/download_queue_persistence_test.dart b/test/download_queue_persistence_test.dart index a6a441d6..08e9fc7f 100644 --- a/test/download_queue_persistence_test.dart +++ b/test/download_queue_persistence_test.dart @@ -83,6 +83,77 @@ void main() { TestWidgetsFlutterBinding.ensureInitialized(); setUp(() => SharedPreferences.setMockInitialValues({})); + test( + 'verification waits survive process restart without reviving terminal failures', + () async { + final store = _Store(); + var items = [ + _item( + 'waiting', + status: DownloadStatus.downloading, + ).copyWith(errorType: DownloadErrorType.verificationRequired), + _item( + 'timeout', + status: DownloadStatus.failed, + ).copyWith(errorType: DownloadErrorType.verificationRequired), + _item( + 'unavailable', + status: DownloadStatus.failed, + ).copyWith(errorType: DownloadErrorType.notFound), + _item('done', status: DownloadStatus.completed), + _item('cancelled', status: DownloadStatus.skipped), + ]; + DownloadQueuePersistence create() => DownloadQueuePersistence( + currentItems: () => items, + onError: (error) => fail('$error'), + loadRows: store.load, + applyChanges: store.apply, + ); + final first = create(); + await first.flush(requireDurable: true); + first.dispose(); + items = []; + final restarted = create(); + addTearDown(restarted.dispose); + items = await restarted.restore(); + expect(items.map((item) => item.id), ['waiting', 'timeout', 'cancelled']); + expect(items.map((item) => item.status), [ + DownloadStatus.queued, + DownloadStatus.queued, + DownloadStatus.skipped, + ]); + expect(store.rows['timeout']!['status'], 'queued'); + expect(items.first.qualityOverride, 'HI_RES'); + }, + ); + + test( + 'browser handoff flush reports disk failure and a later flush retries', + () async { + final store = _Store(); + final errors = []; + var attempts = 0; + final persistence = DownloadQueuePersistence( + currentItems: () => [_item('pending')], + onError: errors.add, + loadRows: store.load, + applyChanges: ({required upserts, required deletedIds}) async { + if (++attempts == 1) throw StateError('disk unavailable'); + await store.apply(upserts: upserts, deletedIds: deletedIds); + }, + ); + addTearDown(persistence.dispose); + await expectLater( + persistence.flush(requireDurable: true), + throwsStateError, + ); + expect(store.rows, isEmpty); + expect(errors, hasLength(1)); + await persistence.flush(requireDurable: true); + expect(store.rows.keys, ['pending']); + }, + ); + test( 'restores active/cancelled items and repairs corrupt or terminal rows', () async { diff --git a/test/download_queue_recovery_test.dart b/test/download_queue_recovery_test.dart index 5bbcca6b..085b1d42 100644 --- a/test/download_queue_recovery_test.dart +++ b/test/download_queue_recovery_test.dart @@ -7,6 +7,7 @@ import 'package:spotiflac_android/models/download_item.dart'; import 'package:spotiflac_android/models/settings.dart'; import 'package:spotiflac_android/models/track.dart'; import 'package:spotiflac_android/providers/download_queue_provider.dart'; +import 'package:spotiflac_android/providers/extension_provider.dart'; import 'package:spotiflac_android/providers/settings_provider.dart'; class _Settings extends SettingsNotifier { @@ -34,6 +35,20 @@ class _Queue extends DownloadQueueNotifier { ); } +class _Extensions extends ExtensionNotifier { + final _started = Completer(); + final _ready = Completer(); + + @override + ExtensionState build() => const ExtensionState(); + + @override + Future ensureInitialized() async { + if (!_started.isCompleted) _started.complete(); + await _ready.future; + } +} + void main() { TestWidgetsFlutterBinding.ensureInitialized(); final messenger = @@ -49,6 +64,7 @@ void main() { }); test('a failed queue setup releases processing and permits retry', () async { + final extensions = _Extensions().._ready.complete(); var pathAttempts = 0; messenger.setMockMethodCallHandler(paths, (_) async { pathAttempts++; @@ -60,6 +76,7 @@ void main() { overrides: [ downloadQueueProvider.overrideWith(() => queue), settingsProvider.overrideWith(_Settings.new), + extensionProvider.overrideWith(() => extensions), ], ); addTearDown(container.dispose); @@ -88,4 +105,41 @@ void main() { expect(pathAttempts, greaterThan(firstAttempts)); expect(container.read(downloadQueueProvider).isProcessing, isFalse); }); + + test( + 'restored downloads wait for extensions and remain queued if initialization fails', + () async { + var pathAttempts = 0; + messenger.setMockMethodCallHandler(paths, (_) async { + pathAttempts++; + throw PlatformException(code: 'folder_unavailable'); + }); + messenger.setMockMethodCallHandler(connectivity, (_) async => null); + final extensions = _Extensions(); + final queue = _Queue(); + final container = ProviderContainer( + overrides: [ + downloadQueueProvider.overrideWith(() => queue), + settingsProvider.overrideWith(_Settings.new), + extensionProvider.overrideWith(() => extensions), + ], + ); + addTearDown(container.dispose); + container.read(downloadQueueProvider); + queue.resumePendingDownloadsOnForeground(); + await extensions._started.future; + expect(pathAttempts, 0); + expect( + container.read(downloadQueueProvider).items.single.status, + DownloadStatus.queued, + ); + extensions._ready.completeError(StateError('temporarily unavailable')); + await Future.delayed(Duration.zero); + final recovered = container.read(downloadQueueProvider); + expect(recovered.items.single.status, DownloadStatus.queued); + expect(recovered.items.single.error, isNull); + expect(recovered.isProcessing, isFalse); + expect(pathAttempts, 0); + }, + ); } diff --git a/test/extension_initialization_test.dart b/test/extension_initialization_test.dart index bb2ee735..ce101413 100644 --- a/test/extension_initialization_test.dart +++ b/test/extension_initialization_test.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'dart:io'; import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -112,6 +113,29 @@ void main() { await root.delete(recursive: true); }); + testWidgets( + 'activity detach releases idle memory without closing extensions', + (tester) async { + final methods = []; + messenger.setMockMethodCallHandler(backend, (call) async { + methods.add(call.method); + return null; + }); + final container = ProviderContainer(); + addTearDown(container.dispose); + container.read(extensionProvider); + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed); + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.inactive); + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.hidden); + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.paused); + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.detached); + await tester.pump(); + expect(methods, contains('releaseMemory')); + expect(methods, isNot(contains('cleanupExtensions'))); + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed); + }, + ); + test( 'storage preparation retries a transient failure and coalesces callers', () async {