Update passwords to use Argon2ID (#695)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

.github/workflows/binaries-publish.yaml

@@ -16,6 +16,9 @@ jobs:
 
       - name: Set up Go
         uses: actions/setup-go@v5
+        with:
+          go-version: "1.23"
+          cache-dependency-path: backend/go.mod
 
       - uses: pnpm/action-setup@v2
         with:

.github/workflows/e2e-partial.yaml

@@ -27,7 +27,8 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version: "1.23"
+          cache-dependency-path: backend/go.mod
 
       - uses: actions/setup-node@v4
         with:

.github/workflows/partial-backend.yaml

@@ -12,7 +12,8 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version: "1.23"
+          cache-dependency-path: backend/go.mod
 
       - name: Install Task
         uses: arduino/setup-task@v1

.github/workflows/partial-frontend.yaml

@@ -60,7 +60,8 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version: "1.23"
+          cache-dependency-path: backend/go.mod
 
       - uses: actions/setup-node@v4
         with:
@@ -110,7 +111,8 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version: "1.23"
+          cache-dependency-path: backend/go.mod
 
       - uses: actions/setup-node@v4
         with:

Taskfile.yml

@@ -95,7 +95,7 @@ tasks:
     desc: Runs all go tests using gotestsum - supports passing gotestsum args
     dir: backend
     cmds:
-      - gotestsum {{ .CLI_ARGS }} ./...
+      - go test {{ .CLI_ARGS }} ./...
 
   go:coverage:
     desc: Runs all go tests with -race flag and generates a coverage report

backend/internal/core/services/service_user.go

@@ -190,10 +190,23 @@ func (svc *UserService) Login(ctx context.Context, username, password string, ex
 		return UserAuthTokenDetail{}, ErrorInvalidLogin
 	}
 
-	if !hasher.CheckPasswordHash(password, usr.PasswordHash) {
+	check, rehash := hasher.CheckPasswordHash(password, usr.PasswordHash)
+
+	if !check {
 		return UserAuthTokenDetail{}, ErrorInvalidLogin
 	}
 
+	if rehash {
+		hash, err := hasher.HashPassword(password)
+		if err != nil {
+			log.Err(err).Msg("Failed to hash password")
+			return UserAuthTokenDetail{}, err
+		}
+		err = svc.repos.Users.ChangePassword(ctx, usr.ID, hash)
+		if err != nil {
+			return UserAuthTokenDetail{}, err
+		}
+	}
 	return svc.createSessionToken(ctx, usr.ID, extendedSession)
 }
 
@@ -227,7 +240,8 @@ func (svc *UserService) ChangePassword(ctx Context, current string, new string)
 		return false
 	}
 
-	if !hasher.CheckPasswordHash(current, usr.PasswordHash) {
+	match, _ := hasher.CheckPasswordHash(current, usr.PasswordHash)
+	if !match {
 		log.Err(errors.New("current password is incorrect")).Msg("Failed to change password")
 		return false
 	}

backend/pkgs/hasher/password.go

@@ -1,14 +1,35 @@
 package hasher
 
 import (
+	"crypto/rand"
+	"crypto/subtle"
+	"encoding/base64"
 	"fmt"
 	"os"
+	"strings"
 
+	"golang.org/x/crypto/argon2"
 	"golang.org/x/crypto/bcrypt"
 )
 
 var enabled = true
 
+type params struct {
+	memory      uint32
+	iterations  uint32
+	parallelism uint8
+	saltLength  uint32
+	keyLength   uint32
+}
+
+var p = &params{
+	memory:      64 * 1024,
+	iterations:  3,
+	parallelism: 2,
+	saltLength:  16,
+	keyLength:   32,
+}
+
 func init() { // nolint: gochecknoinits
 	disableHas := os.Getenv("UNSAFE_DISABLE_PASSWORD_PROJECTION") == "yes_i_am_sure"
 
@@ -18,20 +39,108 @@ func init() { // nolint: gochecknoinits
 	}
 }
 
+func GenerateRandomBytes(n uint32) ([]byte, error) {
+	b := make([]byte, n)
+	_, err := rand.Read(b)
+	if err != nil {
+		return nil, err
+	}
+	return b, nil
+}
+
 func HashPassword(password string) (string, error) {
 	if !enabled {
 		return password, nil
 	}
 
-	bytes, err := bcrypt.GenerateFromPassword([]byte(password), 14)
+	salt, err := GenerateRandomBytes(p.saltLength)
-	return string(bytes), err
+	if err != nil {
+		return "", err
+	}
+	hash := argon2.IDKey([]byte(password), salt, p.iterations, p.memory, p.parallelism, p.keyLength)
+
+	b64Salt := base64.RawStdEncoding.EncodeToString(salt)
+	b64Hash := base64.RawStdEncoding.EncodeToString(hash)
+
+	encodedHash := fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", argon2.Version, 64*1024, 3, 2, b64Salt, b64Hash)
+	return encodedHash, err
 }
 
-func CheckPasswordHash(password, hash string) bool {
+// CheckPasswordHash checks if the provided password matches the hash.
+// Additionally, it returns a boolean indicating whether the password should be rehashed.
+func CheckPasswordHash(password, hash string) (bool, bool) {
 	if !enabled {
-		return password == hash
+		return password == hash, false
 	}
 
+	// Compare Argon2id hash first
+	match, err := comparePasswordAndHash(password, hash)
+	if err != nil || !match {
+		// If argon2id hash fails or doesn't match, try bcrypt
 		err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password))
-	return err == nil
+		if err == nil {
+			// If bcrypt hash matches, return true and indicate rehashing
+			return true, true
+		} else {
+			// If both fail, return false and indicate no rehashing
+			return false, false
+		}
+	}
+	return match, false
+}
+
+func comparePasswordAndHash(password, encodedHash string) (match bool, err error) {
+	// Extract the parameters, salt and derived key from the encoded password
+	// hash.
+	p, salt, hash, err := decodeHash(encodedHash)
+	if err != nil {
+		return false, err
+	}
+
+	// Derive the key from the other password using the same parameters.
+	otherHash := argon2.IDKey([]byte(password), salt, p.iterations, p.memory, p.parallelism, p.keyLength)
+
+	// Check that the contents of the hashed passwords are identical. Note
+	// that we are using the subtle.ConstantTimeCompare() function for this
+	// to help prevent timing attacks.
+	if subtle.ConstantTimeCompare(hash, otherHash) == 1 {
+		return true, nil
+	}
+	return false, nil
+}
+
+func decodeHash(encodedHash string) (p *params, salt, hash []byte, err error) {
+	vals := strings.Split(encodedHash, "$")
+	if len(vals) != 6 {
+		return nil, nil, nil, fmt.Errorf("invalid hash format")
+	}
+
+	var version int
+	_, err = fmt.Sscanf(vals[2], "v=%d", &version)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	if version != argon2.Version {
+		return nil, nil, nil, fmt.Errorf("unsupported argon2 version: %d", version)
+	}
+
+	p = &params{}
+	_, err = fmt.Sscanf(vals[3], "m=%d,t=%d,p=%d", &p.memory, &p.iterations, &p.parallelism)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+
+	salt, err = base64.RawStdEncoding.Strict().DecodeString(vals[4])
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	p.saltLength = uint32(len(salt))
+
+	hash, err = base64.RawStdEncoding.Strict().DecodeString(vals[5])
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	p.keyLength = uint32(len(hash))
+
+	return p, salt, hash, nil
 }

backend/pkgs/hasher/password_test.go

@@ -1,11 +1,14 @@
 package hasher
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestHashPassword(t *testing.T) {
 	t.Parallel()
 	type args struct {
 		password      string
+		invalidInputs []string
 	}
 	tests := []struct {
 		name    string
@@ -16,12 +19,28 @@ func TestHashPassword(t *testing.T) {
 			name: "letters_and_numbers",
 			args: args{
 				password:      "password123456788",
+				invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"},
 			},
 		},
 		{
 			name: "letters_number_and_special",
 			args: args{
 				password:      "!2afj3214pofajip3142j;fa",
+				invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"},
+			},
+		},
+		{
+			name: "extra_long_password",
+			args: args{
+				password:      "this_is_a_very_long_password_that_should_be_hashed_properly_and_still_work_with_the_check_function",
+				invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"},
+			},
+		},
+		{
+			name: "empty_password",
+			args: args{
+				password:      "",
+				invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"},
 			},
 		},
 	}
@@ -32,9 +51,17 @@ func TestHashPassword(t *testing.T) {
 				t.Errorf("HashPassword() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			if !CheckPasswordHash(tt.args.password, got) {
+			check, _ := CheckPasswordHash(tt.args.password, got)
+			if !check {
 				t.Errorf("CheckPasswordHash() failed to validate password=%v against hash=%v", tt.args.password, got)
 			}
+
+			for _, invalid := range tt.args.invalidInputs {
+				check, _ := CheckPasswordHash(invalid, got)
+				if check {
+					t.Errorf("CheckPasswordHash() improperly validated password=%v against hash=%v", invalid, got)
+				}
+			}
 		})
 	}
 }