From 2c96e585247c1e593e4607acf5d54caf9aaa588a Mon Sep 17 00:00:00 2001 From: Murali Date: Wed, 31 Jul 2024 12:25:16 +0530 Subject: [PATCH 1/6] fix: response transformer handle isEncrypted false --- .../notification_response_transformer.dart | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart index 8d307789f..3a151172c 100644 --- a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart +++ b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart @@ -56,11 +56,13 @@ class NotificationResponseTransformer } if (atNotification.messageType.isNotNull && - atNotification.messageType!.toLowerCase().contains('text') && - (atNotification.isEncrypted != null && atNotification.isEncrypted!)) { - // decrypt the text message; - var decryptedValue = await _getDecryptedValue(atKey, atKey.key); - atNotification.key = '${atNotification.to}:$decryptedValue'; + atNotification.messageType!.toLowerCase().contains('text')) { + // decrypt the text message if isEncrypted is true + if (atNotification.isEncrypted != null && atNotification.isEncrypted == true) { + var decryptedValue = await _getDecryptedValue(atKey, atKey.key); + atNotification.key = '${atNotification.to}:$decryptedValue'; + } + // do not decrypt if isEncrypted is null or set to false return atNotification; } else if ((atNotification.value.isNotNull) && (config.shouldDecrypt && atNotification.id != '-1') && From 841a2f338a555485728a2692c5096741e2872125 Mon Sep 17 00:00:00 2001 From: Murali Date: Mon, 5 Aug 2024 15:33:48 +0530 Subject: [PATCH 2/6] fix: skip decrypting notification key if isEncrypted is explicitly set to false --- .../notification_response_transformer.dart | 13 +++++++--- .../test/notification_service_test.dart | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart index 3a151172c..c5ea69088 100644 --- a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart +++ b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart @@ -57,14 +57,13 @@ class NotificationResponseTransformer if (atNotification.messageType.isNotNull && atNotification.messageType!.toLowerCase().contains('text')) { - // decrypt the text message if isEncrypted is true - if (atNotification.isEncrypted != null && atNotification.isEncrypted == true) { + if (_shouldDecryptKey(atNotification.isEncrypted)) { var decryptedValue = await _getDecryptedValue(atKey, atKey.key); atNotification.key = '${atNotification.to}:$decryptedValue'; } - // do not decrypt if isEncrypted is null or set to false return atNotification; - } else if ((atNotification.value.isNotNull) && + } + if ((atNotification.value.isNotNull) && (config.shouldDecrypt && atNotification.id != '-1') && // The shared_key (which is a reserved key) has different decryption process // and is not a user created key. @@ -78,6 +77,12 @@ class NotificationResponseTransformer return atNotification; } + // Decrypt the notification key only if isEncrypted false is set to true or + // not set(for backward compatibility where default behavior is encrypt the notification key) + bool _shouldDecryptKey(bool? isEncrypted) { + return isEncrypted == null || isEncrypted == true; + } + Future _getDecryptedValue(AtKey atKey, String? encryptedValue) async { atKeyDecryption ??= AtKeyDecryptionManager(_atClient).get(atKey, atKey.sharedWith!); diff --git a/packages/at_client/test/notification_service_test.dart b/packages/at_client/test/notification_service_test.dart index fb3f570c3..b84484e70 100644 --- a/packages/at_client/test/notification_service_test.dart +++ b/packages/at_client/test/notification_service_test.dart @@ -275,6 +275,31 @@ void main() { expect(transformedNotification.key, '@bob:encryptedValue'); }); + test( + 'A test to verify notification text is decrypted when isEncrypted is set to null', + () async { + bool? isEncrypted; + var atNotification = at_notification.AtNotification( + '124', + '@bob:encryptedValue', + '@alice', + '@bob', + DateTime.now().millisecondsSinceEpoch, + MessageTypeEnum.text.toString(), + isEncrypted); + var notificationResponseTransformer = + NotificationResponseTransformer(mockAtClientImpl); + notificationResponseTransformer.atKeyDecryption = mockSharedKeyDecryption; + + var transformedNotification = + await notificationResponseTransformer.transform(Tuple() + ..one = atNotification + ..two = (NotificationConfig() + ..regex = '.*' + ..shouldDecrypt = true)); + expect(transformedNotification.key, '@bob:decryptedValue'); + }); + test( 'A test to verify notification key is decrypted when shouldDecrypt is set to true', () async { From c947cf90c09dd45f1cc9762c1dcee55023cdc303 Mon Sep 17 00:00:00 2001 From: Murali Date: Mon, 5 Aug 2024 15:45:46 +0530 Subject: [PATCH 3/6] fix: dart doc minor changes --- .../notification_response_transformer.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart index c5ea69088..3367a8ba5 100644 --- a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart +++ b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart @@ -77,8 +77,8 @@ class NotificationResponseTransformer return atNotification; } - // Decrypt the notification key only if isEncrypted false is set to true or - // not set(for backward compatibility where default behavior is encrypt the notification key) + // Decrypt the notification key only if isEncrypted flag is set to true or + // null (for backward compatibility - default behavior is decrypt the notification key) bool _shouldDecryptKey(bool? isEncrypted) { return isEncrypted == null || isEncrypted == true; } From de0932f3b14f9117cd6786d97ac4f6b61ab7672a Mon Sep 17 00:00:00 2001 From: Murali Date: Mon, 12 Aug 2024 12:32:38 +0530 Subject: [PATCH 4/6] fix: review comments --- .../notification_response_transformer.dart | 22 ++++++++++--------- .../test/notification_service_test.dart | 17 +++++++------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart index 3367a8ba5..e3a1daa48 100644 --- a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart +++ b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart @@ -56,22 +56,24 @@ class NotificationResponseTransformer } if (atNotification.messageType.isNotNull && - atNotification.messageType!.toLowerCase().contains('text')) { - if (_shouldDecryptKey(atNotification.isEncrypted)) { - var decryptedValue = await _getDecryptedValue(atKey, atKey.key); - atNotification.key = '${atNotification.to}:$decryptedValue'; - } + atNotification.messageType!.toLowerCase().contains('text') && + (atNotification.isEncrypted != null && atNotification.isEncrypted!)) { + // decrypt the text message; + var decryptedValue = await _getDecryptedValue(atKey, atKey.key); + atNotification.key = '${atNotification.to}:$decryptedValue'; return atNotification; } - if ((atNotification.value.isNotNull) && - (config.shouldDecrypt && atNotification.id != '-1') && + if (atNotification.value.isNotNull && + atNotification.id != '-1' && // The shared_key (which is a reserved key) has different decryption process // and is not a user created key. // Hence do not decrypt if key's are reserved keys AtKey.getKeyType(atKey.toString()) != KeyType.reservedKey) { - // decrypt the value - atNotification.value = - await _getDecryptedValue(atKey, atNotification.value!); + // decrypt the notification value only if isEncrypted is not set to false + if (atNotification.isEncrypted != false) { + atNotification.value = + await _getDecryptedValue(atKey, atNotification.value!); + } return atNotification; } return atNotification; diff --git a/packages/at_client/test/notification_service_test.dart b/packages/at_client/test/notification_service_test.dart index b84484e70..7494ab32c 100644 --- a/packages/at_client/test/notification_service_test.dart +++ b/packages/at_client/test/notification_service_test.dart @@ -276,17 +276,18 @@ void main() { }); test( - 'A test to verify notification text is decrypted when isEncrypted is set to null', + 'A test to verify notification value is decrypted when isEncrypted is set to null', () async { bool? isEncrypted; var atNotification = at_notification.AtNotification( '124', - '@bob:encryptedValue', + 'key-1', '@alice', '@bob', DateTime.now().millisecondsSinceEpoch, - MessageTypeEnum.text.toString(), - isEncrypted); + MessageTypeEnum.key.toString(), + isEncrypted, + value: 'encryptedValue'); var notificationResponseTransformer = NotificationResponseTransformer(mockAtClientImpl); notificationResponseTransformer.atKeyDecryption = mockSharedKeyDecryption; @@ -297,13 +298,13 @@ void main() { ..two = (NotificationConfig() ..regex = '.*' ..shouldDecrypt = true)); - expect(transformedNotification.key, '@bob:decryptedValue'); + expect(transformedNotification.value, 'decryptedValue'); }); test( - 'A test to verify notification key is decrypted when shouldDecrypt is set to true', + 'A test to verify notification value is decrypted when isEncrypted is set to true', () async { - var isEncrypted = false; + bool isEncrypted = true; var atNotification = at_notification.AtNotification( '124', 'key-1', @@ -327,7 +328,7 @@ void main() { }); test( - 'A test to verify notification key is not decrypted when shouldDecrypt is set to false', + 'A test to verify notification value is not decrypted when isEncrypted is set to false', () async { var isEncrypted = false; var atNotification = at_notification.AtNotification( From ed887c6acd03bf917360cab12645b7a25d663f19 Mon Sep 17 00:00:00 2001 From: Murali Date: Mon, 12 Aug 2024 12:41:52 +0530 Subject: [PATCH 5/6] fix: analyzer issue --- .../notification_response_transformer.dart | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart index e3a1daa48..91ea5be02 100644 --- a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart +++ b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart @@ -22,7 +22,6 @@ class NotificationResponseTransformer Tuple tuple) async { // prepare the atKey from the atNotification object. AtNotification atNotification = tuple.one; - NotificationConfig config = tuple.two; String sharedBy = atNotification.from; String sharedWith = atNotification.to; var key = atNotification.key; @@ -79,12 +78,6 @@ class NotificationResponseTransformer return atNotification; } - // Decrypt the notification key only if isEncrypted flag is set to true or - // null (for backward compatibility - default behavior is decrypt the notification key) - bool _shouldDecryptKey(bool? isEncrypted) { - return isEncrypted == null || isEncrypted == true; - } - Future _getDecryptedValue(AtKey atKey, String? encryptedValue) async { atKeyDecryption ??= AtKeyDecryptionManager(_atClient).get(atKey, atKey.sharedWith!); From c7ceb27877e7ff3b611cba21b9f73de45a24dbd8 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 13 Aug 2024 11:13:16 +0530 Subject: [PATCH 6/6] fix: review comments - add shouldDecrypt check --- .../notification_response_transformer.dart | 6 +++-- .../test/notification_service_test.dart | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart index 91ea5be02..4055d4875 100644 --- a/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart +++ b/packages/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart @@ -22,6 +22,7 @@ class NotificationResponseTransformer Tuple tuple) async { // prepare the atKey from the atNotification object. AtNotification atNotification = tuple.one; + NotificationConfig notificationConfig = tuple.two; String sharedBy = atNotification.from; String sharedWith = atNotification.to; var key = atNotification.key; @@ -68,8 +69,9 @@ class NotificationResponseTransformer // and is not a user created key. // Hence do not decrypt if key's are reserved keys AtKey.getKeyType(atKey.toString()) != KeyType.reservedKey) { - // decrypt the notification value only if isEncrypted is not set to false - if (atNotification.isEncrypted != false) { + // decrypt the notification value only if isEncrypted is not set to false and shouldDecrypt is set to true + if (atNotification.isEncrypted != false && + notificationConfig.shouldDecrypt) { atNotification.value = await _getDecryptedValue(atKey, atNotification.value!); } diff --git a/packages/at_client/test/notification_service_test.dart b/packages/at_client/test/notification_service_test.dart index 7494ab32c..9e3e9093e 100644 --- a/packages/at_client/test/notification_service_test.dart +++ b/packages/at_client/test/notification_service_test.dart @@ -353,6 +353,32 @@ void main() { expect(transformedNotification.value, 'encryptedValue'); }); + test( + 'A test to verify notification value is not decrypted when isEncrypted is set to true and should decrypt is false', + () async { + var isEncrypted = true; + var atNotification = at_notification.AtNotification( + '124', + 'key-1', + '@alice', + '@bob', + DateTime.now().millisecondsSinceEpoch, + MessageTypeEnum.key.toString(), + isEncrypted, + value: 'encryptedValue'); + var notificationResponseTransformer = + NotificationResponseTransformer(mockAtClientImpl); + notificationResponseTransformer.atKeyDecryption = mockSharedKeyDecryption; + + var transformedNotification = + await notificationResponseTransformer.transform(Tuple() + ..one = atNotification + ..two = (NotificationConfig() + ..regex = '.*' + ..shouldDecrypt = false)); + expect(transformedNotification.value, 'encryptedValue'); + }); + test('A test to verify notification is returned as is', () async { var isEncrypted = false; var atNotification = at_notification.AtNotification(