tonyair
/
silverstripe-framework
mirror of https://github.com/silverstripe/silverstripe-framework
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Improve `Member_Validator` to: 

- properly check for existing members.
- allow extensions.
- remove old code and replace with new syntax and add config API.

Fix issue in Group code where Member_Validator was instantiated via "new" which didn't allow injector overrides.
Added unit-tests.

Establish a link between the member and the validator for said member.
This commit is contained in:
Roman Schmid 2016-02-24 13:23:38 +01:00
parent 5f2d3f31d7
commit f691a5da32
3 changed files with 248 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
security/Group.php
View File

@ -115,7 +115,7 @@ class Group extends DataObject {
$config->getComponentByType('GridFieldAddExistingAutocompleter')
->setResultsFormat('$Title ($Email)')->setSearchFields(array('FirstName', 'Surname', 'Email'));
$config->getComponentByType('GridFieldDetailForm')
->setValidator(new Member_Validator())
->setValidator(Member_Validator::create())
->setItemEditFormCallback(function($form, $component) use($group) {
$record = $form->getRecord();
$groupsField = $form->Fields()->dataFieldByName('DirectGroups');

132
security/Member.php
View File

@ -766,6 +766,7 @@ class Member extends DataObject implements TemplateGlobalProvider {
*/
public function getValidator() {
$validator = Injector::inst()->create('Member_Validator');
$validator->setForMember($this);
$this->extend('updateValidator', $validator);
return $validator;
@ -1801,21 +1802,39 @@ class Member_ForgotPasswordEmail extends Email {
* Member Validator
*
* Custom validation for the Member object can be achieved either through an
* {@link DataExtension} on the Member object or, by specifying a subclass of
* {@link DataExtension} on the Member_Validator object or, by specifying a subclass of
* {@link Member_Validator} through the {@link Injector} API.
*
* The Validator can also be modified by adding an Extension to Member and implement the
* <code>updateValidator</code> hook.
* {@see Member::getValidator()}
*
* Additional required fields can also be set via config API, eg.
* <code>
* Member_Validator:
* customRequired:
* - Surname
* </code>
*
* @package framework
* @subpackage security
*/
class Member_Validator extends RequiredFields {
class Member_Validator extends RequiredFields
{
/**
* Fields that are required by this validator
* @config
* @var array
*/
protected $customRequired = array(
'FirstName',
'Email'
);
/**
* Determine what member this validator is meant for
* @var Member
*/
protected $forMember = null;
/**
* Constructor
@ -1829,7 +1848,33 @@ class Member_Validator extends RequiredFields {
$required = array_merge($required, $this->customRequired);
parent::__construct($required);
// check for config API values and merge them in
$config = $this->config()->customRequired;
if(is_array($config)){
$required = array_merge($required, $config);
}
parent::__construct(array_unique($required));
}
/**
* Get the member this validator applies to.
* @return Member
*/
public function getForMember()
{
return $this->forMember;
}
/**
* Set the Member this validator applies to.
* @param Member $value
* @return $this
*/
public function setForMember(Member $value)
{
$this->forMember = $value;
return $this;
}
/**
@ -1842,47 +1887,54 @@ class Member_Validator extends RequiredFields {
* @return bool Returns TRUE if the submitted data is valid, otherwise
* FALSE.
*/
public function php($data) {
public function php($data)
{
$valid = parent::php($data);
$identifierField = Member::config()->unique_identifier_field;
$member = DataObject::get_one('Member', array(
"\"$identifierField\"" => $data[$identifierField]
));
$identifierField = (string)Member::config()->unique_identifier_field;
// if we are in a complex table field popup, use ctf[childID], else use ID
if(isset($_REQUEST['ctf']['childID'])) {
$id = $_REQUEST['ctf']['childID'];
} elseif(isset($_REQUEST['ID'])) {
$id = $_REQUEST['ID'];
} else {
$id = null;
}
if($id && is_object($member) && $member->ID != $id) {
$uniqueField = $this->form->Fields()->dataFieldByName($identifierField);
$this->validationError(
$uniqueField->id(),
_t(
'Member.VALIDATIONMEMBEREXISTS',
'A member already exists with the same %s',
array('identifier' => strtolower($identifierField))
),
'required'
);
$valid = false;
}
// Execute the validators on the extensions
if($this->extension_instances) {
foreach($this->extension_instances as $extension) {
if(method_exists($extension, 'hasMethod') && $extension->hasMethod('updatePHP')) {
$valid &= $extension->updatePHP($data, $this->form);
// Only validate identifier field if it's actually set. This could be the case if
// somebody removes `Email` from the list of required fields.
if(isset($data[$identifierField])){
$id = isset($data['ID']) ? (int)$data['ID'] : 0;
if(!$id && ($ctrl = $this->form->getController())){
// get the record when within GridField (Member editing page in CMS)
if($ctrl instanceof GridFieldDetailForm_ItemRequest && $record = $ctrl->getRecord()){
$id = $record->ID;
}
}
// If there's no ID passed via controller or form-data, use the assigned member (if available)
if(!$id && ($member = $this->getForMember())){
$id = $member->exists() ? $member->ID : 0;
}
// set the found ID to the data array, so that extensions can also use it
$data['ID'] = $id;
$members = Member::get()->filter($identifierField, $data[$identifierField]);
if($id) {
$members = $members->exclude('ID', $id);
}
if($members->count() > 0) {
$this->validationError(
$identifierField,
_t(
'Member.VALIDATIONMEMBEREXISTS',
'A member already exists with the same {identifier}',
array('identifier' => Member::singleton()->fieldLabel($identifierField))
),
'required'
);
$valid = false;
}
}
return $valid;
}
// Execute the validators on the extensions
$results = $this->extend('updatePHP', $data, $this->form);
$results[] = $valid;
return min($results);
}
}

158
tests/security/MemberTest.php
View File

@ -678,7 +678,7 @@ class MemberTest extends FunctionalTest {
$newAdminGroup = new Group(array('Title' => 'newadmin'));
$newAdminGroup->write();
Permission::grant($newAdminGroup->ID, 'ADMIN');
// Test staff member can't be added to admin groups
$this->assertFalse($staffMember->inGroup($newAdminGroup));
$staffMember->Groups()->setByIDList(array($newAdminGroup->ID));
@ -842,7 +842,135 @@ class MemberTest extends FunctionalTest {
}
}
public function testCustomMemberValidator() {
public function testMemberValidator()
{
// clear custom requirements for this test
Config::inst()->update('Member_Validator', 'customRequired', null);
$memberA = $this->objFromFixture('Member', 'admin');
$memberB = $this->objFromFixture('Member', 'test');
// create a blank form
$form = new MemberTest_ValidatorForm();
$validator = new Member_Validator();
$validator->setForm($form);
// Simulate creation of a new member via form, but use an existing member identifier
$fail = $validator->php(array(
'FirstName' => 'Test',
'Email' => $memberA->Email
));
$this->assertFalse(
$fail,
'Member_Validator must fail when trying to create new Member with existing Email.'
);
// populate the form with values from another member
$form->loadDataFrom($memberB);
// Assign the validator to an existing member
// (this is basically the same as passing the member ID with the form data)
$validator->setForMember($memberB);
// Simulate update of a member via form and use an existing member Email
$fail = $validator->php(array(
'FirstName' => 'Test',
'Email' => $memberA->Email
));
// Simulate update to a new Email address
$pass1 = $validator->php(array(
'FirstName' => 'Test',
'Email' => 'membervalidatortest@testing.com'
));
// Pass in the same Email address that the member already has. Ensure that case is valid
$pass2 = $validator->php(array(
'FirstName' => 'Test',
'Surname' => 'User',
'Email' => $memberB->Email
));
$this->assertFalse(
$fail,
'Member_Validator must fail when trying to update existing member with existing Email.'
);
$this->assertTrue(
$pass1,
'Member_Validator must pass when Email is updated to a value that\'s not in use.'
);
$this->assertTrue(
$pass2,
'Member_Validator must pass when Member updates his own Email to the already existing value.'
);
}
public function testMemberValidatorWithExtensions()
{
// clear custom requirements for this test
Config::inst()->update('Member_Validator', 'customRequired', null);
// create a blank form
$form = new MemberTest_ValidatorForm();
// Test extensions
Member_Validator::add_extension('MemberTest_MemberValidator_SurnameMustMatchFirstNameExtension');
$validator = new Member_Validator();
$validator->setForm($form);
// This test should fail, since the extension enforces FirstName == Surname
$fail = $validator->php(array(
'FirstName' => 'Test',
'Surname' => 'User',
'Email' => 'test-member-validator-extension@testing.com'
));
$pass = $validator->php(array(
'FirstName' => 'Test',
'Surname' => 'Test',
'Email' => 'test-member-validator-extension@testing.com'
));
$this->assertFalse(
$fail,
'Member_Validator must fail because of added extension.'
);
$this->assertTrue(
$pass,
'Member_Validator must succeed, since it meets all requirements.'
);
// Add another extension that always fails. This ensures that all extensions are considered in the validation
Member_Validator::add_extension('MemberTest_MemberValidator_AlwaysFailsExtension');
$validator = new Member_Validator();
$validator->setForm($form);
// Even though the data is valid, This test should still fail, since one extension always returns false
$fail = $validator->php(array(
'FirstName' => 'Test',
'Surname' => 'Test',
'Email' => 'test-member-validator-extension@testing.com'
));
$this->assertFalse(
$fail,
'Member_Validator must fail because of added extensions.'
);
// Remove added extensions
Member_Validator::remove_extension('MemberTest_MemberValidator_AlwaysFailsExtension');
Member_Validator::remove_extension('MemberTest_MemberValidator_SurnameMustMatchFirstNameExtension');
}
public function testCustomMemberValidator()
{
// clear custom requirements for this test
Config::inst()->update('Member_Validator', 'customRequired', null);
$member = $this->objFromFixture('Member', 'admin');
$form = new MemberTest_ValidatorForm();
@ -861,7 +989,7 @@ class MemberTest extends FunctionalTest {
'Surname' => ''
));
$this->assertTrue($pass, 'Validator requires on FirstName and Email');
$this->assertTrue($pass, 'Validator requires a FirstName and Email');
$this->assertFalse($fail, 'Missing FirstName');
$ext = new MemberTest_ValidatorExtension();
@ -919,6 +1047,30 @@ class MemberTest_ValidatorExtension extends DataExtension implements TestOnly {
}
}
/**
* Extension that adds additional validation criteria
* @package framework
* @subpackage tests
*/
class MemberTest_MemberValidator_SurnameMustMatchFirstNameExtension extends DataExtension implements TestOnly
{
public function updatePHP($data, $form) {
return $data['FirstName'] == $data['Surname'];
}
}
/**
* Extension that adds additional validation criteria
* @package framework
* @subpackage tests
*/
class MemberTest_MemberValidator_AlwaysFailsExtension extends DataExtension implements TestOnly
{
public function updatePHP($data, $form) {
return false;
}
}
/**
* @package framework
* @subpackage tests