diff --git a/forms/ConfirmedPasswordField.php b/forms/ConfirmedPasswordField.php index 53db507cb..9cf7eda52 100644 --- a/forms/ConfirmedPasswordField.php +++ b/forms/ConfirmedPasswordField.php @@ -54,13 +54,28 @@ class ConfirmedPasswordField extends FormField { */ protected $showOnClick = false; + /** + * Check if the existing password should be entered first + * + * @var bool + */ + protected $requireExistingPassword = false; + /** - * A place to temporarly store the confirm password value + * A place to temporarily store the confirm password value + * * @var string */ protected $confirmValue; + /** + * Store value of "Current Password" field + * + * @var string + */ + protected $currentPasswordValue; + /** * Title for the link that triggers the visibility of password fields. * @@ -105,6 +120,7 @@ class ConfirmedPasswordField extends FormField { // disable auto complete foreach($this->children as $child) { + /** @var FormField $child */ $child->setAttribute('autocomplete', 'off'); } @@ -113,7 +129,7 @@ class ConfirmedPasswordField extends FormField { // we have labels for the subfields $title = false; - parent::__construct($name, $title, null, $form); + parent::__construct($name, $title); $this->setValue($value); } @@ -147,6 +163,7 @@ class ConfirmedPasswordField extends FormField { } foreach($this->children as $field) { + /** @var FormField $field */ $field->setDisabled($this->isDisabled()); $field->setReadonly($this->isReadonly()); @@ -220,6 +237,7 @@ class ConfirmedPasswordField extends FormField { */ public function setRightTitle($title) { foreach($this->children as $field) { + /** @var FormField $field */ $field->setRightTitle($title); } @@ -227,15 +245,20 @@ class ConfirmedPasswordField extends FormField { } /** - * @param array $titles 2 entry array with the customized title for each - * of the 2 children. + * Set child field titles. Titles in order should be: + * - "Current Password" (if getRequireExistingPassword() is set) + * - "Password" + * - "Confirm Password" * - * @return ConfirmedPasswordField + * @param array $titles List of child titles + * @return $this */ public function setChildrenTitles($titles) { - if(is_array($titles) && count($titles) == 2) { + $expectedChildren = $this->getRequireExistingPassword() ? 3 : 2; + if(is_array($titles) && count($titles) == $expectedChildren) { foreach($this->children as $field) { if(isset($titles[0])) { + /** @var FormField $field */ $field->setTitle($titles[0]); array_shift($titles); @@ -251,8 +274,8 @@ class ConfirmedPasswordField extends FormField { * to handle both cases. * * @param mixed $value - * - * @return ConfirmedPasswordField + * @param mixed $data + * @return $this */ public function setValue($value, $data = null) { // If $data is a DataObject, don't use the value, since it's a hashed value @@ -264,6 +287,9 @@ class ConfirmedPasswordField extends FormField { if(is_array($value)) { $this->value = $value['_Password']; $this->confirmValue = $value['_ConfirmPassword']; + $this->currentPasswordValue = ($this->getRequireExistingPassword() && isset($value['_CurrentPassword'])) + ? $value['_CurrentPassword'] + : null; if($this->showOnClick && isset($value['_PasswordFieldVisible'])) { $this->children->fieldByName($this->getName() . '[_PasswordFieldVisible]') @@ -292,6 +318,7 @@ class ConfirmedPasswordField extends FormField { * Update the names of the child fields when updating name of field. * * @param string $name new name to give to the field. + * @return $this */ public function setName($name) { $this->children->fieldByName($this->getName() . '[_Password]') @@ -340,8 +367,7 @@ class ConfirmedPasswordField extends FormField { $validator->validationError( $name, _t('Form.VALIDATIONPASSWORDSDONTMATCH',"Passwords don't match"), - "validation", - false + "validation" ); return false; @@ -353,8 +379,7 @@ class ConfirmedPasswordField extends FormField { $validator->validationError( $name, _t('Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"), - "validation", - false + "validation" ); return false; @@ -363,6 +388,8 @@ class ConfirmedPasswordField extends FormField { // lengths if(($this->minLength || $this->maxLength)) { + $errorMsg = null; + $limit = null; if($this->minLength && $this->maxLength) { $limit = "{{$this->minLength},{$this->maxLength}}"; $errorMsg = _t( @@ -390,8 +417,7 @@ class ConfirmedPasswordField extends FormField { $validator->validationError( $name, $errorMsg, - "validation", - false + "validation" ); } } @@ -402,14 +428,56 @@ class ConfirmedPasswordField extends FormField { $name, _t('Form.VALIDATIONSTRONGPASSWORD', "Passwords must have at least one digit and one alphanumeric character"), - "validation", - false + "validation" ); return false; } } + // Check if current password is valid + if(!empty($value) && $this->getRequireExistingPassword()) { + if(!$this->currentPasswordValue) { + $validator->validationError( + $name, + _t( + 'ConfirmedPasswordField.CURRENT_PASSWORD_MISSING', + "You must enter your current password." + ), + "validation" + ); + return false; + } + + // Check this password is valid for the current user + $member = Member::currentUser(); + if(!$member) { + $validator->validationError( + $name, + _t( + 'ConfirmedPasswordField.LOGGED_IN_ERROR', + "You must be logged in to change your password." + ), + "validation" + ); + return false; + } + + // With a valid user and password, check the password is correct + $checkResult = $member->checkPassword($this->currentPasswordValue); + if(!$checkResult->valid()) { + $validator->validationError( + $name, + _t( + 'ConfirmedPasswordField.CURRENT_PASSWORD_ERROR', + "The current password you have entered is not correct." + ), + "validation" + ); + return false; + } + } + return true; } @@ -442,4 +510,36 @@ class ConfirmedPasswordField extends FormField { return $field; } + + /** + * Check if existing password is required + * + * @return bool + */ + public function getRequireExistingPassword() { + return $this->requireExistingPassword; + } + + /** + * Set if the existing password should be required + * + * @param bool $show Flag to show or hide this field + * @return $this + */ + public function setRequireExistingPassword($show) { + // Don't modify if already added / removed + if((bool)$show === $this->requireExistingPassword) { + return $this; + } + $this->requireExistingPassword = $show; + $name = $this->getName(); + $currentName = "{$name}[_CurrentPassword]"; + if ($show) { + $confirmField = PasswordField::create($currentName, _t('Member.CURRENT_PASSWORD', 'Current Password')); + $this->children->unshift($confirmField); + } else { + $this->children->removeByName($currentName, true); + } + return $this; + } } diff --git a/security/Member.php b/security/Member.php index ecad697e2..c0b1e5ff2 100644 --- a/security/Member.php +++ b/security/Member.php @@ -312,15 +312,23 @@ class Member extends DataObject implements TemplateGlobalProvider { /** * Check if the passed password matches the stored one (if the member is not locked out). * - * @param string $password + * @param string $password * @return ValidationResult */ public function checkPassword($password) { $result = $this->canLogIn(); // Short-circuit the result upon failure, no further checks needed. - if (!$result->valid()) return $result; + if (!$result->valid()) { + return $result; + } + // Allow default admin to login as self + if($this->isDefaultAdmin() && Security::check_default_admin($this->Email, $password)) { + return $result; + } + + // Check a password is set on this member if(empty($this->Password) && $this->exists()) { $result->error(_t('Member.NoPassword','There is no password on this member.')); return $result; @@ -337,6 +345,16 @@ class Member extends DataObject implements TemplateGlobalProvider { return $result; } + /** + * Check if this user is the currently configured default admin + * + * @return bool + */ + public function isDefaultAdmin() { + return Security::has_default_admin() + && $this->Email === Security::default_admin_username(); + } + /** * Returns a valid {@link ValidationResult} if this member can currently log in, or an invalid * one with error messages to display if the member is locked out. @@ -730,14 +748,7 @@ class Member extends DataObject implements TemplateGlobalProvider { public function getMemberFormFields() { $fields = parent::getFrontendFields(); - $fields->replaceField('Password', $password = new ConfirmedPasswordField ( - 'Password', - $this->fieldLabel('Password'), - null, - null, - (bool) $this->ID - )); - $password->setCanBeEmpty(true); + $fields->replaceField('Password', $this->getMemberPasswordField()); $fields->replaceField('Locale', new DropdownField ( 'Locale', @@ -754,6 +765,36 @@ class Member extends DataObject implements TemplateGlobalProvider { return $fields; } + /** + * Builds "Change / Create Password" field for this member + * + * @return ConfirmedPasswordField + */ + public function getMemberPasswordField() { + $editingPassword = $this->isInDB(); + $label = $editingPassword + ? _t('Member.EDIT_PASSWORD', 'New Password') + : $this->fieldLabel('Password'); + /** @var ConfirmedPasswordField $password */ + $password = ConfirmedPasswordField::create( + 'Password', + $label, + null, + null, + $editingPassword + ); + + // If editing own password, require confirmation of existing + if($editingPassword && $this->ID == Member::currentUserID()) { + $password->setRequireExistingPassword(true); + } + + $password->setCanBeEmpty(true); + $this->extend('updateMemberPasswordField', $password); + return $password; + } + + /** * Returns the {@link RequiredFields} instance for the Member object. This * Validator is used when saving a {@link CMSProfileController} or added to @@ -1341,19 +1382,12 @@ class Member extends DataObject implements TemplateGlobalProvider { require_once 'Zend/Date.php'; $self = $this; - $this->beforeUpdateCMSFields(function($fields) use ($self) { - $mainFields = $fields->fieldByName("Root")->fieldByName("Main")->Children; + $this->beforeUpdateCMSFields(function(FieldList $fields) use ($self) { + /** @var FieldList $mainFields */ + $mainFields = $fields->fieldByName("Root")->fieldByName("Main")->getChildren(); - $password = new ConfirmedPasswordField( - 'Password', - null, - null, - null, - true // showOnClick - ); - $password->setCanBeEmpty(true); - if( ! $self->ID) $password->showOnClick = false; - $mainFields->replaceField('Password', $password); + // Build change password field + $mainFields->replaceField('Password', $self->getMemberPasswordField()); $mainFields->replaceField('Locale', new DropdownField( "Locale", diff --git a/tests/behat/features/profile.feature b/tests/behat/features/profile.feature index 9842af666..a13c02c74 100644 --- a/tests/behat/features/profile.feature +++ b/tests/behat/features/profile.feature @@ -19,9 +19,18 @@ Feature: Manage my own settings Then I should see "Jack" And I should see "Johnson" + Scenario: I can't reset the password without the original + Given I follow "Change Password" + And I fill in "Current Password" with "idontknow" + And I fill in "New Password" with "newsecret" + And I fill in "Confirm Password" with "newsecret" + And I press the "Save" button + Then I should see "The current password you have entered is not correct." + Scenario: I can change my password Given I follow "Change Password" - And I fill in "Password" with "newsecret" + And I fill in "Current Password" with "secret" + And I fill in "New Password" with "newsecret" And I fill in "Confirm Password" with "newsecret" And I press the "Save" button And I am not logged in @@ -35,4 +44,4 @@ Feature: Manage my own settings Then I should see "Sprache" # TODO Date/time format - Difficult because its not exposed anywhere in the CMS? - # TODO Group modification as ADMIN user \ No newline at end of file + # TODO Group modification as ADMIN user