Rantanen / node-mumble

Mumble client in Node.js
MIT License
155 stars 48 forks source link

Register user #104

Closed Prior99 closed 6 years ago

Prior99 commented 6 years ago

Is it possible, using node-mumble to register another user if the user which node-mumble authenticated with has the necessary permissions to do so (e.g. connected as SuperUser)? I could neither find a method for this nor something in mumbles Protocol buffer.

Rantanen commented 6 years ago

Hum. It should be possible since the Mumble client does this already and the only thing it has is the protocol buffer interfaces.

Rantanen commented 6 years ago

https://github.com/mumble-voip/mumble/blob/master/src/murmur/Messages.cpp#L730-L749

Seems to be just a case of sending an updated user info with the user_id filled. Don't think the actual ID is used so as long as it's something else than null it should work I'd guess. Can you try this? I would welcome some sort of pull request to have a .register() method on the user objects if you get this working.

Prior99 commented 6 years ago

Sure. I will take a look at it.

On Thu, May 3, 2018, 23:45 Mikko Rantanen notifications@github.com wrote:

https://github.com/mumble-voip/mumble/blob/master/src/murmur/Messages.cpp#L730-L749

Seems to be just a case of sending an updated user info with the user_id filled. Don't think the actual ID is used so as long as it's something else than null it should work I'd guess. Can you try this? I would welcome some sort of pull request to have a .register() method on the user objects if you get this working.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Rantanen/node-mumble/issues/104#issuecomment-386447170, or mute the thread https://github.com/notifications/unsubscribe-auth/ABbCdhEcZbQu5J9A0fZE4BeeOaE6V81Qks5tu3qKgaJpZM4Txu-X .

Prior99 commented 6 years ago

See #105. The id assigned to the user needs to be calculated by the client updating the user information, hence I implemented another utility in that PR: getRegisteredUsers. It can only be called if the ACL allow the user to register other users. I'd be happy if you could review and publish this.

Rantanen commented 6 years ago

The id assigned to the user needs to be calculated by the client updating the user information

Really? That would make self registering painful if the ACL didn't allow getting registered users. It's also contradicting what the murmur code seems to do. The murmur code calls registerUser with a user info. The user info is created few lines above and filled with name, (certificate) hash and e-mail (if available):

https://github.com/mumble-voip/mumble/blob/master/src/murmur/Messages.cpp#L732-L738

Although getRegisteredUsers is definitely useful on its own. :)

I'll merge that in and publish a new version once I get off work. I'll leave the review till then as well, otherwise my GitHub notification is going away and I'll forget it.

Prior99 commented 6 years ago

I also found it weird, but when trying to simply call it with 1 for two users it would succeed only the first time on a clean server. Everytime I re-used an already assigned id I got a PermissionError. Fixing it to assigning truely unique ids worked fine, though.

By the way, I might be mistaken but I think the ACL for registering also allows to list registered users.

Rantanen commented 6 years ago

Yeah, ACL for registering (as opposed to self registering) is needed to retrieve the list of users. And this ACL needs to be defined on the root channel:

https://github.com/mumble-voip/mumble/blob/master/src/murmur/Messages.cpp#L1511-L1516

Prior99 commented 6 years ago

I think this can be closed after #105 was merged, what do you think @Rantanen?

Rantanen commented 6 years ago

Oh, right. These don't get closed automatically from other people's PRs.

Thanks for reminding again. :)