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

Private link feature request #264

Closed
postacik opened this issue Aug 7, 2019 · 21 comments
Closed

Private link feature request #264

postacik opened this issue Aug 7, 2019 · 21 comments

Comments

@postacik
Copy link

postacik commented Aug 7, 2019

Hi,
We have a feature request for the create link function.

When we create a link on the client side and publish a message to that link, the server answers the message but there is no generic way to match the request and the response on the client side and we need an "id" to match the request with the response.

So here is our proposition:

The server app creates a key for the private link main channel:

emitter.keygen({ key: secretKey, channel: "rpc/", type: "e", ttl: 0 });

Emitter generates a key which has permission to create a link on channel "rpc/".

The key is provided to the client application.

The client sends a create link request to emitter:

emitter.link({ key: key, channel: "rpc/", subscribe: true, ttl: 0 });
Note: name, me and private options are truncated since they are not needed.

If subscribe option = true, Emitter automatically subscribes user to channel "rpc/{userid}/".

Emitter creates a key with "w" permission on "rpc/{userid}/#".

Emitter returns the channel name "rpc/{userid}" and the generated key in response to the link request.

Now the client can publish messages to "rpc/{userid}/{requestid}" using the key provided.

emitter.publish({ key: key, channel: "rpc/{userid}/{requestid}", message: "A sample request", me: false });
Using this mechanism, no shortcut is needed. Private messages are sent via normal publish messages.

The server subscribes to channel "rpc/" and responds to messages it receives on this channel with the same channel name which also includes the request id.

This mechanism can be added as an alternative link function with a name like "link2" not to break current implementations.

Does it sound reasonable?

@kelindar
Copy link
Contributor

kelindar commented Aug 8, 2019

Not sure whether the link function is a right approach for this. What's exactly is missing, the generated key which can read/write on the private channel? We could do something like this instead:

Server which can use the secret key to generate an extendable key for a client:

clientKey = emitter.keygen({ key: secretKey, channel: "rpc/", type: "rwe", ttl: 0 });

Client ships with the clientKey which then can be used in a keygen to generate a key for the private channel, in this case for rpc/{userid}:

key, channelName = emitter.keygen({ key: clientKey, type: "rw", ttl: 0 });
emitter.subscribe({ key: key, channel: channelName })
emitter.publish({ key: key, channel: channelName + "{requestid}" })

Would this work for your use-case?

@postacik
Copy link
Author

postacik commented Aug 9, 2019

Your approach seems to establish the same result and it is just simpler.

But it adds a second meaning to the extended key: It can create a link + it can create a key.

If this is not going to confuse the API user, it's perfectly fine.

And you are also changing the keygen function to be able to operate without a channel parameter which may also confuse the API user.

Is it easy on the server side to match this key with the "rpc/{userid}/" channel?

Besides these concerns, your approach looks much better than mine. :)

And does this approach also cover the functionality of "link"?

@postacik
Copy link
Author

postacik commented Aug 9, 2019

Something else:

The following generated key must not read or write on "rpc/" but I think this is what you did in the last PR commit. Am I right?

clientKey = emitter.keygen({ key: secretKey, channel: "rpc/", type: "rwe", ttl: 0 });

@postacik
Copy link
Author

Do you intend to implement this feature?

@kelindar
Copy link
Contributor

Yes, but we need to think a bit more on how to properly design it in a consistent way. This may make private links redundant and there should be only one way of doing the same thing for simplicity.

@postacik
Copy link
Author

Great news.

I think this is a better way of creating private channels with a single "keygen" command.

@postacik
Copy link
Author

Do you have a time plan for this feature?

@postacik
Copy link
Author

Any progress for this issue?

@kelindar
Copy link
Contributor

Around mid September for the timeline. I'm a bit swamped at work these days.

@postacik
Copy link
Author

Ok thanks. :)

@postacik
Copy link
Author

Hi @kelindar , any progress? :)

@kelindar
Copy link
Contributor

@postacik WIP here: #268

@postacik
Copy link
Author

How can I test this new commit?

@Florimond
Copy link
Member

Given that you already are in the folder where you pulled Emitter, just git checkout feat-keygen. Or do you have another issue?

@postacik
Copy link
Author

Ok, I did not notice it was in the feat-keygen branch. Testing...

@postacik
Copy link
Author

postacik commented Sep 16, 2019

I made a small test with the following code:

            var keyResult1 = await emitter.keygen({ key: secretKey, channel: "rpc/", type: "rwe", ttl: 0 });
            var keyResult2 = await emitter.keygen({ key: keyResult1.key, channel: "rpc/", type: "rw", ttl: 0 });
            var rpcKey = keyResult2.key;
            var rpcChannel = keyResult2.channel;
            emitter.subscribe({ key: rpcKey, channel: rpcChannel });
            emitter.publish({ key: rpcKey, channel: rpcChannel, message: "{ }" });
            emitter.publish({ key: rpcKey, channel: rpcChannel + "id123/", message: "{ }" });

The first publish works but the second one returns the following error:

{status: 401, message: "the security key provided is not authorized to perform this operation"}

And in the second keygen request, the server returns an error if I don't specify the channel as "rpc/".

Is there anything wrong in my code?

@kelindar
Copy link
Contributor

kelindar commented Sep 16, 2019

This was expected as it was not supporting extending further. I just made a small change to make it more flexible - could you test? The following code should work:

var keyResult1 = await emitter.keygen({ key: secretKey, channel: "rpc/#/", type: "rwe", ttl: 0 }); <---
var keyResult2 = await emitter.keygen({ key: keyResult1.key, channel: "rpc/#/", type: "rw", ttl: 0 }); <---
var rpcKey = keyResult2.key;
var rpcChannel = keyResult2.channel.slice(0, keyResult2.channel.length-2); <---
emitter.subscribe({ key: rpcKey, channel: rpcChannel });
emitter.publish({ key: rpcKey, channel: rpcChannel, message: "{ }" });
emitter.publish({ key: rpcKey, channel: rpcChannel + "id123/", messag

@postacik
Copy link
Author

It worked this way.

Is this the way this new feature should be used?

Are there any other tests you want to be made?

@postacik
Copy link
Author

Are you planning to merge this into the master branch?

@Florimond
Copy link
Member

@postacik I have to do some tests too this w-e, then it will be merged.

@postacik
Copy link
Author

Thanks. I'm currently testing my RPC functions with the new way. It helped to overcome my security concerns very elegantly. :)

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

No branches or pull requests

3 participants