Merge pull request #26277 from owncloud/stable9.1-backport26031

[Stable9.1] Allow one more origin. Log the reason of occ controller failure
owncloud · Oct 5, 2016 · 5e4233a · 5e4233a
2 parents 361844c + 9dd64d1
commit 5e4233a
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 19 deletions.
diff --git a/core/Application.php b/core/Application.php
@@ -126,18 +126,6 @@ public function __construct(array $urlParams=array()){
 				$c->query('SecureRandom')
 			);
 		});
-		$container->registerService('OccController', function(SimpleContainer $c) {
-			return new OccController(
-				$c->query('AppName'),
-				$c->query('Request'),
-				$c->query('Config'),
-				new \OC\Console\Application(
-					$c->query('Config'),
-					$c->query('ServerContainer')->getEventDispatcher(),
-					$c->query('Request')
-				)
-			);
-		});
 
 		/**
 		 * Core class wrappers

diff --git a/core/Controller/OccController.php b/core/Controller/OccController.php
@@ -26,6 +26,7 @@
 use OC\Console\Application;
 use OCP\IConfig;
 use OCP\IRequest;
+use OCP\ILogger;
 use Symfony\Component\Console\Input\ArrayInput;
 use Symfony\Component\Console\Output\BufferedOutput;
 
@@ -48,6 +49,8 @@ class OccController extends Controller {
 	private $config;
 	/** @var Application */
 	private $console;
+	/** @var ILogger */
+	private $logger;
 
 	/**
 	 * OccController constructor.
@@ -56,12 +59,14 @@ class OccController extends Controller {
 	 * @param IRequest $request
 	 * @param IConfig $config
 	 * @param Application $console
+	 * @param ILogger $logger
 	 */
 	public function __construct($appName, IRequest $request,
-								IConfig $config, Application $console) {
+								IConfig $config, Application $console, ILogger $logger) {
 		parent::__construct($appName, $request);
 		$this->config = $config;
 		$this->console = $console;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -108,6 +113,13 @@ public function execute($command, $token, $params = []) {
 			];
 
 		} catch (\UnexpectedValueException $e){
+			$this->logger->warning(
+				'Invalid request to occ controller. Details: "{details}"',
+				[
+					'app' => 'core',
+					'details' => $e->getMessage()
+				]
+			);
 			$json = [
 				'exitCode' => 126,
 				'response' => 'Not allowed',
@@ -123,8 +135,13 @@ public function execute($command, $token, $params = []) {
 	 * @param $token
 	 */
 	protected function validateRequest($command, $token){
-		if (!in_array($this->request->getRemoteAddress(), ['::1', '127.0.0.1', 'localhost'])) {
-			throw new \UnexpectedValueException('Web executor is not allowed to run from a different host');
+		$allowedHosts = ['::1', '127.0.0.1', 'localhost'];
+		if (isset($this->request->server['SERVER_ADDR'])){
+			array_push($allowedHosts, $this->request->server['SERVER_ADDR']);
+		}
+
+		if (!in_array($this->request->getRemoteAddress(), $allowedHosts)) {
+			throw new \UnexpectedValueException('Web executor is not allowed to run from a host ' . $this->request->getRemoteAddress());
 		}
 
 		if (!in_array($command, $this->allowedCommands)) {

diff --git a/core/routes.php b/core/routes.php
@@ -48,7 +48,7 @@
 		['name' => 'login#showLoginForm', 'url' => '/login', 'verb' => 'GET'],
 		['name' => 'login#logout', 'url' => '/logout', 'verb' => 'GET'],
 		['name' => 'token#generateToken', 'url' => '/token/generate', 'verb' => 'POST'],
-		['name' => 'occ#execute', 'url' => '/occ/{command}', 'verb' => 'POST'],
+		['name' => 'OC\Core\Controller\Occ#execute', 'url' => '/occ/{command}', 'verb' => 'POST'],
 		['name' => 'TwoFactorChallenge#selectChallenge', 'url' => '/login/selectchallenge', 'verb' => 'GET'],
 		['name' => 'TwoFactorChallenge#showChallenge', 'url' => '/login/challenge/{challengeProviderId}', 'verb' => 'GET'],
 		['name' => 'TwoFactorChallenge#solveChallenge', 'url' => '/login/challenge/{challengeProviderId}', 'verb' => 'POST'],

diff --git a/tests/Core/Controller/OccControllerTest.php b/tests/Core/Controller/OccControllerTest.php
@@ -46,7 +46,8 @@ class OccControllerTest extends TestCase {
 	private $console;
 
 	public function testFromInvalidLocation(){
-		$this->getControllerMock('example.org');
+		$fakeHost = 'example.org';
+		$this->getControllerMock($fakeHost);
 
 		$response = $this->controller->execute('status', '');
 		$responseData = $response->getData();
@@ -55,7 +56,7 @@ public function testFromInvalidLocation(){
 		$this->assertEquals(126, $responseData['exitCode']);
 
 		$this->assertArrayHasKey('details', $responseData);
-		$this->assertEquals('Web executor is not allowed to run from a different host', $responseData['details']);
+		$this->assertEquals('Web executor is not allowed to run from a host ' . $fakeHost, $responseData['details']);
 	}
 
 	public function testNotWhiteListedCommand(){
@@ -136,7 +137,10 @@ private function getControllerMock($host){
 			'core',
 			$this->request,
 			$this->config,
-			$this->console
+			$this->console,
+			$this->getMockBuilder('\OCP\ILogger')
+				->disableOriginalConstructor()
+				->getMock()
 		);
 	}