at_server icon indicating copy to clipboard operation
at_server copied to clipboard

Setting `isEncrypted:false` in notify: verb doesn't actually set it to `false`.

Open JeremyTubongbanua opened this issue 9 months ago • 6 comments

image

In the above image, I sent notification with isEncrypted:false

Then in the receiving notification has isEncrypted:true

image

Intended behaviour:

Running notify:isEncrypted:false:<...>

Should have a receiving notification of:

notification:{..., "isEncrypted":false, ...}

JeremyTubongbanua avatar May 08 '24 21:05 JeremyTubongbanua

  • P1 to look at it, then reevaluate the priority of the underlying issue.

XavierChanth avatar May 13 '24 15:05 XavierChanth

Could not get to this in the previous sprint, will be taking this up first in PR88.

srieteja avatar May 28 '24 06:05 srieteja

Hi @srieteja I've been looking at this as it came up in another context. I'll pick it up this sprint.

Summary of the current behaviour for 'update' notifications

  • at_secondary_server NotifyVerbHandler._getIsEncrypted ignores whatever it has been sent and always sets isEncrypted to 'true'
  • at_client NotificationServiceImpl.notify always encrypts the value but does not set isEncrypted in the metadata. Thus, usually (unless app code explicitly sets metadata.isEncrypted to true), metadata.isEncrypted is apparently false (default value) but the value has in fact been encrypted
  • at_commons NotifyVerbBuilder.buildCommand only includes isEncrypted in the generated command if metadata.isEncrypted is true (which it ~never is, right now)
  • at_secondary_server ResourceManager.prepareNotifyCommandBody only includes isEncrypted in the generated command if isEncrypted is true

Summary of the changes I have in mind:

  • at_secondary_server: these are the most important changes. Once implemented, the problem Jeremy encountered will be resolved
    • NotifyVerbHandler._getIsEncrypted
      • if a value for isEncrypted was set in the received command, respect it, otherwise default it to 'true' (current behaviour)
    • ResourceManager.prepareNotifyCommandBody
      • always include isEncrypted in the generated command
  • at_client
    • NotificationServiceImpl.notify
      • add an optional bool parameter encryptValue default it to true
      • set the metadata.isEncrypted to the value of encryptValue
    • NotificationRequestTransformer.transform
      • if metadata.isEncrypted is true then encrypt the value, otherwise do not
  • at_commons
    • NotifyVerbBuilder.buildCommand
      • always include isEncrypted in the command (currently it is only sent if isEncrypted is true, which it typically never is, right now)

In the distant future, once all of the client-side sdks are doing the right thing with their generated notify commands, we can default isEncrypted to 'false' if it is not set in the command but for now we have to continue to default it to 'true' so we don't break the world.

I need to think a little more about the sequencing of the at_commons and at_client changes; what we want to ensure is that we only ever send a notify command with isEncrypted:false when that is genuinely the case. We will likely need to do an at_commons major version change, although we may be able to do something fiddly in at_client to ensure the right behaviour

gkc avatar Jun 10 '24 17:06 gkc

See also https://github.com/atsign-foundation/at_client_sdk/issues/1332

gkc avatar Jun 10 '24 17:06 gkc

gkc avatar Jul 08 '24 07:07 gkc

gkc avatar Jul 22 '24 12:07 gkc

@JeremyTubongbanua please verify this issue with latest published version of at_client

murali-shris avatar Sep 16 '24 06:09 murali-shris

Sure, will pick this up this sprint @murali-shris

JeremyTubongbanua avatar Sep 30 '24 14:09 JeremyTubongbanua

Dart

Summary

I sent a notification with this key (with isEncrypted = true

AtKey sharedRecordID = AtKey()
    ..key = keyName
    ..sharedBy = myAtsign
    ..sharedWith = otherAtsign
    ..namespace = nameSpace
    ..metadata = (Metadata()
      ..isEncrypted = true
      ..ttl = 60000
      ..ttr = -1);

And successfully received it

INFO|2024-10-07 11:31:23.957379| at_notify |message received from @jeremy_0 notification id : d3a8502a-1823-4449-b7f2-541a67276e0d 
@jeremy_0: hi
``

**Full logs**

pubspec.yaml
```yaml
name: dart_playground
description: A sample command-line application.
version: 1.0.0
# repository: https://github.com/my_org/my_repo

environment:
  sdk: ^3.5.0

# Add regular dependencies here.
dependencies:
  args: ^2.5.0
  at_cli_commons: ^1.1.0
  at_client: ^3.2.2
  at_onboarding_cli: ^1.6.4
  # path: ^1.8.0

dev_dependencies:
  lints: ^4.0.0
  test: ^1.24.0

Sender

// dart packages
import 'dart:io';

// atPlatform packages
import 'package:at_client/at_client.dart';
import 'package:at_cli_commons/at_cli_commons.dart';

// External Packages
import 'package:args/args.dart';

Future<void> main(List<String> args) async {
  ArgParser argsParser = CLIBase.argsParser
    ..addOption('other-atsign',
        abbr: 'o',
        mandatory: true,
        help: 'The atSign we want to communicate with')
    ..addOption('message',
        abbr: 'm', mandatory: true, help: 'The message we want to send');

  late final AtClient atClient;
  late final String nameSpace, myAtsign, otherAtsign, message;
  try {
    var parsed = argsParser.parse(args);
    otherAtsign = parsed['other-atsign'];
    message = parsed['message'];

    CLIBase cliBase =
        await CLIBase.fromCommandLineArgs(args, parser: argsParser);
    atClient = cliBase.atClient;

    nameSpace = atClient.getPreferences()!.namespace!;
    myAtsign = atClient.getCurrentAtSign()!;
  } catch (e) {
    print(argsParser.usage);
    print(e);
    exit(1);
  }

  String keyName = 'message';

  AtKey sharedRecordID = AtKey()
    ..key = keyName
    ..sharedBy = myAtsign
    ..sharedWith = otherAtsign
    ..namespace = nameSpace
    ..metadata = (Metadata()
      ..isEncrypted = true
      ..ttl = 60000
      ..ttr = -1);

  await atClient.notificationService.notify(
      NotificationParams.forUpdate(sharedRecordID, value: message),
      waitForFinalDeliveryStatus: false,
      checkForFinalDeliveryStatus: false);

  // Using notifications we get all the updates
  // for (int a = 1; a < 101; a++) {
  //   await atClient.notificationService.notify(
  //       NotificationParams.forUpdate(sharedRecordID, value: a.toString()),
  //       waitForFinalDeliveryStatus: false,
  //       checkForFinalDeliveryStatus: false);
  // }

  await Future.delayed(Duration(seconds: 1));
  exit(0);
}

Receiver

// dart packages
import 'dart:io';

// atPlatform packages
import 'package:at_client/at_client.dart';
import 'package:at_cli_commons/at_cli_commons.dart';
import 'package:at_utils/at_logger.dart';

// External Packages
import 'package:chalkdart/chalk.dart';

Future<void> main(List<String> args) async {
  late final AtClient atClient;
  late final String nameSpace, myAtsign;
  try {
    CLIBase cliBase = await CLIBase.fromCommandLineArgs(args);
    atClient = cliBase.atClient;

    nameSpace = atClient.getPreferences()!.namespace!;
    myAtsign = atClient.getCurrentAtSign()!;
  } catch (e) {
    print(CLIBase.argsParser.usage);
    print(e);
    exit(1);
  }

  final AtSignLogger logger = AtSignLogger(' at_notify ');

  atClient.notificationService
      .subscribe(regex: 'message.$nameSpace@', shouldDecrypt: true)
      .listen(((notification) async {
    stdout.writeln(chalk.blue(notification.key));
    String keyName = notification.key
        .replaceAll('${notification.to}:', '')
        .replaceAll('.$nameSpace${notification.from}', '');
    if (keyName == 'message') {
      logger.info(
          'message received from ${notification.from} notification id : ${notification.id}');
      var talk = notification.value;
      print(chalk.brightGreen.bold('\r\x1b[K${notification.from}: ') +
          chalk.brightGreen(talk));

      stdout.write(chalk.brightYellow('$myAtsign: '));
    }
  }),
          onError: (e) => logger.severe('Notification Failed:$e'),
          onDone: () => logger.info('Notification listener stopped'));
}

Receiver logs

dart_playground dart run bin/receiver.dart -a @smoothalligator -n dart_playground -v
Connecting ... INFO|2024-10-07 11:31:18.625480|AtLookup|Creating new connection 
INFO|2024-10-07 11:31:18.839638|AtLookup|New connection created OK 
INFO|2024-10-07 11:31:18.946834|AtLookup|auth success 
INFO|2024-10-07 11:31:18.948024|AtClientManager|setCurrentAtSign called with atSign @smoothalligator 
INFO|2024-10-07 11:31:18.948202|AtClientManager|Switching atSigns from null to @smoothalligator 
INFO|2024-10-07 11:31:18.969193|HiveBase|commit_log_0fe9fb2745a1a4a3f9c43a7dd8ac860e8291d5bafda9f25df26b45a1ad92726e initialized successfully 
INFO|2024-10-07 11:31:18.978238|HiveBase|0fe9fb2745a1a4a3f9c43a7dd8ac860e8291d5bafda9f25df26b45a1ad92726e initialized successfully 
INFO|2024-10-07 11:31:19.008244|AtClientCommitLogCompaction (@smoothalligator)|Starting commit log compaction job running for every 11 minute(s) 
INFO|2024-10-07 11:31:19.024572|AtClientManager|setCurrentAtSign complete 
Connected
INFO|2024-10-07 11:31:19.050899|Monitor (@smoothalligator)|starting monitor for @smoothalligator with lastNotificationTime: 1728315065048 
INFO|2024-10-07 11:31:19.296285|Monitor (@smoothalligator)|monitor started for @smoothalligator with last notification time: 1728315065048 
@smoothalligator:message.dart_playground@jeremy_0
INFO|2024-10-07 11:31:23.957379| at_notify |message received from @jeremy_0 notification id : d3a8502a-1823-4449-b7f2-541a67276e0d 
@jeremy_0: hi
@smoothalligator:

Sender logs

dart_playground dart run bin/dart_playground.dart -a @jeremy_0 -o @smoothalligator -m hi -n dart_playground -v
Connecting ... INFO|2024-10-07 11:31:23.034396|AtLookup|Creating new connection 
INFO|2024-10-07 11:31:23.294716|AtLookup|New connection created OK 
INFO|2024-10-07 11:31:23.400184|AtLookup|auth success 
INFO|2024-10-07 11:31:23.401164|AtClientManager|setCurrentAtSign called with atSign @jeremy_0 
INFO|2024-10-07 11:31:23.401307|AtClientManager|Switching atSigns from null to @jeremy_0 
INFO|2024-10-07 11:31:23.415247|HiveBase|commit_log_3c074c7d3712aa56c1d3e81561c463442f282dfda4ddcc62a724ea0844c2f0e0 initialized successfully 
INFO|2024-10-07 11:31:23.420138|HiveBase|3c074c7d3712aa56c1d3e81561c463442f282dfda4ddcc62a724ea0844c2f0e0 initialized successfully 
INFO|2024-10-07 11:31:23.432690|AtClientCommitLogCompaction (@jeremy_0)|Starting commit log compaction job running for every 11 minute(s) 
INFO|2024-10-07 11:31:23.448292|AtClientManager|setCurrentAtSign complete 
Connected
INFO|2024-10-07 11:31:23.595595|AtLookup|Creating new connection 
INFO|2024-10-07 11:31:23.765248|AtLookup|New connection created OK 
INFO|2024-10-07 11:31:23.848275|AtLookup|auth success 
➜  dart_playground 

JeremyTubongbanua avatar Oct 07 '24 15:10 JeremyTubongbanua

I plan to do a check with the C repl now this spritn

JeremyTubongbanua avatar Oct 14 '24 14:10 JeremyTubongbanua

Monitor receiving end

[DEBG] 2024-10-15 00:49:57.508558 | connection |        SENT: "monitor"
[DEBG] 2024-10-15 00:49:58.449249 | connection | pos: 0, ret: 389
[DEBG] 2024-10-15 00:49:58.449304 | connection |        RECV: "notification: {"id":"65c71a9c-1c35-477a-a9b0-30dd88d025b0","from":"@jeremy_0","to":"@smoothalligator","key":"@smoothalligator:test.c_playground@jeremy_0","value":"asd","operation":"update","epochMillis":1728953398431,"messageType":"MessageType.key","isEncrypted":true,"metadata":{"encKeyName":null,"encAlgo":null,"ivNonce":null,"skeEncKeyName":null,"skeEncAlgo":null,"sharedKeyEnc":null}}"
recv (388): notification: {"id":"65c71a9c-1c35-477a-a9b0-30dd88d025b0","from":"@jeremy_0","to":"@smoothalligator","key":"@smoothalligator:test.c_playground@jeremy_0","value":"asd","operation":"update","epochMillis":1728953398431,"messageType":"MessageType.key","isEncrypted":true,"metadata":{"encKeyName":null,"encAlgo":null,"ivNonce":null,"skeEncKeyName":null,"skeEncAlgo":null,"sharedKeyEnc":null}}

Notification sending end

[DEBG] 2024-10-15 00:49:57.807150 | connection |        SENT: "notify:update:isEncrypted:true:@smoothalligator:test.c_playground@jeremy_0:asd"
[DEBG] 2024-10-15 00:49:57.847440 | connection | pos: 0, ret: 52
[DEBG] 2024-10-15 00:49:57.847498 | connection |        RECV: "data:65c71a9c-1c35-477a-a9b0-30dd88d025b0"
recv (41): data:65c71a9c-1c35-477a-a9b0-30dd88d025b0

notiec that in the Monitor receiving end, we get isEncrypted":true

which means that the metadata was properly set by the atServer

I will go ahead and close the ticket as I have found the feature to be working from my own 2 manual tests

JeremyTubongbanua avatar Oct 15 '24 00:10 JeremyTubongbanua