Skip to content

Commit

Permalink
fix: Add brute force protection to form endpoints
Browse files Browse the repository at this point in the history 
* fix: Add brute force protection to form endpoints

Endpoints that query for forms are now protected against brute force
attacks to find valid forms, invalid hashes or IDs.

---------

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Christian Hartmann <[email protected]>
Co-authored-by: Christian Hartmann <[email protected]>
  • Loading branch information
@susnux @Chartman123
susnux and Chartman123 authored Jan 23, 2025
1 parent 04bea4c commit 6b8fdad
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 97 deletions.
2 changes: 2 additions & 0 deletions lib/AppInfo/Application.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OCA\Forms\FormsMigrator;
use OCA\Forms\Listener\AnalyticsDatasourceListener;
use OCA\Forms\Listener\UserDeletedListener;
use OCA\Forms\Middleware\ThrottleFormAccessMiddleware;
use OCA\Forms\Search\SearchProvider;
use OCP\AppFramework\App;
use OCP\AppFramework\Bootstrap\IBootContext;
Expand Down Expand Up @@ -43,6 +44,7 @@ public function register(IRegistrationContext $context): void {
$context->registerCapability(Capabilities::class);
$context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class);
$context->registerEventListener(DatasourceEvent::class, AnalyticsDatasourceListener::class);
$context->registerMiddleware(ThrottleFormAccessMiddleware::class);
$context->registerSearchProvider(SearchProvider::class);
$context->registerUserMigrator(FormsMigrator::class);
}
Expand Down
154 changes: 73 additions & 81 deletions lib/Controller/ApiController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use OCA\Forms\Db\SubmissionMapper;
use OCA\Forms\Db\UploadedFile;
use OCA\Forms\Db\UploadedFileMapper;
use OCA\Forms\Exception\NoSuchFormException;
use OCA\Forms\ResponseDefinitions;
use OCA\Forms\Service\ConfigService;
use OCA\Forms\Service\FormsService;
Expand All @@ -31,6 +32,7 @@
use OCP\AppFramework\Db\IMapperException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\ApiRoute;
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
use OCP\AppFramework\Http\Attribute\CORS;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
Expand Down Expand Up @@ -147,6 +149,7 @@ public function getForms(string $type = 'owned'): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'POST', url: '/api/v3/forms')]
public function newForm(?int $fromId = null): DataResponse {
// Check if user is allowed
Expand Down Expand Up @@ -226,19 +229,10 @@ public function newForm(?int $fromId = null): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'GET', url: '/api/v3/forms/{formId}')]
public function getForm(int $formId): DataResponse {
try {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form');
throw new OCSBadRequestException('Could not find form');
}

if (!$this->formsService->hasUserAccess($form)) {
$this->logger->debug('User has no permissions to get this form');
throw new OCSForbiddenException('User has no permissions to get this form');
}
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_SUBMIT);

return new DataResponse($this->formsService->getForm($form));
}
Expand All @@ -259,6 +253,7 @@ public function getForm(int $formId): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}')]
public function updateForm(int $formId, array $keyValuePairs): DataResponse {
$this->logger->debug('Updating form: formId: {formId}, values: {keyValuePairs}', [
Expand Down Expand Up @@ -359,6 +354,7 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'DELETE', url: '/api/v3/forms/{formId}')]
public function deleteForm(int $formId): DataResponse {
$this->logger->debug('Delete Form: {formId}', [
Expand Down Expand Up @@ -472,6 +468,7 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions')]
public function newQuestion(int $formId, ?string $type = null, string $text = '', ?int $fromId = null): DataResponse {
$form = $this->getFormIfAllowed($formId);
Expand Down Expand Up @@ -594,6 +591,7 @@ public function newQuestion(int $formId, ?string $type = null, string $text = ''
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}')]
public function updateQuestion(int $formId, int $questionId, array $keyValuePairs): DataResponse {
$this->logger->debug('Updating question: formId: {formId}, questionId: {questionId}, values: {keyValuePairs}', [
Expand All @@ -602,6 +600,14 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
'keyValuePairs' => $keyValuePairs
]);

// Make sure we query the form first to check the user has permissions
// So the user does not get information about "questions" if they do not even have permissions to the form
$form = $this->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

try {
$question = $this->questionMapper->findById($questionId);
} catch (IMapperException $e) {
Expand All @@ -613,12 +619,6 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
throw new OCSBadRequestException('Question doesn\'t belong to given Form');
}

$form = $this->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

// Don't allow empty array
if (sizeof($keyValuePairs) === 0) {
$this->logger->info('Empty keyValuePairs, will not update.');
Expand Down Expand Up @@ -668,12 +668,20 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'DELETE', url: '/api/v3/forms/{formId}/questions/{questionId}')]
public function deleteQuestion(int $formId, int $questionId): DataResponse {
$this->logger->debug('Mark question as deleted: {questionId}', [
'questionId' => $questionId,
]);


$form = $this->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

try {
$question = $this->questionMapper->findById($questionId);
} catch (IMapperException $e) {
Expand All @@ -685,12 +693,6 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
throw new OCSBadRequestException('Question doesn\'t belong to given Form');
}

$form = $this->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

// Store Order of deleted Question
$deletedOrder = $question->getOrder();

Expand Down Expand Up @@ -732,6 +734,7 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions')]
public function reorderQuestions(int $formId, array $newOrder): DataResponse {
$this->logger->debug('Reordering Questions on Form {formId} as Question-Ids {newOrder}', [
Expand Down Expand Up @@ -829,6 +832,7 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions/{questionId}/options')]
public function newOption(int $formId, int $questionId, array $optionTexts): DataResponse {
$this->logger->debug('Adding new options: formId: {formId}, questionId: {questionId}, text: {text}', [
Expand Down Expand Up @@ -909,6 +913,7 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}/options/{optionId}')]
public function updateOption(int $formId, int $questionId, int $optionId, array $keyValuePairs): DataResponse {
$this->logger->debug('Updating option: form: {formId}, question: {questionId}, option: {optionId}, values: {keyValuePairs}', [
Expand Down Expand Up @@ -978,6 +983,7 @@ public function updateOption(int $formId, int $questionId, int $optionId, array
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'DELETE', url: '/api/v3/forms/{formId}/questions/{questionId}/options/{optionId}')]
public function deleteOption(int $formId, int $questionId, int $optionId): DataResponse {
$this->logger->debug('Deleting option: {optionId}', [
Expand Down Expand Up @@ -1037,6 +1043,7 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}/options')]
public function reorderOptions(int $formId, int $questionId, array $newOrder) {
$form = $this->getFormIfAllowed($formId);
Expand Down Expand Up @@ -1134,19 +1141,10 @@ public function reorderOptions(int $formId, int $questionId, array $newOrder) {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'GET', url: '/api/v3/forms/{formId}/submissions')]
public function getSubmissions(int $formId, ?string $fileFormat = null): DataResponse|DataDownloadResponse {
try {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form');
throw new OCSNotFoundException('Could not find form');
}

if (!$this->formsService->canSeeResults($form)) {
$this->logger->debug('The current user has no permission to get the results for this form');
throw new OCSForbiddenException('The current user has no permission to get the results for this form');
}
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_RESULTS);

if ($fileFormat !== null) {
$submissionsData = $this->submissionService->getSubmissionsData($form, $fileFormat);
Expand Down Expand Up @@ -1205,24 +1203,10 @@ public function getSubmissions(int $formId, ?string $fileFormat = null): DataRes
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'DELETE', url: '/api/v3/forms/{formId}/submissions')]
public function deleteAllSubmissions(int $formId): DataResponse {
$this->logger->debug('Delete all submissions to form: {formId}', [
'formId' => $formId,
]);

try {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form');
throw new OCSNotFoundException('Could not find form');
}

// The current user has permissions to remove submissions
if (!$this->formsService->canDeleteResults($form)) {
$this->logger->debug('This form is not owned by the current user and user has no `results_delete` permission');
throw new OCSForbiddenException('This form is not owned by the current user and user has no `results_delete` permission');
}
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_RESULTS_DELETE);

// Delete all submissions (incl. Answers)
$this->submissionMapper->deleteByForm($formId);
Expand Down Expand Up @@ -1337,31 +1321,26 @@ public function newSubmission(int $formId, array $answers, string $shareHash = '
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'DELETE', url: '/api/v3/forms/{formId}/submissions/{submissionId}')]
public function deleteSubmission(int $formId, int $submissionId): DataResponse {
$this->logger->debug('Delete Submission: {submissionId}', [
'submissionId' => $submissionId,
]);

$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_RESULTS_DELETE);
try {
$submission = $this->submissionMapper->findById($submissionId);
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form or submission');
throw new OCSNotFoundException('Could not find form or submission');
$this->logger->debug('Could not find submission');
throw new OCSNotFoundException('Could not find submission');
}

if ($formId !== $submission->getFormId()) {
$this->logger->debug('Submission doesn\'t belong to given form');
throw new OCSBadRequestException('Submission doesn\'t belong to given form');
}

// The current user has permissions to remove submissions
if (!$this->formsService->canDeleteResults($form)) {
$this->logger->debug('This form is not owned by the current user and user has no `results_delete` permission');
throw new OCSForbiddenException('This form is not owned by the current user and user has no `results_delete` permission');
}

// Delete submission (incl. Answers)
$this->submissionMapper->deleteById($submissionId);
$this->formMapper->update($form);
Expand All @@ -1383,20 +1362,10 @@ public function deleteSubmission(int $formId, int $submissionId): DataResponse {
*/
#[CORS()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/submissions/export')]
public function exportSubmissionsToCloud(int $formId, string $path, string $fileFormat = Constants::DEFAULT_FILE_FORMAT) {
try {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form');
throw new OCSNotFoundException('Could not find form');
}

if (!$this->formsService->canSeeResults($form)) {
$this->logger->debug('The current user has no permission to get the results for this form');
throw new OCSForbiddenException('The current user has no permission to get the results for this form');
}

$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_RESULTS);
$file = $this->submissionService->writeFileToCloud($form, $path, $fileFormat);

return new DataResponse($file->getName());
Expand All @@ -1421,8 +1390,9 @@ public function exportSubmissionsToCloud(int $formId, string $path, string $file
* 200: the file id and name of the uploaded file
*/
#[CORS()]
#[NoAdminRequired()]
#[PublicPage()]
#[NoAdminRequired()]
#[BruteForceProtection(action: 'form')]
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/submissions/files/{questionId}')]
public function uploadFiles(int $formId, int $questionId, string $shareHash = ''): DataResponse {
$this->logger->debug('Uploading files for formId: {formId}, questionId: {questionId}', [
Expand Down Expand Up @@ -1614,7 +1584,7 @@ private function loadFormForSubmission(int $formId, string $shareHash): Form {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form');
throw new OCSNotFoundException('Could not find form');
throw new NoSuchFormException('Could not find form');
}

// Does the user have access to the form (Either by logged-in user, or by providing public share-hash.)
Expand All @@ -1634,7 +1604,7 @@ private function loadFormForSubmission(int $formId, string $shareHash): Form {
} finally {
// Now forbid, if no public share and no direct share.
if (!$isPublicShare && !$this->formsService->hasUserAccess($form)) {
throw new OCSForbiddenException('Not allowed to access this form');
throw new NoSuchFormException('Not allowed to access this form', Http::STATUS_FORBIDDEN);
}
}

Expand All @@ -1650,20 +1620,42 @@ private function loadFormForSubmission(int $formId, string $shareHash): Form {
* Helper that retrieves a form if the current user is allowed to edit it
* This throws an exception in case either the form is not found or permissions are missing.
* @param int $formId The form ID to retrieve
* @throws OCSNotFoundException If the form was not found
* @throws OCSForbiddenException If the current user has no permission to edit
* @throws NoSuchFormException If the form was not found or the current user has no permission to edit
*/
private function getFormIfAllowed(int $formId): Form {
private function getFormIfAllowed(int $formId, string $permissions = 'all'): Form {
try {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form');
throw new OCSNotFoundException('Could not find form');
throw new NoSuchFormException('Could not find form');
}

if ($form->getOwnerId() !== $this->currentUser->getUID()) {
$this->logger->debug('This form is not owned by the current user');
throw new OCSForbiddenException('This form is not owned by the current user');
switch ($permissions) {
case Constants::PERMISSION_SUBMIT:
if (!$this->formsService->hasUserAccess($form)) {
$this->logger->debug('User has no permissions to get this form');
throw new NoSuchFormException('User has no permissions to get this form', Http::STATUS_FORBIDDEN);
}
break;
case Constants::PERMISSION_RESULTS:
if (!$this->formsService->canSeeResults($form)) {
$this->logger->debug('The current user has no permission to get the results for this form');
throw new NoSuchFormException('The current user has no permission to get the results for this form', Http::STATUS_FORBIDDEN);
}
break;
case Constants::PERMISSION_RESULTS_DELETE:
if (!$this->formsService->canDeleteResults($form)) {
$this->logger->debug('This form is not owned by the current user and user has no `results_delete` permission');
throw new NoSuchFormException('This form is not owned by the current user and user has no `results_delete` permission', Http::STATUS_FORBIDDEN);
}
break;
default:
// By default we request full permissions
if ($form->getOwnerId() !== $this->currentUser->getUID()) {
$this->logger->debug('This form is not owned by the current user');
throw new NoSuchFormException('This form is not owned by the current user', Http::STATUS_FORBIDDEN);
}
break;
}
return $form;
}
Expand Down
Loading

0 comments on commit 6b8fdad

Please sign in to comment.