From ea4ba18def6c348018eefdc5e29afa4d03a312fc Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 23 Mar 2026 12:24:12 +0100 Subject: [PATCH] fix(user): handle status errors across the codebase, remove redundant checks --- pkg/cmd/user.go | 4 ++-- pkg/models/project_users.go | 4 ++-- pkg/models/team_members.go | 4 ++-- pkg/models/team_members_permissions.go | 2 +- pkg/modules/auth/ldap/ldap.go | 2 +- pkg/modules/auth/openid/openid.go | 8 ++++---- pkg/routes/api/v1/avatar.go | 4 ++-- pkg/routes/api/v1/login.go | 23 +++++++++++------------ 8 files changed, 25 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/user.go b/pkg/cmd/user.go index 902ac6317..7ec6595dc 100644 --- a/pkg/cmd/user.go +++ b/pkg/cmd/user.go @@ -118,7 +118,7 @@ func getUserFromArg(s *xorm.Session, arg string) *user.User { } u, err := user.GetUserWithEmail(s, &filter) - if err != nil { + if err != nil && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { log.Fatalf("Could not get user: %s", err) } return u @@ -143,7 +143,7 @@ var userListCmd = &cobra.Command{ if userFlagEmail != "" { u, err := user.GetUserWithEmail(s, &user.User{Email: userFlagEmail}) - if err != nil { + if err != nil && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { if user.IsErrUserDoesNotExist(err) { log.Fatalf("No user found with email %s", userFlagEmail) } diff --git a/pkg/models/project_users.go b/pkg/models/project_users.go index 7db2f8b0f..18001e2b1 100644 --- a/pkg/models/project_users.go +++ b/pkg/models/project_users.go @@ -142,7 +142,7 @@ func (lu *ProjectUser) Delete(s *xorm.Session, _ web.Auth) (err error) { // Check if the user exists u, err := user.GetUserByUsername(s, lu.Username) - if err != nil { + if err != nil && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { return } lu.UserID = u.ID @@ -249,7 +249,7 @@ func (lu *ProjectUser) Update(s *xorm.Session, _ web.Auth) (err error) { // Check if the user exists u, err := user.GetUserByUsername(s, lu.Username) - if err != nil { + if err != nil && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { return err } lu.UserID = u.ID diff --git a/pkg/models/team_members.go b/pkg/models/team_members.go index 58c60a286..b911283e0 100644 --- a/pkg/models/team_members.go +++ b/pkg/models/team_members.go @@ -110,7 +110,7 @@ func (tm *TeamMember) Delete(s *xorm.Session, a web.Auth) (err error) { // Find the numeric user id user, err := user2.GetUserByUsername(s, tm.Username) - if err != nil { + if err != nil && !user2.IsErrAccountDisabled(err) && !user2.IsErrAccountLocked(err) { return } tm.UserID = user.ID @@ -149,7 +149,7 @@ func (tm *TeamMember) MembershipExists(s *xorm.Session) (exists bool, err error) func (tm *TeamMember) Update(s *xorm.Session, _ web.Auth) (err error) { // Find the numeric user id user, err := user2.GetUserByUsername(s, tm.Username) - if err != nil { + if err != nil && !user2.IsErrAccountDisabled(err) && !user2.IsErrAccountLocked(err) { return } tm.UserID = user.ID diff --git a/pkg/models/team_members_permissions.go b/pkg/models/team_members_permissions.go index d7135ccdb..383e7ffbe 100644 --- a/pkg/models/team_members_permissions.go +++ b/pkg/models/team_members_permissions.go @@ -30,7 +30,7 @@ func (tm *TeamMember) CanCreate(s *xorm.Session, a web.Auth) (bool, error) { // CanDelete checks if the user can delete a new team member func (tm *TeamMember) CanDelete(s *xorm.Session, a web.Auth) (bool, error) { u, err := user.GetUserByUsername(s, tm.Username) - if err != nil { + if err != nil && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { return false, err } if u.ID == a.GetID() { diff --git a/pkg/modules/auth/ldap/ldap.go b/pkg/modules/auth/ldap/ldap.go index 527adebe3..0a6b56e6d 100644 --- a/pkg/modules/auth/ldap/ldap.go +++ b/pkg/modules/auth/ldap/ldap.go @@ -264,7 +264,7 @@ func getOrCreateLdapUser(s *xorm.Session, entry *ldap.Entry) (u *user.User, err Issuer: user.IssuerLDAP, Subject: username, }) - if err != nil && !user.IsErrUserDoesNotExist(err) { + if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { return nil, err } diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 5fab9f46a..aed18dd46 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -278,10 +278,10 @@ func getOrCreateUser(s *xorm.Session, cl *claims, provider *Provider, idToken *o Issuer: idToken.Issuer, Subject: idToken.Subject, }) - if err != nil && !user.IsErrUserDoesNotExist(err) { + if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { return nil, err } - alreadyCreatedFromIssuer = err == nil // found if no error, not found if we reach it here despite an error + alreadyCreatedFromIssuer = err == nil || user.IsErrAccountDisabled(err) || user.IsErrAccountLocked(err) if !alreadyCreatedFromIssuer && (provider.EmailFallback || provider.UsernameFallback) { @@ -304,10 +304,10 @@ func getOrCreateUser(s *xorm.Session, cl *claims, provider *Provider, idToken *o // Check if the user exists for the given fallback matching options u, err = user.GetUserWithEmail(s, searchUser) - if err != nil && !user.IsErrUserDoesNotExist(err) { + if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { return nil, err } - fallbackMatchFound = err == nil // found if no error, not found if we reach it here despite an error + fallbackMatchFound = err == nil || user.IsErrAccountDisabled(err) || user.IsErrAccountLocked(err) } if !alreadyCreatedFromIssuer && !fallbackMatchFound { diff --git a/pkg/routes/api/v1/avatar.go b/pkg/routes/api/v1/avatar.go index 5efea2f64..98216a3b9 100644 --- a/pkg/routes/api/v1/avatar.go +++ b/pkg/routes/api/v1/avatar.go @@ -55,12 +55,12 @@ func GetAvatar(c *echo.Context) error { // Get the user u, err := user.GetUserWithEmail(s, &user.User{Username: username}) - if err != nil && !user.IsErrUserDoesNotExist(err) { + if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrAccountDisabled(err) && !user.IsErrAccountLocked(err) { log.Errorf("Error getting user for avatar: %v", err) return err } - found := err == nil || !user.IsErrUserDoesNotExist(err) + found := err == nil || user.IsErrAccountDisabled(err) || user.IsErrAccountLocked(err) avatarProvider := avatar.GetProvider(u) diff --git a/pkg/routes/api/v1/login.go b/pkg/routes/api/v1/login.go index 26a098caf..873e1efef 100644 --- a/pkg/routes/api/v1/login.go +++ b/pkg/routes/api/v1/login.go @@ -239,23 +239,22 @@ func RefreshToken(c *echo.Context) (err error) { } u, err := user2.GetUserWithEmail(s, &user2.User{ID: session.UserID}) + if user2.IsErrAccountDisabled(err) || user2.IsErrAccountLocked(err) { + if _, delErr := s.Where("id = ?", session.ID).Delete(&models.Session{}); delErr != nil { + _ = s.Rollback() + return delErr + } + if commitErr := s.Commit(); commitErr != nil { + return commitErr + } + auth.ClearRefreshTokenCookie(c) + return err + } if err != nil { _ = s.Rollback() return err } - if u.Status == user2.StatusDisabled || u.Status == user2.StatusAccountLocked { - if _, err := s.Where("id = ?", session.ID).Delete(&models.Session{}); err != nil { - _ = s.Rollback() - return err - } - if err := s.Commit(); err != nil { - return err - } - auth.ClearRefreshTokenCookie(c) - return &user2.ErrAccountDisabled{UserID: u.ID} - } - if err := s.Commit(); err != nil { _ = s.Rollback() return err