New password element#201
Open
TheSyscall wants to merge 6 commits into
Open
Conversation
Although BaseFormElement implements ValueCandidates for every element, PasswordElement is its only consumer, and the upcoming password rework no longer needs it. Mark the interface deprecated before removing the implementation so downstream code gets a heads-up. No replacement is provided.
Add hasBeenValidated(), a pure check of whether $isValid has been set, so callers can peek at an already-known validity without ever triggering it.
When an element is registered after values have been populated for its name, replay every accumulated value through setValue() in order instead of applying only the last one and stashing the full list via the removed ValueCandidates interface. Elements that only care about their final value are unaffected, as the last setValue() call wins. The reworked PasswordElement, in turn, can observe the seeded value and the request's own value as distinct assignments.
Replace the ValueCandidates/event-listener based tracking with an element that manages its own state directly, now that setValue() is replayed with every populated value in order (see the preceding two commits): - Preserve the real value across DUMMYPASSWORD resubmissions instead of losing it in an array of candidates. - Track the first value received as a baseline to tell an actual change apart from a resubmission of the unchanged placeholder. - Skip validators entirely for the dummy placeholder; it isn't a real password, so a policy validator must not run against it. - Clear the field (rather than mask it) when a changed value fails validation on an actual submission, but keep echoing it back plainly during partial/autosubmit rounds so live typing isn't lost. - Only mask when there's an actual pre-existing secret seeded before the current request's own value (see $assignments); a plain form's password field (e.g. a login form) never has one and is now treated like an ordinary input instead of appearing pre-filled after submission.
Stop implementing the deprecated ValueCandidates interface in BaseFormElement and drop the $valueCandidates property along with its getter and setter. Nothing but PasswordElement ever relied on it, and that element is being reworked to track its own state. The interface itself is retained so any external implementers keep working.
FormElements::registerElement() now replays every populated value through setValue(). FileElement::setValue() moves the uploaded file to its destination, so a name that was populated more than once before the element is registered caused the same upload to be moved twice, throwing "Cannot retrieve stream after it has already been moved". Reuse the already-stored file when it is present on disk instead of moving the (now consumed) upload again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PasswordElementpreviously tracked its state through theValueCandidatesinterface plus form event listeners (ON_VALIDATE/ON_SENT).That indirection was fragile and
PasswordElementwas its only user. This PR replaces it with an element that manages its own state directly, and adjusts the surrounding infrastructure to support that.ValueCandidates. OnlyPasswordElementever used it, even thoughBaseFormElementimplemented it for every element. The interface is kept (deprecated, no replacement) so external implementers keep working.ValueCandidatesimplementation fromBaseFormElement.FormElements::registerElement()now replays every accumulated populated value throughsetValue()in order, instead of applying only the last value and stashing the rest as candidates. Elements that only care about their final value are unaffected (last write wins).PasswordElementuses the ordering to tell a seeded secret apart from the request's own value.Form::hasBeenValidated(): A pure check of whether validity has already been computed, so callers can peek without triggering validation (which would otherwise risk re-entrance).Note
Much of the complexity of this PR comes from the fact that we have no reliable way to tell the following three cases apart:
closes #82