diff --git a/_config/security.yml b/_config/security.yml index 40273a393..d4161b62f 100644 --- a/_config/security.yml +++ b/_config/security.yml @@ -26,9 +26,13 @@ SilverStripe\Core\Injector\Injector: properties: Middlewares: AuthenticationMiddleware: '%$SilverStripe\Security\AuthenticationMiddleware' + BasicAuthMiddleware: '%$SilverStripe\Security\BasicAuthMiddleware' SilverStripe\Security\AuthenticationMiddleware: properties: AuthenticationHandler: '%$SilverStripe\Security\AuthenticationHandler' + SilverStripe\Security\BasicAuthMiddleware: + properties: + URLPatterns: [] SilverStripe\Security\Security: properties: Authenticators: diff --git a/src/Control/Controller.php b/src/Control/Controller.php index f367fc57f..2b2932aaa 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -4,8 +4,10 @@ namespace SilverStripe\Control; use SilverStripe\Core\ClassInfo; use SilverStripe\Dev\Debug; +use SilverStripe\Dev\Deprecation; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\Security\BasicAuth; +use SilverStripe\Security\BasicAuthMiddleware; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\View\SSViewer; @@ -61,6 +63,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider protected $templates = []; /** + * @deprecated 4.1.0...5.0.0 Add this controller's url to + * SilverStripe\Security\BasicAuthMiddleware.URLPatterns injected property instead of setting false * @var bool */ protected $basicAuthEnabled = true; @@ -98,6 +102,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider */ protected function init() { + // @todo This will be removed in 5.0 and will be controlled by middleware instead if ($this->basicAuthEnabled) { BasicAuth::protect_site_if_necessary(); } @@ -528,9 +533,16 @@ class Controller extends RequestHandler implements TemplateGlobalProvider * Call this to disable site-wide basic authentication for a specific controller. This must be * called before Controller::init(). That is, you must call it in your controller's init method * before it calls parent::init(). + * + * @deprecated 4.1.0...5.0.0 Add this controller's url to + * SilverStripe\Security\BasicAuthMiddleware.URLPatterns injected property instead of setting false */ public function disableBasicAuth() { + Deprecation::notice( + '5.0', + 'Add this controller\'s url to ' . BasicAuthMiddleware::class . '.URLPatterns injected property instead' + ); $this->basicAuthEnabled = false; } diff --git a/src/Security/BasicAuth.php b/src/Security/BasicAuth.php index ddb03bfbb..992cae461 100644 --- a/src/Security/BasicAuth.php +++ b/src/Security/BasicAuth.php @@ -9,6 +9,7 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Environment; +use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\Connect\DatabaseException; use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; @@ -200,11 +201,13 @@ class BasicAuth * * If you want to enabled protection (rather than enforcing it), * please use {@link protect_entire_site()}. + * + * @param HTTPRequest|null $request + * @throws HTTPResponse_Exception */ - public static function protect_site_if_necessary() + public static function protect_site_if_necessary(HTTPRequest $request = null) { $config = static::config(); - $request = Controller::curr()->getRequest(); // Check if site is protected if ($config->get('entire_site_protected')) { @@ -220,6 +223,11 @@ class BasicAuth return; } + // Get request + if (!$request && Injector::inst()->has(HTTPRequest::class)) { + $request = Injector::inst()->get(HTTPRequest::class); + } + // Require login static::requireLogin( $request, diff --git a/src/Security/BasicAuthMiddleware.php b/src/Security/BasicAuthMiddleware.php new file mode 100644 index 000000000..47c27f345 --- /dev/null +++ b/src/Security/BasicAuthMiddleware.php @@ -0,0 +1,118 @@ + false, + * '#^secure#i' => true, + * '#^secure/admin#i' => 'ADMIN', + * ] + * + * @see CanonicalURLMiddleware + * @var array + */ + protected $urlPatterns = []; + + /** + * Generate response for the given request + * + * @param HTTPRequest $request + * @param callable $delegate + * @return HTTPResponse + */ + public function process(HTTPRequest $request, callable $delegate) + { + // Check if url matches any patterns + $match = $this->checkMatchingURL($request); + + // Check middleware unless specifically opting out + if ($match !== false) { + try { + // Determine method to check + if ($match) { + // Truthy values are explicit, check with optional permission code + $permission = $match === true ? null : $match; + BasicAuth::requireLogin( + $request, + BasicAuth::config()->get('entire_site_protected_message'), + $permission, + false + ); + } elseif ($match === null) { + // Null implies fall back to default behaviour + BasicAuth::protect_site_if_necessary($request); + } + } catch (HTTPResponse_Exception $ex) { + return $ex->getResponse(); + } + } + + // Pass on to other middlewares + return $delegate($request); + } + + /** + * Get list of url patterns + * + * @return array + */ + public function getURLPatterns() + { + return $this->urlPatterns ?: []; + } + + /** + * @param array $urlPatterns + * @return $this + */ + public function setURLPatterns(array $urlPatterns) + { + $this->urlPatterns = $urlPatterns; + return $this; + } + + /** + * Check if global basic auth is enabled for the given request + * + * @param HTTPRequest $request + * @return bool|string|array|null boolean value if enabled/disabled explicitly for this request, + * or null if should fall back to config value. Can also provide an explicit string / array of permission + * codes to require for this requset. + */ + protected function checkMatchingURL(HTTPRequest $request) + { + // Null if no permissions enabled + $patterns = $this->getURLPatterns(); + if (!$patterns) { + return null; + } + + // Filter redirect based on url + $relativeURL = $request->getURL(true); + foreach ($patterns as $pattern => $result) { + if (preg_match($pattern, $relativeURL)) { + return $result; + } + } + + // No patterns match + return null; + } +} diff --git a/tests/php/Security/BasicAuthTest.php b/tests/php/Security/BasicAuthTest.php index 7c9e75645..ffb95f0bf 100644 --- a/tests/php/Security/BasicAuthTest.php +++ b/tests/php/Security/BasicAuthTest.php @@ -2,11 +2,14 @@ namespace SilverStripe\Security\Tests; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Security\BasicAuth; +use SilverStripe\Security\BasicAuthMiddleware; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\Dev\FunctionalTest; use SilverStripe\Control\Director; +use SilverStripe\Security\Tests\BasicAuthTest\ControllerNotSecured; use SilverStripe\Security\Tests\BasicAuthTest\ControllerSecuredWithoutPermission; use SilverStripe\Security\Tests\BasicAuthTest\ControllerSecuredWithPermission; @@ -15,14 +18,12 @@ use SilverStripe\Security\Tests\BasicAuthTest\ControllerSecuredWithPermission; */ class BasicAuthTest extends FunctionalTest { - - protected static $original_unique_identifier_field; - protected static $fixture_file = 'BasicAuthTest.yml'; protected static $extra_controllers = [ ControllerSecuredWithPermission::class, ControllerSecuredWithoutPermission::class, + ControllerNotSecured::class, ]; protected function setUp() @@ -37,9 +38,13 @@ class BasicAuthTest extends FunctionalTest // Temp disable is_cli() exemption for tests BasicAuth::config()->set('ignore_cli', false); - // Reset statics - BasicAuthTest\ControllerSecuredWithPermission::$index_called = false; - BasicAuthTest\ControllerSecuredWithPermission::$post_init_called = false; + // Set route-specific permissions + /** @var BasicAuthMiddleware $middleware */ + $middleware = Injector::inst()->get(BasicAuthMiddleware::class); + $middleware->setURLPatterns([ + '/^BasicAuthTest_ControllerSecuredWithPermission$/' => 'MYCODE', + '/^BasicAuthTest_ControllerSecuredWithoutPermission$/' => true, + ]); } public function testBasicAuthEnabledWithoutLogin() @@ -48,21 +53,6 @@ class BasicAuthTest extends FunctionalTest $this->assertEquals(401, $response->getStatusCode()); } - public function testBasicAuthDoesntCallActionOrFurtherInitOnAuthFailure() - { - Director::test('BasicAuthTest_ControllerSecuredWithPermission'); - $this->assertFalse(BasicAuthTest\ControllerSecuredWithPermission::$index_called); - $this->assertFalse(BasicAuthTest\ControllerSecuredWithPermission::$post_init_called); - - $headers = [ - 'PHP_AUTH_USER' => 'user-in-mygroup@test.com', - 'PHP_AUTH_PW' => 'test', - ]; - Director::test('BasicAuthTest_ControllerSecuredWithPermission', [], [], null, null, $headers); - $this->assertTrue(BasicAuthTest\ControllerSecuredWithPermission::$index_called); - $this->assertTrue(BasicAuthTest\ControllerSecuredWithPermission::$post_init_called); - } - public function testBasicAuthEnabledWithPermission() { $headers = [ @@ -138,4 +128,26 @@ class BasicAuthTest extends FunctionalTest $check = Member::get()->filter('Email', 'failedlogin@test.com')->first(); $this->assertEquals(0, $check->FailedLoginCount); } + + public function testProtectEntireSite() + { + // Unsecured controller allows access + $response = Director::test('BasicAuthTest_ControllerNotSecured'); + $this->assertEquals(200, $response->getStatusCode()); + + // Globally enable basic auth + BasicAuth::protect_entire_site(); + $response = Director::test('BasicAuthTest_ControllerNotSecured'); + $this->assertEquals(401, $response->getStatusCode()); + $this->assertNotEmpty($response->getHeader('WWW-Authenticate')); + + // Can be excluded via rule + /** @var BasicAuthMiddleware $middleware */ + $middleware = Injector::inst()->get(BasicAuthMiddleware::class); + $middleware->setURLPatterns([ + '/^BasicAuthTest_ControllerNotSecured$/' => false, + ]); + $response = Director::test('BasicAuthTest_ControllerNotSecured'); + $this->assertEquals(200, $response->getStatusCode()); + } } diff --git a/tests/php/Security/BasicAuthTest/ControllerNotSecured.php b/tests/php/Security/BasicAuthTest/ControllerNotSecured.php new file mode 100644 index 000000000..53be3dc8d --- /dev/null +++ b/tests/php/Security/BasicAuthTest/ControllerNotSecured.php @@ -0,0 +1,27 @@ +