Skip to content

Commit

Permalink
fix: magiclink failing due to passwordStrength check (#1769)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?

Bug fix

## What is the current behavior?

#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]>
  • Loading branch information
klajdi369 and cstockton committed Sep 24, 2024
1 parent 5b03fda commit 7a5411f
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 2 deletions.
4 changes: 2 additions & 2 deletions internal/api/magic_link.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"net/http"
"strings"

"github.com/sethvargo/go-password/password"
"github.com/supabase/auth/internal/crypto"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/storage"
)
Expand Down Expand Up @@ -83,7 +83,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error {
if isNewUser {
// User either doesn't exist or hasn't completed the signup process.
// Sign them up with temporary password.
password, err := password.Generate(64, 10, 1, false, true)
password, err := crypto.GeneratePassword(config.Password.RequiredCharacters, 33)
if err != nil {
return internalServerError("error creating user").WithInternalError(err)
}
Expand Down
9 changes: 9 additions & 0 deletions internal/crypto/crypto.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ func GenerateTokenHash(emailOrPhone, otp string) string {
return fmt.Sprintf("%x", sha256.Sum224([]byte(emailOrPhone+otp)))
}

// Generated a random secure integer from [0, max[
func secureRandomInt(max int) (int, error) {
randomInt, err := rand.Int(rand.Reader, big.NewInt(int64(max)))
if err != nil {
return 0, errors.WithMessage(err, "Error generating random integer")
}
return int(randomInt.Int64()), nil
}

func GenerateSignatures(secrets []string, msgID uuid.UUID, currentTime time.Time, inputPayload []byte) ([]string, error) {
SymmetricSignaturePrefix := "v1,"
// TODO(joel): Handle asymmetric case once library has been upgraded
Expand Down
42 changes: 42 additions & 0 deletions internal/crypto/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,45 @@ func GenerateFromPassword(ctx context.Context, password string) (string, error)

return string(hash), nil
}

func GeneratePassword(requiredChars []string, length int) (string, error) {
passwordBuilder := strings.Builder{}
passwordBuilder.Grow(length)

// Add required characters
for _, group := range requiredChars {
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)
const allChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"

// 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
}
65 changes: 65 additions & 0 deletions internal/crypto/password_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package crypto

import (
"context"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -19,3 +20,67 @@ func TestArgon2(t *testing.T) {
assert.NoError(t, CompareHashAndPassword(context.Background(), example, "test"))
}
}

func TestGeneratePassword(t *testing.T) {
tests := []struct {
name string
requiredChars []string
length int
wantErr bool
}{
{
name: "Valid password generation",
requiredChars: []string{"ABC", "123", "@#$"},
length: 12,
wantErr: false,
},
{
name: "Empty required chars",
requiredChars: []string{},
length: 8,
wantErr: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := GeneratePassword(tt.requiredChars, tt.length)

if (err != nil) != tt.wantErr {
t.Errorf("GeneratePassword() error = %v, wantErr %v", err, tt.wantErr)
return
}

if len(got) != tt.length {
t.Errorf("GeneratePassword() returned password of length %d, want %d", len(got), tt.length)
}

// Check if all required characters are present
for _, chars := range tt.requiredChars {
found := false
for _, c := range got {
if strings.ContainsRune(chars, c) {
found = true
break
}
}
if !found && len(chars) > 0 {
t.Errorf("GeneratePassword() missing required character from set %s", chars)
}
}
})
}

// Check for duplicates passwords
passwords := make(map[string]bool)
for i := 0; i < 30; i++ {
p, err := GeneratePassword([]string{"ABC", "123", "@#$"}, 30)
if err != nil {
t.Errorf("GeneratePassword() unexpected error: %v", err)
}
if passwords[p] {
t.Errorf("GeneratePassword() generated duplicate password: %s", p)
}
passwords[p] = true
}
}

0 comments on commit 7a5411f

Please sign in to comment.