Skip to content

Extend OCS Middleware #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickvergessen merged 3 commits into from
Aug 17, 2016
Merged

Extend OCS Middleware #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6d7a62
Move OCS Middleware before security middleware
rullzer Aug 12, 2016
aacb68c
Extend OCSMiddleware
rullzer Aug 11, 2016
a98376f
Add tests
rullzer Aug 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/private/AppFramework/DependencyInjection/DIContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ public function __construct($appName, $urlParams = array()){
$this->registerService('MiddlewareDispatcher', function($c) use (&$middleWares) {
$dispatcher = new MiddlewareDispatcher();
$dispatcher->registerMiddleware($c['CORSMiddleware']);
$dispatcher->registerMiddleware($c['OCSMiddleware']);
$dispatcher->registerMiddleware($c['SecurityMiddleware']);
$dispatcher->registerMiddleWare($c['TwoFactorMiddleware']);

Expand All @@ -404,7 +405,6 @@ public function __construct($appName, $urlParams = array()){
}

$dispatcher->registerMiddleware($c['SessionMiddleware']);
$dispatcher->registerMiddleware($c['OCSMiddleware']);
return $dispatcher;
});

Expand Down
62 changes: 60 additions & 2 deletions lib/private/AppFramework/Middleware/OCSMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,14 @@
namespace OC\AppFramework\Middleware;

use OC\AppFramework\Http;
use OCP\API;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\OCSResponse;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCSController;
use OCP\IRequest;
use OCP\AppFramework\Middleware;
Expand Down Expand Up @@ -54,19 +60,71 @@ public function afterException($controller, $methodName, \Exception $exception)

$code = $exception->getCode();
if ($code === 0) {
$code = Http::STATUS_INTERNAL_SERVER_ERROR;
$code = API::RESPOND_UNKNOWN_ERROR;
}

// Build the response
$response = new OCSResponse($format, $code, $exception->getMessage());

// Forbidden always sets 401 (even on v1.php)
if ($exception instanceof OCSForbiddenException || $code === API::RESPOND_UNAUTHORISED) {
$response->setStatus(Http::STATUS_UNAUTHORIZED);
}

// On v2.php we set actual HTTP error codes
if (substr_compare($this->request->getScriptName(), '/ocs/v2.php', -strlen('/ocs/v2.php')) === 0) {
$response->setStatus($code);
if ($code === API::RESPOND_NOT_FOUND) {
$response->setStatus(Http::STATUS_NOT_FOUND);
} else if ($code === API::RESPOND_SERVER_ERROR) {
$response->setStatus(Http::STATUS_INTERNAL_SERVER_ERROR);
} else if ($code === API::RESPOND_UNKNOWN_ERROR) {
$response->setStatus(Http::STATUS_INTERNAL_SERVER_ERROR);
} else if ($code === API::RESPOND_UNAUTHORISED) {
// Already set
}
// 4xx and 5xx codes are forwarded as is.
else if ($code >= 400 && $code < 600) {
$response->setStatus($code);
} else {
// All other codes get a bad request
$response->setStatus(Http::STATUS_BAD_REQUEST);
}
}
return $response;
}

throw $exception;
}

/**
* @param \OCP\AppFramework\Controller $controller
* @param string $methodName
* @param Response $response
* @return \OCP\AppFramework\Http\Response
*/
public function afterController($controller, $methodName, Response $response) {
/*
* If a different middleware has detected that a request unauthorized or forbidden
* we need to catch the response and convert it to a proper OCS response.
*/
if ($controller instanceof OCSController && !($response instanceof OCSResponse)) {
if ($response->getStatus() === Http::STATUS_UNAUTHORIZED ||
$response->getStatus() === Http::STATUS_FORBIDDEN) {
$format = $this->getFormat($controller);

$message = '';
if ($response instanceof JSONResponse) {
/** @var DataResponse $response */
$message = $response->getData()['message'];
}
$response = new OCSResponse($format, \OCP\API::RESPOND_UNAUTHORISED, $message);
$response->setStatus(Http::STATUS_UNAUTHORIZED);
}
}

return $response;
}

/**
* @param \OCP\AppFramework\Controller $controller
* @return string
Expand Down
76 changes: 70 additions & 6 deletions tests/lib/AppFramework/Middleware/OCSMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OC\AppFramework\Http\Request;
use OCP\IRequest;
use OC\AppFramework\Middleware\OCSMiddleware;


class OCSMiddlewareTest extends \Test\TestCase {

/**
* @var Request
* @var IRequest
*/
private $request;

Expand Down Expand Up @@ -101,8 +101,18 @@ public function testAfterExceptionOCSv1($controller, $exception, $forward, $mess
$this->assertInstanceOf('OCP\AppFramework\Http\OCSResponse', $result);

$this->assertSame($message, $this->invokePrivate($result, 'message'));
$this->assertSame($code, $this->invokePrivate($result, 'statuscode'));
$this->assertSame(200, $result->getStatus());

if ($exception->getCode() === 0) {
$this->assertSame(\OCP\API::RESPOND_UNKNOWN_ERROR, $this->invokePrivate($result, 'statuscode'));
} else {
$this->assertSame($code, $this->invokePrivate($result, 'statuscode'));
}

if ($exception instanceof OCSForbiddenException) {
$this->assertSame(Http::STATUS_UNAUTHORIZED, $result->getStatus());
} else {
$this->assertSame(200, $result->getStatus());
}
} catch (\Exception $e) {
$this->assertTrue($forward);
$this->assertEquals($exception, $e);
Expand Down Expand Up @@ -131,7 +141,11 @@ public function testAfterExceptionOCSv2($controller, $exception, $forward, $mess
$this->assertInstanceOf('OCP\AppFramework\Http\OCSResponse', $result);

$this->assertSame($message, $this->invokePrivate($result, 'message'));
$this->assertSame($code, $this->invokePrivate($result, 'statuscode'));
if ($exception->getCode() === 0) {
$this->assertSame(\OCP\API::RESPOND_UNKNOWN_ERROR, $this->invokePrivate($result, 'statuscode'));
} else {
$this->assertSame($code, $this->invokePrivate($result, 'statuscode'));
}
$this->assertSame($code, $result->getStatus());
} catch (\Exception $e) {
$this->assertTrue($forward);
Expand Down Expand Up @@ -161,12 +175,62 @@ public function testAfterExceptionOCSv2SubFolder($controller, $exception, $forwa
$this->assertInstanceOf('OCP\AppFramework\Http\OCSResponse', $result);

$this->assertSame($message, $this->invokePrivate($result, 'message'));
$this->assertSame($code, $this->invokePrivate($result, 'statuscode'));
if ($exception->getCode() === 0) {
$this->assertSame(\OCP\API::RESPOND_UNKNOWN_ERROR, $this->invokePrivate($result, 'statuscode'));
} else {
$this->assertSame($code, $this->invokePrivate($result, 'statuscode'));
}
$this->assertSame($code, $result->getStatus());
} catch (\Exception $e) {
$this->assertTrue($forward);
$this->assertEquals($exception, $e);
}
}

public function dataAfterController() {
$OCSController = $this->getMockBuilder('OCP\AppFramework\OCSController')
->disableOriginalConstructor()
->getMock();
$controller = $this->getMockBuilder('OCP\AppFramework\Controller')
->disableOriginalConstructor()
->getMock();

return [
[$OCSController, new Http\Response(), false],
[$OCSController, new Http\JSONResponse(), false],
[$OCSController, new Http\JSONResponse(['message' => 'foo']), false],
[$OCSController, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_UNAUTHORIZED), true],
[$OCSController, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_FORBIDDEN), true],

[$controller, new Http\Response(), false],
[$controller, new Http\JSONResponse(), false],
[$controller, new Http\JSONResponse(['message' => 'foo']), false],
[$controller, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_UNAUTHORIZED), false],
[$controller, new Http\JSONResponse(['message' => 'foo'], Http::STATUS_FORBIDDEN), false],

];
}

/**
* @dataProvider dataAfterController
*
* @param Controller $controller
* @param Http\Response $response
* @param bool $converted
*/
public function testAfterController($controller, $response, $converted) {
$OCSMiddleware = new OCSMiddleware($this->request);
$newResponse = $OCSMiddleware->afterController($controller, 'foo', $response);

if ($converted === false) {
$this->assertSame($response, $newResponse);
} else {
$this->assertInstanceOf('\OCP\AppFramework\Http\OCSResponse', $newResponse);
/** @var Http\OCSResponse $newResponse */
$this->assertSame($response->getData()['message'], $this->invokePrivate($newResponse, 'message'));
$this->assertSame(\OCP\API::RESPOND_UNAUTHORISED, $this->invokePrivate($newResponse, 'statuscode'));
$this->assertSame(Http::STATUS_UNAUTHORIZED, $newResponse->getStatus());
}
}

}