fix #6

lib/Dav/SubscriptionManagementPlugin.php

@@ -138,10 +138,10 @@ class SubscriptionManagementPlugin extends ServerPlugin {
 						$errors[] = "options validation error";
 					}
 				} else {
-					$existingSubscriptionId = $transport->getSubscriptionIdFromOptions($subscriptionOptions);
-
 					$user = $this->userSession->getUser();
 
+					$existingSubscriptionId = $transport->getSubscriptionIdFromOptions($user->getUID(), $node->getName(), $subscriptionOptions);
+
 					if(!is_int($existingSubscriptionId)) {
 						// create new subscription entry in db
 						$subscription = $this->subscriptionService->create($user->getUID(), $node->getName(), $subscriptionType, $subscriptionExpires);
@@ -164,8 +164,13 @@ class SubscriptionManagementPlugin extends ServerPlugin {
 							}
 						}
 					} else {
+						// implicitly checks if subscription found by transport is really owned by correct user
 						$subscription = $this->subscriptionService->find($user->getUID(), $existingSubscriptionId);
 						
+						// check if subscription found by transport is really for correct collection
+						if($subscription->getCollectionName() !== $node->getName()) {
+							$errors[] = "subscription update error";
+						} else {
 							[
 								'success' => $updateSuccess,
 								'errors' => $updateErrors,
@@ -185,6 +190,7 @@ class SubscriptionManagementPlugin extends ServerPlugin {
 							}
 						}
 					}
+				}
 			} else {
 				$errors[] = $subscriptionType . " transport does not exist";
 			}

lib/Db/WebPushSubscriptionMapper.php

@@ -10,6 +10,7 @@ use OCP\IDBConnection;
 
 class WebPushSubscriptionMapper extends QBMapper {	
 	public const TABLENAME = 'dav_push_subscriptions_webpush';
+	public const SUBSCRIPTIONS_TABLENAME = "dav_push_subscriptions";
 
 	public function __construct(IDBConnection $db) {
 		parent::__construct($db, self::TABLENAME, WebPushSubscription::class);
@@ -37,12 +38,17 @@ class WebPushSubscriptionMapper extends QBMapper {
 	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
 	 * @throws DoesNotExistException
 	 */
-	public function findByPushResource(string $pushResource): WebPushSubscription {
+	public function findByPushResource(string $userId, string $collectionName, string $pushResource): WebPushSubscription {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
-		$qb->select('*')
+		$qb->select('webpush.*')
-			->from(self::TABLENAME)
+			->from(self::TABLENAME, 'webpush');
-			->where($qb->expr()->eq('push_resource', $qb->createNamedParameter($pushResource)));
+
+		$qb->innerJoin('webpush', self::SUBSCRIPTIONS_TABLENAME, 'subscription', $qb->expr()->eq('webpush.subscription_id', 'subscription.id'));
+
+		$qb->where($qb->expr()->eq('webpush.push_resource', $qb->createNamedParameter($pushResource)))
+			->andWhere($qb->expr()->eq('subscription.user_id', $qb->createNamedParameter($userId)))
+			->andWhere($qb->expr()->eq('subscription.collection_name', $qb->createNamedParameter($collectionName)));
 		
 		return $this->findEntity($qb);
 	}

lib/PushTransports/WebPushTransport.php

@@ -99,11 +99,11 @@ class WebPushTransport extends Transport {
 		$result = file_get_contents($pushResource, false, $context);
 	}
 
-	public function getSubscriptionIdFromOptions($options): ?int {
+	public function getSubscriptionIdFromOptions(string $userId, string $collectionName, $options): ?int {
 		['pushResource' => $pushResource] = $this->parseOptions($options);
 
 		try {
-			return $this->webPushSubscriptionService->findByPushResource($pushResource)->getSubscriptionId();
+			return $this->webPushSubscriptionService->findByPushResource($userId, $collectionName, $pushResource)->getSubscriptionId();
 		} catch (WebPushSubscriptionNotFound $e) {
 			return null;
 		}

lib/Service/WebPushSubscriptionService.php

@@ -35,9 +35,9 @@ class WebPushSubscriptionService {
 		}
     }
 
-    public function findByPushResource(string $pushResource): ?WebPushSubscription {
+    public function findByPushResource(string $userId, string $collectionName, string $pushResource): ?WebPushSubscription {
         try {
-			return $this->mapper->findByPushResource($pushResource);
+			return $this->mapper->findByPushResource($userId, $collectionName, $pushResource);
 		} catch (Exception $e) {
 			$this->handleException($e);
 		}

lib/Transport/Transport.php

@@ -59,10 +59,12 @@ abstract class Transport {
 	*/
 	abstract public function registerSubscription($subsciptionId, $options);
 
-	// Transport needs to be able to map subscription options back to a subscription id.
+	// Transport needs to be able to map subscription options + userId + collectionName back to a subscription id.
 	// API Requests to create and update a subscription are the same, therefore if a subscription id is associated with the given options the subscription is updated, otherwise a new subscription is added.
 	// Which option(s) uniquely identify a subscription is implementation specific.
-	abstract public function getSubscriptionIdFromOptions($options): ?int;
+	// Options themselves do not necessarily uniquely identify a subscription, but combined with userId and collectionName it does.
+	// It is not recommended for transports to save userId and collectionName for themselves for this, you can just use a join with the general subscriptions table.
+	abstract public function getSubscriptionIdFromOptions(string $userId, string $collectionName, $options): ?int;
 
 	// Change mutable options of the subscription (if any exist)
 	abstract public function updateSubscription($subsciptionId, $options);