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

Register user #104

Closed
Prior99 opened this issue May 3, 2018 · 9 comments
Closed

Register user #104

Prior99 opened this issue May 3, 2018 · 9 comments

Comments

@Prior99
Copy link
Contributor

Prior99 commented May 3, 2018

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
Copy link
Owner

Rantanen commented May 3, 2018

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

@Rantanen
Copy link
Owner

Rantanen commented May 3, 2018

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
Copy link
Contributor Author

Prior99 commented May 4, 2018 via email

@Prior99
Copy link
Contributor Author

Prior99 commented May 4, 2018

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
Copy link
Owner

Rantanen commented May 4, 2018

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
Copy link
Contributor Author

Prior99 commented May 4, 2018

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
Copy link
Owner

Rantanen commented May 4, 2018

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
Copy link
Contributor Author

Prior99 commented May 9, 2018

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

@Rantanen
Copy link
Owner

Rantanen commented May 9, 2018

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

Thanks for reminding again. :)

@Rantanen Rantanen closed this as completed May 9, 2018
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

2 participants