diff options
author | KN4CK3R <admin@oldschoolhack.me> | 2024-02-04 14:29:09 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-04 13:29:09 +0000 |
commit | f8b471ace1b59bd3fc3a04c9ddb5f62dd1dd5396 (patch) | |
tree | 371d8477bd0cd4e41881b91fdb670eb33e38e8b1 /services | |
parent | b4513f48ce3e748ac621a6f3ddf082feca67e938 (diff) | |
download | gitea-f8b471ace1b59bd3fc3a04c9ddb5f62dd1dd5396.tar.gz gitea-f8b471ace1b59bd3fc3a04c9ddb5f62dd1dd5396.zip |
Unify user update methods (#28733)
Fixes #28660
Fixes an admin api bug related to `user.LoginSource`
Fixed `/user/emails` response not identical to GitHub api
This PR unifies the user update methods. The goal is to keep the logic
only at one place (having audit logs in mind). For example, do the
password checks only in one method not everywhere a password is updated.
After that PR is merged, the user creation should be next.
Diffstat (limited to 'services')
-rw-r--r-- | services/auth/auth.go | 8 | ||||
-rw-r--r-- | services/auth/source/ldap/source_authenticate.go | 16 | ||||
-rw-r--r-- | services/auth/source/ldap/source_sync.go | 32 | ||||
-rw-r--r-- | services/mailer/mail.go | 8 | ||||
-rw-r--r-- | services/mailer/notify.go | 4 | ||||
-rw-r--r-- | services/user/avatar.go | 2 | ||||
-rw-r--r-- | services/user/email.go | 166 | ||||
-rw-r--r-- | services/user/email_test.go | 129 | ||||
-rw-r--r-- | services/user/update.go | 212 | ||||
-rw-r--r-- | services/user/update_test.go | 120 | ||||
-rw-r--r-- | services/user/user.go | 5 | ||||
-rw-r--r-- | services/user/user_test.go | 2 |
12 files changed, 667 insertions, 37 deletions
diff --git a/services/auth/auth.go b/services/auth/auth.go index 713463a3d4..6746dc2a54 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -14,9 +14,11 @@ import ( "code.gitea.io/gitea/modules/auth/webauthn" gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" + user_service "code.gitea.io/gitea/services/user" ) // Init should be called exactly once when the application starts to allow plugins @@ -85,8 +87,10 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore // If the user does not have a locale set, we save the current one. if len(user.Language) == 0 { lc := middleware.Locale(resp, req) - user.Language = lc.Language() - if err := user_model.UpdateUserCols(req.Context(), user, "language"); err != nil { + opts := &user_service.UpdateOptions{ + Language: optional.Some(lc.Language()), + } + if err := user_service.UpdateUser(req.Context(), user, opts); err != nil { log.Error(fmt.Sprintf("Error updating user language [user: %d, locale: %s]", user.ID, user.Language)) return } diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index a7ea61b81c..8f641ed541 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/auth" user_model "code.gitea.io/gitea/models/user" auth_module "code.gitea.io/gitea/modules/auth" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/util" source_service "code.gitea.io/gitea/services/auth/source" user_service "code.gitea.io/gitea/services/user" @@ -49,20 +50,17 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u } } if user != nil && !user.ProhibitLogin { - cols := make([]string, 0) + opts := &user_service.UpdateOptions{} if len(source.AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin { // Change existing admin flag only if AdminFilter option is set - user.IsAdmin = sr.IsAdmin - cols = append(cols, "is_admin") + opts.IsAdmin = optional.Some(sr.IsAdmin) } - if !user.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { + if !sr.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { // Change existing restricted flag only if RestrictedFilter option is set - user.IsRestricted = sr.IsRestricted - cols = append(cols, "is_restricted") + opts.IsRestricted = optional.Some(sr.IsRestricted) } - if len(cols) > 0 { - err = user_model.UpdateUserCols(ctx, user, cols...) - if err != nil { + if opts.IsAdmin.Has() || opts.IsRestricted.Has() { + if err := user_service.UpdateUser(ctx, user, opts); err != nil { return nil, err } } diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index 5c65ca8dc2..eee7bb585a 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/services/auth/source/ldap/source_sync.go @@ -15,6 +15,7 @@ import ( auth_module "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/util" source_service "code.gitea.io/gitea/services/auth/source" user_service "code.gitea.io/gitea/services/user" @@ -158,23 +159,25 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("SyncExternalUsers[%s]: Updating user %s", source.authSource.Name, usr.Name) - usr.FullName = fullName - emailChanged := usr.Email != su.Mail - usr.Email = su.Mail - // Change existing admin flag only if AdminFilter option is set - if len(source.AdminFilter) > 0 { - usr.IsAdmin = su.IsAdmin + opts := &user_service.UpdateOptions{ + FullName: optional.Some(fullName), + IsActive: optional.Some(true), + } + if source.AdminFilter != "" { + opts.IsAdmin = optional.Some(su.IsAdmin) } // Change existing restricted flag only if RestrictedFilter option is set - if !usr.IsAdmin && len(source.RestrictedFilter) > 0 { - usr.IsRestricted = su.IsRestricted + if !su.IsAdmin && source.RestrictedFilter != "" { + opts.IsRestricted = optional.Some(su.IsRestricted) } - usr.IsActive = true - err = user_model.UpdateUser(ctx, usr, emailChanged, "full_name", "email", "is_admin", "is_restricted", "is_active") - if err != nil { + if err := user_service.UpdateUser(ctx, usr, opts); err != nil { log.Error("SyncExternalUsers[%s]: Error updating user %s: %v", source.authSource.Name, usr.Name, err) } + + if err := user_service.ReplacePrimaryEmailAddress(ctx, usr, su.Mail); err != nil { + log.Error("SyncExternalUsers[%s]: Error updating user %s primary email %s: %v", source.authSource.Name, usr.Name, su.Mail, err) + } } if usr.IsUploadAvatarChanged(su.Avatar) { @@ -215,9 +218,10 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.authSource.Name, usr.Name) - usr.IsActive = false - err = user_model.UpdateUserCols(ctx, usr, "is_active") - if err != nil { + opts := &user_service.UpdateOptions{ + IsActive: optional.Some(false), + } + if err := user_service.UpdateUser(ctx, usr, opts); err != nil { log.Error("SyncExternalUsers[%s]: Error deactivating user %s: %v", source.authSource.Name, usr.Name, err) } } diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 16c30088cd..ca27336f92 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -108,7 +108,7 @@ func SendResetPasswordMail(u *user_model.User) { } // SendActivateEmailMail sends confirmation email to confirm new email address -func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { +func SendActivateEmailMail(u *user_model.User, email string) { if setting.MailService == nil { // No mail service configured return @@ -118,8 +118,8 @@ func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { "locale": locale, "DisplayName": u.DisplayName(), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), - "Code": u.GenerateEmailActivateCode(email.Email), - "Email": email.Email, + "Code": u.GenerateEmailActivateCode(email), + "Email": email, "Language": locale.Language(), } @@ -130,7 +130,7 @@ func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { return } - msg := NewMessage(email.Email, locale.Tr("mail.activate_email"), content.String()) + msg := NewMessage(email, locale.Tr("mail.activate_email"), content.String()) msg.Info = fmt.Sprintf("UID: %d, activate email", u.ID) SendAsync(msg) diff --git a/services/mailer/notify.go b/services/mailer/notify.go index cc4e6baf0b..e48b5d399d 100644 --- a/services/mailer/notify.go +++ b/services/mailer/notify.go @@ -114,7 +114,7 @@ func (m *mailNotifier) PullRequestCodeComment(ctx context.Context, pr *issues_mo func (m *mailNotifier) IssueChangeAssignee(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, assignee *user_model.User, removed bool, comment *issues_model.Comment) { // mail only sent to added assignees and not self-assignee - if !removed && doer.ID != assignee.ID && assignee.EmailNotifications() != user_model.EmailNotificationsDisabled { + if !removed && doer.ID != assignee.ID && assignee.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { ct := fmt.Sprintf("Assigned #%d.", issue.Index) if err := SendIssueAssignedMail(ctx, issue, doer, ct, comment, []*user_model.User{assignee}); err != nil { log.Error("Error in SendIssueAssignedMail for issue[%d] to assignee[%d]: %v", issue.ID, assignee.ID, err) @@ -123,7 +123,7 @@ func (m *mailNotifier) IssueChangeAssignee(ctx context.Context, doer *user_model } func (m *mailNotifier) PullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { - if isRequest && doer.ID != reviewer.ID && reviewer.EmailNotifications() != user_model.EmailNotificationsDisabled { + if isRequest && doer.ID != reviewer.ID && reviewer.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { ct := fmt.Sprintf("Requested to review %s.", issue.HTMLURL()) if err := SendIssueAssignedMail(ctx, issue, doer, ct, comment, []*user_model.User{reviewer}); err != nil { log.Error("Error in SendIssueAssignedMail for issue[%d] to reviewer[%d]: %v", issue.ID, reviewer.ID, err) diff --git a/services/user/avatar.go b/services/user/avatar.go index 4130d07c38..2d6c3faf9a 100644 --- a/services/user/avatar.go +++ b/services/user/avatar.go @@ -57,7 +57,7 @@ func DeleteAvatar(ctx context.Context, u *user_model.User) error { u.UseCustomAvatar = false u.Avatar = "" if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar, use_custom_avatar").Update(u); err != nil { - return fmt.Errorf("UpdateUser: %w", err) + return fmt.Errorf("DeleteAvatar: %w", err) } return nil } diff --git a/services/user/email.go b/services/user/email.go new file mode 100644 index 0000000000..07e19bc688 --- /dev/null +++ b/services/user/email.go @@ -0,0 +1,166 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "context" + "errors" + "strings" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" +) + +func AddOrSetPrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { + if strings.EqualFold(u.Email, emailStr) { + return nil + } + + if err := user_model.ValidateEmail(emailStr); err != nil { + return err + } + + // Check if address exists already + email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if email != nil && email.UID != u.ID { + return user_model.ErrEmailAlreadyUsed{Email: emailStr} + } + + // Update old primary address + primary, err := user_model.GetPrimaryEmailAddressOfUser(ctx, u.ID) + if err != nil { + return err + } + + primary.IsPrimary = false + if err := user_model.UpdateEmailAddress(ctx, primary); err != nil { + return err + } + + // Insert new or update existing address + if email != nil { + email.IsPrimary = true + email.IsActivated = true + if err := user_model.UpdateEmailAddress(ctx, email); err != nil { + return err + } + } else { + email = &user_model.EmailAddress{ + UID: u.ID, + Email: emailStr, + IsActivated: true, + IsPrimary: true, + } + if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { + return err + } + } + + u.Email = emailStr + + return user_model.UpdateUserCols(ctx, u, "email") +} + +func ReplacePrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { + if strings.EqualFold(u.Email, emailStr) { + return nil + } + + if err := user_model.ValidateEmail(emailStr); err != nil { + return err + } + + if !u.IsOrganization() { + // Check if address exists already + email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if email != nil { + if email.IsPrimary && email.UID == u.ID { + return nil + } + return user_model.ErrEmailAlreadyUsed{Email: emailStr} + } + + // Remove old primary address + primary, err := user_model.GetPrimaryEmailAddressOfUser(ctx, u.ID) + if err != nil { + return err + } + if _, err := db.DeleteByID[user_model.EmailAddress](ctx, primary.ID); err != nil { + return err + } + + // Insert new primary address + email = &user_model.EmailAddress{ + UID: u.ID, + Email: emailStr, + IsActivated: true, + IsPrimary: true, + } + if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { + return err + } + } + + u.Email = emailStr + + return user_model.UpdateUserCols(ctx, u, "email") +} + +func AddEmailAddresses(ctx context.Context, u *user_model.User, emails []string) error { + for _, emailStr := range emails { + if err := user_model.ValidateEmail(emailStr); err != nil { + return err + } + + // Check if address exists already + email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if email != nil { + return user_model.ErrEmailAlreadyUsed{Email: emailStr} + } + + // Insert new address + email = &user_model.EmailAddress{ + UID: u.ID, + Email: emailStr, + IsActivated: !setting.Service.RegisterEmailConfirm, + IsPrimary: false, + } + if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { + return err + } + } + + return nil +} + +func DeleteEmailAddresses(ctx context.Context, u *user_model.User, emails []string) error { + for _, emailStr := range emails { + // Check if address exists + email, err := user_model.GetEmailAddressOfUser(ctx, emailStr, u.ID) + if err != nil { + return err + } + if email.IsPrimary { + return user_model.ErrPrimaryEmailCannotDelete{Email: emailStr} + } + + // Remove address + if _, err := db.DeleteByID[user_model.EmailAddress](ctx, email.ID); err != nil { + return err + } + } + + return nil +} diff --git a/services/user/email_test.go b/services/user/email_test.go new file mode 100644 index 0000000000..8f419b69f9 --- /dev/null +++ b/services/user/email_test.go @@ -0,0 +1,129 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + organization_model "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func TestAddOrSetPrimaryEmailAddress(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 27}) + + emails, err := user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 1) + + primary, err := user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.NotEqual(t, "new-primary@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + assert.NoError(t, AddOrSetPrimaryEmailAddress(db.DefaultContext, user, "new-primary@example.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "new-primary@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 2) + + assert.NoError(t, AddOrSetPrimaryEmailAddress(db.DefaultContext, user, "user27@example.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "user27@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 2) +} + +func TestReplacePrimaryEmailAddress(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("User", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 13}) + + emails, err := user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 1) + + primary, err := user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.NotEqual(t, "primary-13@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + assert.NoError(t, ReplacePrimaryEmailAddress(db.DefaultContext, user, "primary-13@example.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "primary-13@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 1) + + assert.NoError(t, ReplacePrimaryEmailAddress(db.DefaultContext, user, "primary-13@example.com")) + }) + + t.Run("Organization", func(t *testing.T) { + org := unittest.AssertExistsAndLoadBean(t, &organization_model.Organization{ID: 3}) + + assert.Equal(t, "org3@example.com", org.Email) + + assert.NoError(t, ReplacePrimaryEmailAddress(db.DefaultContext, org.AsUser(), "primary-org@example.com")) + + assert.Equal(t, "primary-org@example.com", org.Email) + }) +} + +func TestAddEmailAddresses(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + assert.Error(t, AddEmailAddresses(db.DefaultContext, user, []string{" invalid email "})) + + emails := []string{"user1234@example.com", "user5678@example.com"} + + assert.NoError(t, AddEmailAddresses(db.DefaultContext, user, emails)) + + err := AddEmailAddresses(db.DefaultContext, user, emails) + assert.Error(t, err) + assert.True(t, user_model.IsErrEmailAlreadyUsed(err)) +} + +func TestDeleteEmailAddresses(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + emails := []string{"user2-2@example.com"} + + err := DeleteEmailAddresses(db.DefaultContext, user, emails) + assert.NoError(t, err) + + err = DeleteEmailAddresses(db.DefaultContext, user, emails) + assert.Error(t, err) + assert.True(t, user_model.IsErrEmailAddressNotExist(err)) + + emails = []string{"user2@example.com"} + + err = DeleteEmailAddresses(db.DefaultContext, user, emails) + assert.Error(t, err) + assert.True(t, user_model.IsErrPrimaryEmailCannotDelete(err)) +} diff --git a/services/user/update.go b/services/user/update.go new file mode 100644 index 0000000000..849757c8b0 --- /dev/null +++ b/services/user/update.go @@ -0,0 +1,212 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/models" + auth_model "code.gitea.io/gitea/models/auth" + user_model "code.gitea.io/gitea/models/user" + password_module "code.gitea.io/gitea/modules/auth/password" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" +) + +type UpdateOptions struct { + KeepEmailPrivate optional.Option[bool] + FullName optional.Option[string] + Website optional.Option[string] + Location optional.Option[string] + Description optional.Option[string] + AllowGitHook optional.Option[bool] + AllowImportLocal optional.Option[bool] + MaxRepoCreation optional.Option[int] + IsRestricted optional.Option[bool] + Visibility optional.Option[structs.VisibleType] + KeepActivityPrivate optional.Option[bool] + Language optional.Option[string] + Theme optional.Option[string] + DiffViewStyle optional.Option[string] + AllowCreateOrganization optional.Option[bool] + IsActive optional.Option[bool] + IsAdmin optional.Option[bool] + EmailNotificationsPreference optional.Option[string] + SetLastLogin bool + RepoAdminChangeTeamAccess optional.Option[bool] +} + +func UpdateUser(ctx context.Context, u *user_model.User, opts *UpdateOptions) error { + cols := make([]string, 0, 20) + + if opts.KeepEmailPrivate.Has() { + u.KeepEmailPrivate = opts.KeepEmailPrivate.Value() + + cols = append(cols, "keep_email_private") + } + + if opts.FullName.Has() { + u.FullName = opts.FullName.Value() + + cols = append(cols, "full_name") + } + if opts.Website.Has() { + u.Website = opts.Website.Value() + + cols = append(cols, "website") + } + if opts.Location.Has() { + u.Location = opts.Location.Value() + + cols = append(cols, "location") + } + if opts.Description.Has() { + u.Description = opts.Description.Value() + + cols = append(cols, "description") + } + if opts.Language.Has() { + u.Language = opts.Language.Value() + + cols = append(cols, "language") + } + if opts.Theme.Has() { + u.Theme = opts.Theme.Value() + + cols = append(cols, "theme") + } + if opts.DiffViewStyle.Has() { + u.DiffViewStyle = opts.DiffViewStyle.Value() + + cols = append(cols, "diff_view_style") + } + + if opts.AllowGitHook.Has() { + u.AllowGitHook = opts.AllowGitHook.Value() + + cols = append(cols, "allow_git_hook") + } + if opts.AllowImportLocal.Has() { + u.AllowImportLocal = opts.AllowImportLocal.Value() + + cols = append(cols, "allow_import_local") + } + + if opts.MaxRepoCreation.Has() { + u.MaxRepoCreation = opts.MaxRepoCreation.Value() + + cols = append(cols, "max_repo_creation") + } + + if opts.IsActive.Has() { + u.IsActive = opts.IsActive.Value() + + cols = append(cols, "is_active") + } + if opts.IsRestricted.Has() { + u.IsRestricted = opts.IsRestricted.Value() + + cols = append(cols, "is_restricted") + } + if opts.IsAdmin.Has() { + if !opts.IsAdmin.Value() && user_model.IsLastAdminUser(ctx, u) { + return models.ErrDeleteLastAdminUser{UID: u.ID} + } + + u.IsAdmin = opts.IsAdmin.Value() + + cols = append(cols, "is_admin") + } + + if opts.Visibility.Has() { + if !u.IsOrganization() && !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(opts.Visibility.Value()) { + return fmt.Errorf("visibility mode not allowed: %s", opts.Visibility.Value().String()) + } + u.Visibility = opts.Visibility.Value() + + cols = append(cols, "visibility") + } + if opts.KeepActivityPrivate.Has() { + u.KeepActivityPrivate = opts.KeepActivityPrivate.Value() + + cols = append(cols, "keep_activity_private") + } + + if opts.AllowCreateOrganization.Has() { + u.AllowCreateOrganization = opts.AllowCreateOrganization.Value() + + cols = append(cols, "allow_create_organization") + } + if opts.RepoAdminChangeTeamAccess.Has() { + u.RepoAdminChangeTeamAccess = opts.RepoAdminChangeTeamAccess.Value() + + cols = append(cols, "repo_admin_change_team_access") + } + + if opts.EmailNotificationsPreference.Has() { + u.EmailNotificationsPreference = opts.EmailNotificationsPreference.Value() + + cols = append(cols, "email_notifications_preference") + } + + if opts.SetLastLogin { + u.SetLastLogin() + + cols = append(cols, "last_login_unix") + } + + return user_model.UpdateUserCols(ctx, u, cols...) +} + +type UpdateAuthOptions struct { + LoginSource optional.Option[int64] + LoginName optional.Option[string] + Password optional.Option[string] + MustChangePassword optional.Option[bool] + ProhibitLogin optional.Option[bool] +} + +func UpdateAuth(ctx context.Context, u *user_model.User, opts *UpdateAuthOptions) error { + if opts.LoginSource.Has() { + source, err := auth_model.GetSourceByID(ctx, opts.LoginSource.Value()) + if err != nil { + return err + } + + u.LoginType = source.Type + u.LoginSource = source.ID + } + if opts.LoginName.Has() { + u.LoginName = opts.LoginName.Value() + } + + if opts.Password.Has() && (u.IsLocal() || u.IsOAuth2()) { + password := opts.Password.Value() + + if len(password) < setting.MinPasswordLength { + return password_module.ErrMinLength + } + if !password_module.IsComplexEnough(password) { + return password_module.ErrComplexity + } + if err := password_module.IsPwned(ctx, password); err != nil { + return err + } + + if err := u.SetPassword(password); err != nil { + return err + } + } + + if opts.MustChangePassword.Has() { + u.MustChangePassword = opts.MustChangePassword.Value() + } + if opts.ProhibitLogin.Has() { + u.ProhibitLogin = opts.ProhibitLogin.Value() + } + + return user_model.UpdateUserCols(ctx, u, "login_type", "login_source", "login_name", "passwd", "passwd_hash_algo", "salt", "must_change_password", "prohibit_login") +} diff --git a/services/user/update_test.go b/services/user/update_test.go new file mode 100644 index 0000000000..7ed764b539 --- /dev/null +++ b/services/user/update_test.go @@ -0,0 +1,120 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + password_module "code.gitea.io/gitea/modules/auth/password" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func TestUpdateUser(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + assert.Error(t, UpdateUser(db.DefaultContext, admin, &UpdateOptions{ + IsAdmin: optional.Some(false), + })) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + + opts := &UpdateOptions{ + KeepEmailPrivate: optional.Some(false), + FullName: optional.Some("Changed Name"), + Website: optional.Some("https://gitea.com/"), + Location: optional.Some("location"), + Description: optional.Some("description"), + AllowGitHook: optional.Some(true), + AllowImportLocal: optional.Some(true), + MaxRepoCreation: optional.Some[int](10), + IsRestricted: optional.Some(true), + IsActive: optional.Some(false), + IsAdmin: optional.Some(true), + Visibility: optional.Some(structs.VisibleTypePrivate), + KeepActivityPrivate: optional.Some(true), + Language: optional.Some("lang"), + Theme: optional.Some("theme"), + DiffViewStyle: optional.Some("split"), + AllowCreateOrganization: optional.Some(false), + EmailNotificationsPreference: optional.Some("disabled"), + SetLastLogin: true, + } + assert.NoError(t, UpdateUser(db.DefaultContext, user, opts)) + + assert.Equal(t, opts.KeepEmailPrivate.Value(), user.KeepEmailPrivate) + assert.Equal(t, opts.FullName.Value(), user.FullName) + assert.Equal(t, opts.Website.Value(), user.Website) + assert.Equal(t, opts.Location.Value(), user.Location) + assert.Equal(t, opts.Description.Value(), user.Description) + assert.Equal(t, opts.AllowGitHook.Value(), user.AllowGitHook) + assert.Equal(t, opts.AllowImportLocal.Value(), user.AllowImportLocal) + assert.Equal(t, opts.MaxRepoCreation.Value(), user.MaxRepoCreation) + assert.Equal(t, opts.IsRestricted.Value(), user.IsRestricted) + assert.Equal(t, opts.IsActive.Value(), user.IsActive) + assert.Equal(t, opts.IsAdmin.Value(), user.IsAdmin) + assert.Equal(t, opts.Visibility.Value(), user.Visibility) + assert.Equal(t, opts.KeepActivityPrivate.Value(), user.KeepActivityPrivate) + assert.Equal(t, opts.Language.Value(), user.Language) + assert.Equal(t, opts.Theme.Value(), user.Theme) + assert.Equal(t, opts.DiffViewStyle.Value(), user.DiffViewStyle) + assert.Equal(t, opts.AllowCreateOrganization.Value(), user.AllowCreateOrganization) + assert.Equal(t, opts.EmailNotificationsPreference.Value(), user.EmailNotificationsPreference) + + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + assert.Equal(t, opts.KeepEmailPrivate.Value(), user.KeepEmailPrivate) + assert.Equal(t, opts.FullName.Value(), user.FullName) + assert.Equal(t, opts.Website.Value(), user.Website) + assert.Equal(t, opts.Location.Value(), user.Location) + assert.Equal(t, opts.Description.Value(), user.Description) + assert.Equal(t, opts.AllowGitHook.Value(), user.AllowGitHook) + assert.Equal(t, opts.AllowImportLocal.Value(), user.AllowImportLocal) + assert.Equal(t, opts.MaxRepoCreation.Value(), user.MaxRepoCreation) + assert.Equal(t, opts.IsRestricted.Value(), user.IsRestricted) + assert.Equal(t, opts.IsActive.Value(), user.IsActive) + assert.Equal(t, opts.IsAdmin.Value(), user.IsAdmin) + assert.Equal(t, opts.Visibility.Value(), user.Visibility) + assert.Equal(t, opts.KeepActivityPrivate.Value(), user.KeepActivityPrivate) + assert.Equal(t, opts.Language.Value(), user.Language) + assert.Equal(t, opts.Theme.Value(), user.Theme) + assert.Equal(t, opts.DiffViewStyle.Value(), user.DiffViewStyle) + assert.Equal(t, opts.AllowCreateOrganization.Value(), user.AllowCreateOrganization) + assert.Equal(t, opts.EmailNotificationsPreference.Value(), user.EmailNotificationsPreference) +} + +func TestUpdateAuth(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + copy := *user + + assert.NoError(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + LoginName: optional.Some("new-login"), + })) + assert.Equal(t, "new-login", user.LoginName) + + assert.NoError(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + Password: optional.Some("%$DRZUVB576tfzgu"), + MustChangePassword: optional.Some(true), + })) + assert.True(t, user.MustChangePassword) + assert.NotEqual(t, copy.Passwd, user.Passwd) + assert.NotEqual(t, copy.Salt, user.Salt) + + assert.NoError(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + ProhibitLogin: optional.Some(true), + })) + assert.True(t, user.ProhibitLogin) + + assert.ErrorIs(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + Password: optional.Some("aaaa"), + }), password_module.ErrMinLength) +} diff --git a/services/user/user.go b/services/user/user.go index 8bf083192f..f2648db409 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -41,10 +41,7 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string) err } if newUserName == u.Name { - return user_model.ErrUsernameNotChanged{ - UID: u.ID, - Name: u.Name, - } + return nil } if err := user_model.IsUsableUsername(newUserName); err != nil { diff --git a/services/user/user_test.go b/services/user/user_test.go index 73f1491c12..2ebcded925 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -107,7 +107,7 @@ func TestRenameUser(t *testing.T) { }) t.Run("Same username", func(t *testing.T) { - assert.ErrorIs(t, RenameUser(db.DefaultContext, user, user.Name), user_model.ErrUsernameNotChanged{UID: user.ID, Name: user.Name}) + assert.NoError(t, RenameUser(db.DefaultContext, user, user.Name)) }) t.Run("Non usable username", func(t *testing.T) { |