Conversation
Member
Author
|
@pajgo & @belkinadasha - please take a look, as for me - LGTM |
Collaborator
|
@AVVS Restored plugin behavior and added additional checks. |
* skipFieldNames parameter * additional tests
pajgo
force-pushed
the
pajgo-feat/passwordLimits
branch
from
25186e7 to
c5147e3
Compare
bludzhd
reviewed
Sep 30, 2019
| ``` | ||
|
|
||
| ### forceCheckFieldNames __string[]__ | ||
| Allows to force Validation event if the Validator disabled: |
Contributor
There was a problem hiding this comment.
if the Validator is disabled
bludzhd
reviewed
Sep 30, 2019
| const validatorConfig = { | ||
| minStrength: 4, | ||
| skipFieldName: 'validate', // if `data.validate` eq true 'data.myfield' is skipped during validation | ||
| enabled:false, // Will validate the `data` object anyway |
Contributor
There was a problem hiding this comment.
missing space after the semicolon
bludzhd
reviewed
Sep 30, 2019
| minStrength: 4, | ||
| skipFieldName: 'validate', // if `data.validate` eq true 'data.myfield' is skipped during validation | ||
| enabled:false, // Will validate the `data` object anyway | ||
| forceCheckFieldNames: ['forceValidate'], // if `data.validate` not exists 'data.myfield' is skipped during validation |
Contributor
There was a problem hiding this comment.
What does the comment mean?
bludzhd
reviewed
Sep 30, 2019
|
|
||
| // Force validation | ||
| const data = { | ||
| myfield: 'fooBar', // in schema "myfield":{"password": true} keyword. |
Contributor
There was a problem hiding this comment.
I suggest to rename myfield to camelCase everywhere for a neat look
bludzhd
reviewed
Sep 30, 2019
| * Validation skipped if `Object[field]` set. | ||
| * Validation performed if `Object[field]` not exists. | ||
|
|
||
| Eg: |
bludzhd
reviewed
Sep 30, 2019
| forceCheckFieldName: ['checkPassword'], // force enable password to check if the object field value set.. | ||
| inputFieldNames: [ // Linked fields list, allow filter case sensitive data in the password from the parent object. | ||
| skipCheckFieldNames: ['skipPassword'], // Force disable password to check if the object field value exists. | ||
| forceCheckFieldNames: ['checkPassword'], // Force enable password check if the object field value exists. |
Contributor
There was a problem hiding this comment.
How about "forces password check <...>" and "disables password check <...>" (one line above)?
bludzhd
reviewed
Sep 30, 2019
| inputFieldNames: [ // Linked fields list, allow filter case sensitive data in the password from the parent object. | ||
| skipCheckFieldNames: ['skipPassword'], // Force disable password to check if the object field value exists. | ||
| forceCheckFieldNames: ['checkPassword'], // Force enable password check if the object field value exists. | ||
| inputFieldNames: [ // Linked fields list, allow filter the sensitive data in the password from the parent object. |
Contributor
There was a problem hiding this comment.
"allows to filter sensitive data <...>" or "allows sensitive data filtering <...>"
bludzhd
reviewed
Sep 30, 2019
|
|
||
| ## OTP and Empty Password | ||
| The `password` field is not defined as `required` in current `schemas`. | ||
| Validation does not happen because This field does not exist in the OTP registration request. |
Contributor
There was a problem hiding this comment.
Validation is not performed because this field <...>
bludzhd
reviewed
Sep 30, 2019
| ## OTP and Empty Password | ||
| The `password` field is not defined as `required` in current `schemas`. | ||
| Validation does not happen because This field does not exist in the OTP registration request. | ||
| When the Service performs password generation, `skipPassword` property does not exist in the registration request, so validation not executing on generated passwords. |
Contributor
There was a problem hiding this comment.
- double space before
skipPassword - <...> so the validator does not check these generated passwords.
bludzhd
reviewed
Sep 30, 2019
| }); | ||
| }); | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
I think it could be nice to add a test that uses checkPassword field that forces validation so that forceCheckFieldNames feature could have its deserved test coverage :)
* doc typos * additional tests
Collaborator
|
@belkinadasha Fixed. Please re ^) |
bludzhd
approved these changes
Oct 2, 2019
Member
Author
|
🎉 This PR is included in version 11.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
3 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.
No description provided.