Add subscription permission by source and name #1043

lukasIO · 2025-04-11T12:42:38Z

.. and deprecate trackSid based permissions.

allows for permissions that are stable across full reconnects, where usually the trackSid changes due to the tracks being republished.

changeset-bot · 2025-04-11T12:42:41Z

⚠️ No Changeset found

Latest commit: db50077

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR

Some errors occurred when validating the changesets config:
The package or glob expression "github.com/livekit/protocol" specified in the `fixed` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.

lukasIO · 2025-04-11T12:44:32Z

protobufs/livekit_rtc.proto

@@ -353,8 +353,10 @@ message SubscriptionPermission {

 message SubscriptionPermissionUpdate {


wait, this is the wrong message 🤦

boks1971 · 2025-04-11T12:45:12Z

protobufs/livekit_rtc.proto

@@ -353,8 +353,10 @@ message SubscriptionPermission {

 message SubscriptionPermissionUpdate {
  string participant_sid = 1;
-  string track_sid = 2;
+  string track_sid = 2 [deprecated = true];


this is a response message, shouldn't this be in request?

Also, checking with a customer about this also. Maybe, keep track_sid also? I am on the fence too on that.

haha jinx, commented above

keeping track_sid seems like a footgun now that we always generate new track ids on republish

Should the SubscriptionPermissionUpdate sent from the server down still translate to track_sid in the response?
or should we match the tracks client side also by source and/or name?

yeah true, I am just trying to think if it will be useful in any situation as that gives a very specific thing and that is already in the system.

Tracks can be published without having a source or name. But, track_sid is always there.

I cannot think of a case, but just feels like there is something that would need that specificity. We can remove and bring it back later if needed I guess.

Should the SubscriptionPermissionUpdate sent from the server down still translate to track_sid in the response? or should we match the tracks client side also by source and/or name?

jinx again, I was adding a note about it too.

Should the SubscriptionPermissionUpdate sent from the server down still translate to track_sid in the response?

ideally both server/client would only have to think about name/source for permissions but idk if we can do this without breaking the api...? also to Raja's point even though we're deprecating this i don't think we're going to clean up the code immediately and if customers have use cases that the new model doesn't support we'll get push-back.

…l into lukas/source-subscr-perm

boks1971 · 2025-04-11T12:50:05Z

protobufs/livekit_rtc.proto

@@ -340,10 +340,12 @@ message SubscribedQualityUpdate {

 message TrackPermission {
  // permission could be granted either by participant sid or identity


change comment if participant_sid is marked deprecated?

boks1971 · 2025-04-11T12:51:31Z

protobufs/livekit_rtc.proto

@@ -353,8 +355,10 @@ message SubscriptionPermission {

 message SubscriptionPermissionUpdate {
  string participant_sid = 1;
-  string track_sid = 2;
+  string track_sid = 2 [deprecated = true];


maybe not deprecate here. This is just an info message. It can have the current SID server sees. Also not sure if we want to include source/name in this message. I guess we can, but not sure if it is needed. It is optional, but if unused, might as well not have it?

it's not purely informational, we also use it client side to setAllowed on publications: https://github.com/livekit/client-sdk-js/blob/0c1c18cce46ec5e78c50912277b378fae04997f1/src/room/Room.ts#L1723-L1734

ah true, so I guess having SID is a good thing then?

I don't have an opinion on whether or not track_sid should stay here, it's just an additional step for the server to translate between source + name on the request to the trackSid

boks1971 · 2025-04-11T12:53:44Z

protobufs/livekit_rtc.proto

  string participant_identity = 4;
+  repeated TrackSource track_sources = 5;
+  repeated string track_names = 6;


I would make this another message to have source, name pairing explicit. If both are repeated and if we are doing intersection match, we would have to check all combinations of the two and allow it. Is that the intention? Or should we have explicit pairs (and the fields inside the pairing can be optional)?

oh, that's a good point!

maybe it doesn't even have to be repeated 🤔

I think a repeated pairing makes sense. That will allow something like camera, front and mic, back to be allowed, but not permitting camera, back, mic, front.

it has to be repeated to support multiple sources...?

allowed: [{source: MIC}, {source: CAM}, {etc...}]

Add subscription permission by source and name

a7d5c05

lukasIO requested a review from a team April 11, 2025 12:42

generated protobuf

3f5874c

paulwe approved these changes Apr 11, 2025

View reviewed changes

lukasIO commented Apr 11, 2025

View reviewed changes

boks1971 reviewed Apr 11, 2025

View reviewed changes

lukasIO and others added 3 commits April 11, 2025 14:45

change fields on TrackPermission

7ef2003

Merge branch 'lukas/source-subscr-perm' of github.com:livekit/protoco…

3a6cb84

…l into lukas/source-subscr-perm

generated protobuf

db50077

boks1971 reviewed Apr 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subscription permission by source and name #1043

Add subscription permission by source and name #1043

lukasIO commented Apr 11, 2025

changeset-bot bot commented Apr 11, 2025 •

edited

Loading

lukasIO Apr 11, 2025

boks1971 Apr 11, 2025

lukasIO Apr 11, 2025

paulwe Apr 11, 2025

lukasIO Apr 11, 2025

boks1971 Apr 11, 2025

boks1971 Apr 11, 2025

paulwe Apr 11, 2025

boks1971 Apr 11, 2025

boks1971 Apr 11, 2025

lukasIO Apr 11, 2025

boks1971 Apr 11, 2025

lukasIO Apr 11, 2025

boks1971 Apr 11, 2025

lukasIO Apr 11, 2025

lukasIO Apr 11, 2025

boks1971 Apr 11, 2025

paulwe Apr 11, 2025

boks1971 Apr 11, 2025

		@@ -353,8 +353,10 @@ message SubscriptionPermission {

		message SubscriptionPermissionUpdate {

		@@ -340,10 +340,12 @@ message SubscribedQualityUpdate {

		message TrackPermission {
		// permission could be granted either by participant sid or identity

Add subscription permission by source and name #1043

Are you sure you want to change the base?

Add subscription permission by source and name #1043

Conversation

lukasIO commented Apr 11, 2025

changeset-bot bot commented Apr 11, 2025 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Apr 11, 2025 •

edited

Loading