imbajin commented on code in PR #2918:
URL:
https://github.com/apache/incubator-hugegraph/pull/2918#discussion_r2618207747
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/StringEncoding.java:
##########
@@ -140,7 +140,7 @@ public static String decompress(byte[] value, float
bufferRatio) {
}
public static String hashPassword(String password) {
- return BCrypt.hashpw(password, BCrypt.gensalt(4));
+ return BCrypt.hashpw(password, BCrypt.gensalt(12));
Review Comment:
> ⚠️ **Important: BCrypt Work Factor Change Needs Migration Strategy**
>
> Increasing BCrypt rounds from 4 to 12 is a good security improvement, but
this will break authentication for all existing users whose passwords were
hashed with the old work factor (4 rounds).
>
> **Impact:** Existing users won't be able to log in after this change.
>
> **Recommendation:** Implement a gradual migration strategy:
>
> 1. Keep verifying old hashes (4 rounds) in `checkPassword()`
> 2. Re-hash passwords with 12 rounds on successful login
> 3. Consider adding a configuration option for the work factor
>
> Example approach:
>
> ```java
> public static String hashPassword(String password) {
> // New hashes use 12 rounds
> return BCrypt.hashpw(password, BCrypt.gensalt(12));
> }
>
> public static boolean checkPassword(String candidatePassword, String
dbPassword) {
> boolean valid = BCrypt.checkpw(candidatePassword, dbPassword);
> // TODO: Consider re-hashing with new work factor on successful login
> return valid;
> }
> ```
Also, we **lack tests** here to avoid any data `breaking change` in our
system
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
