Skip to content

Commit 7a5411f

Browse files
klajdi369cstockton
andauthored
fix: magiclink failing due to passwordStrength check (supabase#1769)
## What kind of change does this PR introduce? Bug fix ## What is the current behavior? supabase#1761 ## What is the new behavior? Now the password should generate secure enough with the necessary password requirements specified in environment variables. ## Additional context Basically this line in /internal/api/magic_link.go password.Generate(64, 10, 1, false, true) Generates an invalid value for this line in /internal/api/signup.go if err := a.checkPasswordStrength(ctx, p.Password); err != nil { --------- Co-authored-by: Chris Stockton <[email protected]>
1 parent 5b03fda commit 7a5411f

File tree

4 files changed

+118
-2
lines changed

4 files changed

+118
-2
lines changed

internal/api/magic_link.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"net/http"
99
"strings"
1010

11-
"github.com/sethvargo/go-password/password"
11+
"github.com/supabase/auth/internal/crypto"
1212
"github.com/supabase/auth/internal/models"
1313
"github.com/supabase/auth/internal/storage"
1414
)
@@ -83,7 +83,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error {
8383
if isNewUser {
8484
// User either doesn't exist or hasn't completed the signup process.
8585
// Sign them up with temporary password.
86-
password, err := password.Generate(64, 10, 1, false, true)
86+
password, err := crypto.GeneratePassword(config.Password.RequiredCharacters, 33)
8787
if err != nil {
8888
return internalServerError("error creating user").WithInternalError(err)
8989
}

internal/crypto/crypto.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@ func GenerateTokenHash(emailOrPhone, otp string) string {
5151
return fmt.Sprintf("%x", sha256.Sum224([]byte(emailOrPhone+otp)))
5252
}
5353

54+
// Generated a random secure integer from [0, max[
55+
func secureRandomInt(max int) (int, error) {
56+
randomInt, err := rand.Int(rand.Reader, big.NewInt(int64(max)))
57+
if err != nil {
58+
return 0, errors.WithMessage(err, "Error generating random integer")
59+
}
60+
return int(randomInt.Int64()), nil
61+
}
62+
5463
func GenerateSignatures(secrets []string, msgID uuid.UUID, currentTime time.Time, inputPayload []byte) ([]string, error) {
5564
SymmetricSignaturePrefix := "v1,"
5665
// TODO(joel): Handle asymmetric case once library has been upgraded

internal/crypto/password.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,3 +234,45 @@ func GenerateFromPassword(ctx context.Context, password string) (string, error)
234234

235235
return string(hash), nil
236236
}
237+
238+
func GeneratePassword(requiredChars []string, length int) (string, error) {
239+
passwordBuilder := strings.Builder{}
240+
passwordBuilder.Grow(length)
241+
242+
// Add required characters
243+
for _, group := range requiredChars {
244+
if len(group) > 0 {
245+
randomIndex, err := secureRandomInt(len(group))
246+
if err != nil {
247+
return "", err
248+
}
249+
passwordBuilder.WriteByte(group[randomIndex])
250+
}
251+
}
252+
253+
// Define a default character set for random generation (if needed)
254+
const allChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
255+
256+
// Fill the rest of the password
257+
for passwordBuilder.Len() < length {
258+
randomIndex, err := secureRandomInt(len(allChars))
259+
if err != nil {
260+
return "", err
261+
}
262+
passwordBuilder.WriteByte(allChars[randomIndex])
263+
}
264+
265+
// Convert to byte slice for shuffling
266+
passwordBytes := []byte(passwordBuilder.String())
267+
268+
// Secure shuffling
269+
for i := len(passwordBytes) - 1; i > 0; i-- {
270+
j, err := secureRandomInt(i + 1)
271+
if err != nil {
272+
return "", err
273+
}
274+
passwordBytes[i], passwordBytes[j] = passwordBytes[j], passwordBytes[i]
275+
}
276+
277+
return string(passwordBytes), nil
278+
}

internal/crypto/password_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package crypto
22

33
import (
44
"context"
5+
"strings"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
@@ -19,3 +20,67 @@ func TestArgon2(t *testing.T) {
1920
assert.NoError(t, CompareHashAndPassword(context.Background(), example, "test"))
2021
}
2122
}
23+
24+
func TestGeneratePassword(t *testing.T) {
25+
tests := []struct {
26+
name string
27+
requiredChars []string
28+
length int
29+
wantErr bool
30+
}{
31+
{
32+
name: "Valid password generation",
33+
requiredChars: []string{"ABC", "123", "@#$"},
34+
length: 12,
35+
wantErr: false,
36+
},
37+
{
38+
name: "Empty required chars",
39+
requiredChars: []string{},
40+
length: 8,
41+
wantErr: false,
42+
},
43+
}
44+
45+
for _, tt := range tests {
46+
t.Run(tt.name, func(t *testing.T) {
47+
got, err := GeneratePassword(tt.requiredChars, tt.length)
48+
49+
if (err != nil) != tt.wantErr {
50+
t.Errorf("GeneratePassword() error = %v, wantErr %v", err, tt.wantErr)
51+
return
52+
}
53+
54+
if len(got) != tt.length {
55+
t.Errorf("GeneratePassword() returned password of length %d, want %d", len(got), tt.length)
56+
}
57+
58+
// Check if all required characters are present
59+
for _, chars := range tt.requiredChars {
60+
found := false
61+
for _, c := range got {
62+
if strings.ContainsRune(chars, c) {
63+
found = true
64+
break
65+
}
66+
}
67+
if !found && len(chars) > 0 {
68+
t.Errorf("GeneratePassword() missing required character from set %s", chars)
69+
}
70+
}
71+
})
72+
}
73+
74+
// Check for duplicates passwords
75+
passwords := make(map[string]bool)
76+
for i := 0; i < 30; i++ {
77+
p, err := GeneratePassword([]string{"ABC", "123", "@#$"}, 30)
78+
if err != nil {
79+
t.Errorf("GeneratePassword() unexpected error: %v", err)
80+
}
81+
if passwords[p] {
82+
t.Errorf("GeneratePassword() generated duplicate password: %s", p)
83+
}
84+
passwords[p] = true
85+
}
86+
}

0 commit comments

Comments
 (0)