acl->request(Acl::PERMISSION_POLL_VIEW) is obsolete

Signed-off-by: dartcafe <github@dartcafe.de>

9 files changed, 115 insertions, 112 deletions

diff --git a/lib/Controller/PollController.php b/lib/Controller/PollController.php
index 7ade65d6..d27deccb 100644
--- a/lib/Controller/PollController.php
+++ b/lib/Controller/PollController.php
@@ -80,11 +80,10 @@ class PollController extends Controller {
 	 */
 	public function get(int $pollId): DataResponse {
 		return $this->response(function () use ($pollId) {
-			$this->poll = $this->pollService->get($pollId);
-			$this->acl->setPoll($this->poll)->request(Acl::PERMISSION_POLL_VIEW);
+			$this->acl->setPollId($pollId);
 			return [
 				'acl' => $this->acl,
-				'poll' => $this->poll,
+				'poll' => $this->acl->getPoll(),
 			];
 		});
 	}
diff --git a/lib/Controller/PublicController.php b/lib/Controller/PublicController.php
index 52706bc2..2aeb41ea 100644
--- a/lib/Controller/PublicController.php
+++ b/lib/Controller/PublicController.php
@@ -144,12 +144,10 @@ class PublicController extends Controller {
 	 */
 	public function getPoll(string $token): DataResponse {
 		return $this->response(function () use ($token) {
-			$this->share = $this->shareService->get($token, true);
-			$this->acl->setShare($this->share);
-			$this->poll = $this->pollService->get($this->share->getPollId());
+			$this->acl->setToken($token);
 			return [
 				'acl' => $this->acl,
-				'poll' => $this->poll,
+				'poll' => $this->acl->getPoll(),
 			];
 		});
 	}
diff --git a/lib/Model/Acl.php b/lib/Model/Acl.php
index 03d51aab..83b4f9c1 100644
--- a/lib/Model/Acl.php
+++ b/lib/Model/Acl.php
@@ -96,45 +96,41 @@ class Acl implements JsonSerializable {
 	 */
 	public function setToken(string $token = ''): Acl {
 		try {
-			return $this->setShare($this->shareMapper->findByToken($token));
+			$this->share = $this->shareMapper->findByToken($token);
+			$this->poll = $this->pollMapper->find($this->share->getPollId());
+			$this->validateShareAccess();
+			$this->request(self::PERMISSION_POLL_VIEW);
+
 		} catch (DoesNotExistException $e) {
 			throw new NotAuthorizedException('Error loading share ' . $token);
 		}
-	}
 
-	/**
-	 * setShare - sets and validates the share
-	 * read access is
-	 */
-	public function setShare(Share $share): Acl {
-		$this->share = $share;
-		$this->validateShareAccess();
-		$this->setPollId($share->getPollId());
-		$this->request(self::PERMISSION_POLL_VIEW);
 		return $this;
 	}
 
-	public function getToken(): string {
-		return strval($this->share->getToken());
-	}
-
 	public function setPollId(?int $pollId = 0): Acl {
 		try {
-			return $this->setPoll($this->pollMapper->find($pollId));
+			$this->poll = $this->pollMapper->find($pollId);
+			$this->request(self::PERMISSION_POLL_VIEW);
 		} catch (DoesNotExistException $e) {
 			throw new NotAuthorizedException('Error loading poll ' . $pollId);
 		}
-	}
 
-	public function setPoll(Poll $poll): Acl {
-		$this->poll = $poll;
 		return $this;
 	}
 
+	public function getToken(): string {
+		return strval($this->share->getToken());
+	}
+
 	public function getPollId(): int {
 		return $this->poll->getId();
 	}
 
+	public function getPoll(): Poll {
+		return $this->poll;
+	}
+
 	public function getUserId(): string {
 		return $this->getLoggedIn() ? \OC::$server->getUserSession()->getUser()->getUID() : $this->share->getUserId();
 	}
@@ -159,50 +155,60 @@ class Acl implements JsonSerializable {
 	public function isAllowed(string $permission): bool {
 		switch ($permission) {
 			case self::PERMISSION_POLL_VIEW:
-				if ($this->getIsOwner() || $this->hasAdminAccess()) {
-					// always grant access, if user has edit rights
-					return true;
-				} elseif ($this->poll->getDeleted()) {
-					// always deny access, if poll is deleted
-					return false;
-				} elseif ($this->poll->getAccess() === Poll::ACCESS_PUBLIC) {
-					// grant access if poll poll is public
-					return true;
-				} elseif ($this->getUserIsInvolved()) {
-					// grant access if user is involved in poll in any way
-					return true;
-				} elseif ($this->getToken()) {
-					// user has token
-					return true;
+			// if ($this->getIsOwner() || $this->hasAdminAccess()) {
+				if ($this->isAllowed(self::PERMISSION_POLL_EDIT)) {
+					return true; // always grant access, if user has edit rights
+				}
+
+				if ($this->poll->getDeleted()) {
+					return false; // always deny access, if poll is deleted
+				}
+
+				if ($this->poll->getAccess() === Poll::ACCESS_PUBLIC) {
+					return true; // grant access if poll poll is public
+				}
+
+				if ($this->getUserIsInvolved()) {
+					return true; // grant access if user is involved in poll in any way
+				}
+
+				if ($this->getToken()) {
+					return true; // user has token
 				}
-				break;
 
 			case self::PERMISSION_POLL_EDIT:
 				return $this->getIsOwner() || $this->hasAdminAccess();
-			case self::PERMISSION_OPTIONS_ADD:
-				return $this->getIsOwner()
-					|| $this->hasAdminAccess()
-					|| ($this->poll->getAllowProposals() === Poll::PROPOSAL_ALLOW
-					&& !$this->poll->getProposalsExpired());
+
 			case self::PERMISSION_POLL_DELETE:
-				return $this->getIsOwner() || $this->hasAdminAccess() || $this->getIsAdmin();
-			case self::PERMISSION_COMMENT_ADD:
-				return $this->share->getType() !== Share::TYPE_PUBLIC && $this->poll->getallowComment();
+				return $this->isAllowed(self::PERMISSION_POLL_EDIT) || $this->getIsAdmin();
+
+			case self::PERMISSION_POLL_TAKEOVER:
+				return $this->getIsAdmin() && !$this->getIsOwner();
+
 			case self::PERMISSION_POLL_SUBSCRIBE:
 				return $this->hasEmail();
-			case self::PERMISSION_VOTE_EDIT:
-				return !$this->poll->getExpired() && $this->share->getType() !== Share::TYPE_PUBLIC;
+
 			case self::PERMISSION_POLL_RESULTS_VIEW:
 				return $this->getIsOwner()
 					|| $this->poll->getShowResults() === Poll::SHOW_RESULTS_ALWAYS
 					|| $this->poll->getShowResults() === Poll::SHOW_RESULTS_CLOSED && $this->poll->getExpired();
+
 			case self::PERMISSION_POLL_USERNAMES_VIEW:
 				return $this->getIsOwner() || !$this->poll->getAnonymous();
-			case self::PERMISSION_POLL_TAKEOVER:
-				return $this->getIsAdmin();
-			default:
-				break;
+
+			case self::PERMISSION_OPTIONS_ADD:
+				return $this->isAllowed(self::PERMISSION_POLL_EDIT)
+					|| ($this->poll->getAllowProposals() === Poll::PROPOSAL_ALLOW
+					&& !$this->poll->getProposalsExpired());
+
+			case self::PERMISSION_COMMENT_ADD:
+				return $this->share->getType() !== Share::TYPE_PUBLIC && $this->poll->getallowComment();
+
+			case self::PERMISSION_VOTE_EDIT:
+				return !$this->poll->getExpired() && $this->share->getType() !== Share::TYPE_PUBLIC;
+
 		}
+
 		return false;
 	}
 
@@ -261,14 +267,15 @@ class Acl implements JsonSerializable {
 
 	/**
 	 * getUserIsInvolved - Is user involved?
-	 * Returns true, if the current user is involved in the share via share or if he is a participant.
+	 * Returns true, if the current user is involved in the poll via share,
+	 * as a participant or as the poll owner.
 	 */
 	private function getUserIsInvolved(): bool {
 		return (
 			   $this->getIsOwner()
 			|| $this->getUserHasVoted()
-			|| $this->getGroupShare()
-			|| $this->getPersonalShare());
+			|| $this->isInvitedViaGroupShare()
+			|| $this->isPersonallyInvited());
 	}
 
 	/**
@@ -282,57 +289,59 @@ class Acl implements JsonSerializable {
 	}
 
 	/**
-	 * getGroupShare - Is the poll shared via group share?
+	 * isInvitedViaGroupShare - Is the poll shared via group share?
 	 * Returns true, if the current poll contains a group share with a group,
-	 * where the current user is member of. This only affects logged users.
+	 * where the current user is member of. This only affects logged in users.
 	 */
-	private function getGroupShare(): int {
-		if (!$this->getLoggedIn()) {
-			return 0;
+	private function isInvitedViaGroupShare(): bool {
+		if ($this->getLoggedIn()) {
+			return !!count(
+				array_filter($this->shareMapper->findByPoll($this->getPollId()), function ($item) {
+					return ($item->getType() === Share::TYPE_GROUP && $this->groupManager->isInGroup($this->getUserId(), $item->getUserId()));
+				})
+			);
 		}
-		return count(
-			array_filter($this->shareMapper->findByPoll($this->getPollId()), function ($item) {
-				return ($item->getType() === Share::TYPE_GROUP && $this->groupManager->isInGroup($this->getUserId(), $item->getUserId()));
-			})
-		);
+
+		return false;
 	}
 
 	/**
-	 * getPersonalShare - Is the poll shared via user share?
-	 * Returns >0, if the current poll contains a user share for the current user.
-	 * This only affects logged users.
+	 * isPersonallyInvited - Is the poll shared via user share?
+	 * Returns true, if the current poll contains a user share for the current user.
+	 * This only affects logged in users.
 	 */
-	private function getPersonalShare(): int {
-		if (!$this->getLoggedIn()) {
-			return 0;
+	private function isPersonallyInvited(): bool {
+		if ($this->getLoggedIn()) {
+			return !!count(
+				array_filter($this->shareMapper->findByPoll($this->getPollId()), function ($item) {
+					return ($item->getUserId() === $this->getUserId()
+						&& in_array($item->getType(), [
+							Share::TYPE_USER,
+							Share::TYPE_EXTERNAL,
+							Share::TYPE_EMAIL,
+							Share::TYPE_CONTACT
+						])
+					);
+				})
+			);
 		}
-		return count(
-			array_filter($this->shareMapper->findByPoll($this->getPollId()), function ($item) {
-				return ($item->getUserId() === $this->getUserId()
-					&& in_array($item->getType(), [
-						Share::TYPE_USER,
-						Share::TYPE_EXTERNAL,
-						Share::TYPE_EMAIL,
-						Share::TYPE_CONTACT
-					])
-				);
-			})
-		);
+
+		return false;
 	}
 
 	private function validateShareAccess(): void {
 		if ($this->getLoggedIn()) {
-			if (!$this->getValidAuthenticatedShare()) {
-				throw new NotAuthorizedException('Share type "' . $this->share->getType() . '"only valid for external users');
+			if (!$this->isShareValidForUsers()) {
+				throw new NotAuthorizedException('Share type "' . $this->share->getType() . '"only valid for guests');
 			};
 		} else {
-			if (!$this->getValidPublicShare()) {
-				throw new NotAuthorizedException('Share type "' . $this->share->getType() . '"only valid for internal users');
+			if (!$this->isShareValidForGuests()) {
+				throw new NotAuthorizedException('Share type "' . $this->share->getType() . '"only valid for registered users');
 			};
 		}
 	}
 
-	private function getValidPublicShare(): bool {
+	private function isShareValidForGuests(): bool {
 		return in_array($this->share->getType(), [
 			Share::TYPE_PUBLIC,
 			Share::TYPE_EMAIL,
@@ -341,7 +350,7 @@ class Acl implements JsonSerializable {
 		]);
 	}
 
-	private function getValidAuthenticatedShare(): bool {
+	private function isShareValidForUsers(): bool {
 		return in_array($this->share->getType(), [
 			Share::TYPE_PUBLIC,
 			Share::TYPE_USER,
diff --git a/lib/Service/CommentService.php b/lib/Service/CommentService.php
index a8cb5a4c..9ce0c51a 100644
--- a/lib/Service/CommentService.php
+++ b/lib/Service/CommentService.php
@@ -67,7 +67,7 @@ class CommentService {
 		if ($token) {
 			$this->acl->setToken($token);
 		} else {
-			$this->acl->setPollId($pollId)->request(Acl::PERMISSION_POLL_VIEW);
+			$this->acl->setPollId($pollId);
 		}
 
 		if ($this->acl->isAllowed(Acl::PERMISSION_POLL_USERNAMES_VIEW)) {
diff --git a/lib/Service/OptionService.php b/lib/Service/OptionService.php
index 80add2fe..6d29823d 100644
--- a/lib/Service/OptionService.php
+++ b/lib/Service/OptionService.php
@@ -109,11 +109,11 @@ class OptionService {
 	 */
 	public function list(int $pollId = 0, string $token = ''): array {
 		if ($token) {
-			$this->acl->setToken($token)->request(Acl::PERMISSION_POLL_VIEW);
-			$pollId = $this->acl->getPollId();
+			$this->acl->setToken($token);
 		} else {
-			$this->acl->setPollId($pollId)->request(Acl::PERMISSION_POLL_VIEW);
+			$this->acl->setPollId($pollId);
 		}
+		$this->poll = $this->acl->getPoll();
 
 		try {
 			$this->poll = $this->pollMapper->find($pollId);
@@ -142,9 +142,9 @@ class OptionService {
 	 * @return Option
 	 */
 	public function get(int $optionId): Option {
-		$this->acl->setPollId($this->optionMapper->find($optionId)->getPollId())
-			->request(Acl::PERMISSION_POLL_VIEW);
-		return $this->optionMapper->find($optionId);
+		$option = $this->optionMapper->find($optionId);
+		$this->acl->setPollId($option->getPollId());
+		return $option;
 	}
 
 
diff --git a/lib/Service/PollService.php b/lib/Service/PollService.php
index 584f964a..0e2c2d5e 100644
--- a/lib/Service/PollService.php
+++ b/lib/Service/PollService.php
@@ -104,7 +104,7 @@ class PollService {
 
 			foreach ($polls as $poll) {
 				try {
-					$this->acl->setPoll($poll)->request(Acl::PERMISSION_POLL_VIEW);
+					$this->acl->setPollId($poll->getId());
 					// TODO: Not the elegant way. Improvement neccessary
 					$pollList[] = (object) array_merge(
 						(array) json_decode(json_encode($poll)),
@@ -165,13 +165,10 @@ class PollService {
 
 	/**
 	 * get poll configuration
-	 *
-	 * @return Poll
 	 */
 	public function get(int $pollId): Poll {
-		$this->poll = $this->pollMapper->find($pollId);
-		$this->acl->setPoll($this->poll)->request(Acl::PERMISSION_POLL_VIEW);
-		return $this->poll;
+		$this->acl->setPollId($pollId);
+		return $this->acl->getPoll();
 	}
 
 	/**
@@ -314,8 +311,8 @@ class PollService {
 	 * @return Poll
 	 */
 	public function clone(int $pollId): Poll {
-		$origin = $this->pollMapper->find($pollId);
-		$this->acl->setPoll($origin)->request(Acl::PERMISSION_POLL_VIEW);
+		$this->acl->setPollId($pollId);
+		$origin = $this->acl->getPoll();
 
 		$this->poll = new Poll();
 		$this->poll->setCreated(time());
diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php
index fd45c96f..c34251b0 100644
--- a/lib/Service/ShareService.php
+++ b/lib/Service/ShareService.php
@@ -165,7 +165,7 @@ class ShareService {
 		if ($this->share->getType() === Share::TYPE_PUBLIC && \OC::$server->getUserSession()->isLoggedIn()) {
 			try {
 				// Test if the user has already access.
-				$this->acl->setPollId($this->share->getPollId())->request(Acl::PERMISSION_POLL_VIEW);
+				$this->acl->setPollId($this->share->getPollId());
 			} catch (NotAuthorizedException $e) {
 				// If he is not authorized until now, create a new personal share for this user.
 				// Return the created share
diff --git a/lib/Service/SubscriptionService.php b/lib/Service/SubscriptionService.php
index 251cfe49..f24a2606 100644
--- a/lib/Service/SubscriptionService.php
+++ b/lib/Service/SubscriptionService.php
@@ -50,7 +50,7 @@ class SubscriptionService {
 		if ($token) {
 			$this->acl->setToken($token);
 		} else {
-			$this->acl->setPollId($pollId)->request(Acl::PERMISSION_POLL_VIEW);
+			$this->acl->setPollId($pollId);
 		}
 
 		try {
@@ -73,7 +73,7 @@ class SubscriptionService {
 		if ($token) {
 			$this->acl->setToken($token);
 		} else {
-			$this->acl->setPollId($pollId)->request(Acl::PERMISSION_POLL_VIEW);
+			$this->acl->setPollId($pollId);
 		}
 
 		if (!$subscribed) {
diff --git a/lib/Service/VoteService.php b/lib/Service/VoteService.php
index 04987721..b2a026d4 100644
--- a/lib/Service/VoteService.php
+++ b/lib/Service/VoteService.php
@@ -89,7 +89,7 @@ class VoteService {
 		if ($token) {
 			$this->acl->setToken($token);
 		} else {
-			$this->acl->setPollId($pollId)->request(Acl::PERMISSION_POLL_VIEW);
+			$this->acl->setPollId($pollId);
 		}
 
 		try {