fix: Return multiple uris in getTurnAuth response #7
Conversation
Pull Request Test Coverage Report for Build 11654490031Details
💛 - Coveralls |
levaitamas
force-pushed
the
turnauth-multiple-uris
branch
from
65ec8c6 to
f667459
Compare
levaitamas
force-pushed
the
turnauth-multiple-uris
branch
from
f667459 to
d1308ca
Compare
| for _, s := range servers { | ||
| for _, uri := range *s.Urls { | ||
| uris = append(uris, uri) | ||
| } | ||
| } | ||
|
|
||
| turnAuthToken := types.TurnAuthenticationToken{ | ||
| Username: servers[0].Username, |
There was a problem hiding this comment.
I'm afraid this is more difficult than that. The problem is that the auth-service can generate the TURN/ICE credentials for multiple gateways in a single call (e.g., all gateways in a namespace, or all gateways in a cluster), and each can use a different auth realm with different users/passwords.
The way this should be implemented is to collect only the URIs with the same credentials and return only those. But in any way this may still turn out to be wrong since we never know which particular gateway to return (unless the caller chooses one with the proper HTTP queries).
There was a problem hiding this comment.
Thanks for starting this discussion! I agree. I had to sleep on it and came up with the idea to combine iceServers that have the same (username, password, cluster) tuple. The same username and password are not sufficient since they are not unique (see our demo cluster for an example). Likewise, keeping only the cluster (e.g., a media server) is not unique either. We can define multiple gateways for a single cluster (I learned this the hard way with the "multiple configs" test case, which might be not correct in this PR). My next idea is to collect URIs sharing the same (username, password, cluster). Wdyt?
There was a problem hiding this comment.
Here is one thing we have to understand first. The only time when we must provide multiple URIs per TURN auth token is when a single Gateway has multiple listeners. Now the way this is implemented is that the TURN auth request handler calls the ICE server config generation routine, which returns an ICE auth token with one URI per listener, i.e., it does what we want it to do (see this test). Once the TURN handler has the resultant ICE config, it just copies the URIs as is. My understanding is that this should contain multiple URIs, so why this is not happening?
There was a problem hiding this comment.
You are correct. Having a single gateway with multiple listeners indeed results multiple URIs:
{"password":"pass-1","ttl":86400,"uris":["turn:172.18.0.6:3478?transport=udp","turn:0.0.0.0:3478?transport=tcp"],"username":"user-1"}I guess my misleading comes from one gateways per protocol approach (e.g., simple-tunnel example).
|
Closing this for now. Hovewer, I plan to add the getTurnAuth tests in the near future. |
|
Please, add them right away, they're super-useful and I guess they also mostly pass already. |
|
done in dced359 |
Fixes #6