Closes: https://todo.sr.ht/~emersion/soju/136
---
 database/database.go | 16 ++++++++++++++++
 downstream.go        |  6 ++++++
 2 files changed, 22 insertions(+)

diff --git a/database/database.go b/database/database.go
index 977d1b7..dd0d9bc 100644
--- a/database/database.go
+++ b/database/database.go
@@ -82,6 +82,22 @@ func (u *User) CheckPassword(password string) error {
 		return fmt.Errorf("wrong password: %v", err)
 	}
 
+	passCost, err := bcrypt.Cost([]byte(u.Password))
+	if err != nil {
+		return fmt.Errorf("invalid password cost: %v", err)
+	}
+
+	if passCost >= bcrypt.DefaultCost {
+		return nil
+	}
+
+	hashed, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
+	if err != nil {
+		return fmt.Errorf("failed to hash password: %v", err)
+	}
+
+	u.Password = string(hashed)
+
 	return nil
 }
 
diff --git a/downstream.go b/downstream.go
index 417b513..4575f43 100644
--- a/downstream.go
+++ b/downstream.go
@@ -1306,10 +1306,16 @@ func (dc *downstreamConn) authenticate(ctx context.Context, username, password s
 		return newInvalidUsernameOrPasswordError(fmt.Errorf("user not found: %w", err))
 	}
 
+	hashedpassword := u.Password
+
 	if err := u.CheckPassword(password); err != nil {
 		return newInvalidUsernameOrPasswordError(err)
 	}
 
+	if hashedpassword != u.Password {
+		dc.srv.db.StoreUser(ctx, u)
+	}
+
 	dc.user = dc.srv.getUser(username)
 	if dc.user == nil {
 		return fmt.Errorf("user exists in the DB but hasn't been loaded by the bouncer -- a restart may help")
-- 
2.36.1

Thanks for the patch! Here are a few comments.

On Wednesday, June 29th, 2022 at 01:35, gildarts <gildarts@orbital.rocks> wrote:

> Closes: https://todo.sr.ht/~emersion/soju/136
> ---
>  database/database.go | 16 ++++++++++++++++
>  downstream.go        |  6 ++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/database/database.go b/database/database.go
> index 977d1b7..dd0d9bc 100644
> --- a/database/database.go
> +++ b/database/database.go
> @@ -82,6 +82,22 @@ func (u *User) CheckPassword(password string) error {
>  		return fmt.Errorf("wrong password: %v", err)
>  	}
>
> +	passCost, err := bcrypt.Cost([]byte(u.Password))
> +	if err != nil {
> +		return fmt.Errorf("invalid password cost: %v", err)
> +	}
> +
> +	if passCost >= bcrypt.DefaultCost {
> +		return nil
> +	}
> +
> +	hashed, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
> +	if err != nil {
> +		return fmt.Errorf("failed to hash password: %v", err)
> +	}
> +
> +	u.Password = string(hashed)

Hm, I'm not a fan of comparing the hash before/after CheckPassword. Can we
return a bool indicating whether the password hash has been upgraded? e.g.

    func (u *User) CheckPassword(password string) (upgraded bool, err error)

BTW, we have User.SetPassword -- we can use that instead of repeating the
logic.

>  	return nil
>  }
>
> diff --git a/downstream.go b/downstream.go
> index 417b513..4575f43 100644
> --- a/downstream.go
> +++ b/downstream.go
> @@ -1306,10 +1306,16 @@ func (dc *downstreamConn) authenticate(ctx context.Context, username, password s
>  		return newInvalidUsernameOrPasswordError(fmt.Errorf("user not found: %w", err))
>  	}
>
> +	hashedpassword := u.Password

Style: use camelCase (hashPassword).

> +
>  	if err := u.CheckPassword(password); err != nil {
>  		return newInvalidUsernameOrPasswordError(err)
>  	}
>
> +	if hashedpassword != u.Password {
> +		dc.srv.db.StoreUser(ctx, u)

Please log the error, if any.

> +	}
> +
>  	dc.user = dc.srv.getUser(username)
>  	if dc.user == nil {
>  		return fmt.Errorf("user exists in the DB but hasn't been loaded by the bouncer -- a restart may help")
> --
> 2.36.1