-
Notifications
You must be signed in to change notification settings - Fork 348
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: magiclink failing due to passwordStrength check #1769
base: master
Are you sure you want to change the base?
Changes from all commits
331e37c
1ee831f
943cba6
655c5bd
db57285
07a44bc
fb1a58c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,65 @@ | ||||||||||||||||||||||
package utilities | ||||||||||||||||||||||
|
||||||||||||||||||||||
import ( | ||||||||||||||||||||||
"crypto/rand" | ||||||||||||||||||||||
"math/big" | ||||||||||||||||||||||
"strings" | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
// parseGroups processes the required character groups from a slice of strings. | ||||||||||||||||||||||
func parseGroups(requiredChars []string) []string { | ||||||||||||||||||||||
var groups []string | ||||||||||||||||||||||
groups = append(groups, requiredChars...) | ||||||||||||||||||||||
return groups | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
func GeneratePassword(requiredChars []string, length int) (string, error) { | ||||||||||||||||||||||
groups := parseGroups(requiredChars) | ||||||||||||||||||||||
Comment on lines
+9
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can reference the parameter directly since we don't mutate the slice.
Suggested change
|
||||||||||||||||||||||
passwordBuilder := strings.Builder{} | ||||||||||||||||||||||
passwordBuilder.Grow(length) | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Add required characters | ||||||||||||||||||||||
for _, group := range groups { | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
if len(group) > 0 { | ||||||||||||||||||||||
randomIndex, err := secureRandomInt(len(group)) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
return "", err | ||||||||||||||||||||||
} | ||||||||||||||||||||||
passwordBuilder.WriteByte(group[randomIndex]) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Define a default character set for random generation (if needed) | ||||||||||||||||||||||
allChars := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
// Fill the rest of the password | ||||||||||||||||||||||
for passwordBuilder.Len() < length { | ||||||||||||||||||||||
randomIndex, err := secureRandomInt(len(allChars)) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
return "", err | ||||||||||||||||||||||
} | ||||||||||||||||||||||
passwordBuilder.WriteByte(allChars[randomIndex]) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Convert to byte slice for shuffling | ||||||||||||||||||||||
passwordBytes := []byte(passwordBuilder.String()) | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Secure shuffling | ||||||||||||||||||||||
for i := len(passwordBytes) - 1; i > 0; i-- { | ||||||||||||||||||||||
j, err := secureRandomInt(i + 1) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
return "", err | ||||||||||||||||||||||
} | ||||||||||||||||||||||
passwordBytes[i], passwordBytes[j] = passwordBytes[j], passwordBytes[i] | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
return string(passwordBytes), nil | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
func secureRandomInt(max int) (int, error) { | ||||||||||||||||||||||
randomInt, err := rand.Int(rand.Reader, big.NewInt(int64(max))) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
return 0, err | ||||||||||||||||||||||
} | ||||||||||||||||||||||
return int(randomInt.Int64()), nil | ||||||||||||||||||||||
} |
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.
If we can move this into
internal/crypto/password.go
I think the change will look good to me. A unit test would be a nice addition if time permits.