feat(audit): attribute failed logins to the originating request

Thread the request context through CheckUserCredentials so the
LoginFailedEvent carries IP, user agent and request id — without it,
failed logins were the one auth event useless for brute-force tracing.
All four callers have the request at hand.
This commit is contained in:
kolaente 2026-06-11 21:34:45 +02:00 committed by kolaente
parent 3291556821
commit f33cde82e2
9 changed files with 31 additions and 24 deletions

View File

@ -17,6 +17,8 @@
package models
import (
"context"
"code.vikunja.io/api/pkg/modules/avatar"
"code.vikunja.io/api/pkg/user"
@ -66,12 +68,12 @@ func NewUserGeneralSettings(u *user.User) *UserGeneralSettings {
// ChangeUserPassword verifies the old password, sets the new one, and
// invalidates all of the user's sessions. Lives here (not in pkg/user) because
// it needs DeleteAllUserSessions, which pkg/user cannot import.
func ChangeUserPassword(s *xorm.Session, u *user.User, oldPassword, newPassword string) error {
func ChangeUserPassword(ctx context.Context, s *xorm.Session, u *user.User, oldPassword, newPassword string) error {
if oldPassword == "" {
return user.ErrEmptyOldPassword{}
}
if _, err := user.CheckUserCredentials(s, &user.Login{Username: u.Username, Password: oldPassword}); err != nil {
if _, err := user.CheckUserCredentials(ctx, s, &user.Login{Username: u.Username, Password: oldPassword}); err != nil {
return err
}

View File

@ -77,7 +77,7 @@ func Login(c *echo.Context) (err error) {
}
// This allows us to still have local users while ldap is enabled
user, err = user2.CheckUserCredentials(s, &u)
user, err = user2.CheckUserCredentials(c.Request().Context(), s, &u)
if err != nil {
_ = s.Rollback()
return err

View File

@ -62,7 +62,7 @@ func UpdateUserEmail(c *echo.Context) (err error) {
s := db.NewSession()
defer s.Close()
if err := user.ChangeUserEmail(s, emailUpdate.User, emailUpdate.Password, emailUpdate.NewEmail); err != nil {
if err := user.ChangeUserEmail(c.Request().Context(), s, emailUpdate.User, emailUpdate.Password, emailUpdate.NewEmail); err != nil {
_ = s.Rollback()
return err
}

View File

@ -66,7 +66,7 @@ func UserChangePassword(c *echo.Context) error {
s := db.NewSession()
defer s.Close()
if err := models.ChangeUserPassword(s, doer, newPW.OldPassword, newPW.NewPassword); err != nil {
if err := models.ChangeUserPassword(c.Request().Context(), s, doer, newPW.OldPassword, newPW.NewPassword); err != nil {
_ = s.Rollback()
return err
}

View File

@ -176,7 +176,7 @@ func userChangePassword(ctx context.Context, in *struct {
s := db.NewSession()
defer s.Close()
if err := models.ChangeUserPassword(s, doer, in.Body.OldPassword, in.Body.NewPassword); err != nil {
if err := models.ChangeUserPassword(ctx, s, doer, in.Body.OldPassword, in.Body.NewPassword); err != nil {
_ = s.Rollback()
return nil, translateDomainError(err)
}
@ -206,7 +206,7 @@ func userUpdateEmail(ctx context.Context, in *struct {
s := db.NewSession()
defer s.Close()
if err := user.ChangeUserEmail(s, doer, in.Body.Password, in.Body.NewEmail); err != nil {
if err := user.ChangeUserEmail(ctx, s, doer, in.Body.Password, in.Body.NewEmail); err != nil {
_ = s.Rollback()
return nil, translateDomainError(err)
}

View File

@ -88,7 +88,7 @@ func BasicAuth(c *echo.Context, username, password string) (bool, error) {
return false, nil
}
if u == nil {
u, err = user.CheckUserCredentials(s, credentials)
u, err = user.CheckUserCredentials(c.Request().Context(), s, credentials)
if err != nil {
log.Errorf("Error during basic auth for caldav: %v", err)
return false, nil

View File

@ -17,6 +17,8 @@
package user
import (
"context"
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/notifications"
"xorm.io/xorm"
@ -34,8 +36,8 @@ type EmailUpdate struct {
// ChangeUserEmail verifies the user's password, then sets a new email address
// (kicking off confirmation when the mailer is enabled). Shared by the v1 and
// v2 email-update handlers; only HTTP input binding stays in the handlers.
func ChangeUserEmail(s *xorm.Session, u *User, password, newEmail string) error {
verified, err := CheckUserCredentials(s, &Login{Username: u.Username, Password: password})
func ChangeUserEmail(ctx context.Context, s *xorm.Session, u *User, password, newEmail string) error {
verified, err := CheckUserCredentials(ctx, s, &Login{Username: u.Username, Password: password})
if err != nil {
return err
}

View File

@ -17,6 +17,7 @@
package user
import (
"context"
"encoding/json"
"errors"
"fmt"
@ -363,8 +364,9 @@ func getUserByUsernameOrEmail(s *xorm.Session, usernameOrEmail string) (u *User,
return
}
// CheckUserCredentials checks user credentials
func CheckUserCredentials(s *xorm.Session, u *Login) (*User, error) {
// CheckUserCredentials checks user credentials. The context carries request
// metadata for the audit trail of failed attempts.
func CheckUserCredentials(ctx context.Context, s *xorm.Session, u *Login) (*User, error) {
// Check if we have any credentials
if u.Password == "" || u.Username == "" {
return nil, ErrNoUsernamePassword{}
@ -391,7 +393,7 @@ func CheckUserCredentials(s *xorm.Session, u *Login) (*User, error) {
err = CheckUserPassword(user, u.Password)
if err != nil {
if IsErrWrongUsernameOrPassword(err) {
handleFailedPassword(user)
handleFailedPassword(ctx, user)
}
return user, err
}
@ -411,8 +413,8 @@ func (u *User) IsLocalUser() bool {
return u.Issuer == IssuerLocal
}
func handleFailedPassword(user *User) {
if err := events.Dispatch(&LoginFailedEvent{User: user}); err != nil {
func handleFailedPassword(ctx context.Context, user *User) {
if err := events.DispatchWithContext(ctx, &LoginFailedEvent{User: user}); err != nil {
log.Errorf("Could not dispatch login failed event: %s", err)
}

View File

@ -17,6 +17,7 @@
package user
import (
"context"
"testing"
"code.vikunja.io/api/pkg/db"
@ -357,7 +358,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Username: "user1", Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user1", Password: "12345678"})
require.NoError(t, err)
})
t.Run("unverified email", func(t *testing.T) {
@ -365,7 +366,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Username: "user5", Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user5", Password: "12345678"})
require.Error(t, err)
assert.True(t, IsErrEmailNotConfirmed(err))
})
@ -374,7 +375,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Username: "user1", Password: "12345"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user1", Password: "12345"})
require.Error(t, err)
assert.True(t, IsErrWrongUsernameOrPassword(err))
})
@ -383,7 +384,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Username: "dfstestuu", Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "dfstestuu", Password: "12345678"})
require.Error(t, err)
assert.True(t, IsErrWrongUsernameOrPassword(err))
})
@ -392,7 +393,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Username: "user1"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user1"})
require.Error(t, err)
assert.True(t, IsErrNoUsernamePassword(err))
})
@ -401,7 +402,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Password: "12345678"})
require.Error(t, err)
assert.True(t, IsErrNoUsernamePassword(err))
})
@ -410,7 +411,7 @@ func TestCheckUserCredentials(t *testing.T) {
s := db.NewSession()
defer s.Close()
_, err := CheckUserCredentials(s, &Login{Username: "user1@example.com", Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user1@example.com", Password: "12345678"})
require.NoError(t, err)
})
t.Run("disabled user", func(t *testing.T) {
@ -419,7 +420,7 @@ func TestCheckUserCredentials(t *testing.T) {
defer s.Close()
// user17 is disabled (status=2), password is "12345678"
_, err := CheckUserCredentials(s, &Login{Username: "user17", Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user17", Password: "12345678"})
require.Error(t, err)
assert.True(t, IsErrAccountDisabled(err))
})
@ -429,7 +430,7 @@ func TestCheckUserCredentials(t *testing.T) {
defer s.Close()
// user18 is locked (status=3), password is "12345678"
_, err := CheckUserCredentials(s, &Login{Username: "user18", Password: "12345678"})
_, err := CheckUserCredentials(context.Background(), s, &Login{Username: "user18", Password: "12345678"})
require.Error(t, err)
assert.True(t, IsErrAccountLocked(err))
})