Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX Allow logged in user with permissions to bypass basic auth #119

Merged
merged 1 commit into from
Jan 16, 2025
Merged

FIX Allow logged in user with permissions to bypass basic auth #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions _config/routes.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
Name: environmentcheckroutes
Before: coreroutes
---
SilverStripe\Control\Director:
rules:
Expand Down
5 changes: 4 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@
],
"require": {
"php": "^8.1",
"silverstripe/framework": "^5",
"silverstripe/framework": "^5.3",
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
"silverstripe/versioned": "^2",
"guzzlehttp/guzzle": "^7"
},
"require-dev": {
"phpunit/phpunit": "^9.6",
"squizlabs/php_codesniffer": "^3"
},
"conflict": {
"cwp/cwp-core": "<3.1"
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
},
"extra": [],
"autoload": {
"psr-4": {
Expand Down
57 changes: 37 additions & 20 deletions src/EnvironmentChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
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;
}
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
// If we get to this point, user has passed basic auth
}

/**
Expand Down
198 changes: 198 additions & 0 deletions tests/EnvironmentCheckerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace SilverStripe\EnvironmentCheck\Tests;

use ReflectionProperty;
use Monolog\Logger;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}
Loading