From 7ed7ad0254195980f8226d1dbb542debe5288d85 Mon Sep 17 00:00:00 2001
From: Steve Boyd <emteknetnz@gmail.com>
Date: Thu, 17 Jun 2021 12:05:20 +1200
Subject: [PATCH] FIX Ensure changing a password to blank is validated

---
 src/Security/Member.php           |  6 +++---
 src/Security/MemberPassword.php   |  2 +-
 tests/php/Security/MemberTest.php | 12 +++++++++++-
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/Security/Member.php b/src/Security/Member.php
index aac772bb3..c88ea357d 100644
--- a/src/Security/Member.php
+++ b/src/Security/Member.php
@@ -875,7 +875,7 @@ class Member extends DataObject
         if ($this->Email) {
             $this->Email = trim($this->Email);
         }
-        
+
         // If a member with the same "unique identifier" already exists with a different ID, don't allow merging.
         // Note: This does not a full replacement for safeguards in the controller layer (e.g. in a registration form),
         // but rather a last line of defense against data inconsistencies.
@@ -1705,8 +1705,8 @@ class Member extends DataObject
         $valid = parent::validate();
         $validator = static::password_validator();
 
-        if (!$this->ID || $this->isChanged('Password')) {
-            if ($this->Password && $validator) {
+        if ($validator) {
+            if ((!$this->ID && $this->Password) || $this->isChanged('Password')) {
                 $userValid = $validator->validate($this->Password, $this);
                 $valid->combineAnd($userValid);
             }
diff --git a/src/Security/MemberPassword.php b/src/Security/MemberPassword.php
index de71c6e23..845110446 100644
--- a/src/Security/MemberPassword.php
+++ b/src/Security/MemberPassword.php
@@ -53,6 +53,6 @@ class MemberPassword extends DataObject
     public function checkPassword($password)
     {
         $encryptor = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
-        return $encryptor->check($this->Password, $password, $this->Salt, $this->Member());
+        return $encryptor->check($this->Password ?? '', $password, $this->Salt, $this->Member());
     }
 }
diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php
index 96a30041b..52f9a3c10 100644
--- a/tests/php/Security/MemberTest.php
+++ b/tests/php/Security/MemberTest.php
@@ -1593,7 +1593,7 @@ class MemberTest extends FunctionalTest
 
         $this->assertSame('Johnson', $member->getLastName(), 'getLastName should proxy to Surname');
     }
-    
+
     public function testEmailIsTrimmed()
     {
         $member = new Member();
@@ -1601,4 +1601,14 @@ class MemberTest extends FunctionalTest
         $member->write();
         $this->assertNotNull(Member::get()->find('Email', 'trimmed@test.com'));
     }
+
+    public function testChangePasswordToBlankIsValidated()
+    {
+        // override setup() function which setMinLength(0)
+        PasswordValidator::singleton()->setMinLength(8);
+        // 'test' member has a password defined in yml
+        $member = $this->objFromFixture(Member::class, 'test');
+        $result = $member->changePassword('');
+        $this->assertFalse($result->isValid());
+    }
 }