Stronger Passwords #590
Stronger Passwords #590
Conversation
jleandroperez
changed the base branch from
develop
to
issue/updating-auth-interface
| @property (nonatomic, assign) NSUInteger legacyMinimumPasswordLength; | ||
|
|
||
| - (BOOL)validateUsername:(NSString *)username | ||
| error:(NSError **)error; |
There was a problem hiding this comment.
It's a small thing, but I'm diggin' the use of pointer references :)
There was a problem hiding this comment.
Old school! Now you can just return a... touple?
| - (BOOL)validatePasswordWithUsername:(NSString *)username password:(NSString *)password error:(NSError **)error { | ||
| if (password.length < self.strongMinimumPasswordLength) { | ||
| if (error) { | ||
| NSString *description = NSLocalizedString(@"Password must contain at least %d characters", comment: @"Message displayed when password is too short. Please preserve the Percent D!"); |
There was a problem hiding this comment.
Please preserve the Percent D!
I like it, but would it be more clear to our polyglots to say something like The "%d" is a placeholder for a numeral. Please preserve the Percent D!
There was a problem hiding this comment.
Done, thank you!!
|
|
||
| - (BOOL)mustPerformPasswordResetWithUsername:(NSString *)username password:(NSString *)password { | ||
| BOOL isValidUsername = [self validateUsername:username error:nil]; | ||
| BOOL isValidLegacyPassword = (password.length >= self.legacyMinimumPasswordLength); |
There was a problem hiding this comment.
Do we need this check because legacy passwords might contain tabs or new lines? Not sure there is any other need for it so I wanted to double check.
There was a problem hiding this comment.
Actually the logic behind this one is:
A. Is the password now considered unsafe?
B. Was the password then considered safe?
C. Is the password valid (does the backend let us thru?)
In that case, we'd want to enter the PW Reset flow. I'll add a comment to document this, thanks for asking!!
There was a problem hiding this comment.
Hmm... Well... the check for isValidLegacyPassword is just checking the length. So if the length is too short, then its also going to be too short when checking isValidStrongPassword, so if isValidLegacyPassword is false then isValidStrongPassword will be false, so in this scenario the check is unnecessary.
On the other hand, if isValidLegacyPassword is true, isValidStrongPassword can still be false.
I guess my confusion about this check is that there never seems to be a scenario where we get:
isValidUsername = true
isValidLegacyPassword = false
isValidStrongPassword = true
which would be why we'd want to have that check.
What am I missing?
There was a problem hiding this comment.
OH!!!! You're absolutely right. That scenario is not possible, yet, isValidStrongPassword is calculated anyways (and will be false).
I'll prettify the checks (I was aiming a bit for compact), but you're 100% on point.
Thank yoouu!!
| NSString *username = @"something@here.com"; | ||
| NSString *password = @"1234568"; | ||
|
|
||
| XCTAssertTrue([self.validator mustPerformPasswordResetWithUsername:username password:password]); |
There was a problem hiding this comment.
Should this be XCTAssertFalse (along with adding a 7 to the password above)?
There was a problem hiding this comment.
The only correct thing about this unit test... was the method description. Everything is good now, thank you!!!
There was a problem hiding this comment.
I mean... the email was right too! :D
|
@aerych Thanks A LOT for your time!!! |
!
|
Thank you sir!! |
Details:
In this PR we're enhancing the Password Strength requirements:
newline/tabcharactersIncludes Unit Tests and 🔋
@aerych B side of Simplenote PR 550!. Actually, B Side "Mark 2"?
Thank you!!!!
Testing:
Testing Steps outlined here