From 95f5c94b4d01a9bce8d80310fd409312fe57e367 Mon Sep 17 00:00:00 2001
From: Raja Subramanian <raja.gobi@tutanota.com>
Date: Fri, 22 Mar 2024 23:22:20 +0530
Subject: [PATCH] Notify initial permissions (#2595)

* Notify initial permissions

NOTE: This does add an initial subscription permission notification
which should be fine, but something to watch for.

A stress test combining
- mute/unmute on publisher side.
- allowing/revoking permission for subscriber from publisher side.
- subscribing/unsubscribing from subscriber side.
results in a scenario where a subscription permission update of
`not_allowed` being sent and on a re-subscribe, an `allowed` update does
not happen.

It happens like so
- Subscription revoke cloes the down track of subscriber.
- The subscription is still desired.
- So, a subscription reconcile runs and sees `permission: false`. This
  sends subscription permission of `not_allowed`.
- Unsubscribe request comes in and sets `desired: false`.
- Reconsiler runs again and sees `desired: false` and `subscribedTrack:
  nil`. This cleans up the subscription.
- Publisher grants permission for the subscriber.
- Subscriber subscribes to the track again. A new subscription is
  created.
- Reconciler runs and sees `permission: true`, but there is no
  permission change as it is a new subscription object. So, `allowed`
  subscription permission update is not sent and the client is stuck at
  `not_allowed`.

Fix, maintain if permission has been initialized. Has the effect of
sending an initial update which should be fine.

* clean up comment

* no default
---
 pkg/rtc/subscriptionmanager.go | 35 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/pkg/rtc/subscriptionmanager.go b/pkg/rtc/subscriptionmanager.go
index 8232c7527..a7dfa6608 100644
--- a/pkg/rtc/subscriptionmanager.go
+++ b/pkg/rtc/subscriptionmanager.go
@@ -497,8 +497,6 @@ func (m *SubscriptionManager) subscribe(s *trackSubscription) error {
 
 	s.setPublisher(res.PublisherIdentity, res.PublisherID)
 
-	// since hasPermission defaults to true, we will want to send a message to the client the first time
-	// that we discover permissions were denied
 	permChanged := s.setHasPermission(res.HasPermission)
 	if permChanged {
 		m.params.Participant.SubscriptionPermissionUpdate(s.getPublisherID(), trackID, res.HasPermission)
@@ -722,19 +720,20 @@ type trackSubscription struct {
 	trackID      livekit.TrackID
 	logger       logger.Logger
 
-	lock              sync.RWMutex
-	desired           bool
-	publisherID       livekit.ParticipantID
-	publisherIdentity livekit.ParticipantIdentity
-	settings          *livekit.UpdateTrackSettings
-	changedNotifier   types.ChangeNotifier
-	removedNotifier   types.ChangeNotifier
-	hasPermission     bool
-	subscribedTrack   types.SubscribedTrack
-	eventSent         atomic.Bool
-	numAttempts       atomic.Int32
-	bound             bool
-	kind              atomic.Pointer[livekit.TrackType]
+	lock                     sync.RWMutex
+	desired                  bool
+	publisherID              livekit.ParticipantID
+	publisherIdentity        livekit.ParticipantIdentity
+	settings                 *livekit.UpdateTrackSettings
+	changedNotifier          types.ChangeNotifier
+	removedNotifier          types.ChangeNotifier
+	hasPermissionInitialized bool
+	hasPermission            bool
+	subscribedTrack          types.SubscribedTrack
+	eventSent                atomic.Bool
+	numAttempts              atomic.Int32
+	bound                    bool
+	kind                     atomic.Pointer[livekit.TrackType]
 
 	// the later of when subscription was requested OR when the first failure was encountered OR when permission is granted
 	// this timestamp determines when failures are reported
@@ -746,8 +745,6 @@ func newTrackSubscription(subscriberID livekit.ParticipantID, trackID livekit.Tr
 		subscriberID: subscriberID,
 		trackID:      trackID,
 		logger:       l,
-		// default allow
-		hasPermission: true,
 	}
 }
 
@@ -796,9 +793,11 @@ func (s *trackSubscription) setDesired(desired bool) bool {
 func (s *trackSubscription) setHasPermission(perm bool) bool {
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	if s.hasPermission == perm {
+	if s.hasPermissionInitialized && s.hasPermission == perm {
 		return false
 	}
+
+	s.hasPermissionInitialized = true
 	s.hasPermission = perm
 	if s.hasPermission {
 		// when permission is granted, reset the timer so it has sufficient time to reconcile