Prevent leaking personnal infos on forms

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

lib/AppInfo/Application.php

@@ -6,6 +6,7 @@ declare(strict_types=1);
  * @copyright Copyright (c) 2017 Vinzenz Rosenkranz <vinzenz.rosenkranz@gmail.com>
  *
  * @author affan98 <affan98@gmail.com>
+ * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  *
  * @license GNU AGPL version 3 or any later version

lib/Controller/ApiController.php

@@ -128,13 +128,18 @@ class ApiController extends Controller {
 	 */
 	public function getForm(int $id): Http\JSONResponse {
 		try {
-			$results = $this->formsService->getForm($id);
+			$form = $this->formsService->getForm($id);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form');
 			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
 		}
+	
+		if (!$this->formsService->hasUserAccess($id)) {
+			$this->logger->debug('User has no permissions to get this form');
+			return new Http\JSONResponse([], Http::STATUS_FORBIDDEN);
+		}
 
-		return new Http\JSONResponse($results);
+		return new Http\JSONResponse($form);
 	}
 
 	/**
@@ -297,7 +302,7 @@ class ApiController extends Controller {
 			$form = $this->formMapper->findById($formId);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			return new Http\JSONResponse(['message' => 'Could not find form'], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($form->getOwnerId() !== $this->userId) {
@@ -307,15 +312,15 @@ class ApiController extends Controller {
 
 		// Check if array contains duplicates
 		if (array_unique($newOrder) !== $newOrder) {
-			$this->logger->debug('The given Array contains duplicates.');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			$this->logger->debug('The given Array contains duplicates');
+			return new Http\JSONResponse(['message' => 'The given Array contains duplicates'], Http::STATUS_BAD_REQUEST);
 		}
 
 		// Check if all questions are given in Array.
 		$questions = $this->questionMapper->findByForm($formId);
 		if (sizeof($questions) !== sizeof($newOrder)) {
 			$this->logger->debug('The length of the given array does not match the number of stored questions');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			return new Http\JSONResponse(['message' => 'The length of the given array does not match the number of stored questions'], Http::STATUS_BAD_REQUEST);
 		}
 
 		$questions = []; // Clear Array of Entities
@@ -385,8 +390,8 @@ class ApiController extends Controller {
 			$question = $this->questionMapper->findById($id);
 			$form = $this->formMapper->findById($question->getFormId());
 		} catch (IMapperException $e) {
-			$this->logger->debug('Could not find question or form');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			$this->logger->debug('Could not find form or question');
+			return new Http\JSONResponse(['message' => 'Could not find form or question'], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($form->getOwnerId() !== $this->userId) {
@@ -396,7 +401,7 @@ class ApiController extends Controller {
 
 		if (array_key_exists('order', $keyValuePairs)) {
 			$this->logger->debug('Key \'order\' is not allowed on updateQuestion. Please use reorderQuestions() to change order.');
-			return new Http\JSONResponse([], Http::STATUS_FORBIDDEN);
+			return new Http\JSONResponse(['message' => 'Please use reorderQuestions() to change order'], Http::STATUS_FORBIDDEN);
 		}
 
 		// Create QuestionEntity with given Params & Id.
@@ -422,7 +427,7 @@ class ApiController extends Controller {
 			$form = $this->formMapper->findById($question->getFormId());
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form or question');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			return new Http\JSONResponse(['message' => 'Could not find form or question'], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($form->getOwnerId() !== $this->userId) {
@@ -463,8 +468,8 @@ class ApiController extends Controller {
 			$question = $this->questionMapper->findById($questionId);
 			$form = $this->formMapper->findById($question->getFormId());
 		} catch (IMapperException $e) {
-			$this->logger->debug('Could not find form or question so option can\'t be added');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			$this->logger->debug('Could not find form or question');
+			return new Http\JSONResponse(['message' => 'Could not find form or question'], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($form->getOwnerId() !== $this->userId) {
@@ -535,7 +540,7 @@ class ApiController extends Controller {
 			$form = $this->formMapper->findById($question->getFormId());
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form or option');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			return new Http\JSONResponse(['message' => 'Could not find form or option'], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($form->getOwnerId() !== $this->userId) {
@@ -556,7 +561,7 @@ class ApiController extends Controller {
 			$form = $this->formMapper->findByHash($hash);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form');
-			return new Http\JSONResponse([], Http::STATUS_BAD_REQUEST);
+			return new Http\JSONResponse(['message' => 'Could not find form'], Http::STATUS_BAD_REQUEST);
 		}
 
 		if ($form->getOwnerId() !== $this->userId) {
@@ -592,7 +597,7 @@ class ApiController extends Controller {
 	 * @param int $formId
 	 * @param array $answers [question_id => arrayOfString]
 	 */
-	public function insertSubmission(int $formId, array $answers) {
+	public function insertSubmission(int $formId, array $answers): Http\JSONResponse {
 		$this->logger->debug('Inserting submission: formId: {formId}, answers: {answers}', [
 			'formId' => $formId,
 			'answers' => $answers,
@@ -606,13 +611,16 @@ class ApiController extends Controller {
 			return new Http\JSONResponse(['message' => 'Could not find form'], Http::STATUS_BAD_REQUEST);
 		}
 
-		$user = $this->userSession->getUser();
-		// TODO check again hasUserAccess?!
+		// Does the user have permissions to display
+		if (!$this->formsService->canSubmit($form->getId())) {
+			return new Http\JSONResponse(['message' => 'Already submitted'], Http::STATUS_FORBIDDEN);
+		}
 
 		// Create Submission
 		$submission = new Submission();
 		$submission->setFormId($formId);
 		$submission->setTimestamp(time());
+		$user = $this->userSession->getUser();
 
 		// If not logged in or anonymous use anonID
 		if (!$user || $form->getIsAnonymous()) {

lib/Controller/PageController.php

@@ -5,7 +5,6 @@
  * @author affan98 <affan98@gmail.com>
  * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  * @author Jonas Rittershofer <jotoeri@users.noreply.github.com>
- * @author Marcel Klehr <mklehr@gmx.net>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  *
  * @license GNU AGPL version 3 or any later version
@@ -185,7 +184,12 @@ class PageController extends Controller {
 		}
 
 		// Does the user have permissions to display
-		if (!$this->hasUserAccess($form)) {
+		if (!$this->formsService->canSubmit($form->getId())) {
+			return new TemplateResponse('forms', 'nosubmit');
+		}
+
+		// Does the user have permissions to display
+		if (!$this->formsService->hasUserAccess($form->getId())) {
 			return new TemplateResponse('forms', 'notfound');
 		}
 
@@ -197,66 +201,8 @@ class PageController extends Controller {
 		$renderAs = $this->userSession->isLoggedIn() ? 'user' : 'public';
 
 		Util::addScript($this->appName, 'submit');
-		$this->initialStateService->provideInitialState($this->appName, 'form', $this->formsService->getForm($form->getId()));
+		$this->initialStateService->provideInitialState($this->appName, 'form', $this->formsService->getPublicForm($form->getId()));
 		$this->initialStateService->provideInitialState($this->appName, 'maxStringLengths', $this->maxStringLengths);
 		return new TemplateResponse($this->appName, 'main', [], $renderAs);
 	}
-
-	/**
-	 * @NoAdminRequired
-	 * Check if user has access to this form
-	 *
-	 * @param Form $form
-	 * @return boolean
-	 */
-	private function hasUserAccess(Form $form): bool {
-		$access = $form->getAccess();
-		$ownerId = $form->getOwnerId();
-		$user = $this->userSession->getUser();
-
-		if ($access['type'] === 'public') {
-			return true;
-		}
-		
-		// Refuse access, if not public and no user logged in.
-		if (!$user) {
-			return false;
-		}
-
-		// Always grant access to owner.
-		if ($ownerId === $user->getUID()) {
-			return true;
-		}
-
-		// Refuse access, if SubmitOnce is set and user already has taken part.
-		if ($form->getSubmitOnce()) {
-			$participants = $this->submissionMapper->findParticipantsByForm($form->getId());
-			foreach ($participants as $participant) {
-				if ($participant === $user->getUID()) {
-					return false;
-				}
-			}
-		}
-
-		// Now all remaining users are allowed, if access-type 'registered'.
-		if ($access['type'] === 'registered') {
-			return true;
-		}
-
-		// Selected Access remains.
-		// Grant Access, if user is in users-Array.
-		if (in_array($user->getUID(), $access['users'])) {
-			return true;
-		}
-
-		// Check if access granted by group.
-		foreach ($access['groups'] as $group) {
-			if ($this->groupManager->isInGroup($user->getUID(), $group)) {
-				return true;
-			}
-		}
-
-		// None of the possible access-options matched.
-		return false;
-	}
 }

lib/Controller/SystemController.php

@@ -3,6 +3,7 @@
  * @copyright Copyright (c) 2017 Vinzenz Rosenkranz <vinzenz.rosenkranz@gmail.com>
  *
  * @author affan98 <affan98@gmail.com>
+ * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  *
  * @license GNU AGPL version 3 or any later version

lib/Db/FormMapper.php

@@ -3,6 +3,7 @@
  * @copyright Copyright (c) 2017 Vinzenz Rosenkranz <vinzenz.rosenkranz@gmail.com>
  *
  * @author affan98 <affan98@gmail.com>
+ * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  * @author Jonas Rittershofer <jotoeri@users.noreply.github.com>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  *

lib/Db/OptionMapper.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2019 Inigo Jiron <ijiron@terpmail.umd.edu>
  *
+ * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  * @author Jonas Rittershofer <jotoeri@users.noreply.github.com>
  *
  * @license GNU AGPL version 3 or any later version

lib/Db/SubmissionMapper.php

@@ -3,6 +3,7 @@
  * @copyright Copyright (c) 2020 Jonas Rittershofer <jotoeri@users.noreply.github.com>
  *
  * @author affan98 <affan98@gmail.com>
+ * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  * @author Jonas Rittershofer <jotoeri@users.noreply.github.com>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  *

lib/Service/FormsService.php

@@ -26,8 +26,11 @@ namespace OCA\Forms\Service;
 use OCA\Forms\Db\FormMapper;
 use OCA\Forms\Db\OptionMapper;
 use OCA\Forms\Db\QuestionMapper;
+use OCA\Forms\Db\SubmissionMapper;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\IMapperException;
+use OCP\IGroupManager;
+use OCP\IUserSession;
 
 /**
  * Trait for getting forms information in a service
@@ -43,12 +46,27 @@ class FormsService {
 	/** @var OptionMapper */
 	private $optionMapper;
 
+	/** @var SubmissionMapper */
+	private $submissionMapper;
+	
+	/** @var IGroupManager */
+	private $groupManager;
+
+	/** @var IUserSession */
+	private $userSession;
+
 	public function __construct(FormMapper $formMapper,
 								QuestionMapper $questionMapper,
-								OptionMapper $optionMapper) {
+								OptionMapper $optionMapper,
+								SubmissionMapper $submissionMapper,
+								IGroupManager $groupManager,
+								IUserSession $userSession) {
 		$this->formMapper = $formMapper;
 		$this->questionMapper = $questionMapper;
 		$this->optionMapper = $optionMapper;
+		$this->submissionMapper = $submissionMapper;
+		$this->groupManager = $groupManager;
+		$this->userSession = $userSession;
 	}
 
 
@@ -96,4 +114,95 @@ class FormsService {
 
 		return $result;
 	}
+
+	/**
+	 * Get a form data without sensitive informations
+	 *
+	 * @param integer $id
+	 * @return array
+	 * @throws IMapperException
+	 */
+	public function getPublicForm(int $id): array {
+		$form = $this->getForm($id);
+
+		// Remove sensitive data
+		unset($form['access']);
+		unset($form['ownerId']);
+
+		return $form;
+	}
+
+	/**
+	 * Can the user submit a form
+	 */
+	public function canSubmit($formId) {
+		$form = $this->formMapper->findById($formId);
+		$access = $form->getAccess();
+		$user = $this->userSession->getUser();
+
+		// We cannot control how many time users can submit in public mode
+		if ($access['type'] === 'public') {
+			return true;
+		}
+
+		// Refuse access, if SubmitOnce is set and user already has taken part.
+		if ($form->getSubmitOnce()) {
+			$participants = $this->submissionMapper->findParticipantsByForm($form->getId());
+			foreach ($participants as $participant) {
+				if ($participant === $user->getUID()) {
+					return false;
+				}
+			}
+		}
+
+		return true;
+	}
+
+	/**
+	 * Check if user has access to this form
+	 *
+	 * @param integer $formId
+	 * @return boolean
+	 */
+	public function hasUserAccess(int $formId): bool {
+		$form = $this->formMapper->findById($formId);
+		$access = $form->getAccess();
+		$ownerId = $form->getOwnerId();
+		$user = $this->userSession->getUser();
+
+		if ($access['type'] === 'public') {
+			return true;
+		}
+		
+		// Refuse access, if not public and no user logged in.
+		if (!$user) {
+			return false;
+		}
+
+		// Always grant access to owner.
+		if ($ownerId === $user->getUID()) {
+			return true;
+		}
+
+		// Now all remaining users are allowed, if access-type 'registered'.
+		if ($access['type'] === 'registered') {
+			return true;
+		}
+
+		// Selected Access remains.
+		// Grant Access, if user is in users-Array.
+		if (in_array($user->getUID(), $access['users'])) {
+			return true;
+		}
+
+		// Check if access granted by group.
+		foreach ($access['groups'] as $group) {
+			if ($this->groupManager->isInGroup($user->getUID(), $group)) {
+				return true;
+			}
+		}
+
+		// None of the possible access-options matched.
+		return false;
+	}
 }

templates/expired.php

@@ -1,8 +1,9 @@
 <?php
 /**
- * @copyright Copyright (c) 2019 Inigo Jiron <ijiron@terpmail.umd.edu>
+ * @copyright Copyright (c) 2020 John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  *
  * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
+ * @author rakekniven <mark.ziegler@rakekniven.de>
  *
  * @license GNU AGPL version 3 or any later version
  *
@@ -22,6 +23,7 @@
  */
 
 ?>
+
 <div id="emptycontent" class="">
 	<div class="icon-forms"></div>
 	<h2><?php p($l->t('Form expired')); ?></h2>

templates/nosubmit.php

@@ -0,0 +1,29 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020 John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
+ *
+ * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+?>
+
+<div id="emptycontent" class="">
+	<div class="icon-checkmark"></div>
+	<h2><?php p($l->t('Thank you for completing the form!')); ?></h2>
+</div>

templates/notfound.php

@@ -1,8 +1,9 @@
 <?php
 /**
- * @copyright Copyright (c) 2019 Inigo Jiron <ijiron@terpmail.umd.edu>
+ * @copyright Copyright (c) 2020 John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
  *
  * @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
+ * @author rakekniven <mark.ziegler@rakekniven.de>
  *
  * @license GNU AGPL version 3 or any later version
  *
@@ -22,6 +23,7 @@
  */
 
 ?>
+
 <div id="emptycontent" class="">
 	<div class="icon-forms"></div>
 	<h2><?php p($l->t('Form not found')); ?></h2>