Feature: Hash password for security & migrates legacy user passwords!#25
Open
PavanCodes05 wants to merge 3 commits intoEvolutionary-Algorithms-On-Click:mainfrom
Open
Feature: Hash password for security & migrates legacy user passwords!#25PavanCodes05 wants to merge 3 commits intoEvolutionary-Algorithms-On-Click:mainfrom
PavanCodes05 wants to merge 3 commits intoEvolutionary-Algorithms-On-Click:mainfrom
Conversation
- Import bcrpyt lib - Hash password before inserting to db - Insert hashed password to db
- Import bcrypt lib - Get user and password hash - Verify password with hash
- Added bcrypt hash detection via helper - Implemented secure password comparison and rehashing for legacy users
Ashrockzzz2003
requested changes
Oct 31, 2025
Comment on lines
+87
to
+100
| } else { | ||
| // Compare plain text for legacy users | ||
| if storedPasswordHash != l.Password { | ||
| logger.Info("Login: invalid password attempt (legacy)") | ||
| return nil, fmt.Errorf("invalid username/email or password") | ||
| } | ||
|
|
||
| // Rehash and update DB for this user | ||
| newHash, err := bcrypt.GenerateFromPassword([]byte(l.Password), bcrypt.DefaultCost) | ||
| if err == nil { | ||
| _, _ = db.Exec(ctx, "UPDATE users SET password = $1 WHERE id = $2", string(newHash), id) | ||
| logger.Info(fmt.Sprintf("Upgraded password hash for user %v", id)) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
it's great that you thought about this. But a better way is where you write a migration script and add it as part of the initialization of the service.
In that migration script you migrate all passwords to the hashed form. Your approach fails in cases where users have set actual passwords starting with $2b ... etc
Maintain a new column that says legacy/v2, maybe version numbers for each record in the user table. default value is v1, new passwords gets v2?
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
2 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.
Enhanced Login Flow with Secure Bcrypt Handling
Closes #21
Summary
This PR enhances the authentication logic by introducing bcrypt-based password hashing while maintaining backward compatibility for legacy users who still have plain-text passwords.
Key Changes
isBcryptHashto detect bcrypt-hashed passwords.bcrypt.CompareHashAndPassword.Why This Change
Testing
Impact
Migration Note
It’s recommended to back up the
userstable before deployment.Legacy user passwords will be upgraded to bcrypt hashes automatically after successful logins.