fix: public health/check should not include full details

This commit is contained in:
Will Rossiter 2022-07-01 15:13:27 +12:00
parent d3d20e5539
commit 3e3907d14c
No known key found for this signature in database
GPG Key ID: 7FD2A809B22259EF
6 changed files with 72 additions and 10 deletions

View File

@ -43,8 +43,11 @@ class DevCheckController extends Controller
$suite = $name;
}
$checker = new EnvironmentChecker($suite, 'Environment status');
$checker->setRequest($request);
/** @var EnvironmentChecker */
$checker = EnvironmentChecker::create($suite, 'Environment status')
->setRequest($request)
->setIncludeDetails(true);
$checker->init($this->config()->permission);
return $checker;

View File

@ -28,7 +28,6 @@ class DevHealthController extends Controller
public function index()
{
// health check does not require permission to run
$checker = new EnvironmentChecker('health', 'Site health');
$checker->setErrorCode(500);

View File

@ -40,6 +40,11 @@ class EnvironmentChecker extends RequestHandler
*/
protected $title;
/**
* @var bool
*/
protected $includeDetails = false;
/**
* @var int
*/
@ -172,19 +177,31 @@ class EnvironmentChecker extends RequestHandler
$response->setStatusCode($this->errorCode);
}
$resultText = $result->customise([
$data = [
'URL' => Director::absoluteBaseURL(),
'Title' => $this->title,
'Name' => $this->checkSuiteName,
'ErrorCode' => $this->errorCode,
])->renderWith(__CLASS__);
'ErrorCode' => $this->errorCode
];
$emailContent = $result->customise(array_merge($data, [
'IncludeDetails' => true
]))->renderWith(__CLASS__);
if (!$this->includeDetails) {
$webContent = $result->customise(array_merge($data, [
'IncludeDetails' => false
]))->renderWith(__CLASS__);
} else {
$webContent = $emailContent;
}
if ($this->config()->get('email_results') && !$result->ShouldPass()) {
$email = new Email(
$this->config()->get('from_email_address'),
$this->config()->get('to_email_address'),
$this->title,
$resultText
$emailContent
);
$email->send();
}
@ -213,7 +230,7 @@ class EnvironmentChecker extends RequestHandler
return $response;
}
$response->setBody($resultText);
$response->setBody($webContent);
return $response;
}
@ -229,16 +246,36 @@ class EnvironmentChecker extends RequestHandler
Injector::inst()->get(LoggerInterface::class)->log($level, $message);
}
/**
* Set the HTTP status code that should be returned when there's an error.
*
* @param int $errorCode
*
* @return $this
*/
public function setErrorCode($errorCode)
{
$this->errorCode = $errorCode;
return $this;
}
/**
* Set whether to include the full breakdown of services
*
* @param bool $includeDetails
*
* @return $this
*/
public function setIncludeDetails($includeDetails)
{
$this->includeDetails = $includeDetails;
return $this;
}
/**
* @deprecated
* @param string $from

View File

@ -64,16 +64,18 @@
<h1 class="$Status">$Title: $Status</h1>
<h2 class="website">Site: $URL</h2>
<% if $IncludeDetails %>
<table>
<tr><th>Check</th> <th>Status</th> <th>Message</th></tr>
<% loop $Details %>
<tr><td>$Check</td> <td class="$Status">$Status</td> <td>$Message.XML</td></tr>
<% end_loop %>
</table>
<% end_if %>
<% if $ShouldPass %>
<p>Site is available</p>
<p class="subtext">(you may check for the presence of the text 'Site is available' rather than an HTTP $ErrorCode error on this page, if you prefer.)</p>
<p>Site is available</p>
<p class="subtext">(you may check for the presence of the text 'Site is available' rather than an HTTP $ErrorCode error on this page, if you prefer.<% if not $IncludeDetails %> Full details are available for logged in users at <a href="{$AbsoluteBaseURL}dev/check/">dev/check</a><% end_if %>)</p>
<% else %>
<% if $Name == "check" %>
<p><b>A subsystem of the site is unavailable, but the site remains operational</b></p>

View File

@ -28,4 +28,14 @@ class DevCheckControllerTest extends SapphireTest
$this->assertInstanceOf(EnvironmentChecker::class, $controller->index($request));
}
public function testCheckIncludesDetails()
{
$controller = new DevCheckController();
$request = new HTTPRequest('GET', 'example.com');
$response = $controller->index($request)->index();
$this->assertStringContainsString('<table>', $response->getBody());
}
}

View File

@ -37,4 +37,15 @@ class DevHealthControllerTest extends SapphireTest
$this->assertInstanceOf(EnvironmentChecker::class, $controller->index($request));
}
public function testHealthDoesNotIncludeDetails()
{
$controller = new DevHealthController();
$request = new HTTPRequest('GET', 'example.com');
$response = $controller->index($request)->index();
$this->assertFalse(strpos($response->getBody(), '<table>'));
}
}