Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can not set RealTimeChannelOptions params #182

Closed
1N50MN14 opened this issue Oct 1, 2021 · 7 comments · Fixed by #190
Closed

Can not set RealTimeChannelOptions params #182

1N50MN14 opened this issue Oct 1, 2021 · 7 comments · Fixed by #190

Comments

@1N50MN14
Copy link

1N50MN14 commented Oct 1, 2021

Because cipher is not nullable I'm unable to pass {'rewind':'1'} to RealtimeChannelOptions without using cipher.

RealtimeChannelOptions RealtimeChannelOptions(
  Object cipher, {
  Map<String, String>? params,
  List<ChannelMode>? modes,
})

Otherwise I could not find a reference within the flutter example how to use one...

Upon further investigation, CipherParams seems to be abstract. Crypto interface doesn't seem to be implemented so I couldn't use getDefaultParams using my server-side generated key. So I tried to manually pass cipherParams as a Map but that threw a methodChannel error as well.

Basically locked out of symmetric encryption as well as channel options...

┆Issue is synchronized with this Jira Uncategorised by Unito

@ben-xD
Copy link
Contributor

ben-xD commented Oct 6, 2021

Thanks for reporting this @1N50MN14. I'm working on the fix for this: to let you use channel options properly. The symmetric encryption feature is currently not implemented, and you should not be forced to set cipher. However, channel options (Params and mode) should already be working. Unfortunately, in Ably-java they are both not handled correctly at the moment.

In Ably-java: Unfortunately, the params passed by you (the user) is used as CipherParams instead, and the modes is simply not set.

In Ably-cocoa: It looks like this should work fine.

So I tried to manually pass cipherParams as a Map but that threw a methodChannel error as well.

I'm not sure what you mean by passing cipherParams manually, can you clarify?

@1N50MN14
Copy link
Author

1N50MN14 commented Oct 6, 2021

I'm not sure what you mean by passing cipherParams manually, can you clarify?

My understanding reading the documentation cipher is an object that consists of key, algorithm, keyLength and mode. It's referred to as CipherParams here https://ably.com/documentation/realtime/encryption#cipher-params. Both server and client need to use the same object to correctly decode messages.

In Node.js I can generate a key using Crypto.generateRandomKey() and pass it to Crypto.getDefaultParams() to get that. From there, I can utilize it like so channels.get(channelName, {cipher: cipherParams, rewind:1, ...}).

In ably-flutter the equivalent of that would be channels.get(String channelName)..setOptions(RealtimeOption options) and I'm trying to either pass the same cipher object or re-recreate it (directly using the server generated cipher key) so I could pass that to channels.get().

RealtimeChannelOptions exposes Object cipher and optional Map<String, String>? params, List<ChannelMode>? modes. Following the above logic, and seeing you do actually expose a CipherParams class, my first instinct was to pass that but the class CipherParams is not implemented and Object cipher should've been CipherParams cipher. Object can be anything so I tried to manually construct a Map that consists of key, algorithm, keyLength and mode and pass that, didn't work either. params is where I thought my {'rewind':'1'} goes. I ignored List<ChannelMode because my auth callback defines that.

In Ably-java: Unfortunately, the params passed by you (the user) is used as CipherParams instead, and the modes is simply not set.

That's the thing, if params is used as CipherParams then what should one pass Object cipher (and how would passing something like {'rewind':'1'} work).

I see you have your hands busy with a big PR (push notifications) to merge, I hope this one doesn't create a headache for you, if so we can work around this one until after the next release.

Many thanks

@ben-xD
Copy link
Contributor

ben-xD commented Oct 6, 2021

Unfortunately Ably Flutter doesn't support symmetric encryption yet, which is why you faced those issues. If you want Ably Flutter to read those messages currently, you cannot encrypt messages. After investigating the bugs above, it looks like symmetric encryption will be relatively easy to implement (maybe within the next release). The bugs I mentioned above will be fixed first though.

That's the thing, if params is used as CipherParams then what should one pass Object cipher (and how would passing something like {'rewind':'1'} work).

The bug means the params you pass is completely useless, so IMHO it is quite serious bug. Which is why I have fixed it but the PR is not yet up.


I found another issue with ChannelOptions. It simply doesn't get sent to the Android side at all. This is because the following code:

int? getCodecType(final Object? value) {
if (value is ClientOptions) {
return CodecTypes.clientOptions;
} else if (value is TokenDetails) {
return CodecTypes.tokenDetails;
} else if (value is TokenParams) {
return CodecTypes.tokenParams;
} else if (value is TokenRequest) {
return CodecTypes.tokenRequest;
} else if (value is MessageData) {
return CodecTypes.messageData;
} else if (value is MessageExtras) {
return CodecTypes.messageExtras;
} else if (value is Message) {
return CodecTypes.message;
} else if (value is RealtimeHistoryParams) {
return CodecTypes.realtimeHistoryParams;
} else if (value is RestHistoryParams) {
return CodecTypes.restHistoryParams;
} else if (value is RestPresenceParams) {
return CodecTypes.restPresenceParams;
} else if (value is RealtimePresenceParams) {
return CodecTypes.realtimePresenceParams;
} else if (value is ErrorInfo) {
return CodecTypes.errorInfo;
} else if (value is AblyMessage) {
return CodecTypes.ablyMessage;
} else if (value is AblyEventMessage) {
return CodecTypes.ablyEventMessage;
}
// ignore: avoid_returning_null
return null;
}

is missing

else if (value is RealtimeChannelOptions) {
      return CodecTypes.realtimeChannelOptions;
    } else if (value is ChannelOptions) {
      return CodecTypes.restChannelOptions;
    }

This results in you getting an error: Invalid argument: Instance of 'RealtimeChannelOptions'

I've actually fixed these bugs and will have a PR up asap.

@1N50MN14
Copy link
Author

1N50MN14 commented Oct 7, 2021

This results in you getting an error: Invalid argument: Instance of 'RealtimeChannelOptions'

Ah that totally explains why it didn't work all together, I am indeed using the Android simulator.

@1N50MN14
Copy link
Author

Hey @ben-xD just wanted to follow up on pull #184 and hear if there's an ETA for a new version pub.dev ;) Thank you ;)

@ben-xD
Copy link
Contributor

ben-xD commented Oct 27, 2021

Hey @1N50MN14

I'm sorry for the delay. I've been away for a while, but merging this bug fix, and creating a release with this fix is my priority. As such, today I've just fixed a failing test affecting a PR. Now, I will merge most outstanding PRs, and create the release. The release will not include symmetric encryption.

@1N50MN14
Copy link
Author

No worries at all @ben-xD - getting rid of the "scary" remote auth error message is good start for me to use Ably in production, hopefully this one is included in the next release. I'll switch over to using symmetric encryption when it's ready ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants