meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.17k stars 2.47k forks source link

Add `opaque_id` to event messages along with `sender` #1531

Closed manifest closed 5 years ago

manifest commented 5 years ago

There is already a way to pass optional opaque_id string identifier in "attach" request that may be used for different purposes.

The problem, it seems to be only accessible through Admin/Monitor API. In that case to map sender message field (or handle_Id) of event messages like "webrtcup", "hangup", "slowlink", etc. is only possible by performing a list_handles request per each event message.

If we had opaque_id along with sender field in event messages that would make performing additional list_handles request unnecessary and the operation itself more efficient.

lminiero commented 5 years ago

I don't see why that's needed in the regular API. The opaque_id only makes sense for stats purposes, which is why it's only in the Admin API and Event Handlers.

manifest commented 5 years ago

In our case we have a web service in front of Janus Gateway implementing authorization and some additional API. Users connect to the web service and it relays request messages forward to Janus Gateway and response and event messages back to the users. In order to deliver response and event messages within active plugin handle a user created, we need to map session_id and handle_id to the particular user_id. For request and response messages we can embed our user_id into transaction. At the same time, we may also embed user_id into opaque_id and if it will be accessible as a field of event message, we won't need any global state (session_id + handle_id mapping to the user_id) to deliver the event messages to the particular user.

lminiero commented 5 years ago

Got it, make sense, thanks for the clarification :+1: I'll still have to think about the possible implications, since opaque_id is supposed to be an application specific identifier, meaning applications may insert user-specific info there that's not supposed to be known to others than the user and the application itself (hence why it's only in Admin API and event handlers right now, for correlation purposes). Making it available in another context too may reveal that information to others as well. I guess that if it's only added to responses and events to a handle related to the opaque ID anyway, it shouldn't be an issue, but I want to give this a bit more thought before deciding whether to add it or not.

manifest commented 5 years ago

Thanks. IMHO, there shouldn't be any secrets on that level of API. I mean application developers should never mix secrets or authorization with data.

lminiero commented 5 years ago

Thanks. IMHO, there shouldn't be any secrets on that level of API. I mean application developers should never mix secrets or authorization with data.

Well, that's up to applications, though, not me :smile: If that's what they need for correlation purposes (e.g., to identify users when evaluating stats), the API and opaque ID allow for this.

Anyway, I'm adding support for this in an optional way via janus.jcfg: by default we behave as we've always done, unless you set opaqueid_in_api = true in the general configuration. That should cover your requirement and still ensure backwards compatibility; I'll commit in a few minutes. As a side note, this only covers requests with a "sender" field, of course: not all Janus API messages include one, so you won't get the opaque in all messages. I guess that's fine for your needs, correct?

lminiero commented 5 years ago

Forgot to tag the issue in the commit: this is it, https://github.com/meetecho/janus-gateway/commit/ddbf37fef43ade61d73173c7661a2449c13582d4 Closing.