Minor fixes on List sharing permissions. (#1981)

* first pass

* second pass

* more tentative

* more tentative

* more work

* how about we don't execute tests twice?

* fix tests?

* forgot to revert that

app/Http/Requests/Sharing/ListSharingRequest.php

@@ -5,6 +5,7 @@
 use App\Contracts\Http\Requests\HasBaseAlbum;
 use App\Contracts\Http\Requests\RequestAttribute;
 use App\Contracts\Models\AbstractAlbum;
+use App\Exceptions\UnauthenticatedException;
 use App\Http\Requests\BaseApiRequest;
 use App\Http\Requests\Traits\HasBaseAlbumTrait;
 use App\Models\User;
@@ -50,13 +51,20 @@ class ListSharingRequest extends BaseApiRequest implements HasBaseAlbum
 	 */
 	public function authorize(): bool
 	{
-		if (Gate::check(AlbumPolicy::CAN_SHARE_WITH_USERS, [AbstractAlbum::class, $this->album])) {
+		/** @var User $user */
+		$user = Auth::user() ?? throw new UnauthenticatedException();
+
+		if (!Gate::check(AlbumPolicy::CAN_SHARE_WITH_USERS, [AbstractAlbum::class, $this->album])) {
+			return false;
+		}
+
+		if ($user->may_administrate === true) {
 			return true;
 		}
 
 		if (
-			($this->owner !== null && $this->owner->id === Auth::id()) ||
-			($this->participant !== null && $this->participant->id === Auth::id())
+			($this->owner?->id === $user->id) ||
+			($this->participant?->id === $user->id)
 		) {
 			return true;
 		}

app/Policies/AlbumPolicy.php

@@ -7,6 +7,7 @@
 use App\Exceptions\ConfigurationKeyMissingException;
 use App\Exceptions\Internal\LycheeAssertionError;
 use App\Exceptions\Internal\QueryBuilderException;
+use App\Models\AccessPermission;
 use App\Models\BaseAlbumImpl;
 use App\Models\Configs;
 use App\Models\Extensions\BaseAlbum;
@@ -21,6 +22,7 @@ class AlbumPolicy extends BasePolicy
 	public const CAN_SEE = 'canSee';
 	public const CAN_ACCESS = 'canAccess';
 	public const CAN_DOWNLOAD = 'canDownload';
+	public const CAN_DELETE = 'canDelete';
 	public const CAN_UPLOAD = 'canUpload';
 	public const CAN_EDIT = 'canEdit';
 	public const CAN_EDIT_ID = 'canEditById';
@@ -107,9 +109,11 @@ public function canAccess(?User $user, ?AbstractAlbum $album): bool
 					return true;
 				}
 
-				if ($album->public_permissions() !== null &&
+				if (
+					$album->public_permissions() !== null &&
 					($album->public_permissions()->password === null ||
-					$this->isUnlocked($album))) {
+						$this->isUnlocked($album))
+				) {
 					return true;
 				}
 
@@ -233,13 +237,36 @@ public function canEdit(User $user, AbstractAlbum|null $album): bool
 
 		if ($album instanceof BaseAlbum) {
 			return $this->isOwner($user, $album) ||
-			$album->current_user_permissions()?->grants_edit === true ||
-			$album->public_permissions()?->grants_edit === true;
+				$album->current_user_permissions()?->grants_edit === true ||
+				$album->public_permissions()?->grants_edit === true;
 		}
 
 		return false;
 	}
 
+	/**
+	 * Check if user is allowed to delete in current albumn.
+	 *
+	 * @param User               $user
+	 * @param AbstractAlbum|null $abstractAlbum
+	 *
+	 * @return bool
+	 *
+	 * @throws ConfigurationKeyMissingException
+	 */
+	public function canDelete(User $user, ?AbstractAlbum $abstractAlbum = null): bool
+	{
+		if (!$user->may_upload) {
+			return false;
+		}
+
+		if (!$abstractAlbum instanceof BaseAlbum) {
+			return false;
+		}
+
+		return $this->isOwner($user, $abstractAlbum);
+	}
+
 	/**
 	 * Checks whether the designated albums are editable by the current user.
 	 *
@@ -274,12 +301,30 @@ public function canEditById(User $user, array $albumIDs): bool
 			[null]
 		);
 
-		return
-			count($albumIDs) === 0 ||
-			BaseAlbumImpl::query()
+		$num_albums = count($albumIDs);
+
+		if ($num_albums === 0) {
+			return true;
+		}
+
+		if (BaseAlbumImpl::query()
 			->whereIn('id', $albumIDs)
-			->where('owner_id', $user->id)
-			->count() === count($albumIDs);
+			->where('owner_id', '=', $user->id)
+			->count() === $num_albums
+		) {
+			return true;
+		}
+
+		if (AccessPermission::query()
+			->whereIn('base_album_id', $albumIDs)
+			->where('user_id', '=', $user->id)
+			->where('grants_edit', '=', true)
+			->count() === $num_albums
+		) {
+			return true;
+		}
+
+		return false;
 	}
 
 	/**
@@ -294,12 +339,13 @@ public function canEditById(User $user, array $albumIDs): bool
 	 */
 	public function canShareWithUsers(?User $user, ?AbstractAlbum $abstractAlbum): bool
 	{
-		if ($abstractAlbum === null) {
+		if ($user?->may_upload !== true) {
 			return false;
 		}
 
-		if ($user?->may_upload !== true) {
-			return false;
+		// If this is null, this means that we are looking at the list.
+		if ($abstractAlbum === null) {
+			return true;
 		}
 
 		if (SmartAlbumType::tryFrom($abstractAlbum->id) !== null) {
@@ -321,7 +367,33 @@ public function canShareWithUsers(?User $user, ?AbstractAlbum $abstractAlbum): b
 	 */
 	public function canShareById(User $user, array $albumIDs): bool
 	{
-		return $this->canEditById($user, $albumIDs);
+		if (!$user->may_upload) {
+			return false;
+		}
+
+		// Remove root and smart albums, as they get a pass.
+		// Make IDs unique as otherwise count will fail.
+		$albumIDs = array_diff(
+			array_unique($albumIDs),
+			array_keys(SmartAlbumType::values()),
+			[null]
+		);
+
+		$num_albums = count($albumIDs);
+
+		if ($num_albums === 0) {
+			return true;
+		}
+
+		if (BaseAlbumImpl::query()
+			->whereIn('id', $albumIDs)
+			->where('owner_id', '=', $user->id)
+			->count() === $num_albums
+		) {
+			return true;
+		}
+
+		return false;
 	}
 
 	/**

tests/Feature/SharingBasicTest.php

@@ -12,9 +12,10 @@
 
 namespace Tests\Feature;
 
+use Tests\Feature\Base\BaseSharingTest;
 use Tests\Feature\Constants\TestConstants;
 
-class SharingBasicTest extends Base\BaseSharingTest
+class SharingBasicTest extends BaseSharingTest
 {
 	/**
 	 * @return void