api: check ownership in unsubscribe endpoint

Let a user unsubscribe only from his/her own subscriptions. Prior to this change, anyone could unsubscribe anybody else from whatever active subscription. Signed-off-by: Ricardo Cañuelo <[email protected]>
kernelci · Dec 1, 2023 · 8be9929 · 8be9929
1 parent 4305111
commit 8be9929
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/api/main.py b/api/main.py
@@ -598,12 +598,17 @@ async def subscribe(channel: str, user: User = Depends(get_current_user)):
 async def unsubscribe(sub_id: int, user: User = Depends(get_current_user)):
     """Unsubscribe handler for Pub/Sub channel"""
     try:
-        await pubsub.unsubscribe(sub_id)
+        await pubsub.unsubscribe(sub_id, user.username)
     except KeyError as error:
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,
             detail=f"Subscription id not found: {str(error)}"
         ) from error
+    except RuntimeError as error:
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail=str(error)
+        ) from error
 
 
 @app.get('/listen/{sub_id}')

diff --git a/api/pubsub.py b/api/pubsub.py
@@ -107,14 +107,21 @@ async def subscribe(self, channel, user=None):
             self._start_keep_alive_timer()
             return sub
 
-    async def unsubscribe(self, sub_id):
+    async def unsubscribe(self, sub_id, user=None):
         """Unsubscribe from a Pub/Sub channel
 
         Unsubscribe from a channel using the provided subscription id as found
         in a Subscription object.
         """
         async with self._lock:
-            sub = self._subscriptions.pop(sub_id)
+            if sub_id not in self._subscriptions:
+                raise KeyError(sub_id)
+            sub = self._subscriptions[sub_id]
+            if user and user != sub['sub'].user:
+                # Subscription is from a different user
+                raise RuntimeError(f"Subscription {sub_id} "
+                                   f"not owned by {user}")
+            self._subscriptions.pop(sub_id)
             self._update_channels()
             await sub['redis_sub'].unsubscribe()