From 44ee87dccb84a6da6a7a6d7a4a9b6c3f403a9ea4 Mon Sep 17 00:00:00 2001 From: Ralph Slooten Date: Tue, 19 Dec 2023 15:42:04 +1300 Subject: [PATCH 1/2] Fix logging of failed logins & unknown users Fixes the `authenticationFailed()` method to correctly return the login email --- code/AuditHook.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/code/AuditHook.php b/code/AuditHook.php index 28c722a..86a4c1f 100644 --- a/code/AuditHook.php +++ b/code/AuditHook.php @@ -382,7 +382,7 @@ public function authenticationFailed($data) // LDAP authentication uses a "Login" POST field instead of Email. $login = isset($data['Login']) ? $data['Login'] - : (isset($data[Email::class]) ? $data[Email::class] : ''); + : (isset($data['Email']) ? $data['Email'] : ''); if (empty($login)) { return $this->getAuditLogger()->warning( @@ -394,6 +394,14 @@ public function authenticationFailed($data) $this->getAuditLogger()->info(sprintf('Failed login attempt using email "%s"', $login)); } + /** + * Log failed login attempts when the email address doesn't map to an existing member record + */ + public function authenticationFailedUnknownUser($data) + { + $this->authenticationFailed($data); + } + /** * @deprecated 2.1.0 Use tractorcow/silverstripe-proxy-db instead */ From d5f2f0b6eb9173cec78f297000f2e3bbc5fb332d Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Wed, 20 Dec 2023 16:19:27 +1300 Subject: [PATCH 2/2] MNT Test failed login is logged correctly --- tests/AuditHookTest.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/AuditHookTest.php b/tests/AuditHookTest.php index 0f93255..341c620 100644 --- a/tests/AuditHookTest.php +++ b/tests/AuditHookTest.php @@ -333,4 +333,31 @@ public function testRestoreToStage() $this->assertStringContainsString('deleted Page', $message); $this->assertStringContainsString('My page', $message); } + + public function testFailedLogin() + { + $member = $this->createMemberWithPermission('ADMIN'); + $this->get('Security/login'); + $this->submitForm( + 'MemberLoginForm_LoginForm', + null, + ['Email' => $member->Email, 'Password' => 'clearly wrong password'] + ); + + $message = $this->writer->getLastMessage(); + $this->assertStringContainsString('Failed login attempt using email "' . $member->Email . '"', $message); + } + + public function testFailedLoginWithoutMember() + { + $this->get('Security/login'); + $this->submitForm( + 'MemberLoginForm_LoginForm', + null, + ['Email' => '__NO VALID USER__', 'Password' => 'clearly wrong password'] + ); + + $message = $this->writer->getLastMessage(); + $this->assertStringContainsString('Failed login attempt using email "__NO VALID USER__"', $message); + } }