From 34141e69bd36e1cb42b12c37c2727696316f65c0 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Wed, 8 Jun 2016 09:30:49 -0400
Subject: [PATCH] Smarter use of Forbidden exceptions.

---
 .../src/VuFind/Controller/AbstractBase.php    | 11 +++++++++++
 .../src/VuFind/Controller/AbstractRecord.php  | 19 +++++++++++++++----
 .../src/VuFind/Controller/AjaxController.php  | 18 ++++++++++++++++++
 .../VuFind/Controller/BrowseController.php    |  3 ++-
 .../src/VuFind/Controller/CartController.php  |  5 +++--
 .../Controller/MyResearchController.php       | 11 ++++++-----
 .../src/VuFind/Controller/TagController.php   |  3 ++-
 .../src/VuFind/Exception/ListPermission.php   |  2 +-
 .../src/VuFind/Exception/LoginRequired.php    |  2 +-
 9 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/module/VuFind/src/VuFind/Controller/AbstractBase.php b/module/VuFind/src/VuFind/Controller/AbstractBase.php
index dbf4da32eaf..58b2428cde0 100644
--- a/module/VuFind/src/VuFind/Controller/AbstractBase.php
+++ b/module/VuFind/src/VuFind/Controller/AbstractBase.php
@@ -525,6 +525,17 @@ class AbstractBase extends AbstractActionController
         return $this->getServiceLocator()->get('VuFind\Search\Memory');
     }
 
+    /**
+     * Are comments enabled?
+     *
+     * @return bool
+     */
+    protected function commentsEnabled()
+    {
+        $check = $this->getServiceLocator()->get('VuFind\AccountCapabilities');
+        return $check->getCommentSetting() !== 'disabled';
+    }
+
     /**
      * Are lists enabled?
      *
diff --git a/module/VuFind/src/VuFind/Controller/AbstractRecord.php b/module/VuFind/src/VuFind/Controller/AbstractRecord.php
index 44b3e3b292c..78d2949eeb1 100644
--- a/module/VuFind/src/VuFind/Controller/AbstractRecord.php
+++ b/module/VuFind/src/VuFind/Controller/AbstractRecord.php
@@ -26,7 +26,8 @@
  * @link     https://vufind.org/wiki/development:plugins:controllers Wiki
  */
 namespace VuFind\Controller;
-use VuFind\Exception\Mail as MailException,
+use VuFind\Exception\Forbidden as ForbiddenException,
+    VuFind\Exception\Mail as MailException,
     VuFind\RecordDriver\AbstractBase as AbstractRecordDriver;
 
 /**
@@ -104,6 +105,11 @@ class AbstractRecord extends AbstractBase
      */
     public function addcommentAction()
     {
+        // Make sure comments are enabled:
+        if (!$this->commentsEnabled()) {
+            throw new ForbiddenException('Comments disabled');
+        }
+
         // Force login:
         if (!($user = $this->getUser())) {
             // Remember comment since POST data will be lost:
@@ -145,6 +151,11 @@ class AbstractRecord extends AbstractBase
      */
     public function deletecommentAction()
     {
+        // Make sure comments are enabled:
+        if (!$this->commentsEnabled()) {
+            throw new ForbiddenException('Comments disabled');
+        }
+
         // Force login:
         if (!($user = $this->getUser())) {
             return $this->forceLogin();
@@ -168,7 +179,7 @@ class AbstractRecord extends AbstractBase
     {
         // Make sure tags are enabled:
         if (!$this->tagsEnabled()) {
-            throw new \Exception('Tags disabled');
+            throw new ForbiddenException('Tags disabled');
         }
 
         // Force login:
@@ -203,7 +214,7 @@ class AbstractRecord extends AbstractBase
     {
         // Make sure tags are enabled:
         if (!$this->tagsEnabled()) {
-            throw new \Exception('Tags disabled');
+            throw new ForbiddenException('Tags disabled');
         }
 
         // Force login:
@@ -311,7 +322,7 @@ class AbstractRecord extends AbstractBase
     {
         // Fail if lists are disabled:
         if (!$this->listsEnabled()) {
-            throw new \Exception('Lists disabled');
+            throw new ForbiddenException('Lists disabled');
         }
 
         // Process form submission:
diff --git a/module/VuFind/src/VuFind/Controller/AjaxController.php b/module/VuFind/src/VuFind/Controller/AjaxController.php
index 0393c52166d..23e0d2d2c79 100644
--- a/module/VuFind/src/VuFind/Controller/AjaxController.php
+++ b/module/VuFind/src/VuFind/Controller/AjaxController.php
@@ -1026,6 +1026,15 @@ class AjaxController extends AbstractBase
      */
     protected function commentRecordAjax()
     {
+        // Make sure comments are enabled:
+        if (!$this->commentsEnabled()) {
+            return $this->output(
+                $this->translate('Comments disabled'),
+                self::STATUS_ERROR,
+                403
+            );
+        }
+
         $user = $this->getUser();
         if ($user === false) {
             return $this->output(
@@ -1061,6 +1070,15 @@ class AjaxController extends AbstractBase
      */
     protected function deleteRecordCommentAjax()
     {
+        // Make sure comments are enabled:
+        if (!$this->commentsEnabled()) {
+            return $this->output(
+                $this->translate('Comments disabled'),
+                self::STATUS_ERROR,
+                403
+            );
+        }
+
         $user = $this->getUser();
         if ($user === false) {
             return $this->output(
diff --git a/module/VuFind/src/VuFind/Controller/BrowseController.php b/module/VuFind/src/VuFind/Controller/BrowseController.php
index cd966ba0dbd..e27045bb136 100644
--- a/module/VuFind/src/VuFind/Controller/BrowseController.php
+++ b/module/VuFind/src/VuFind/Controller/BrowseController.php
@@ -26,6 +26,7 @@
  * @link     https://vufind.org/wiki/development:plugins:controllers Wiki
  */
 namespace VuFind\Controller;
+use VuFind\Exception\Forbidden as ForbiddenException;
 
 /**
  * BrowseController Class
@@ -283,7 +284,7 @@ class BrowseController extends AbstractBase
     public function tagAction()
     {
         if (!$this->tagsEnabled()) {
-            throw new \Exception('Tags disabled.');
+            throw new ForbiddenException('Tags disabled.');
         }
 
         $this->setCurrentAction('Tag');
diff --git a/module/VuFind/src/VuFind/Controller/CartController.php b/module/VuFind/src/VuFind/Controller/CartController.php
index dfadc07a704..0b96f80d62c 100644
--- a/module/VuFind/src/VuFind/Controller/CartController.php
+++ b/module/VuFind/src/VuFind/Controller/CartController.php
@@ -26,7 +26,8 @@
  * @link     https://vufind.org Main Site
  */
 namespace VuFind\Controller;
-use VuFind\Exception\Mail as MailException;
+use VuFind\Exception\Forbidden as ForbiddenException,
+    VuFind\Exception\Mail as MailException;
 
 /**
  * Book Bag / Bulk Action Controller
@@ -403,7 +404,7 @@ class CartController extends AbstractBase
     {
         // Fail if lists are disabled:
         if (!$this->listsEnabled()) {
-            throw new \Exception('Lists disabled');
+            throw new ForbiddenException('Lists disabled');
         }
 
         // Load record information first (no need to prompt for login if we just
diff --git a/module/VuFind/src/VuFind/Controller/MyResearchController.php b/module/VuFind/src/VuFind/Controller/MyResearchController.php
index 21d48a5bfa2..059b8ff828f 100644
--- a/module/VuFind/src/VuFind/Controller/MyResearchController.php
+++ b/module/VuFind/src/VuFind/Controller/MyResearchController.php
@@ -28,6 +28,7 @@
 namespace VuFind\Controller;
 
 use VuFind\Exception\Auth as AuthException,
+    VuFind\Exception\Forbidden as ForbiddenException,
     VuFind\Exception\Mail as MailException,
     VuFind\Exception\ListPermission as ListPermissionException,
     VuFind\Exception\RecordMissing as RecordMissingException,
@@ -295,7 +296,7 @@ class MyResearchController extends AbstractBase
         $sessId = $this->getServiceLocator()->get('VuFind\SessionManager')->getId();
         $row = $searchTable->getOwnedRowById($searchId, $sessId, $userId);
         if (empty($row)) {
-            throw new \Exception('Access denied.');
+            throw new ForbiddenException('Access denied.');
         }
         $row->saved = $saved ? 1 : 0;
         $row->user_id = $userId;
@@ -312,7 +313,7 @@ class MyResearchController extends AbstractBase
         // Fail if saved searches are disabled.
         $check = $this->getServiceLocator()->get('VuFind\AccountCapabilities');
         if ($check->getSavedSearchSetting() === 'disabled') {
-            throw new \Exception('Saved searches disabled.');
+            throw new ForbiddenException('Saved searches disabled.');
         }
 
         $user = $this->getUser();
@@ -643,7 +644,7 @@ class MyResearchController extends AbstractBase
     {
         // Fail if lists are disabled:
         if (!$this->listsEnabled()) {
-            throw new \Exception('Lists disabled');
+            throw new ForbiddenException('Lists disabled');
         }
 
         // Check for "delete item" request; parameter may be in GET or POST depending
@@ -775,7 +776,7 @@ class MyResearchController extends AbstractBase
     {
         // Fail if lists are disabled:
         if (!$this->listsEnabled()) {
-            throw new \Exception('Lists disabled');
+            throw new ForbiddenException('Lists disabled');
         }
 
         // User must be logged in to edit list:
@@ -816,7 +817,7 @@ class MyResearchController extends AbstractBase
     {
         // Fail if lists are disabled:
         if (!$this->listsEnabled()) {
-            throw new \Exception('Lists disabled');
+            throw new ForbiddenException('Lists disabled');
         }
 
         // Get requested list ID:
diff --git a/module/VuFind/src/VuFind/Controller/TagController.php b/module/VuFind/src/VuFind/Controller/TagController.php
index e12730e24be..a844d6634ee 100644
--- a/module/VuFind/src/VuFind/Controller/TagController.php
+++ b/module/VuFind/src/VuFind/Controller/TagController.php
@@ -26,6 +26,7 @@
  * @link     https://vufind.org Main Site
  */
 namespace VuFind\Controller;
+use VuFind\Exception\Forbidden as ForbiddenException;
 
 /**
  * Tag Controller
@@ -55,7 +56,7 @@ class TagController extends AbstractSearch
     public function homeAction()
     {
         if (!$this->tagsEnabled()) {
-            throw new \Exception('Tags disabled');
+            throw new ForbiddenException('Tags disabled');
         }
         return $this->resultsAction();
     }
diff --git a/module/VuFind/src/VuFind/Exception/ListPermission.php b/module/VuFind/src/VuFind/Exception/ListPermission.php
index 337ad6056a1..44da98acb00 100644
--- a/module/VuFind/src/VuFind/Exception/ListPermission.php
+++ b/module/VuFind/src/VuFind/Exception/ListPermission.php
@@ -36,6 +36,6 @@ namespace VuFind\Exception;
  * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
  * @link     https://vufind.org/wiki/development Wiki
  */
-class ListPermission extends \Exception
+class ListPermission extends Forbidden
 {
 }
diff --git a/module/VuFind/src/VuFind/Exception/LoginRequired.php b/module/VuFind/src/VuFind/Exception/LoginRequired.php
index 4f44b4e7d8e..e4f82e1a687 100644
--- a/module/VuFind/src/VuFind/Exception/LoginRequired.php
+++ b/module/VuFind/src/VuFind/Exception/LoginRequired.php
@@ -36,6 +36,6 @@ namespace VuFind\Exception;
  * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
  * @link     https://vufind.org/wiki/development Wiki
  */
-class LoginRequired extends \Exception
+class LoginRequired extends Forbidden
 {
 }
-- 
GitLab