Skip to content

Commit 49254bb

Browse files
authored
feature: automatically redact passwords in logs (#728)
1 parent 57f52b4 commit 49254bb

3 files changed

Lines changed: 125 additions & 5 deletions

File tree

config.default.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ log_static_requests=false
3131
log_404_to_error_log=false
3232
log_not_modified=false
3333
log_redirects=false
34+
ignore_post_fields=password,pass,passwd,password_confirm,password_confirmation,current_password,new_password,old_password,secret,client_secret,token,access_token,refresh_token,id_token,api_key,apikey,authorization,auth,bearer,otp,totp,mfa_code,verification_code,recovery_code,card_number,cc_number,credit_card,cvv,cvc,pin
3435
debug_to_javascript=true
3536
stderr_level=ERROR
3637
type=stdout

src/Application.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
* @SuppressWarnings("PHPMD.ExcessiveClassComplexity")
4242
*/
4343
class Application {
44+
private const string REDACTED_LOG_VALUE = "[redacted]";
45+
4446
private Redirect $redirect;
4547
private Timer $timer;
4648
private OutputBuffer $outputBuffer;
@@ -575,6 +577,59 @@ private function shouldLogRequest(Response $response):bool {
575577
*/
576578
private function filterLoggedPostBody(array $postBody):array {
577579
unset($postBody[HTMLDocumentProtector::TOKEN_NAME]);
580+
return $this->redactIgnoredPostFields(
581+
$postBody,
582+
$this->getIgnoredPostFieldLookup(),
583+
);
584+
}
585+
586+
/** @return array<string, true> */
587+
private function getIgnoredPostFieldLookup():array {
588+
$configuredFields = explode(
589+
",",
590+
$this->config->getString("logger.ignore_post_fields") ?? "",
591+
);
592+
593+
$lookup = [];
594+
foreach($configuredFields as $field) {
595+
$field = strtolower(trim($field));
596+
if($field === "") {
597+
continue;
598+
}
599+
600+
$lookup[$field] = true;
601+
}
602+
603+
return $lookup;
604+
}
605+
606+
/**
607+
* @param array<array-key, mixed> $postBody
608+
* @param array<string, true> $ignoredFieldLookup
609+
* @return array<array-key, mixed>
610+
*/
611+
private function redactIgnoredPostFields(
612+
array $postBody,
613+
array $ignoredFieldLookup,
614+
):array {
615+
if(!$ignoredFieldLookup) {
616+
return $postBody;
617+
}
618+
619+
foreach($postBody as $key => $value) {
620+
if(is_string($key) && isset($ignoredFieldLookup[strtolower($key)])) {
621+
$postBody[$key] = self::REDACTED_LOG_VALUE;
622+
continue;
623+
}
624+
625+
if(is_array($value)) {
626+
$postBody[$key] = $this->redactIgnoredPostFields(
627+
$value,
628+
$ignoredFieldLookup,
629+
);
630+
}
631+
}
632+
578633
return $postBody;
579634
}
580635

test/phpunit/ApplicationTest.php

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ public function testDefaultConfig_usesHtmlRoutingAndDoesNotLogNotModifiedRespons
4141

4242
self::assertSame("text/html", $config["router"]["default_content_type"]);
4343
self::assertSame("false", $config["logger"]["log_not_modified"]);
44+
self::assertStringContainsString("password", $config["logger"]["ignore_post_fields"]);
45+
self::assertStringContainsString("pass", $config["logger"]["ignore_post_fields"]);
4446
}
4547

4648
public function testStart_callsRedirectExecute():void {
@@ -860,7 +862,7 @@ public function testBuildLogContext_omitsObjectParsedBodyAndKeepsQueryContext():
860862
self::assertArrayNotHasKey("post", $context);
861863
}
862864

863-
public function testBuildLogContext_filtersConfiguredCsrfTokenNameOnly():void {
865+
public function testBuildLogContext_filtersCsrfTokenBeforeRedactingIgnoredFields():void {
864866
$sut = new Application(
865867
config: $this->createTestConfig([]),
866868
requestFactory: $this->createRequestFactory(),
@@ -872,14 +874,76 @@ public function testBuildLogContext_filtersConfiguredCsrfTokenNameOnly():void {
872874
$sut,
873875
"buildLogContext",
874876
"/send",
877+
[],
878+
[
879+
HTMLDocumentProtector::TOKEN_NAME => "CSRF_secret",
880+
"token" => "business-token",
881+
],
882+
);
883+
884+
self::assertSame(["token" => "[redacted]"], $context["post"]);
885+
}
886+
887+
public function testBuildLogContext_redactsIgnoredPostFields():void {
888+
$sut = new Application(
889+
config: $this->createTestConfig([]),
890+
requestFactory: $this->createRequestFactory(),
891+
dispatcherFactory: self::createStub(DispatcherFactory::class),
892+
globalProtection: self::createStub(Protection::class),
893+
);
894+
895+
$context = $this->invokePrivateMethod(
896+
$sut,
897+
"buildLogContext",
898+
"/login",
875899
[],
876900
[
877-
HTMLDocumentProtector::TOKEN_NAME => "CSRF_secret",
878-
"token" => "business-token",
901+
"username" => "ada",
902+
"password" => "correct horse battery staple",
903+
"pass" => "secret",
879904
],
880905
);
881906

882-
self::assertSame(["token" => "business-token"], $context["post"]);
907+
self::assertSame([
908+
"username" => "ada",
909+
"password" => "[redacted]",
910+
"pass" => "[redacted]",
911+
], $context["post"]);
912+
}
913+
914+
public function testBuildLogContext_redactsConfiguredPostFieldsRecursively():void {
915+
$sut = new Application(
916+
config: $this->createTestConfig([
917+
"logger.ignore_post_fields" => " password , secret , ",
918+
]),
919+
requestFactory: $this->createRequestFactory(),
920+
dispatcherFactory: self::createStub(DispatcherFactory::class),
921+
globalProtection: self::createStub(Protection::class),
922+
);
923+
924+
$context = $this->invokePrivateMethod(
925+
$sut,
926+
"buildLogContext",
927+
"/account",
928+
[],
929+
[
930+
"user" => [
931+
"name" => "Grace",
932+
"password" => "hidden",
933+
],
934+
"SECRET" => "hidden",
935+
"message" => "hello",
936+
],
937+
);
938+
939+
self::assertSame([
940+
"user" => [
941+
"name" => "Grace",
942+
"password" => "[redacted]",
943+
],
944+
"SECRET" => "[redacted]",
945+
"message" => "hello",
946+
], $context["post"]);
883947
}
884948

885949
public function testStart_logsAllRequestsWithRequestContext():void {
@@ -924,7 +988,7 @@ public function testStart_logsAllRequestsWithRequestContext():void {
924988
self::assertSame("HTTP 204", TestLogHandler::$records[0]["message"]);
925989
self::assertSame("/search", TestLogHandler::$records[0]["context"]["uri"]);
926990
self::assertSame(["q" => "php"], TestLogHandler::$records[0]["context"]["query"]);
927-
self::assertSame(["token" => "abc"], TestLogHandler::$records[0]["context"]["post"]);
991+
self::assertSame(["token" => "[redacted]"], TestLogHandler::$records[0]["context"]["post"]);
928992
self::assertSame("127.0.0.1:", TestLogHandler::$records[0]["context"]["id"]);
929993
}
930994

0 commit comments

Comments
 (0)