diff options
-rw-r--r-- | apps/files/ajax/upload.php | 4 | ||||
-rw-r--r-- | apps/files_sharing/appinfo/update.php | 8 | ||||
-rw-r--r-- | core/command/upgrade.php | 3 | ||||
-rw-r--r-- | lib/private/appframework/dependencyinjection/dicontainer.php | 7 | ||||
-rw-r--r-- | lib/private/appframework/middleware/security/corsmiddleware.php | 46 | ||||
-rw-r--r-- | lib/private/appframework/utility/controllermethodreflector.php | 2 | ||||
-rw-r--r-- | lib/repair/collation.php | 1 | ||||
-rw-r--r-- | tests/lib/appframework/middleware/security/CORSMiddlewareTest.php | 73 | ||||
-rw-r--r-- | tests/lib/appframework/utility/ControllerMethodReflectorTest.php | 14 |
9 files changed, 143 insertions, 15 deletions
diff --git a/apps/files/ajax/upload.php b/apps/files/ajax/upload.php index b737d5f8710..6e190b03192 100644 --- a/apps/files/ajax/upload.php +++ b/apps/files/ajax/upload.php @@ -49,6 +49,10 @@ if (empty($_POST['dirToken'])) { // The token defines the target directory (security reasons) $path = \OC\Files\Filesystem::getPath($linkItem['file_source']); + if($path === null) { + OCP\JSON::error(array('data' => array_merge(array('message' => $l->t('Unable to set upload directory.'))))); + die(); + } $dir = sprintf( "/%s/%s", $path, diff --git a/apps/files_sharing/appinfo/update.php b/apps/files_sharing/appinfo/update.php index e393b1575af..2d2d78a6454 100644 --- a/apps/files_sharing/appinfo/update.php +++ b/apps/files_sharing/appinfo/update.php @@ -87,8 +87,14 @@ function removeSharedFolder($mkdirs = true, $chunkSize = 99) { // create folder Shared for each user if ($mkdirs) { + $logger = \OC::$server->getLogger(); foreach ($unique_users as $user) { - \OC\Files\Filesystem::initMountPoints($user); + try { + \OC\Files\Filesystem::initMountPoints($user); + } catch(\OC\User\NoUserException $e) { + $logger->warning("Update: removeSharedFolder - user '$user' is not present anymore" , array('app' => 'files_sharing')); + continue; + } if (!$view->file_exists('/' . $user . '/files/Shared')) { $view->mkdir('/' . $user . '/files/Shared'); } diff --git a/core/command/upgrade.php b/core/command/upgrade.php index d037082c5e8..22f4293149f 100644 --- a/core/command/upgrade.php +++ b/core/command/upgrade.php @@ -101,6 +101,9 @@ class Upgrade extends Command { $updater->listen('\OC\Updater', 'disabledApps', function ($appList) use($output) { $output->writeln('<info>Disabled incompatible apps: ' . implode(', ', $appList) . '</info>'); }); + $updater->listen('\OC\Repair', 'info', function ($message) use($output) { + $output->writeln('<info>Repair info: ' . $message . '</info>'); + }); $updater->listen('\OC\Updater', 'failure', function ($message) use($output) { $output->writeln($message); diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php index 61a2333ecee..74b0a98f09f 100644 --- a/lib/private/appframework/dependencyinjection/dicontainer.php +++ b/lib/private/appframework/dependencyinjection/dicontainer.php @@ -108,18 +108,19 @@ class DIContainer extends SimpleContainer implements IAppContainer{ ); }); - $this['CORSMiddleware'] = $this->share(function($c) { + $this['CORSMiddleware'] = $this->share(function($c) use ($app){ return new CORSMiddleware( $c['Request'], - $c['ControllerMethodReflector'] + $c['ControllerMethodReflector'], + $app->getServer()->getUserSession() ); }); $middleWares = &$this->middleWares; $this['MiddlewareDispatcher'] = $this->share(function($c) use (&$middleWares) { $dispatcher = new MiddlewareDispatcher(); - $dispatcher->registerMiddleware($c['SecurityMiddleware']); $dispatcher->registerMiddleware($c['CORSMiddleware']); + $dispatcher->registerMiddleware($c['SecurityMiddleware']); foreach($middleWares as $middleWare) { $dispatcher->registerMiddleware($c[$middleWare]); diff --git a/lib/private/appframework/middleware/security/corsmiddleware.php b/lib/private/appframework/middleware/security/corsmiddleware.php index dca3996ea2e..2d768e1a0db 100644 --- a/lib/private/appframework/middleware/security/corsmiddleware.php +++ b/lib/private/appframework/middleware/security/corsmiddleware.php @@ -13,30 +13,66 @@ namespace OC\AppFramework\Middleware\Security; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\IRequest; +use OCP\IUserSession; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; /** - * This middleware sets the correct CORS headers on a response if the + * This middleware sets the correct CORS headers on a response if the * controller has the @CORS annotation. This is needed for webapps that want - * to access an API and dont run on the same domain, see + * to access an API and dont run on the same domain, see * https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS */ class CORSMiddleware extends Middleware { + /** + * @var IRequest + */ private $request; + /** + * @var ControllerMethodReflector + */ private $reflector; + /** + * @var IUserSession + */ + private $session; /** * @param IRequest $request * @param ControllerMethodReflector $reflector + * @param IUserSession $session */ - public function __construct(IRequest $request, - ControllerMethodReflector $reflector) { + public function __construct(IRequest $request, + ControllerMethodReflector $reflector, + IUserSession $session) { $this->request = $request; $this->reflector = $reflector; + $this->session = $session; } + /** + * This is being run in normal order before the controller is being + * called which allows several modifications and checks + * + * @param Controller $controller the controller that is being called + * @param string $methodName the name of the method that will be called on + * the controller + * @since 7.0.0 + */ + public function beforeController($controller, $methodName){ + // ensure that @CORS annotated API routes are not used in conjunction + // with session authentication since this enables CSRF attack vectors + if ($this->reflector->hasAnnotation('CORS') && + !$this->reflector->hasAnnotation('PublicPage')) { + $user = $this->request->server['PHP_AUTH_USER']; + $pass = $this->request->server['PHP_AUTH_PW']; + $this->session->logout(); + if(!$this->session->login($user, $pass)) { + throw new SecurityException('CORS requires basic auth'); + } + } + } /** * This is being run after a successful controllermethod call and allows @@ -54,7 +90,7 @@ class CORSMiddleware extends Middleware { if(isset($this->request->server['HTTP_ORIGIN']) && $this->reflector->hasAnnotation('CORS')) { - // allow credentials headers must not be true or CSRF is possible + // allow credentials headers must not be true or CSRF is possible // otherwise foreach($response->getHeaders() as $header => $value ) { if(strtolower($header) === 'access-control-allow-credentials' && diff --git a/lib/private/appframework/utility/controllermethodreflector.php b/lib/private/appframework/utility/controllermethodreflector.php index 045bfc8b9ba..b454640c1ed 100644 --- a/lib/private/appframework/utility/controllermethodreflector.php +++ b/lib/private/appframework/utility/controllermethodreflector.php @@ -54,7 +54,7 @@ class ControllerMethodReflector { $this->annotations = $matches[1]; // extract type parameter information - preg_match_all('/@param (?P<type>\w+) \$(?P<var>\w+)/', $docs, $matches); + preg_match_all('/@param\h+(?P<type>\w+)\h+\$(?P<var>\w+)/', $docs, $matches); // this is just a fix for PHP 5.3 (array_combine raises warning if called with // two empty arrays if($matches['var'] === array() && $matches['type'] === array()) { diff --git a/lib/repair/collation.php b/lib/repair/collation.php index 2247cf82d0a..315969b7313 100644 --- a/lib/repair/collation.php +++ b/lib/repair/collation.php @@ -48,6 +48,7 @@ class Collation extends BasicEmitter implements \OC\RepairStep { foreach ($tables as $table) { $query = $this->connection->prepare('ALTER TABLE `' . $table . '` CONVERT TO CHARACTER SET utf8 COLLATE utf8_bin;'); $query->execute(); + $this->emit('\OC\Repair', 'info', array("Changed collation for $table")); } } diff --git a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php index 79cd3b278af..9e314d68207 100644 --- a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php +++ b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php @@ -21,9 +21,11 @@ use OCP\AppFramework\Http\Response; class CORSMiddlewareTest extends \PHPUnit_Framework_TestCase { private $reflector; + private $session; protected function setUp() { $this->reflector = new ControllerMethodReflector(); + $this->session = $this->getMock('\OCP\IUserSession'); } /** @@ -34,7 +36,7 @@ class CORSMiddlewareTest extends \PHPUnit_Framework_TestCase { array('server' => array('HTTP_ORIGIN' => 'test')) ); $this->reflector->reflect($this, __FUNCTION__); - $middleware = new CORSMiddleware($request, $this->reflector); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); $response = $middleware->afterController($this, __FUNCTION__, new Response()); $headers = $response->getHeaders(); @@ -46,7 +48,7 @@ class CORSMiddlewareTest extends \PHPUnit_Framework_TestCase { $request = new Request( array('server' => array('HTTP_ORIGIN' => 'test')) ); - $middleware = new CORSMiddleware($request, $this->reflector); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); $response = $middleware->afterController($this, __FUNCTION__, new Response()); $headers = $response->getHeaders(); @@ -60,14 +62,13 @@ class CORSMiddlewareTest extends \PHPUnit_Framework_TestCase { public function testNoOriginHeaderNoCORSHEADER() { $request = new Request(); $this->reflector->reflect($this, __FUNCTION__); - $middleware = new CORSMiddleware($request, $this->reflector); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); $response = $middleware->afterController($this, __FUNCTION__, new Response()); $headers = $response->getHeaders(); $this->assertFalse(array_key_exists('Access-Control-Allow-Origin', $headers)); } - /** * @CORS * @expectedException \OC\AppFramework\Middleware\Security\SecurityException @@ -77,11 +78,73 @@ class CORSMiddlewareTest extends \PHPUnit_Framework_TestCase { array('server' => array('HTTP_ORIGIN' => 'test')) ); $this->reflector->reflect($this, __FUNCTION__); - $middleware = new CORSMiddleware($request, $this->reflector); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); $response = new Response(); $response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE'); $response = $middleware->afterController($this, __FUNCTION__, $response); } + /** + * @CORS + * @PublicPage + */ + public function testNoCORSShouldAllowCookieAuth() { + $request = new Request( + array(), + $this->getMock('\OCP\Security\ISecureRandom'), + $this->getMock('\OCP\IConfig') + ); + $this->reflector->reflect($this, __FUNCTION__); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); + $middleware->beforeController($this, __FUNCTION__, new Response()); + } + + /** + * @CORS + */ + public function testCORSShouldRelogin() { + $request = new Request( + array('server' => array( + 'PHP_AUTH_USER' => 'user', + 'PHP_AUTH_PW' => 'pass' + )), + $this->getMock('\OCP\Security\ISecureRandom'), + $this->getMock('\OCP\IConfig') + ); + $this->session->expects($this->once()) + ->method('logout'); + $this->session->expects($this->once()) + ->method('login') + ->with($this->equalTo('user'), $this->equalTo('pass')) + ->will($this->returnValue(true)); + $this->reflector->reflect($this, __FUNCTION__); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); + $middleware->beforeController($this, __FUNCTION__, new Response()); + } + + /** + * @CORS + * @expectedException \OC\AppFramework\Middleware\Security\SecurityException + */ + public function testCORSShouldNotAllowCookieAuth() { + $request = new Request( + array('server' => array( + 'PHP_AUTH_USER' => 'user', + 'PHP_AUTH_PW' => 'pass' + )), + $this->getMock('\OCP\Security\ISecureRandom'), + $this->getMock('\OCP\IConfig') + ); + $this->session->expects($this->once()) + ->method('logout'); + $this->session->expects($this->once()) + ->method('login') + ->with($this->equalTo('user'), $this->equalTo('pass')) + ->will($this->returnValue(false)); + $this->reflector->reflect($this, __FUNCTION__); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); + $middleware->beforeController($this, __FUNCTION__, new Response()); + } + } diff --git a/tests/lib/appframework/utility/ControllerMethodReflectorTest.php b/tests/lib/appframework/utility/ControllerMethodReflectorTest.php index 8939a203edb..feb6ac5a352 100644 --- a/tests/lib/appframework/utility/ControllerMethodReflectorTest.php +++ b/tests/lib/appframework/utility/ControllerMethodReflectorTest.php @@ -87,6 +87,20 @@ class ControllerMethodReflectorTest extends \PHPUnit_Framework_TestCase { $this->assertEquals('double', $reader->getType('test')); } + /** + * @Annotation + * @param string $foo + */ + public function testReadTypeWhitespaceAnnotations(){ + $reader = new ControllerMethodReflector(); + $reader->reflect( + '\OC\AppFramework\Utility\ControllerMethodReflectorTest', + 'testReadTypeWhitespaceAnnotations' + ); + + $this->assertEquals('string', $reader->getType('foo')); + } + public function arguments($arg, $arg2='hi') {} public function testReflectParameters() { |