Move photo deletion from ViewPhoto to EditAsset

Removes the intermediate confirmation page, instead using JavaScript for confirmation.

Fixes an XSS issue, in that the previous method was not passing or checking the session (!)
This commit is contained in:
Aaron van Geffen 2023-11-11 15:29:32 +01:00
parent 83da4a26ac
commit e28fcd8b03
5 changed files with 37 additions and 42 deletions

View File

@ -22,8 +22,14 @@ class EditAsset extends HTMLController
if (!($user->isAdmin() || $asset->isOwnedBy($user))) if (!($user->isAdmin() || $asset->isOwnedBy($user)))
throw new NotAllowedException(); throw new NotAllowedException();
if (isset($_REQUEST['delete'])) if (isset($_REQUEST['delete']) && Session::validateSession('get'))
throw new Exception('Not implemented.'); {
$redirectUrl = BASEURL . '/' . $asset->getSubdir();
$asset->delete();
header('Location: ' . $redirectUrl);
exit;
}
if (!empty($_POST)) if (!empty($_POST))
{ {

View File

@ -21,39 +21,14 @@ class ViewPhoto extends HTMLController
parent::__construct($photo->getTitle() . ' - ' . SITE_TITLE); parent::__construct($photo->getTitle() . ' - ' . SITE_TITLE);
$author = $photo->getAuthor();
if (isset($_REQUEST['confirm_delete']) || isset($_REQUEST['delete_confirmed']))
$this->handleConfirmDelete($user, $author, $photo);
else
$this->handleViewPhoto($user, $author, $photo);
}
private function handleConfirmDelete(User $user, User $author, Asset $photo)
{
if (!($user->isAdmin() || $user->getUserId() === $author->getUserId()))
throw new NotAllowedException();
if (isset($_REQUEST['confirm_delete']))
{
$page = new ConfirmDeletePage($photo->getImage());
$this->page->adopt($page);
}
elseif (isset($_REQUEST['delete_confirmed']))
{
$album_url = $photo->getSubdir();
$photo->delete();
header('Location: ' . BASEURL . '/' . $album_url);
exit;
}
}
private function handleViewPhoto(User $user, User $author, Asset $photo)
{
if (!empty($_POST)) if (!empty($_POST))
$this->handleTagging($photo->getImage()); $this->handleTagging($photo->getImage());
else
$this->handleViewPhoto($photo);
}
private function handleViewPhoto(Asset $photo)
{
$page = new PhotoPage($photo->getImage()); $page = new PhotoPage($photo->getImage());
// Exif data? // Exif data?
@ -75,9 +50,6 @@ class ViewPhoto extends HTMLController
if ($next_url) if ($next_url)
$page->setNextPhotoUrl($next_url); $page->setNextPhotoUrl($next_url);
if ($user->isAdmin() || $user->getUserId() === $author->getUserId())
$page->setIsAssetOwner(true);
$this->page->adopt($page); $this->page->adopt($page);
$this->page->setCanonicalUrl($photo->getPageUrl()); $this->page->setCanonicalUrl($photo->getPageUrl());
} }

View File

@ -289,6 +289,16 @@ class Asset
return $this->date_captured; return $this->date_captured;
} }
public function getDeleteUrl()
{
return BASEURL . '/editasset/?id=' . $this->id_asset . '&delete';
}
public function getEditUrl()
{
return BASEURL . '/editasset/?id=' . $this->id_asset;
}
public function getFilename() public function getFilename()
{ {
return $this->filename; return $this->filename;

View File

@ -23,7 +23,10 @@ class EditAssetForm extends Template
<form id="asset_form" action="" method="post" enctype="multipart/form-data"> <form id="asset_form" action="" method="post" enctype="multipart/form-data">
<div class="content-box"> <div class="content-box">
<div class="float-end"> <div class="float-end">
<a class="btn btn-danger" href="', BASEURL, '/', $this->asset->getSlug(), '?delete_confirmed">Delete asset</a> <a class="btn btn-danger" href="', $this->asset->getDeleteUrl(), '&',
Session::getSessionTokenKey(), '=', Session::getSessionToken(),
'" onclick="return confirm(\'Are you sure you want to delete this asset?\');">',
'Delete asset</a>
<button class="btn btn-primary" type="submit">Save asset data</button> <button class="btn btn-primary" type="submit">Save asset data</button>
</div> </div>
<h2>Edit asset \'', $this->asset->getTitle(), '\' (', $this->asset->getFilename(), ')</h2> <h2>Edit asset \'', $this->asset->getTitle(), '\' (', $this->asset->getFilename(), ')</h2>

View File

@ -54,9 +54,7 @@ class PhotoPage extends Template
<div class="col-lg-4">'; <div class="col-lg-4">';
$this->photoMeta(); $this->photoMeta();
$this->userActions();
if ($this->is_asset_owner)
$this->addUserActions();
echo ' echo '
</div> </div>
@ -259,13 +257,19 @@ class PhotoPage extends Template
$this->exif = $exif; $this->exif = $exif;
} }
public function addUserActions() public function userActions()
{ {
if (!$this->photo->isOwnedBy(Registry::get('user')))
return;
echo ' echo '
<div id="user_actions_box" class="content-box"> <div id="user_actions_box" class="content-box">
<h3>Actions</h3> <h3>Actions</h3>
<a class="btn btn-primary" href="', BASEURL, '/editasset/?id=', $this->photo->getId(), '">Edit photo</a> <a class="btn btn-primary" href="', $this->photo->getEditUrl(), '">Edit photo</a>
<a class="btn btn-danger" href="', BASEURL, '/', $this->photo->getSlug(), '/?confirm_delete">Delete photo</a> <a class="btn btn-danger" href="', $this->photo->getDeleteUrl(), '&',
Session::getSessionTokenKey(), '=', Session::getSessionToken(),
'" onclick="return confirm(\'Are you sure you want to delete this photo?\');"',
'">Delete photo</a>
</div>'; </div>';
} }
} }