From 3a7c457f7390678c5d72721b88cbf23dc7508d0e Mon Sep 17 00:00:00 2001 From: provokateurin Date: Wed, 20 Nov 2024 14:08:59 +0100 Subject: [PATCH 1/2] feat(account_repository): Allow registering callback to be executed before an account is logged out Signed-off-by: provokateurin --- .../lib/src/account_repository.dart | 22 +++++++++++++++- .../test/account_repository_test.dart | 25 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/neon_framework/packages/account_repository/lib/src/account_repository.dart b/packages/neon_framework/packages/account_repository/lib/src/account_repository.dart index a8bdc465372..f176d25326b 100644 --- a/packages/neon_framework/packages/account_repository/lib/src/account_repository.dart +++ b/packages/neon_framework/packages/account_repository/lib/src/account_repository.dart @@ -14,6 +14,9 @@ import 'package:rxdart/rxdart.dart'; part 'account_storage.dart'; +/// Signature of a callback that executed right before the [account] is logged out. +typedef BeforeLogOutCallback = Future Function(Account account); + /// {@template account_failure} /// A base failure for the account repository failures. /// {@endtemplate} @@ -86,6 +89,7 @@ class AccountRepository { final String _userAgent; final http.Client _httpClient; final AccountStorage _storage; + final List _beforeLogOutCallbacks = []; final BehaviorSubject<({String? active, BuiltMap accounts})> _accounts = BehaviorSubject.seeded((active: null, accounts: BuiltMap())); @@ -272,6 +276,18 @@ class AccountRepository { ]); } + /// Registers a new [callback] that is executed right before an account is logged out. + /// + /// The callback must not throw any exceptions and handle them gracefully itself. + void registerBeforeLogOutCallback(BeforeLogOutCallback callback) { + _beforeLogOutCallbacks.add(callback); + } + + /// Unregisters a [callback] that was previously registered with [registerBeforeLogOutCallback]. + void unregisterBeforeLogOutCallback(BeforeLogOutCallback callback) { + _beforeLogOutCallbacks.remove(callback); + } + /// Logs out the user from the server. /// /// May throw a [DeleteCredentialsFailure]. @@ -299,8 +315,12 @@ class AccountRepository { _accounts.add((active: active, accounts: accounts)); + for (final callback in _beforeLogOutCallbacks) { + await callback(account!); + } + try { - await account?.client.authentication.appPassword.deleteAppPassword(); + await account!.client.authentication.appPassword.deleteAppPassword(); } on http.ClientException catch (error, stackTrace) { Error.throwWithStackTrace(DeleteCredentialsFailure(error), stackTrace); } diff --git a/packages/neon_framework/packages/account_repository/test/account_repository_test.dart b/packages/neon_framework/packages/account_repository/test/account_repository_test.dart index 3a2c0c0a647..af755b49118 100644 --- a/packages/neon_framework/packages/account_repository/test/account_repository_test.dart +++ b/packages/neon_framework/packages/account_repository/test/account_repository_test.dart @@ -40,6 +40,12 @@ class _UsersClientMock extends Mock implements provisioning_api.$UsersClient {} class _AccountStorageMock extends Mock implements AccountStorage {} +class _FakeAccount extends Fake implements Account {} + +class _BeforeLogOutCallbackMock extends Mock { + Future call(Account account); +} + typedef _AccountStream = ({BuiltList accounts, Account? active}); void main() { @@ -54,6 +60,7 @@ void main() { setUpAll(() { registerFallbackValue(_FakeUri()); registerFallbackValue(_FakePollRequest()); + registerFallbackValue(_FakeAccount()); MockNeonStorage(); }); @@ -506,6 +513,20 @@ void main() { when(() => appPassword.deleteAppPassword()).thenAnswer( (_) async => _DynamiteResponseMock(), ); + + final callback1 = _BeforeLogOutCallbackMock(); + when(() => callback1.call(any())).thenAnswer((_) async {}); + repository.registerBeforeLogOutCallback(callback1.call); + + final callback2 = _BeforeLogOutCallbackMock(); + repository + ..registerBeforeLogOutCallback(callback2.call) + ..unregisterBeforeLogOutCallback(callback2.call); + + final callback3 = _BeforeLogOutCallbackMock(); + when(() => callback3.call(any())).thenAnswer((_) async {}); + repository.registerBeforeLogOutCallback(callback3.call); + await repository.logOut(credentialsList.first.id); await expectLater( @@ -517,6 +538,10 @@ void main() { ), ); + verify(() => callback1(accountsList.first)).called(1); + verifyNever(() => callback2(accountsList.first)); + verify(() => callback3(accountsList.first)).called(1); + verify(() => appPassword.deleteAppPassword()).called(1); verify(() => storage.saveLastAccount(credentialsList[1].id)).called(1); verify(() => storage.saveCredentials(any(that: equals([credentialsList[1]])))).called(1); From 20c0c0df97f25a246a8f2c76761d4571c62f5c71 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Wed, 20 Nov 2024 14:15:57 +0100 Subject: [PATCH 2/2] fix(notifications_push_repository): Delete push subscription when account is deleted Signed-off-by: provokateurin --- .../src/notifications_push_repository.dart | 47 +++++-- .../notifications_push_repository_test.dart | 125 ++++++++++++++++++ 2 files changed, 162 insertions(+), 10 deletions(-) diff --git a/packages/neon_framework/packages/notifications_push_repository/lib/src/notifications_push_repository.dart b/packages/neon_framework/packages/notifications_push_repository/lib/src/notifications_push_repository.dart index 5e1b6eeb10e..3cc2efa9bbc 100644 --- a/packages/neon_framework/packages/notifications_push_repository/lib/src/notifications_push_repository.dart +++ b/packages/neon_framework/packages/notifications_push_repository/lib/src/notifications_push_repository.dart @@ -29,7 +29,9 @@ class NotificationsPushRepository { required OnMessageCallback onMessage, }) : _accountRepository = accountRepository, _storage = storage, - _onMessage = onMessage; + _onMessage = onMessage { + _accountRepository.registerBeforeLogOutCallback(_beforeLogOutCallback); + } final AccountRepository _accountRepository; final NotificationsPushStorage _storage; @@ -48,6 +50,28 @@ class NotificationsPushRepository { /// Closes all open resources of the repository. void close() { unawaited(_accountsListener?.cancel()); + _accountRepository.unregisterBeforeLogOutCallback(_beforeLogOutCallback); + } + + Future _beforeLogOutCallback(Account account) async { + final subscriptions = await _storage.readSubscriptions(); + var subscription = subscriptions[account.id]; + if (subscription == null) { + return; + } + + final pushDevice = subscription.pushDevice; + if (pushDevice != null) { + await _unregisterNextcloud(account.id, account, pushDevice); + subscription = subscription.rebuild((b) => b.pushDevice = null); + } + + if (subscription.endpoint != null) { + await _unregisterUnifiedPush(account.id); + subscription = subscription.rebuild((b) => b.endpoint = null); + } + + await _storage.updateSubscription(account.id, subscription); } /// Changes the used distributor to the new [distributor]. @@ -146,21 +170,21 @@ class NotificationsPushRepository { if (_selectedDistributor == null) { _log.fine('Push notifications disabled, removing all subscriptions'); - await _unregisterUnifiedPush(); + await _unregisterAllUnifiedPush(); return; } if (distributorChanged) { _log.finer('UnifiedPush distributor changed to $_selectedDistributor'); - await _unregisterUnifiedPush(); + await _unregisterAllUnifiedPush(); await UnifiedPush.saveDistributor(_selectedDistributor!); } - await _registerUnifiedPush(); + await _registerAllUnifiedPush(); } - Future _registerUnifiedPush() async { + Future _registerAllUnifiedPush() async { // Notifications will only work on accounts with app password final accounts = (await _accountRepository.accounts.first).accounts; for (final account in accounts.where((a) => a.credentials.appPassword != null)) { @@ -170,7 +194,7 @@ class NotificationsPushRepository { } } - Future _unregisterUnifiedPush() async { + Future _unregisterAllUnifiedPush() async { final subscriptions = await _storage.readSubscriptions(); for (final entry in subscriptions.entries) { final accountID = entry.key; @@ -184,10 +208,7 @@ class NotificationsPushRepository { } if (subscription.endpoint != null) { - _log.finer('Unregistering $accountID from UnifiedPush'); - - await UnifiedPush.unregister(accountID); - + await _unregisterUnifiedPush(accountID); subscription = subscription.rebuild((b) => b.endpoint = null); } @@ -229,4 +250,10 @@ class NotificationsPushRepository { _log.warning('Failed to unregister $accountID at Nextcloud', error); } } + + Future _unregisterUnifiedPush(String accountID) async { + _log.finer('Unregistering $accountID from UnifiedPush'); + + await UnifiedPush.unregister(accountID); + } } diff --git a/packages/neon_framework/packages/notifications_push_repository/test/notifications_push_repository_test.dart b/packages/neon_framework/packages/notifications_push_repository/test/notifications_push_repository_test.dart index dfa8f9285eb..b2a4f504d61 100644 --- a/packages/neon_framework/packages/notifications_push_repository/test/notifications_push_repository_test.dart +++ b/packages/neon_framework/packages/notifications_push_repository/test/notifications_push_repository_test.dart @@ -60,6 +60,7 @@ void main() { void Function(String, String)? unifiedPushOnNewEndpoint; void Function(String)? unifiedPushOnUnregistered; void Function(Uint8List, String)? unifiedPushOnMessage; + late List beforeLogOutCallbacks; setUpAll(() { registerFallbackValue(RSAPrivateKey(BigInt.zero, BigInt.zero, BigInt.zero, BigInt.zero)); @@ -72,6 +73,7 @@ void main() { }); setUp(() { + beforeLogOutCallbacks = []; accountsSubject = BehaviorSubject(); accountRepository = _AccountRepositoryMock(); when(() => accountRepository.accounts).thenAnswer((_) => accountsSubject.map((e) => (active: null, accounts: e))); @@ -81,6 +83,12 @@ void main() { return accountsSubject.value.singleWhereOrNull((account) => account.id == accountID); }, ); + when(() => accountRepository.registerBeforeLogOutCallback(any())).thenAnswer((invocation) { + beforeLogOutCallbacks.add(invocation.positionalArguments.first as BeforeLogOutCallback); + }); + when(() => accountRepository.unregisterBeforeLogOutCallback(any())).thenAnswer((invocation) { + beforeLogOutCallbacks.remove(invocation.positionalArguments.first as BeforeLogOutCallback); + }); storage = _StorageMock(); @@ -104,6 +112,7 @@ void main() { tearDown(() { repository.close(); + expect(beforeLogOutCallbacks, isEmpty); unawaited(accountsSubject.close()); }); @@ -1728,6 +1737,122 @@ void main() { }); }); }); + + group('Removes subscription when account is deleted', () { + test('Success at Nextcloud', () async { + when( + () => httpRequest( + 'DELETE', + Uri.parse('https://cloud.example.com:8443/nextcloud/ocs/v2.php/apps/notifications/api/v2/push'), + any(), + any(), + ), + ).thenAnswer( + (_) => http.StreamedResponse( + Stream.value( + utf8.encode( + json.encode( + { + 'ocs': { + 'meta': { + 'status': '', + 'statuscode': 0, + }, + 'data': {}, + }, + }, + ), + ), + ), + 200, + headers: { + 'content-type': 'application/json; charset=utf-8', + }, + ), + ); + + repository = NotificationsPushRepository( + accountRepository: accountRepository, + storage: storage, + onMessage: onMessageCallback, + ); + await repository.initialize(); + + verify(() => unifiedPushPlatform.registerApp(account.id, [])).called(1); + verifyNever(() => unifiedPushPlatform.registerApp(any(), any())); + + for (final callback in beforeLogOutCallbacks) { + await callback(account); + } + + verify(() => unifiedPushPlatform.unregister(account.id)).called(1); + verifyNever(() => unifiedPushPlatform.unregister(any())); + verify( + () => httpRequest( + 'DELETE', + Uri.parse('https://cloud.example.com:8443/nextcloud/ocs/v2.php/apps/notifications/api/v2/push'), + BuiltMap( + { + 'Accept': 'application/json', + 'Authorization': 'Bearer user1', + 'OCS-APIRequest': 'true', + 'user-agent': 'neon', + }, + ), + Uint8List(0), + ), + ).called(1); + verifyNever(() => httpRequest(any(), any(), any(), any())); + verify(() => storage.updateSubscription(account.id, PushSubscription())).called(1); + verifyNever(() => storage.updateSubscription(any(), any())); + }); + + test('Failure at Nextcloud', () async { + when( + () => httpRequest( + 'DELETE', + Uri.parse('https://cloud.example.com:8443/nextcloud/ocs/v2.php/apps/notifications/api/v2/push'), + any(), + any(), + ), + ).thenAnswer((_) => http.StreamedResponse(const Stream.empty(), 500)); + + repository = NotificationsPushRepository( + accountRepository: accountRepository, + storage: storage, + onMessage: onMessageCallback, + ); + await repository.initialize(); + + verify(() => unifiedPushPlatform.registerApp(account.id, [])).called(1); + verifyNever(() => unifiedPushPlatform.registerApp(any(), any())); + + for (final callback in beforeLogOutCallbacks) { + await callback(account); + } + + verify(() => unifiedPushPlatform.unregister(account.id)).called(1); + verifyNever(() => unifiedPushPlatform.unregister(any())); + verify( + () => httpRequest( + 'DELETE', + Uri.parse('https://cloud.example.com:8443/nextcloud/ocs/v2.php/apps/notifications/api/v2/push'), + BuiltMap( + { + 'Accept': 'application/json', + 'Authorization': 'Bearer user1', + 'OCS-APIRequest': 'true', + 'user-agent': 'neon', + }, + ), + Uint8List(0), + ), + ).called(1); + verifyNever(() => httpRequest(any(), any(), any(), any())); + verify(() => storage.updateSubscription(account.id, PushSubscription())).called(1); + verifyNever(() => storage.updateSubscription(any(), any())); + }); + }); }); }); });