Add BCrypt Revision Support

[lin199231]

From issue #3320
I find that jBCrypt are already support
https://code.google.com/archive/p/jbcrypt/issues/9
So I try to add bcrypt revision support for BCryptPasswordEncoder, and compatible with old version.
Please merge my request, thanks.

[rwinch]

Thanks for the PR! I provided feedback inline.

I've also noticed that the checkstyle is failing. You can run checkstyle using ./gradlew checkstyleMain checkstyleTest. I don't mind fixing that up if it is too much trouble for you. However, you are able to clean it up it would be appreciated. Here is what the report states:

File /home/rwinch/code/spring-projects/spring-security/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java

Error Description | Line
-- | --
'if' is not followed by whitespace. | 105

File /home/rwinch/code/spring-projects/spring-security/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java

Error Description | Line
-- | --
'typecast' is not followed by whitespace. | 426
'typecast' is not followed by whitespace. | 426
'typecast' is not followed by whitespace. | 428
Line has leading space characters; indentation should be performed with tabs only. | 440
'typecast' is not followed by whitespace. | 455
'typecast' is not followed by whitespace. | 457
'typecast' is not followed by whitespace. | 463
'typecast' is not followed by whitespace. | 465
'typecast' is not followed by whitespace. | 469
'typecast' is not followed by whitespace. | 471
'typecast' is not followed by whitespace. | 477
'typecast' is not followed by whitespace. | 527
'typecast' is not followed by whitespace. | 565
'typecast' is not followed by whitespace. | 566
Line has leading space characters; indentation should be performed with tabs only. | 690
'typecast' is not followed by whitespace. | 692
'typecast' is not followed by whitespace. | 716
'typecast' is not followed by whitespace. | 717
'typecast' is not followed by whitespace. | 718
'typecast' is not followed by whitespace. | 719
'typecast' is not followed by whitespace. | 754

[rwinch]

Let's change version inputs from the user to be a inner nested enum named BCryptVersion so that an invalid version cannot be provided.

[rwinch]

Remove the println statement

[rwinch]

Rather than having this check and two different code paths, let's make it so that version is final and always non-null with a default of $2y

[lin199231]

Hi, Thanks for the reply, that's very useful advice. I will change it and update later. BR Lin Rob Winch <[email protected]> 于2018年10月18日周四 下午10:39写道：

…

***@***.**** requested changes on this pull request. Thanks for the PR! I provided feedback inline. I've also noticed that the checkstyle is failing. You can run checkstyle using ./gradlew checkstyleMain checkstyleTest. *I don't mind fixing that up if it is too much trouble for you*. However, you are able to clean it up it would be appreciated. Here is what the report states: File /home/rwinch/code/spring-projects/spring-security/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java Error Description | Line -- | -- 'if' is not followed by whitespace. | 105 File /home/rwinch/code/spring-projects/spring-security/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java Error Description | Line -- | -- 'typecast' is not followed by whitespace. | 426 'typecast' is not followed by whitespace. | 426 'typecast' is not followed by whitespace. | 428 Line has leading space characters; indentation should be performed with tabs only. | 440 'typecast' is not followed by whitespace. | 455 'typecast' is not followed by whitespace. | 457 'typecast' is not followed by whitespace. | 463 'typecast' is not followed by whitespace. | 465 'typecast' is not followed by whitespace. | 469 'typecast' is not followed by whitespace. | 471 'typecast' is not followed by whitespace. | 477 'typecast' is not followed by whitespace. | 527 'typecast' is not followed by whitespace. | 565 'typecast' is not followed by whitespace. | 566 Line has leading space characters; indentation should be performed with tabs only. | 690 'typecast' is not followed by whitespace. | 692 'typecast' is not followed by whitespace. | 716 'typecast' is not followed by whitespace. | 717 'typecast' is not followed by whitespace. | 718 'typecast' is not followed by whitespace. | 719 'typecast' is not followed by whitespace. | 754 ------------------------------ In crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java <#5992 (comment)> : > @@ -51,32 +53,77 @@ public BCryptPasswordEncoder(int strength) { this(strength, null); } + /** + * @param version the version of bcrypt, can be 2a,2b,2y + */ + public BCryptPasswordEncoder(String version) { Let's change version inputs from the user to be a inner nested enum named BCryptVersion so that an invalid version cannot be provided. ------------------------------ In crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java <#5992 (comment)> : > this.strength = strength; this.random = random; } public String encode(CharSequence rawPassword) { String salt; - if (strength > 0) { - if (random != null) { - salt = BCrypt.gensalt(strength, random); + if(version != null) { + if (strength > 0) { + if (random != null) { + salt = BCrypt.gensalt(version, strength, random); + } else { + System.out.println(version); Remove the println statement ------------------------------ In crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java <#5992 (comment)> : > this.strength = strength; this.random = random; } public String encode(CharSequence rawPassword) { String salt; - if (strength > 0) { - if (random != null) { - salt = BCrypt.gensalt(strength, random); + if(version != null) { Rather than having this check and two different code paths, let's make it so that version is final and always non-null with a default of $2y ------------------------------ In crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java <#5992 (comment)> : > @@ -33,6 +33,14 @@ public void matches() { String result = encoder.encode("password"); Generally with these tests please split up these tests so that there is a different test for each method ------------------------------ In crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java <#5992 (comment)> : > @@ -13,252 +13,363 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. package org.springframework.security.crypto.bcrypt; -import java.io.ByteArrayOutputStream; Please remove unnecessary formatting changes in here so it is easier to follow what has changed. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5992 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEIV4j0lpVQxEGafzHcgD0xSoo8Nuefzks5umJKEgaJpZM4Xs4D6> .

[lin199231]

I have a question, if use $2y as default version. The old version of Spring Security's default version is $2a. Is this OK to change BCryptPasswordEncoder 's default version from $2a to $2y?

[rwinch]

@lin199231 Thanks for pointing that out. Please use $2a as the default version.

[lin199231]

OK，the default version is $2a now.

[rwinch]

Thanks for the updates @lin199231! The feature is merged into master and will be available in 5.2.0.M1 Note I applied a polish commit to fix the last checkstyle error f56f55d