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 eb18e45..036405d 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" }, @@ -31,6 +31,9 @@ "silverstripe/standards": "^1", "phpstan/extension-installer": "^1.3" }, + "conflict": { + "cwp/cwp-core": "<3.1" + }, "extra": [], "autoload": { "psr-4": { diff --git a/src/EnvironmentChecker.php b/src/EnvironmentChecker.php index 4005d81..23bb749 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,28 +104,44 @@ public function __construct($checkSuiteName, $title) */ public function init($permission = 'ADMIN') { - // 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') - ) { - // Fail check with basic auth challenge - $response = new HTTPResponse(null, 401); - $response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\""); - throw new HTTPResponse_Exception($response); + $canAccess = $this->canAccess(null, $permission); + // Allow bypassing basic-auth check if the user is logged in with the required permission + if ($canAccess) { + return; + } + // If basic auth is not configured, raise a permission failure + $basicAuthUsername = Environment::getEnv('ENVCHECK_BASICAUTH_USERNAME'); + $basicAuthPassword = Environment::getEnv('ENVCHECK_BASICAUTH_PASSWORD'); + if (!$basicAuthUsername || !$basicAuthPassword) { + $response = Security::permissionFailure(); + throw new HTTPResponse_Exception($response); + } + // Check basic auth + $request = $this->getRequest(); + $user = $request->getHeader('PHP_AUTH_USER'); + $pw = $request->getHeader('PHP_AUTH_PW'); + // Check that submitted basic auth credentials match + if ($user !== $basicAuthUsername || $pw !== $basicAuthPassword) { + // Fail check with basic auth challenge + $response = new HTTPResponse(null, 401); + $response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\""); + 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.' + )); } - } 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); + // Exception is caught by RequestHandler->handleRequest() and will halt further execution + $e = new HTTPResponse_Exception(null, 401); + $e->setResponse($response); + throw $e; } + // If we get to this point, user has passed basic auth } /** diff --git a/tests/EnvironmentCheckerTest.php b/tests/EnvironmentCheckerTest.php index f8e2c02..ecaef61 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,197 @@ 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, + ], + 'logged-out-no-creds' => [ + 'user' => 'logged-out', + 'validCreds' => null, + 'expectedSuccess' => false, + ], + 'non-admin-valid-creds' => [ + 'user' => 'non-admin', + 'validCreds' => true, + 'expectedSuccess' => true, + ], + 'non-admin-invalid-creds' => [ + 'user' => 'non-admin', + 'validCreds' => false, + 'expectedSuccess' => false, + ], + 'non-admin-no-creds' => [ + 'user' => 'non-admin', + 'validCreds' => null, + 'expectedSuccess' => false, + ], + 'admin-valid-creds' => [ + 'user' => 'admin', + 'validCreds' => true, + 'expectedSuccess' => true, + ], + 'admin-invalid-creds' => [ + 'user' => 'admin', + 'validCreds' => false, + 'expectedSuccess' => true, + ], + 'admin-no-creds' => [ + 'user' => 'admin', + 'validCreds' => null, + 'expectedSuccess' => true, + ], + ]; + } + + /** + * @dataProvider provideBasicAuthAdminBypass + */ + public function testBasicAuthAdminBypass( + string $user, + ?bool $validCreds, + bool $expectedSuccess, + ): void { + $this->setupBasicAuthEnv(true, true); + // Log in or out + if ($user === 'admin') { + $this->logInWithPermission('ADMIN'); + } elseif ($user === 'non-admin') { + $this->logInWithPermission('NOT_AN_ADMIN'); + } else { + $this->logOut(); + } + $request = new HTTPRequest('GET', 'dev/check'); + if ($validCreds !== null) { + // Simulate passing in basic auth creds + $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); + } + + public static function provideBasicAuthEnvVariables(): array + { + return [ + 'logged-in-env-vars-set' => [ + 'loggedIn' => true, + 'setUsernameEnvVar' => true, + 'setPwEnvVar' => true, + 'expected' => true, + ], + 'logged-in-env-no-username' => [ + 'loggedIn' => true, + 'setUsernameEnvVar' => false, + 'setPwEnvVar' => true, + 'expected' => true, + ], + 'logged-in-env-no-password' => [ + 'loggedIn' => true, + 'setUsernameEnvVar' => true, + 'setPwEnvVar' => false, + 'expected' => true, + ], + 'logged-in-env-no-username-or-pw' => [ + 'loggedIn' => true, + 'setUsernameEnvVar' => false, + 'setPwEnvVar' => false, + 'expected' => true, + ], + 'logged-out-env-vars-set' => [ + 'loggedIn' => false, + 'setUsernameEnvVar' => true, + 'setPwEnvVar' => true, + 'expected' => true, + ], + 'logged-out-env-no-username' => [ + 'loggedIn' => false, + 'setUsernameEnvVar' => false, + 'setPwEnvVar' => true, + 'expected' => false, + ], + 'logged-out-env-no-password' => [ + 'loggedIn' => false, + 'setUsernameEnvVar' => true, + 'setPwEnvVar' => false, + 'expected' => false, + ], + 'logged-out-env-no-username-or-pw' => [ + 'loggedIn' => false, + 'setUsernameEnvVar' => false, + 'setPwEnvVar' => false, + 'expected' => false, + ], + ]; + } + + /** + * @dataProvider provideBasicAuthEnvVariables + */ + public function testBasicAuthEnvVariables( + bool $loggedIn, + bool $setUsernameEnvVar, + bool $setPwEnvVar, + bool $expected, + ): void { + $this->setupBasicAuthEnv($setUsernameEnvVar, $setPwEnvVar); + if ($loggedIn) { + $this->logInWithPermission('ADMIN'); + } else { + $this->logOut(); + } + $request = new HTTPRequest('GET', 'dev/check'); + $request->addHeader('PHP_AUTH_USER', 'test'); + $request->addHeader('PHP_AUTH_PW', 'password'); + // 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(302, $response->getStatusCode()); + } + $this->assertSame($expected, $success); + } + + /** + * Setup the test environment so that we can use basic auth + */ + private function setupBasicAuthEnv(bool $setUsernameEnvVar, bool $setPwEnvVar): 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'); + // Set the basic auth env vars + Environment::setEnv('ENVCHECK_BASICAUTH_USERNAME', $setUsernameEnvVar ? 'test' : null); + Environment::setEnv('ENVCHECK_BASICAUTH_PASSWORD', $setPwEnvVar ? 'password' : null); + } }