Rework the controllers to close CSRF attack vectors

.github/actions/deploy/update-data.sh

@@ -36,5 +36,5 @@ version_arr="$major, $minor, $patch"
 if [ -n "$suffix" ]; then
 	version_arr="$version_arr, '-$suffix'"
 fi
-sed "/VERSION_TAG/s@[[].*[]]@[$version_arr]@" -i lib/Controller/MainController.php
+sed "/VERSION_TAG/s@[[].*[]]@[$version_arr]@" -i lib/Controller/UtilApiController.php
 git add lib/Controller/MainController.php

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## [Unreleased]
 
+### Fixed
+- Close security issue by enabling CSRF protection on most endpoints
+  [#1190](https://github.com/nextcloud/cookbook/pull/1190) @christianlupus
+
 ### Documentation
 - Defining new API interface to fix security issue
   [#1186](https://github.com/nextcloud/cookbook/pull/1186) @christianlupus

appinfo/routes.php

@@ -14,24 +14,72 @@
 		 * If you add new features here, increase the minor version of the API.
 		 * If you change the behavior or remove functionality, increase the major version there.
 		 */
-		['name' => 'main#getApiVersion', 'url' => '/api/version', 'verb' => 'GET'],
+
+		// The static HTML template
 		['name' => 'main#index', 'url' => '/', 'verb' => 'GET'],
-		['name' => 'main#keywords', 'url' => '/keywords', 'verb' => 'GET'],
-		['name' => 'main#categories', 'url' => '/categories', 'verb' => 'GET'],
-		['name' => 'main#import', 'url' => '/import', 'verb' => 'POST'],
-		['name' => 'recipe#image', 'url' => '/recipes/{id}/image', 'verb' => 'GET', 'requirements' => ['id' => '\d+']],
-		['name' => 'config#reindex', 'url' => '/reindex', 'verb' => 'POST'],
-		['name' => 'config#list', 'url' => '/config', 'verb' => 'GET'],
-		['name' => 'config#config', 'url' => '/config', 'verb' => 'POST'],
+
+		// The /webapp routes
+		['name' => 'recipe#image', 'url' => '/webapp/recipes/{id}/image', 'verb' => 'GET', 'requirements' => ['id' => '\d+']],
+		['name' => 'recipe#import', 'url' => '/webapp/import', 'verb' => 'POST'],
+		['name' => 'recipe#category', 'url' => '/webapp/category/{category}', 'verb' => 'GET'],
+		['name' => 'recipe#tags', 'url' => '/webapp/tags/{keywords}', 'verb' => 'GET'],
+		['name' => 'recipe#search', 'url' => '/webapp/search/{query}', 'verb' => 'GET'],
+
+		['name' => 'keyword#keywords', 'url' => '/webapp/keywords', 'verb' => 'GET'],
+
+		['name' => 'category#categories', 'url' => '/webapp/categories', 'verb' => 'GET'],
+		['name' => 'category#rename', 'url' => '/webapp/category/{category}', 'verb' => 'PUT'],
+
+		['name' => 'config#list', 'url' => '/webapp/config', 'verb' => 'GET'],
+		['name' => 'config#config', 'url' => '/webapp/config', 'verb' => 'POST'],
+		['name' => 'config#reindex', 'url' => '/webapp/reindex', 'verb' => 'POST'],
+
 		/* API routes */
-		['name' => 'main#category', 'url' => '/api/category/{category}', 'verb' => 'GET'],
-		['name' => 'main#categoryUpdate', 'url' => '/api/category/{category}', 'verb' => 'PUT'],
-		['name' => 'main#tags', 'url' => '/api/tags/{keywords}', 'verb' => 'GET'],
-		['name' => 'main#search', 'url' => '/api/search/{query}', 'verb' => 'GET'],
+
+		// Generic routes on /api
+		['name' => 'util_api#getApiVersion', 'url' => '/api/version', 'verb' => 'GET'],
+
+		// APIv1 routes under /api/v1
+		['name' => 'recipe_api#image', 'url' => '/api/v1/recipes/{id}/image', 'verb' => 'GET', 'requirements' => ['id' => '\d+']],
+		['name' => 'recipe_api#import', 'url' => '/api/v1/import', 'verb' => 'POST'],
+		['name' => 'recipe_api#category', 'url' => '/api/v1/category/{category}', 'verb' => 'GET'],
+		['name' => 'recipe_api#tags', 'url' => '/api/v1/tags/{keywords}', 'verb' => 'GET'],
+		['name' => 'recipe_api#search', 'url' => '/api/v1/search/{query}', 'verb' => 'GET'],
+
+		['name' => 'keyword_api#keywords', 'url' => '/api/v1/keywords', 'verb' => 'GET'],
+
+		['name' => 'category_api#categories', 'url' => '/api/v1/categories', 'verb' => 'GET'],
+		['name' => 'category_api#rename', 'url' => '/api/v1/category/{category}', 'verb' => 'PUT'],
+
+		['name' => 'config_api#list', 'url' => '/api/v1/config', 'verb' => 'GET'],
+		['name' => 'config_api#config', 'url' => '/api/v1/config', 'verb' => 'POST'],
+		['name' => 'config_api#reindex', 'url' => '/api/v1/reindex', 'verb' => 'POST'],
+
+		// DEPRECATED ROUTES
+		// These routes are here only to avoid breaking the current 3rd party apps. They will be removed with the next release.
+
+		['name' => 'recipe_api#import', 'url' => '/import', 'verb' => 'POST', 'postfix' => '_legacy'],
+		['name' => 'recipe_api#image', 'url' => '/recipes/{id}/image', 'verb' => 'GET', 'requirements' => ['id' => '\d+'], 'postfix' => '_legacy'],
+		['name' => 'recipe_api#category', 'url' => '/api/category/{category}', 'verb' => 'GET', 'postfix' => '_legacy'],
+		['name' => 'recipe_api#tags', 'url' => '/api/tags/{keywords}', 'verb' => 'GET', 'postfix' => '_legacy'],
+		['name' => 'recipe_api#search', 'url' => '/api/search/{query}', 'verb' => 'GET', 'postfix' => '_legacy'],
+
+		['name' => 'keyword_api#keywords', 'url' => '/keywords', 'verb' => 'GET', 'postfix' => '_legacy'],
+
+		['name' => 'category_api#categories', 'url' => '/categories', 'verb' => 'GET', 'postfix' => '_legacy'],
+		['name' => 'category_api#rename', 'url' => '/api/category/{category}', 'verb' => 'PUT', 'postfix' => '_legacy'],
+
+		['name' => 'config_api#list', 'url' => '/config', 'verb' => 'GET', 'postfix' => '_legacy'],
+		['name' => 'config_api#config', 'url' => '/config', 'verb' => 'POST', 'postfix' => '_legacy'],
+		['name' => 'config_api#reindex', 'url' => '/reindex', 'verb' => 'POST', 'postfix' => '_legacy'],
+
+		// Preflight option for CORS API
+		['name' => 'util_api#preflighted_cors', 'url' => '/api/{path}', 'verb' => 'OPTIONS', 'requirements' => ['path' => '.+']],
 	],
 
 	/* API resources */
 	'resources' => [
-		'recipe' => ['url' => '/api/recipes']
+		'recipe' => ['url' => '/webapp/recipes'],
+		'recipe_api' => ['url' => '/api/v1/recipes'],
 	]
 ];

lib/Controller/CategoryApiController.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace OCA\Cookbook\Controller;
+
+use OCA\Cookbook\Controller\Implementation\CategoryImplementation;
+use OCP\AppFramework\ApiController;
+use OCP\AppFramework\Http\JSONResponse;
+use OCP\IRequest;
+
+class CategoryApiController extends ApiController {
+	/** @var CategoryImplementation */
+	private $impl;
+
+	public function __construct(
+		string $AppName,
+		IRequest $request,
+		CategoryImplementation $categoryImplementation
+	) {
+		parent::__construct($AppName, $request);
+
+		$this->impl = $categoryImplementation;
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 * @CORS
+	 *
+	 * @return JSONResponse
+	 */
+	public function categories() {
+		return $this->impl->index();
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 * @CORS
+	 * @param string $category
+	 * @return JSONResponse
+	 */
+	public function rename($category) {
+		return $this->impl->rename($category);
+	}
+}

lib/Controller/CategoryController.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace OCA\Cookbook\Controller;
+
+use OCP\IRequest;
+use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\JSONResponse;
+use OCA\Cookbook\Controller\Implementation\CategoryImplementation;
+
+class CategoryController extends Controller {
+	/** @var CategoryImplementation */
+	private $impl;
+
+	public function __construct(
+		string $AppName,
+		IRequest $request,
+		CategoryImplementation $categoryImplementation
+	) {
+		parent::__construct($AppName, $request);
+
+		$this->impl = $categoryImplementation;
+	}
+
+	/**
+	 * @NoAdminRequired
+	 *
+	 * @return JSONResponse
+	 */
+	public function categories() {
+		return $this->impl->index();
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @param string $category
+	 * @return JSONResponse
+	 */
+	public function rename($category) {
+		return $this->impl->rename($category);
+	}
+}

lib/Controller/ConfigApiController.php

@@ -0,0 +1,56 @@
+<?php
+
+namespace OCA\Cookbook\Controller;
+
+use OCA\Cookbook\Controller\Implementation\ConfigImplementation;
+use OCP\IRequest;
+use OCP\AppFramework\ApiController;
+use OCP\AppFramework\Http\JSONResponse;
+
+class ConfigApiController extends ApiController {
+	/** @var ConfigImplementation */
+	private $implementation;
+
+	public function __construct(
+		$AppName,
+		IRequest $request,
+		ConfigImplementation $configImplementation
+	) {
+		parent::__construct($AppName, $request);
+
+		$this->implementation = $configImplementation;
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 * @CORS
+	 *
+	 * @return JSONResponse
+	 */
+	public function list() {
+		return $this->implementation->list();
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 * @CORS
+	 *
+	 * @return JSONResponse
+	 */
+	public function config() {
+		return $this->implementation->config();
+	}
+
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 * @CORS
+	 *
+	 * @return JSONResponse
+	 */
+	public function reindex() {
+		return $this->implementation->reindex();
+	}
+}

lib/Controller/ConfigController.php

@@ -3,98 +3,48 @@
 namespace OCA\Cookbook\Controller;
 
 use OCP\IRequest;
-use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Controller;
 
-use OCA\Cookbook\Service\RecipeService;
-use OCA\Cookbook\Service\DbCacheService;
-use OCA\Cookbook\Helper\RestParameterParser;
-use OCA\Cookbook\Helper\UserFolderHelper;
+use OCA\Cookbook\Controller\Implementation\ConfigImplementation;
 
 class ConfigController extends Controller {
-	/**
-	 * @var RecipeService
-	 */
-	private $service;
-
-	/**
-	 * @var DbCacheService
-	 */
-	private $dbCacheService;
-
-	/**
-	 * @var RestParameterParser
-	 */
-	private $restParser;
-
-	/**
-	 * @var UserFolderHelper
-	 */
-	private $userFolder;
+	/** @var ConfigImplementation */
+	private $implementation;
 
 	public function __construct(
 		$AppName,
 		IRequest $request,
-		RecipeService $recipeService,
-		DbCacheService $dbCacheService,
-		RestParameterParser $restParser,
-		UserFolderHelper $userFolder
+		ConfigImplementation $configImplementation
 	) {
 		parent::__construct($AppName, $request);
 
-		$this->service = $recipeService;
-		$this->dbCacheService = $dbCacheService;
-		$this->restParser = $restParser;
-		$this->userFolder = $userFolder;
+		$this->implementation = $configImplementation;
 	}
 
 	/**
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
+	 *
+	 * @return JSONResponse
 	 */
 	public function list() {
-		$this->dbCacheService->triggerCheck();
-
-		return new DataResponse([
-			'folder' => $this->userFolder->getPath(),
-			'update_interval' => $this->dbCacheService->getSearchIndexUpdateInterval(),
-			'print_image' => $this->service->getPrintImage(),
-		], Http::STATUS_OK);
+		return $this->implementation->list();
 	}
 
 	/**
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
+	 *
+	 * @return JSONResponse
 	 */
 	public function config() {
-		$data = $this->restParser->getParameters();
-
-		if (isset($data['folder'])) {
-			$this->userFolder->setPath($data['folder']);
-			$this->dbCacheService->updateCache();
-		}
-
-		if (isset($data['update_interval'])) {
-			$this->service->setSearchIndexUpdateInterval($data['update_interval']);
-		}
-
-		if (isset($data['print_image'])) {
-			$this->service->setPrintImage((bool)$data['print_image']);
-		}
-
-		$this->dbCacheService->triggerCheck();
-
-		return new DataResponse('OK', Http::STATUS_OK);
+		return $this->implementation->config();
 	}
 
 	/**
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
+	 *
+	 * @return JSONResponse
 	 */
 	public function reindex() {
-		$this->dbCacheService->updateCache();
-
-		return new DataResponse('Search index rebuilt successfully', Http::STATUS_OK);
+		return $this->implementation->reindex();
 	}
 }