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
fix(users): add password validations #4555
Conversation
let mut rng = rand::thread_rng(); | ||
|
||
let special_chars: Vec<char> = "!@#$%^&*()-_=+[]{}|;:,.<>?".chars().collect(); | ||
let specialchars: char = *special_chars.choose(&mut rng).unwrap_or(&'@'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name special_char
, its a single char
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to specialchars
to special_char
} | ||
} | ||
|
||
pub fn new_without_validation(password: Secret<String>) -> UserResult<Self> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be new_password_without_validation
,
we are not validating the password in merchant create request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're retrieving user details from the database to create a new merchant. It's possible that some old users didn't followed the password convention. Therefore, we've implemented a workaround specifically for this scenario to bypass validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The password must be between 8 and 50 characters in length
Please change description, as per code the maximum length allowed is 70
Also if possible swap description and motivation and context content.
let mut rng = rand::thread_rng(); | ||
|
||
let special_chars: Vec<char> = "!@#$%^&*()-_=+[]{}|;:,.<>?".chars().collect(); | ||
let special_char: char = *special_chars.choose(&mut rng).unwrap_or(&'@'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
special_char
can be a &char
, lets not force it to be owned type.
let special_char: char = *special_chars.choose(&mut rng).unwrap_or(&'@'); | |
let special_char = special_chars.choose(&mut rng).unwrap_or(&'@'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed
@@ -38,3 +39,20 @@ pub fn is_correct_password( | |||
} | |||
.change_context(UserErrors::InternalServerError) | |||
} | |||
|
|||
pub fn get_temp_password() -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets make return type as Secret<String>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Type of Change
Description
The mandatory condition for the password field was just not be empty . Following the PR, the password field will be subject to the following checks:
1.The password must be between 8 and 70 characters in length.
2.It must include at least one uppercase character.
3.It must include at least one lowercase character.
4.It must include at least one special character.
5.It must include at least one numeric character.
6.It must not contain whitespace.
Additional Changes
Motivation and Context
Closes #4412
How did you test it?
1.Signup API ( local testing)
Failure case response
2.Reset Password API (local testing)
Failure case response
Checklist
cargo +nightly fmt --all
cargo clippy