Skip to content

Commit

Permalink
Cleanup middleware registering
Browse files Browse the repository at this point in the history
Fixes #12224

Since we only use the middleware at 1 location it makes no sense to
register them in each and every container.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
  • Loading branch information
rullzer committed Dec 27, 2018
1 parent bb3a7ad commit 499668b
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 106 deletions.
172 changes: 75 additions & 97 deletions lib/private/AppFramework/DependencyInjection/DIContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,11 @@
use OCP\GlobalScale\IConfig;
use OCP\IL10N;
use OCP\ILogger;
use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserSession;
use OCA\WorkflowEngine\Manager;

Expand Down Expand Up @@ -180,15 +182,37 @@ public function __construct($appName, $urlParams = array(), ServerContainer $ser
* Middleware
*/
$app = $this;
$this->registerService('SecurityMiddleware', function($c) use ($app){
/** @var \OC\Server $server */
$server = $app->getServer();

return new SecurityMiddleware(
$c['Request'],
$server->query(IControllerMethodReflector::class),
$server->getNavigationManager(),
$server->getURLGenerator(),
$middleWares = &$this->middleWares;
$this->registerService('MiddlewareDispatcher', function(SimpleContainer $c) use (&$middleWares, $app) {
$server = $app->getServer();

$dispatcher = new MiddlewareDispatcher();
$dispatcher->registerMiddleware(
new OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware(
$c->query(IRequest::class),
$c->query(IControllerMethodReflector::class)
)
);
$dispatcher->registerMiddleware(
new CORSMiddleware(
$c->query(IRequest::class),
$c->query(IControllerMethodReflector::class),
$c->query(IUserSession::class),
$c->query(OC\Security\Bruteforce\Throttler::class)
)
);
$dispatcher->registerMiddleware(
new OCSMiddleware(
$c->query(IRequest::class)
)
);

$securityMiddleware = new SecurityMiddleware(
$c->query(IRequest::class),
$c->query(IControllerMethodReflector::class),
$c->query(INavigationManager::class),
$c->query(IURLGenerator::class),
$server->getLogger(),
$c['AppName'],
$server->getUserSession()->isLoggedIn(),
Expand All @@ -199,105 +223,59 @@ public function __construct($appName, $urlParams = array(), ServerContainer $ser
$server->getAppManager(),
$server->getL10N('lib')
);
});

$this->registerService(OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware::class, function ($c) use ($app) {
/** @var \OC\Server $server */
$server = $app->getServer();

return new OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware(
$c->query(IControllerMethodReflector::class),
$server->getSession(),
$server->getUserSession(),
$server->query(ITimeFactory::class)
);
});

$this->registerService('BruteForceMiddleware', function($c) use ($app) {
/** @var \OC\Server $server */
$server = $app->getServer();

return new OC\AppFramework\Middleware\Security\BruteForceMiddleware(
$c->query(IControllerMethodReflector::class),
$server->getBruteForceThrottler(),
$server->getRequest()
);
});

$this->registerService('RateLimitingMiddleware', function($c) use ($app) {
/** @var \OC\Server $server */
$server = $app->getServer();

return new RateLimitingMiddleware(
$server->getRequest(),
$server->getUserSession(),
$c->query(IControllerMethodReflector::class),
$c->query(OC\Security\RateLimiting\Limiter::class)
$dispatcher->registerMiddleware($securityMiddleware);
$dispatcher->registerMiddleware(
new OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware(
$c->query(IControllerMethodReflector::class),
$c->query(ISession::class),
$c->query(IUserSession::class),
$c->query(ITimeFactory::class)
)
);
});

$this->registerService('CORSMiddleware', function($c) {
return new CORSMiddleware(
$c['Request'],
$c->query(IControllerMethodReflector::class),
$c->query(IUserSession::class),
$c->getServer()->getBruteForceThrottler()
$dispatcher->registerMiddleware(
new TwoFactorMiddleware(
$c->query(OC\Authentication\TwoFactorAuth\Manager::class),
$c->query(IUserSession::class),
$c->query(ISession::class),
$c->query(IURLGenerator::class),
$c->query(IControllerMethodReflector::class),
$c->query(IRequest::class)
)
);
});

$this->registerService('SessionMiddleware', function($c) use ($app) {
return new SessionMiddleware(
$c['Request'],
$c->query(IControllerMethodReflector::class),
$app->getServer()->getSession()
$dispatcher->registerMiddleware(
new OC\AppFramework\Middleware\Security\BruteForceMiddleware(
$c->query(IControllerMethodReflector::class),
$c->query(OC\Security\Bruteforce\Throttler::class),
$c->query(IRequest::class)
)
);
});

$this->registerService('TwoFactorMiddleware', function (SimpleContainer $c) use ($app) {
$twoFactorManager = $c->getServer()->getTwoFactorAuthManager();
$userSession = $app->getServer()->getUserSession();
$session = $app->getServer()->getSession();
$urlGenerator = $app->getServer()->getURLGenerator();
$reflector = $c->query(IControllerMethodReflector::class);
$request = $app->getServer()->getRequest();
return new TwoFactorMiddleware($twoFactorManager, $userSession, $session, $urlGenerator, $reflector, $request);
});

$this->registerService('OCSMiddleware', function (SimpleContainer $c) {
return new OCSMiddleware(
$c['Request']
$dispatcher->registerMiddleware(
new RateLimitingMiddleware(
$c->query(IRequest::class),
$c->query(IUserSession::class),
$c->query(IControllerMethodReflector::class),
$c->query(OC\Security\RateLimiting\Limiter::class)
)
);
});

$this->registerService(OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware::class, function (SimpleContainer $c) {
return new OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware(
$c['Request'],
$c->query(IControllerMethodReflector::class)
$dispatcher->registerMiddleware(
new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware(
$c->query(IRequest::class),
$c->query(ISession::class),
$c->query(\OCP\IConfig::class)
)
);
});

$middleWares = &$this->middleWares;
$this->registerService('MiddlewareDispatcher', function(SimpleContainer $c) use (&$middleWares) {
$dispatcher = new MiddlewareDispatcher();
$dispatcher->registerMiddleware($c[OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware::class]);
$dispatcher->registerMiddleware($c['CORSMiddleware']);
$dispatcher->registerMiddleware($c['OCSMiddleware']);
$dispatcher->registerMiddleware($c['SecurityMiddleware']);
$dispatcher->registerMiddleware($c[OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware::class]);
$dispatcher->registerMiddleware($c['TwoFactorMiddleware']);
$dispatcher->registerMiddleware($c['BruteForceMiddleware']);
$dispatcher->registerMiddleware($c['RateLimitingMiddleware']);
$dispatcher->registerMiddleware(new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware(
$c['Request'],
$c->query(ISession::class),
$c->query(\OCP\IConfig::class)
));

foreach($middleWares as $middleWare) {
$dispatcher->registerMiddleware($c[$middleWare]);
}

$dispatcher->registerMiddleware($c['SessionMiddleware']);
$dispatcher->registerMiddleware(
new SessionMiddleware(
$c->query(IRequest::class),
$c->query(IControllerMethodReflector::class),
$c->query(ISession::class)
)
);
return $dispatcher;
});

Expand Down
19 changes: 10 additions & 9 deletions tests/lib/AppFramework/DependencyInjection/DIContainerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

use OC\AppFramework\DependencyInjection\DIContainer;
use \OC\AppFramework\Http\Request;
use OC\AppFramework\Middleware\Security\SecurityMiddleware;
use OCP\AppFramework\QueryException;
use OCP\IConfig;
use OCP\Security\ISecureRandom;
Expand All @@ -54,17 +55,10 @@ public function testProvidesRequest(){
$this->assertTrue(isset($this->container['Request']));
}


public function testProvidesSecurityMiddleware(){
$this->assertTrue(isset($this->container['SecurityMiddleware']));
}


public function testProvidesMiddlewareDispatcher(){
$this->assertTrue(isset($this->container['MiddlewareDispatcher']));
}


public function testProvidesAppName(){
$this->assertTrue(isset($this->container['AppName']));
}
Expand All @@ -80,10 +74,17 @@ public function testMiddlewareDispatcherIncludesSecurityMiddleware(){
$this->createMock(ISecureRandom::class),
$this->createMock(IConfig::class)
);
$security = $this->container['SecurityMiddleware'];
$dispatcher = $this->container['MiddlewareDispatcher'];
$middlewares = $dispatcher->getMiddlewares();

$found = false;
foreach ($middlewares as $middleware) {
if ($middleware instanceof SecurityMiddleware) {
$found = true;
}
}

$this->assertContains($security, $dispatcher->getMiddlewares());
$this->assertTrue($found);
}

public function testInvalidAppClass() {
Expand Down

0 comments on commit 499668b

Please sign in to comment.