feat: add validating user password as trace region (#7981)

- Password hashing can take a measurable amount of time, make this more visible in the trace by capturing the computations done in the password hash in their own region.
- Ref: forgejo/forgejo#6470

## Screenshot

![image](/attachments/9834b094-a78f-4ac2-847e-91f221a84833)

The upper part are where the tasks are shown (and nothing else). The bottom part is where the interesting execution tracing happens and the part where the user password hashing happens is now properly indicated/highlighted and does not need to be inferred by looking at the stack traces.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7981
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Gusted 2025-05-28 14:46:23 +02:00 committed by Earl Warren
parent 4c4fe595c2
commit 7d2a7b8559
6 changed files with 8 additions and 6 deletions

View file

@ -15,6 +15,7 @@ import (
"net/url" "net/url"
"path/filepath" "path/filepath"
"regexp" "regexp"
"runtime/trace"
"strings" "strings"
"time" "time"
"unicode" "unicode"
@ -397,7 +398,8 @@ func (u *User) SetPassword(passwd string) (err error) {
} }
// ValidatePassword checks if the given password matches the one belonging to the user. // ValidatePassword checks if the given password matches the one belonging to the user.
func (u *User) ValidatePassword(passwd string) bool { func (u *User) ValidatePassword(ctx context.Context, passwd string) bool {
defer trace.StartRegion(ctx, "Validate user password").End()
return hash.Parse(u.PasswdHashAlgo).VerifyPassword(passwd, u.Passwd, u.Salt) return hash.Parse(u.PasswdHashAlgo).VerifyPassword(passwd, u.Passwd, u.Salt)
} }

View file

@ -267,7 +267,7 @@ func TestHashPasswordDeterministic(t *testing.T) {
r2 := u.Passwd r2 := u.Passwd
assert.NotEqual(t, r1, r2) assert.NotEqual(t, r1, r2)
assert.True(t, u.ValidatePassword(pass)) assert.True(t, u.ValidatePassword(t.Context(), pass))
} }
} }
} }

View file

@ -764,7 +764,7 @@ func ActivatePost(ctx *context.Context) {
ctx.HTML(http.StatusOK, TplActivate) ctx.HTML(http.StatusOK, TplActivate)
return return
} }
if !user.ValidatePassword(password) { if !user.ValidatePassword(ctx, password) {
ctx.Data["IsPasswordInvalid"] = true ctx.Data["IsPasswordInvalid"] = true
ctx.HTML(http.StatusOK, TplActivate) ctx.HTML(http.StatusOK, TplActivate)
return return

View file

@ -57,7 +57,7 @@ func AccountPost(ctx *context.Context) {
return return
} }
if ctx.Doer.IsPasswordSet() && !ctx.Doer.ValidatePassword(form.OldPassword) { if ctx.Doer.IsPasswordSet() && !ctx.Doer.ValidatePassword(ctx, form.OldPassword) {
ctx.Flash.Error(ctx.Tr("settings.password_incorrect")) ctx.Flash.Error(ctx.Tr("settings.password_incorrect"))
} else if form.Password != form.Retype { } else if form.Password != form.Retype {
ctx.Flash.Error(ctx.Tr("form.password_not_match")) ctx.Flash.Error(ctx.Tr("form.password_not_match"))

View file

@ -50,7 +50,7 @@ func Authenticate(ctx context.Context, user *user_model.User, login, password st
if !user.IsPasswordSet() { if !user.IsPasswordSet() {
return nil, ErrUserPasswordNotSet{UID: user.ID, Name: user.Name} return nil, ErrUserPasswordNotSet{UID: user.ID, Name: user.Name}
} else if !user.ValidatePassword(password) { } else if !user.ValidatePassword(ctx, password) {
return nil, ErrUserPasswordInvalid{UID: user.ID, Name: user.Name} return nil, ErrUserPasswordInvalid{UID: user.ID, Name: user.Name}
} }

View file

@ -1062,7 +1062,7 @@ func TestUserPasswordReset(t *testing.T) {
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusSeeOther)
unittest.AssertNotExistsBean(t, &auth_model.AuthorizationToken{ID: authToken.ID}) unittest.AssertNotExistsBean(t, &auth_model.AuthorizationToken{ID: authToken.ID})
assert.True(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).ValidatePassword("new_password")) assert.True(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).ValidatePassword(t.Context(), "new_password"))
} }
func TestActivateEmailAddress(t *testing.T) { func TestActivateEmailAddress(t *testing.T) {