From 7059fa97b1c48e499827c7569bc66a6a06ba0766 Mon Sep 17 00:00:00 2001 From: Simon Spannagel Date: Sun, 22 Jan 2023 15:04:24 +0100 Subject: [PATCH 1/6] AlbumMapper: add method to check if user is collaborator for album Signed-off-by: Simon Spannagel --- lib/Album/AlbumMapper.php | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/Album/AlbumMapper.php b/lib/Album/AlbumMapper.php index 9be017d0..c19e9296 100644 --- a/lib/Album/AlbumMapper.php +++ b/lib/Album/AlbumMapper.php @@ -332,6 +332,40 @@ class AlbumMapper { return array_values(array_filter($collaborators, fn ($c) => $c !== null)); } + + /** + * @param int $albumId + * @param string $userId + * @return bool + */ + public function isCollaborator(int $albumId, string $userId): bool { + $query = $this->connection->getQueryBuilder(); + $query->select("collaborator_id", "collaborator_type") + ->from("photos_albums_collabs") + ->where($query->expr()->eq('album_id', $query->createNamedParameter($albumId, IQueryBuilder::PARAM_INT))); + + $rows = $query->executeQuery()->fetchAll(); + + foreach ($rows as $row) { + switch ($row['collaborator_type']) { + case self::TYPE_USER: + if ($row['collaborator_id'] === $userId) { + return true; + } + break; + case self::TYPE_GROUP: + if ($$this->groupManager->isInGroup($userId, $row['collaborator_id'])) { + return true; + } + break; + default: + break; + } + } + + return false; + } + /** * @param int $albumId * @param array{'id': string, 'type': int} $collaborators From b89d47d1eb75b60b09a0fc353a19b25670f9967c Mon Sep 17 00:00:00 2001 From: Simon Spannagel Date: Sun, 22 Jan 2023 15:04:41 +0100 Subject: [PATCH 2/6] SharedAlbumRoot: correctly check if user is collaborator Signed-off-by: Simon Spannagel --- lib/Sabre/Album/SharedAlbumRoot.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/Sabre/Album/SharedAlbumRoot.php b/lib/Sabre/Album/SharedAlbumRoot.php index d5dc9909..bc935698 100644 --- a/lib/Sabre/Album/SharedAlbumRoot.php +++ b/lib/Sabre/Album/SharedAlbumRoot.php @@ -47,11 +47,7 @@ class SharedAlbumRoot extends AlbumRoot { throw new Conflict("File $sourceId is already in the folder"); } - $collaboratorIds = array_map( - fn ($collaborator) => $collaborator['type'].':'.$collaborator['id'], - $this->albumMapper->getCollaborators($this->album->getAlbum()->getId()), - ); - if (!in_array(AlbumMapper::TYPE_USER.':'.$this->userId, $collaboratorIds)) { + if(!$this->albumMapper->isCollaborator($this->album->getAlbum()->getId(), $this->userId)) { return false; } From bff6581467983e73e15452c9a9ebc88a0ddd06bb Mon Sep 17 00:00:00 2001 From: Simon Spannagel Date: Sun, 22 Jan 2023 15:18:57 +0100 Subject: [PATCH 3/6] Fix comparisons in isCollaborator Signed-off-by: Simon Spannagel --- lib/Album/AlbumMapper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Album/AlbumMapper.php b/lib/Album/AlbumMapper.php index c19e9296..d1727c45 100644 --- a/lib/Album/AlbumMapper.php +++ b/lib/Album/AlbumMapper.php @@ -349,12 +349,12 @@ class AlbumMapper { foreach ($rows as $row) { switch ($row['collaborator_type']) { case self::TYPE_USER: - if ($row['collaborator_id'] === $userId) { + if (strcmp($row['collaborator_id'], $userId)) { return true; } break; case self::TYPE_GROUP: - if ($$this->groupManager->isInGroup($userId, $row['collaborator_id'])) { + if ($this->groupManager->isInGroup($userId, $row['collaborator_id'])) { return true; } break; From 69dcd886b029404a412637980b51045db867edbc Mon Sep 17 00:00:00 2001 From: Simon Spannagel Date: Sun, 22 Jan 2023 16:06:26 +0100 Subject: [PATCH 4/6] Fix sharing with individual user Signed-off-by: Simon Spannagel --- lib/Album/AlbumMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Album/AlbumMapper.php b/lib/Album/AlbumMapper.php index d1727c45..0562d8e5 100644 --- a/lib/Album/AlbumMapper.php +++ b/lib/Album/AlbumMapper.php @@ -349,7 +349,7 @@ class AlbumMapper { foreach ($rows as $row) { switch ($row['collaborator_type']) { case self::TYPE_USER: - if (strcmp($row['collaborator_id'], $userId)) { + if (!strcmp($row['collaborator_id'], $userId)) { return true; } break; From 7bc21922a7aed1f287f4ce5ebae00e49803fb08c Mon Sep 17 00:00:00 2001 From: Simon Spannagel Date: Sun, 22 Jan 2023 16:10:40 +0100 Subject: [PATCH 5/6] Satisfy linter Signed-off-by: Simon Spannagel --- lib/Sabre/Album/SharedAlbumRoot.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sabre/Album/SharedAlbumRoot.php b/lib/Sabre/Album/SharedAlbumRoot.php index bc935698..a6f84510 100644 --- a/lib/Sabre/Album/SharedAlbumRoot.php +++ b/lib/Sabre/Album/SharedAlbumRoot.php @@ -25,7 +25,6 @@ namespace OCA\Photos\Sabre\Album; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\Conflict; -use OCA\Photos\Album\AlbumMapper; class SharedAlbumRoot extends AlbumRoot { /** @@ -47,7 +46,7 @@ class SharedAlbumRoot extends AlbumRoot { throw new Conflict("File $sourceId is already in the folder"); } - if(!$this->albumMapper->isCollaborator($this->album->getAlbum()->getId(), $this->userId)) { + if (!$this->albumMapper->isCollaborator($this->album->getAlbum()->getId(), $this->userId)) { return false; } From f3365d9ef75a1fa372f5a1007b28f5095a0c980c Mon Sep 17 00:00:00 2001 From: Simon Spannagel Date: Mon, 23 Jan 2023 19:28:40 +0100 Subject: [PATCH 6/6] Go for === in favor of strcmp for clarity Signed-off-by: Simon Spannagel --- lib/Album/AlbumMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Album/AlbumMapper.php b/lib/Album/AlbumMapper.php index 0562d8e5..8b7ed7df 100644 --- a/lib/Album/AlbumMapper.php +++ b/lib/Album/AlbumMapper.php @@ -349,7 +349,7 @@ class AlbumMapper { foreach ($rows as $row) { switch ($row['collaborator_type']) { case self::TYPE_USER: - if (!strcmp($row['collaborator_id'], $userId)) { + if ($row['collaborator_id'] === $userId) { return true; } break;