Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify password element #82

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions src/Contract/ValueCandidates.php

This file was deleted.

3 changes: 1 addition & 2 deletions src/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
class Form extends BaseHtmlElement
{
use FormElements {
FormElements::remove as private removeElement;
remove as private removeElement;
}
use Messages;

Expand All @@ -21,7 +21,6 @@ class Form extends BaseHtmlElement
public const ON_REQUEST = 'request';
public const ON_SUCCESS = 'success';
public const ON_SENT = 'sent';
public const ON_VALIDATE = 'validate';

/** @var string Form submission URL */
protected $action;
Expand Down
18 changes: 1 addition & 17 deletions src/FormElement/BaseFormElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
use ipl\Html\Attributes;
use ipl\Html\BaseHtmlElement;
use ipl\Html\Contract\FormElement;
use ipl\Html\Contract\ValueCandidates;
use ipl\Html\Form;
use ipl\Stdlib\Messages;
use ipl\Validator\ValidatorChain;
use ReflectionProperty;

abstract class BaseFormElement extends BaseHtmlElement implements FormElement, ValueCandidates
abstract class BaseFormElement extends BaseHtmlElement implements FormElement
{
use Messages;

Expand All @@ -37,9 +36,6 @@ abstract class BaseFormElement extends BaseHtmlElement implements FormElement, V
/** @var mixed Value of the element */
protected $value;

/** @var array Value candidates of the element */
protected $valueCandidates = [];

/**
* @deprecated STOP: Do not use this property!!
*
Expand Down Expand Up @@ -255,18 +251,6 @@ public function setValue($value)
return $this;
}

public function getValueCandidates()
{
return $this->valueCandidates;
}

public function setValueCandidates(array $values)
{
$this->valueCandidates = $values;

return $this;
}

public function onRegistered(Form $form)
{
}
Expand Down
14 changes: 3 additions & 11 deletions src/FormElement/FormElements.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use InvalidArgumentException;
use ipl\Html\Contract\FormElement;
use ipl\Html\Contract\FormElementDecorator;
use ipl\Html\Contract\ValueCandidates;
use ipl\Html\Form;
use ipl\Html\FormDecorator\DecoratorInterface;
use ipl\Html\ValidHtml;
Expand Down Expand Up @@ -194,11 +193,7 @@ public function registerElement(FormElement $element)
$this->elements[$name] = $element;

if (array_key_exists($name, $this->populatedValues)) {
$element->setValue($this->populatedValues[$name][count($this->populatedValues[$name]) - 1]);

if ($element instanceof ValueCandidates) {
$element->setValueCandidates($this->populatedValues[$name]);
}
$element->setValue($this->populatedValues[$name]);
}

$this->onElementRegistered($element);
Expand Down Expand Up @@ -316,7 +311,7 @@ public function getValues()
public function populate($values)
{
foreach ($values as $name => $value) {
$this->populatedValues[$name][] = $value;
$this->populatedValues[$name] = $value;
if ($this->hasElement($name)) {
$this->getElement($name)->setValue($value);
}
Expand All @@ -338,7 +333,7 @@ public function populate($values)
public function getPopulatedValue($name, $default = null)
{
return isset($this->populatedValues[$name])
? $this->populatedValues[$name][count($this->populatedValues[$name]) - 1]
? $this->populatedValues[$name]
: $default;
}

Expand Down Expand Up @@ -481,8 +476,6 @@ public function isValid()
{
if ($this->isValid === null) {
$this->validate();

$this->emit(Form::ON_VALIDATE, [$this]);
}

return $this->isValid;
Expand Down Expand Up @@ -539,7 +532,6 @@ public function isValidEvent($event)
Form::ON_SENT,
Form::ON_ERROR,
Form::ON_REQUEST,
Form::ON_VALIDATE,
Form::ON_ELEMENT_REGISTERED,
]);
}
Expand Down
54 changes: 17 additions & 37 deletions src/FormElement/PasswordElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,34 @@

namespace ipl\Html\FormElement;

use ipl\Html\Attributes;
use ipl\Html\Form;

class PasswordElement extends InputElement
{
/** @var string Dummy passwd of this element to be rendered */
public const DUMMYPASSWORD = '_ipl_form_5847ed1b5b8ca';

protected $type = 'password';
protected const OBSCURE_PASSWORD = '_ipl_form_5847ed1b5b8ca';

/** @var bool Status of the form */
protected $isFormValid = true;
protected $password;

protected function registerAttributeCallbacks(Attributes $attributes)
{
parent::registerAttributeCallbacks($attributes);

$attributes->registerAttributeCallback(
'value',
function () {
if ($this->hasValue() && count($this->getValueCandidates()) === 1 && $this->isFormValid) {
return self::DUMMYPASSWORD;
}

if (parent::getValue() === self::DUMMYPASSWORD) {
return self::DUMMYPASSWORD;
}

return null;
}
);
}
protected $type = 'password';

public function onRegistered(Form $form)
public function getValue()
{
$form->on(Form::ON_VALIDATE, function ($form) {
$this->isFormValid = $form->isValid();
});
return $this->password;
}

public function getValue()
public function setValue($value)
{
parent::setValue($value);
// Consider any changes to the password made by the parent setValue() call.
$value = parent::getValue();
$candidates = $this->getValueCandidates();
while ($value === self::DUMMYPASSWORD) {
$value = array_pop($candidates);

if ($value !== static::OBSCURE_PASSWORD) {
$this->password = $value;
}

return $value;
return $this;
}

public function getValueAttribute()
{
return $this->hasValue() ? static::OBSCURE_PASSWORD : null;
}
}
157 changes: 157 additions & 0 deletions tests/FormElement/PasswordElementTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
<?php

namespace ipl\Tests\Html;

use ipl\Html\Form;
use ipl\Html\FormElement\PasswordElement;
use ipl\Tests\Html\Lib\ElementWithAutosubmitInFieldsetForm;
use ipl\Tests\Html\TestDummy\ObscurePassword;

class PasswordElementTest extends TestCase
{
public function testPasswordValueRenderedObscured()
{
$form = (new Form())
->populate(['password' => 'secret'])
->addElement('password', 'password');

$html = <<<'HTML'
<form method="POST">
<input type="password" name="password" value="%s">
</form>
HTML;
$this->assertHtml(sprintf($html, ObscurePassword::get()), $form);
}

public function testGetValueReturnsNullIfNoPasswordSet()
{
$password = new PasswordElement('password');
$this->assertNull($password->getValue());
$this->assertFalse($password->hasValue());
}

public function testGetValueReturnsPassword()
{
$form = (new Form())
->populate(['password' => 'secret'])
->addElement('password', 'password');

$password = $form->getElement('password');
$this->assertEquals($password->getValue(), 'secret');
$this->assertTrue($password->hasValue());
}

public function testGetValueReturnsNewPassword()
{
$form = (new Form())
->populate(['password' => 'secret'])
->populate(['password' => 'topsecret'])
->addElement('password', 'password');

$password = $form->getElement('password');
$this->assertEquals($password->getValue(), 'topsecret');
$this->assertTrue($password->hasValue());
}

public function testGetValueReturnsNullIfPasswordReset()
{
$form = (new Form())
->populate(['password' => 'secret'])
->populate(['password' => ''])
->addElement('password', 'password');

$password = $form->getElement('password');
$this->assertNull($password->getValue());
$this->assertFalse($password->hasValue());
}

public function testGetValueReturnsNullIfNotChanged()
{
$form = (new Form())
->populate(['password' => ObscurePassword::get()])
->addElement('password', 'password');

$password = $form->getElement('password');
$this->assertNull($password->getValue());
}

/**
* Represents a form that requires a password to be set as confirmation.
* If another element is invalid, the password element should have no
* value, as otherwise the user thinks the password is still set, although
* it's the obscured password.
*
* @return void
*/
public function testObscuredValueNotVisibleAfterFormValidationFailed()
{
$form = (new Form())
->populate(['password' => 'secret'])
->addElement('password', 'password')
->addElement('text', 'username', ['required' => true]);

$this->assertFalse($form->isValid());

$html = <<<'HTML'
<form method="POST">
<input type="password" name="password">
<input name="username" required type="text"/>
</form>
HTML;
$this->assertHtml($html, $form);
}

/**
* Represents a controller action that populates saved data and
* allows the user to change it. If the password isn't changed,
* the saved password must be preserved.
*
* @return void
*/
public function testOriginalPasswordMustBePreserved()
{
$form = (new Form());

// The action populates always first
$form->populate(['password' => 'secret']);

// handleRequest() then another time
$form->populate(['password' => ObscurePassword::get()]);

// assemble() then registers the element
$form->addElement('password', 'password');

$password = $form->getElement('password');
$this->assertEquals('secret', $password->getValue());
}

/**
* Represents a controller action that populates saved data and
* allows the user to change it. If the password isn't changed,
* the password must persist when the value of an element
* with autosubmit class is changed.
*
* @return void
*/
public function testOriginalPasswordMustPersistOnAutoSubmit()
{
$form = (new ElementWithAutosubmitInFieldsetForm());

// The action populates always first
$form->populate([
'foo1' => ['select' => 'option1'],
'foo2' => ['password' => 'secret']
]);

// handleRequest() then another time
$form->populate([
'foo1' => ['select' => 'option2'],
'foo2' => ['password' => ObscurePassword::get()]
]);

$form->ensureAssembled();

$password = $form->getElement('foo2')->getValue('password');
$this->assertEquals('secret', $password);
}
}
Loading