refactor(user): share user-search logic between v1 and v2

Extract the duplicated user-search business logic into two helpers both API
versions call, and refactor v1's handlers onto them:
- user.SearchUsers wraps ListUsers + email obfuscation (global search)
- models.SearchUsersForProject wraps the project read check + ListUsersFromProject

Each handler keeps its own forbidden mapping (v1 echo.ErrForbidden vs v2
huma) so v1 stays byte-identical on the wire.
This commit is contained in:
kolaente 2026-06-11 21:08:41 +02:00 committed by kolaente
parent 5dcc501d54
commit 5807f2e7b4
4 changed files with 47 additions and 34 deletions

View File

@ -18,10 +18,30 @@ package models
import ( import (
"code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/user"
"code.vikunja.io/api/pkg/web"
"xorm.io/builder" "xorm.io/builder"
"xorm.io/xorm" "xorm.io/xorm"
) )
// SearchUsersForProject performs the per-project user search shared by both API
// versions: it checks the caller can read the project, then lists the users
// with access to it. canRead is false (with no error) when the caller lacks
// read access, so each handler can map that to its own forbidden response.
func SearchUsersForProject(s *xorm.Session, project *Project, a web.Auth, currentUser *user.User, search string) (users []*user.User, canRead bool, err error) {
canRead, _, err = project.CanRead(s, a)
if err != nil {
return nil, false, err
}
if !canRead {
return nil, false, nil
}
users, err = ListUsersFromProject(s, project, currentUser, search)
if err != nil {
return nil, true, err
}
return users, true, nil
}
// ProjectUIDs hold all kinds of user IDs from accounts who have access to a project // ProjectUIDs hold all kinds of user IDs from accounts who have access to a project
type ProjectUIDs struct { type ProjectUIDs struct {
ProjectOwnerID int64 `xorm:"projectOwner"` ProjectOwnerID int64 `xorm:"projectOwner"`

View File

@ -52,17 +52,12 @@ func UserList(c *echo.Context) error {
return err return err
} }
users, err := user.ListUsers(s, search, currentUser, nil) users, err := user.SearchUsers(s, search, currentUser)
if err != nil { if err != nil {
_ = s.Rollback() _ = s.Rollback()
return err return err
} }
// Obfuscate the mailadresses
for in := range users {
users[in].Email = ""
}
return c.JSON(http.StatusOK, users) return c.JSON(http.StatusOK, users)
} }
@ -98,15 +93,6 @@ func ListUsersForProject(c *echo.Context) error {
s := db.NewSession() s := db.NewSession()
defer s.Close() defer s.Close()
canRead, _, err := project.CanRead(s, auth)
if err != nil {
_ = s.Rollback()
return err
}
if !canRead {
return echo.ErrForbidden
}
currentUser, err := user.GetCurrentUser(c) currentUser, err := user.GetCurrentUser(c)
if err != nil { if err != nil {
_ = s.Rollback() _ = s.Rollback()
@ -114,11 +100,14 @@ func ListUsersForProject(c *echo.Context) error {
} }
search := c.QueryParam("s") search := c.QueryParam("s")
users, err := models.ListUsersFromProject(s, &project, currentUser, search) users, canRead, err := models.SearchUsersForProject(s, &project, auth, currentUser, search)
if err != nil { if err != nil {
_ = s.Rollback() _ = s.Rollback()
return err return err
} }
if !canRead {
return echo.ErrForbidden
}
if err := s.Commit(); err != nil { if err := s.Commit(); err != nil {
_ = s.Rollback() _ = s.Rollback()

View File

@ -72,7 +72,7 @@ func usersSearch(ctx context.Context, in *struct {
return nil, translateDomainError(err) return nil, translateDomainError(err)
} }
users, err := user.ListUsers(s, in.Q, currentUser, nil) users, err := user.SearchUsers(s, in.Q, currentUser)
if err != nil { if err != nil {
_ = s.Rollback() _ = s.Rollback()
return nil, translateDomainError(err) return nil, translateDomainError(err)
@ -81,10 +81,6 @@ func usersSearch(ctx context.Context, in *struct {
return nil, translateDomainError(err) return nil, translateDomainError(err)
} }
for i := range users {
users[i].Email = ""
}
return &userListBody{Body: NewPaginated(users, int64(len(users)), 1, len(users))}, nil return &userListBody{Body: NewPaginated(users, int64(len(users)), 1, len(users))}, nil
} }
@ -100,8 +96,14 @@ func projectUsersSearch(ctx context.Context, in *struct {
s := db.NewSession() s := db.NewSession()
defer s.Close() defer s.Close()
currentUser, err := models.GetUserOrLinkShareUser(s, a)
if err != nil {
_ = s.Rollback()
return nil, translateDomainError(err)
}
project := &models.Project{ID: in.ProjectID} project := &models.Project{ID: in.ProjectID}
canRead, _, err := project.CanRead(s, a) users, canRead, err := models.SearchUsersForProject(s, project, a, currentUser, in.Q)
if err != nil { if err != nil {
_ = s.Rollback() _ = s.Rollback()
return nil, translateDomainError(err) return nil, translateDomainError(err)
@ -110,18 +112,6 @@ func projectUsersSearch(ctx context.Context, in *struct {
_ = s.Rollback() _ = s.Rollback()
return nil, huma.Error403Forbidden("forbidden") return nil, huma.Error403Forbidden("forbidden")
} }
currentUser, err := models.GetUserOrLinkShareUser(s, a)
if err != nil {
_ = s.Rollback()
return nil, translateDomainError(err)
}
users, err := models.ListUsersFromProject(s, project, currentUser, in.Q)
if err != nil {
_ = s.Rollback()
return nil, translateDomainError(err)
}
if err := s.Commit(); err != nil { if err := s.Commit(); err != nil {
return nil, translateDomainError(err) return nil, translateDomainError(err)
} }

View File

@ -34,6 +34,20 @@ type ProjectUserOpts struct {
MatchFuzzily bool MatchFuzzily bool
} }
// SearchUsers performs the global user search shared by both API versions:
// it lists users matching the search string and obfuscates their email
// addresses before returning.
func SearchUsers(s *xorm.Session, search string, currentUser *User) (users []*User, err error) {
users, err = ListUsers(s, search, currentUser, nil)
if err != nil {
return nil, err
}
for i := range users {
users[i].Email = ""
}
return users, nil
}
// ListUsers returns a list with all users, filtered by an optional search string // ListUsers returns a list with all users, filtered by an optional search string
func ListUsers(s *xorm.Session, search string, currentUser *User, opts *ProjectUserOpts) (users []*User, err error) { func ListUsers(s *xorm.Session, search string, currentUser *User, opts *ProjectUserOpts) (users []*User, err error) {
if opts == nil { if opts == nil {