Skip to content

Commit 9481126

Browse files
author
dereuromark
committed
Make sure fieldList is updated for PasswordableBehavior
1 parent 86adf81 commit 9481126

File tree

3 files changed

+85
-43
lines changed

3 files changed

+85
-43
lines changed

docs/Behavior/Passwordable.md

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Also capable of:
2929
| maxLength | PWD_MAX_LENGTH | |
3030
| validator | 'default' | |
3131
| customValidation | null | Custom validation rule(s) for the formField on top |
32+
| forceFieldList | false | Force field list to overwrite entity accessibility |
3233

3334
You can either pass those to the behavior at runtime, or globally via Configure and `app.php`:
3435
```php
@@ -112,7 +113,7 @@ class UsersController extends Controller {
112113

113114
if ($this->request->is(['put', 'post'])) {
114115
$options = [
115-
'fieldList' => [ 'pwd', 'pwd_repeat', ... ]
116+
'fieldList' => [...]
116117
];
117118
$user = $this->Users->patchEntity($user, $this->request->data, $options);
118119
if ($this->Users->save($user)) {
@@ -188,3 +189,28 @@ Always allow [a-z], [0-9] and ALL special chars a user can possibly type in.
188189
Regex rules can be useful to assert that the password is strong enough, though. That means, that it contains not just letters/numbers, but
189190
also some special chars. This would be way more secure and useful. But also try to be reasonable here, some developers tend to overreach here,
190191
making it very annoying to set up passwords.
192+
193+
### Field list and Accessibility
194+
The behavior will automatically add the internally needed fields to the `'fieldList'` options array, if you provided one on patching.
195+
So you only need to pass in the other non-password-related fields:
196+
```php
197+
$options = [
198+
'fieldList' => ['id', 'name']
199+
];
200+
$user = $this->Users->patchEntity($user, $this->request->data, $options);
201+
```
202+
203+
If the config `forceFieldList` is set to true, it will even create the fieldList for you on the fly.
204+
Otherwise it will use the entity accessible config to determine if the password can be assigned.
205+
So if you do not want to force it, make sure your entity has those fields not protected:
206+
```php
207+
// Inside the entity
208+
protected $_accessible = [
209+
'*' => false,
210+
'pwd' => true,
211+
...
212+
];
213+
214+
// Or from the outside before patching
215+
$user->accessible('*', false); // Mark all properties as protected
216+
$user->accessible(['pwd', ...], true); // Allow certain fields

src/Model/Behavior/PasswordableBehavior.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ class PasswordableBehavior extends Behavior {
5353
'minLength' => PWD_MIN_LENGTH,
5454
'maxLength' => PWD_MAX_LENGTH,
5555
'validator' => 'default',
56-
'customValidation' => null // Custom validation rule(s) for the formField
56+
'customValidation' => null, // Custom validation rule(s) for the formField
57+
'forceFieldList' => false,
5758
];
5859

5960
/**
@@ -221,6 +222,21 @@ public function beforeMarshal(Event $event, ArrayObject $data, ArrayObject $opti
221222
$formFieldRepeat = $this->_config['formFieldRepeat'];
222223
$formFieldCurrent = $this->_config['formFieldCurrent'];
223224

225+
if (!isset($options['fieldList']) && $this->_config['forceFieldList']) {
226+
$options['fieldList'] = array_keys((array)$data);
227+
}
228+
if (isset($options['fieldList'])) {
229+
if (!in_array($formField, $options['fieldList'])) {
230+
$options['fieldList'][] = $formField;
231+
}
232+
if (!in_array($formFieldRepeat, $options['fieldList'])) {
233+
$options['fieldList'][] = $formFieldRepeat;
234+
}
235+
if (!in_array($formFieldCurrent, $options['fieldList'])) {
236+
$options['fieldList'][] = $formFieldCurrent;
237+
}
238+
}
239+
224240
// Make sure fields are set and validation rules are triggered - prevents tempering of form data
225241
if (!isset($data[$formField])) {
226242
$data[$formField] = '';

tests/TestCase/Model/Behavior/PasswordableBehaviorTest.php

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ class PasswordableBehaviorTest extends TestCase {
1818
'plugin.tools.tools_users', 'plugin.tools.roles',
1919
];
2020

21+
/**
22+
* @var \TestApp\Model\Table\ToolsUsersTable
23+
*/
24+
public $Users;
25+
2126
/**
2227
* SetUp method
2328
*
@@ -391,11 +396,13 @@ public function testValidateCurrent() {
391396
'pwd_repeat' => '123456'
392397
];
393398
$user->accessible('*', false); // Mark all properties as protected
394-
$user->accessible(['id', 'pwd', 'pwd_repeat', 'pwd_current'], true);
395-
$this->Users->patchEntity($user, $data);
396-
// Test whitelist setting - only "password" needs to gets auto-added
397-
$options = ['validate' => true, 'fieldList' => ['id', 'pwd', 'pwd_repeat', 'pwd_current']];
399+
$user->accessible(['id'], true); // Allow id to be accessible by default
400+
$user = $this->Users->patchEntity($user, $data, ['fieldList' => ['id']]);
398401

402+
$this->assertNotSame($is['password'], $user['password']);
403+
$this->assertTrue($user->dirty('pwd'));
404+
405+
$options = ['validate' => true];
399406
$is = $this->Users->save($user, $options);
400407
$this->assertTrue(!empty($is));
401408

@@ -416,11 +423,11 @@ public function testValidateCurrent() {
416423
];
417424
$user->accessible('*', false); // Mark all properties as protected
418425
$user->accessible(['id', 'name'], true);
419-
$this->Users->patchEntity($user, $data);
426+
$this->Users->patchEntity($user, $data, ['fieldList' => ['id', 'name']]);
420427
// Test whitelist setting - only "password" gets auto-added, pwd, pwd_repeat etc need to be added manually
421428
// NOTE that I had to remove the code for adding those fields from the behavior (as it was not functional)
422429
// So of course, this won't work now as expected. But feel free to try to add them in the behavior. Results will be the same.
423-
$options = ['validate' => true, 'fieldList' => ['id', 'name']];
430+
$options = ['validate' => true];
424431
$is = $this->Users->save($user, $options);
425432

426433
// Validation errors triggered - as expected
@@ -429,17 +436,12 @@ public function testValidateCurrent() {
429436
}
430437

431438
/**
432-
* Test cake2.4 passwordHasher feature
433-
*
434439
* @return void
435440
*/
436-
public function testPasswordHasher() {
437-
$this->skipIf((float)Configure::version() < 2.4, 'Needs 2.4 and above');
438-
441+
public function testPatchWithFieldList() {
439442
$this->Users->addBehavior('Tools.Passwordable', [
440443
'formField' => 'pwd',
441444
'formFieldRepeat' => 'pwd_repeat',
442-
'allowSame' => false,
443445
'current' => false,
444446
'passwordHasher' => 'Complex',
445447
]);
@@ -448,42 +450,40 @@ public function testPasswordHasher() {
448450
'pwd' => 'somepwd',
449451
'pwd_repeat' => 'somepwd'
450452
];
451-
$this->Users->patchEntity($user, $data);
453+
$user->accessible('*', false); // Mark all properties as protected
454+
$user->accessible(['id'], true);
455+
$this->Users->patchEntity($user, $data, ['fieldList' => ['id']]);
452456
$result = $this->Users->save($user);
453457
$this->assertTrue((bool)$result);
454-
$userCopy = clone($user);
458+
}
455459

456-
$this->Users->removeBehavior('Passwordable');
457-
$this->Users->addBehavior('Tools.Passwordable', ['current' => true]);
458-
$user = clone($userCopy);
460+
/**
461+
* @return void
462+
*/
463+
public function testPatchWithoutFieldList() {
464+
$this->Users->addBehavior('Tools.Passwordable', [
465+
'formField' => 'pwd',
466+
'formFieldRepeat' => 'pwd_repeat',
467+
'current' => false,
468+
'passwordHasher' => 'Complex',
469+
'forceFieldList' => true
470+
]);
471+
$user = $this->Users->newEntity();
459472
$data = [
460-
'pwd' => '123456',
461-
'pwd_repeat' => '12345678',
473+
'name' => 'x',
474+
'pwd' => 'somepwd',
475+
'pwd_repeat' => 'somepwd'
462476
];
463-
$this->Users->patchEntity($user, $data);
464-
$this->assertTrue($this->Users->behaviors()->has('Passwordable'));
465-
$is = $this->Users->save($user);
466-
$this->assertFalse($is);
477+
$user->accessible('*', false); // Mark all properties as protected
478+
$user->accessible(['id'], true);
479+
$user = $this->Users->patchEntity($user, $data);
480+
$result = $this->Users->save($user);
481+
$this->assertTrue((bool)$result);
467482

468-
$user = clone($userCopy);
469-
$data = [
470-
'pwd_current' => 'somepwdx',
471-
'pwd' => '123456',
472-
'pwd_repeat' => '123456'
473-
];
474-
$this->Users->patchEntity($user, $data);
475-
$is = $this->Users->save($user);
476-
$this->assertFalse($is);
483+
$savedUser = $this->Users->get($user->id);
477484

478-
$user = clone($userCopy);
479-
$data = [
480-
'pwd_current' => 'somepwd',
481-
'pwd' => '123456',
482-
'pwd_repeat' => '123456'
483-
];
484-
$this->Users->patchEntity($user, $data);
485-
$is = $this->Users->save($user);
486-
$this->assertTrue(!empty($is));
485+
$this->assertSame($data['name'], $savedUser->name);
486+
$this->assertSame($user->password, $savedUser->password);
487487
}
488488

489489
/**

0 commit comments

Comments
 (0)