diff --git a/README.md b/README.md index bddc32b..b12209a 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ SilverStripe\EnvironmentCheck\EnvironmentChecker: ## Authentication By default, accessing the `dev/check` URL will not require authentication on CLI and dev environments, but if you're -trying to access it on a live or test environment, it will respond with a 403 HTTP status unless you're logged in as +trying to access it on a live or test environment, it will respond with a 401 HTTP status unless you're logged in as an administrator on the site. You may wish to have an automated service check `dev/check` periodically, but not want to open it up for public access. diff --git a/_config/routes.yml b/_config/routes.yml index 88c3c06..d71337a 100644 --- a/_config/routes.yml +++ b/_config/routes.yml @@ -1,5 +1,6 @@ --- Name: environmentcheckroutes +Before: coreroutes --- SilverStripe\Control\Director: rules: diff --git a/composer.json b/composer.json index d3d793e..e81f9f3 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ ], "require": { "php": "^8.1", - "silverstripe/framework": "^5", + "silverstripe/framework": "^5.3", "silverstripe/versioned": "^2", "guzzlehttp/guzzle": "^7" }, @@ -29,6 +29,9 @@ "phpunit/phpunit": "^9.6", "squizlabs/php_codesniffer": "^3" }, + "conflict": { + "cwp/cwp-core": "<3.1" + }, "extra": [], "autoload": { "psr-4": { diff --git a/src/EnvironmentChecker.php b/src/EnvironmentChecker.php index 4005d81..75f21cb 100644 --- a/src/EnvironmentChecker.php +++ b/src/EnvironmentChecker.php @@ -14,6 +14,7 @@ use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; +use SilverStripe\Security\BasicAuth; /** * Provides an interface for checking the given EnvironmentCheckSuite. @@ -103,27 +104,42 @@ public function __construct($checkSuiteName, $title) */ public function init($permission = 'ADMIN') { + $canAccess = $this->canAccess(null, $permission); + // Allow bypassing basic-auth check if the user is logged in with the required permission + if ($canAccess) { + return; + } // if the environment supports it, provide a basic auth challenge and see if it matches configured credentials if (Environment::getEnv('ENVCHECK_BASICAUTH_USERNAME') && Environment::getEnv('ENVCHECK_BASICAUTH_PASSWORD') ) { - // Check that details are both provided, and match - if (empty($_SERVER['PHP_AUTH_USER']) || empty($_SERVER['PHP_AUTH_PW']) - || $_SERVER['PHP_AUTH_USER'] != Environment::getEnv('ENVCHECK_BASICAUTH_USERNAME') - || $_SERVER['PHP_AUTH_PW'] != Environment::getEnv('ENVCHECK_BASICAUTH_PASSWORD') + $request = $this->getRequest(); + $user = $request->getHeader('PHP_AUTH_USER'); + $pw = $request->getHeader('PHP_AUTH_PW'); + // Check that details match + if (empty($user) || empty($pw) + || $user !== Environment::getEnv('ENVCHECK_BASICAUTH_USERNAME') + || $pw !== Environment::getEnv('ENVCHECK_BASICAUTH_PASSWORD') ) { // Fail check with basic auth challenge $response = new HTTPResponse(null, 401); $response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\""); - throw new HTTPResponse_Exception($response); + if ($user) { + $response->setBody(_t( + BasicAuth::class . '.ERRORNOTREC', + "That username / password isn't recognised" + )); + } else { + $response->setBody(_t( + BasicAuth::class . '.ENTERINFO', + 'Please enter a username and password.' + )); + } + // Exception is caught by RequestHandler->handleRequest() and will halt further execution + $e = new HTTPResponse_Exception(null, 401); + $e->setResponse($response); + throw $e; } - } elseif (!$this->canAccess(null, $permission)) { - // Fail check with silverstripe login challenge - $result = Security::permissionFailure( - $this, - "You must have the {$permission} permission to access this check" - ); - throw new HTTPResponse_Exception($result); } } diff --git a/tests/EnvironmentCheckerTest.php b/tests/EnvironmentCheckerTest.php index f8e2c02..ccd0dea 100644 --- a/tests/EnvironmentCheckerTest.php +++ b/tests/EnvironmentCheckerTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\EnvironmentCheck\Tests; +use ReflectionProperty; use Monolog\Logger; use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; @@ -10,6 +11,10 @@ use SilverStripe\Dev\SapphireTest; use SilverStripe\EnvironmentCheck\EnvironmentChecker; use SilverStripe\EnvironmentCheck\EnvironmentCheckSuite; +use SilverStripe\Core\Environment; +use SilverStripe\Core\Kernel; +use SilverStripe\Control\HTTPResponse_Exception; +use SilverStripe\Control\HTTPRequest; /** * Class EnvironmentCheckerTest @@ -94,4 +99,85 @@ public function testLogsWithErrors() (new EnvironmentChecker('test suite', 'test'))->index(); } + + public static function provideBasicAuthAdminBypass(): array + { + return [ + 'logged-out-valid-creds' => [ + 'user' => 'logged-out', + 'validCreds' => true, + 'expectedSuccess' => true, + ], + 'logged-out-invalid-creds' => [ + 'user' => 'logged-out', + 'validCreds' => false, + 'expectedSuccess' => false, + ], + 'non-admin-valid-creds' => [ + 'user' => 'non-admin', + 'validCreds' => true, + 'expectedSuccess' => true, + ], + 'non-admin-invalid-creds' => [ + 'user' => 'non-admin', + 'validCreds' => false, + 'expectedSuccess' => false, + ], + 'admin-valid-creds' => [ + 'user' => 'admin', + 'validCreds' => true, + 'expectedSuccess' => true, + ], + 'admin-invalid-creds' => [ + 'user' => 'admin', + 'validCreds' => false, + 'expectedSuccess' => true, + ], + ]; + } + + /** + * @dataProvider provideBasicAuthAdminBypass + */ + public function testBasicAuthAdminBypass( + string $user, + bool $validCreds, + bool $expectedSuccess, + ): void { + // Pretend we're not using CLI which will bypass basic auth, + $reflectionCli = new ReflectionProperty(Environment::class, 'isCliOverride'); + $reflectionCli->setAccessible(true); + $reflectionCli->setValue(false); + // Change from dev to test mode as dev mode will bypass basic auth + $kernel = Injector::inst()->get(Kernel::class); + $kernel->setEnvironment('test'); + // Setup basic auth env variables + Environment::setEnv('ENVCHECK_BASICAUTH_USERNAME', 'test'); + Environment::setEnv('ENVCHECK_BASICAUTH_PASSWORD', 'password'); + // Log in or out + if ($user === 'admin') { + $this->logInWithPermission('ADMIN'); + } elseif ($user === 'non-admin') { + $this->logInWithPermission('NOT_AN_ADMIN'); + } else { + $this->logOut(); + } + // Simulate passing in basic auth creds + $request = new HTTPRequest('GET', 'dev/check'); + $request->addHeader('PHP_AUTH_USER', 'test'); + $request->addHeader('PHP_AUTH_PW', $validCreds ? 'password' : 'invalid'); + // Run test + $checker = new EnvironmentChecker('test suite', 'test'); + $checker->setRequest($request); + $success = null; + try { + $checker->init(); + $success = true; + } catch (HTTPResponse_Exception $e) { + $success = false; + $response = $e->getResponse(); + $this->assertEquals(401, $response->getStatusCode()); + } + $this->assertSame($expectedSuccess, $success); + } }