From 6b52412693eaf13d0ae60bb807180069d9cb7eec Mon Sep 17 00:00:00 2001
From: Robbie Averill <robbie@silverstripe.com>
Date: Tue, 3 Oct 2017 17:57:52 +1300
Subject: [PATCH] NEW Make Member::changePassword extensible

---
 src/Security/Member.php                        | 10 ++++++++--
 tests/php/Security/MemberTest.php              | 17 +++++++++++++++++
 .../ExtendedChangePasswordExtension.php        | 18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 tests/php/Security/MemberTest/ExtendedChangePasswordExtension.php

diff --git a/src/Security/Member.php b/src/Security/Member.php
index 0b725f11c..b77764341 100644
--- a/src/Security/Member.php
+++ b/src/Security/Member.php
@@ -1680,8 +1680,10 @@ class Member extends DataObject
     }
 
     /**
-     * Change password. This will cause rehashing according to
-     * the `PasswordEncryption` property.
+     * Change password. This will cause rehashing according to the `PasswordEncryption` property. This method will
+     * allow extensions to perform actions and augment the validation result if required before the password is written
+     * and can check it after the write also. Note that the onAfterChangePassword extension point receives a clone of
+     * the validation result which cannot be modified.
      *
      * @param string $password Cleartext password
      * @return ValidationResult
@@ -1691,11 +1693,15 @@ class Member extends DataObject
         $this->Password = $password;
         $valid = $this->validate();
 
+        $this->extend('onBeforeChangePassword', $password, $valid);
+
         if ($valid->isValid()) {
             $this->AutoLoginHash = null;
             $this->write();
         }
 
+        $this->extend('onAfterChangePassword', $password, $valid);
+
         return $valid;
     }
 
diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php
index 55c69df6a..7dce42fe0 100644
--- a/tests/php/Security/MemberTest.php
+++ b/tests/php/Security/MemberTest.php
@@ -11,6 +11,7 @@ use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\ValidationException;
+use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Group;
 use SilverStripe\Security\IdentityStore;
 use SilverStripe\Security\Member;
@@ -1462,4 +1463,20 @@ class MemberTest extends FunctionalTest
         });
         $this->assertEmpty($member);
     }
+
+    public function testChangePasswordWithExtensionsThatModifyValidationResult()
+    {
+        // Default behaviour
+        $member = $this->objFromFixture(Member::class, 'admin');
+        $result = $member->changePassword('my-secret-new-password');
+        $this->assertInstanceOf(ValidationResult::class, $result);
+        $this->assertTrue($result->isValid());
+
+        // With an extension added
+        Member::add_extension(MemberTest\ExtendedChangePasswordExtension::class);
+        $member = $this->objFromFixture(Member::class, 'admin');
+        $result = $member->changePassword('my-second-secret-password');
+        $this->assertInstanceOf(ValidationResult::class, $result);
+        $this->assertFalse($result->isValid());
+    }
 }
diff --git a/tests/php/Security/MemberTest/ExtendedChangePasswordExtension.php b/tests/php/Security/MemberTest/ExtendedChangePasswordExtension.php
new file mode 100644
index 000000000..fb25e467b
--- /dev/null
+++ b/tests/php/Security/MemberTest/ExtendedChangePasswordExtension.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace SilverStripe\Security\Tests\MemberTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataExtension;
+use SilverStripe\ORM\ValidationResult;
+
+/**
+ * Extension that does something extra when changing a member's password
+ */
+class ExtendedChangePasswordExtension extends DataExtension implements TestOnly
+{
+    public function onBeforeChangePassword($newPassword, $valid)
+    {
+        $valid->addError('Extension failed to handle Mary changing her password');
+    }
+}