From 564e701f407c0e110f3c7a4102bf7ed7902b815f Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 17 Nov 2023 00:17:40 +0100 Subject: [PATCH] [GITEA] Allow user to select email for file operations in Web UI - Add a dropdown to the web interface for changing files to select which Email should be used for the commit. It only shows (and verifies) that a activated mail can be used, while this isn't necessary, it's better to have this already in place. - Added integration testing. - Resolves https://codeberg.org/forgejo/forgejo/issues/281 --- models/user/email_address.go | 19 ++ models/user/email_address_test.go | 35 ++++ options/locale/locale_en-US.ini | 1 + routers/web/repo/editor.go | 66 ++++++- services/forms/repo_form.go | 1 + services/repository/files/file.go | 4 + templates/repo/editor/commit_form.tmpl | 8 + tests/integration/api_repo_languages_test.go | 11 +- tests/integration/editor_test.go | 174 +++++++++++++++++-- tests/integration/empty_repo_test.go | 9 +- 10 files changed, 303 insertions(+), 25 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index f1ed6692cf..d252e10dec 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -189,6 +189,25 @@ func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error) return emails, nil } +type ActivatedEmailAddress struct { + ID int64 + Email string +} + +func GetActivatedEmailAddresses(ctx context.Context, uid int64) ([]*ActivatedEmailAddress, error) { + emails := make([]*ActivatedEmailAddress, 0, 8) + if err := db.GetEngine(ctx). + Table("email_address"). + Select("id, email"). + Where("uid=?", uid). + And("is_activated=?", true). + Asc("id"). + Find(&emails); err != nil { + return nil, err + } + return emails, nil +} + // GetEmailAddressByID gets a user's email address by ID func GetEmailAddressByID(ctx context.Context, uid, id int64) (*EmailAddress, error) { // User ID is required for security reasons diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index 7f3ca75cfd..b20797d700 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -4,6 +4,7 @@ package user_test import ( + "fmt" "testing" "code.gitea.io/gitea/models/db" @@ -309,3 +310,37 @@ func TestEmailAddressValidate(t *testing.T) { }) } } + +func TestGetActivatedEmailAddresses(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + testCases := []struct { + UID int64 + expected []*user_model.ActivatedEmailAddress + }{ + { + UID: 1, + expected: []*user_model.ActivatedEmailAddress{{ID: 9, Email: "user1@example.com"}, {ID: 33, Email: "user1-2@example.com"}, {ID: 34, Email: "user1-3@example.com"}}, + }, + { + UID: 2, + expected: []*user_model.ActivatedEmailAddress{{ID: 3, Email: "user2@example.com"}}, + }, + { + UID: 4, + expected: []*user_model.ActivatedEmailAddress{{ID: 11, Email: "user4@example.com"}}, + }, + { + UID: 11, + expected: []*user_model.ActivatedEmailAddress{}, + }, + } + + for _, testCase := range testCases { + t.Run(fmt.Sprintf("User %d", testCase.UID), func(t *testing.T) { + emails, err := user_model.GetActivatedEmailAddresses(db.DefaultContext, testCase.UID) + assert.NoError(t, err) + assert.Equal(t, testCase.expected, emails) + }) + } +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1cabfb4c69..da1ab92781 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1242,6 +1242,7 @@ editor.new_branch_name_desc = New branch name… editor.cancel = Cancel editor.filename_cannot_be_empty = The filename cannot be empty. editor.filename_is_invalid = The filename is invalid: "%s". +editor.invalid_commit_mail = Invalid mail for creating a commit. editor.branch_does_not_exist = Branch "%s" does not exist in this repository. editor.branch_already_exists = Branch "%s" already exists in this repository. editor.directory_is_a_file = Directory name "%s" is already used as a filename in this repository. diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 5e7cd1caa3..c19e0aa32d 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -14,6 +14,7 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/context" @@ -99,6 +100,27 @@ func getParentTreeFields(treePath string) (treeNames, treePaths []string) { return treeNames, treePaths } +// getSelectableEmailAddresses returns which emails can be used by the user as +// email for a Git commiter. +func getSelectableEmailAddresses(ctx *context.Context) ([]*user_model.ActivatedEmailAddress, error) { + // Retrieve emails that the user could use for commiter identity. + commitEmails, err := user_model.GetActivatedEmailAddresses(ctx, ctx.Doer.ID) + if err != nil { + return nil, fmt.Errorf("GetActivatedEmailAddresses: %w", err) + } + + // Allow for the placeholder mail to be used. Use -1 as ID to identify + // this entry to be the placerholder mail of the user. + placeholderMail := &user_model.ActivatedEmailAddress{ID: -1, Email: ctx.Doer.GetPlaceholderEmail()} + if ctx.Doer.KeepEmailPrivate { + commitEmails = append([]*user_model.ActivatedEmailAddress{placeholderMail}, commitEmails...) + } else { + commitEmails = append(commitEmails, placeholderMail) + } + + return commitEmails, nil +} + func editFile(ctx *context.Context, isNewFile bool) { ctx.Data["PageIsEdit"] = true ctx.Data["IsNewFile"] = isNewFile @@ -177,6 +199,12 @@ func editFile(ctx *context.Context, isNewFile bool) { treeNames = append(treeNames, fileName) } + commitEmails, err := getSelectableEmailAddresses(ctx) + if err != nil { + ctx.ServerError("getSelectableEmailAddresses", err) + return + } + ctx.Data["TreeNames"] = treeNames ctx.Data["TreePaths"] = treePaths ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() @@ -192,6 +220,8 @@ func editFile(ctx *context.Context, isNewFile bool) { ctx.Data["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",") ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath) + ctx.Data["CommitMails"] = commitEmails + ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail() ctx.HTML(http.StatusOK, tplEditFile) } @@ -227,6 +257,12 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b branchName = form.NewBranchName } + commitEmails, err := getSelectableEmailAddresses(ctx) + if err != nil { + ctx.ServerError("getSelectableEmailAddresses", err) + return + } + ctx.Data["PageIsEdit"] = true ctx.Data["PageHasPosted"] = true ctx.Data["IsNewFile"] = isNewFile @@ -243,6 +279,8 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b ctx.Data["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",") ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, form.TreePath) + ctx.Data["CommitMails"] = commitEmails + ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail() if ctx.HasError() { ctx.HTML(http.StatusOK, tplEditFile) @@ -277,6 +315,30 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b operation = "create" } + gitIdentity := &files_service.IdentityOptions{ + Name: ctx.Doer.Name, + } + + // -1 is defined as placeholder email. + if form.CommitMailID == -1 { + gitIdentity.Email = ctx.Doer.GetPlaceholderEmail() + } else { + // Check if the given email is activated. + email, err := user_model.GetEmailAddressByID(ctx, ctx.Doer.ID, form.CommitMailID) + if err != nil { + ctx.ServerError("GetEmailAddressByID", err) + return + } + + if email == nil || !email.IsActivated { + ctx.Data["Err_CommitMailID"] = true + ctx.RenderWithErr(ctx.Tr("repo.editor.invalid_commit_mail"), tplEditFile, &form) + return + } + + gitIdentity.Email = email.Email + } + if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{ LastCommitID: form.LastCommit, OldBranch: ctx.Repo.BranchName, @@ -290,7 +352,9 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b ContentReader: strings.NewReader(strings.ReplaceAll(form.Content, "\r", "")), }, }, - Signoff: form.Signoff, + Signoff: form.Signoff, + Author: gitIdentity, + Committer: gitIdentity, }); err != nil { // This is where we handle all the errors thrown by files_service.ChangeRepoFiles if git.IsErrNotExist(err) { diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 5df7ec8fd6..2542d64cd2 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -766,6 +766,7 @@ type EditRepoFileForm struct { CommitChoice string `binding:"Required;MaxSize(50)"` NewBranchName string `binding:"GitRefName;MaxSize(100)"` LastCommit string + CommitMailID int64 `binding:"Required"` Signoff bool } diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 16783f5b5f..852cca0371 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -123,6 +123,8 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m if committer.Name != "" { committerUser.FullName = committer.Name } + // Use the provided email and not revert to placeholder mail. + committerUser.KeepEmailPrivate = false } else { committerUser = &user_model.User{ FullName: committer.Name, @@ -136,6 +138,8 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m if authorUser.Name != "" { authorUser.FullName = author.Name } + // Use the provided email and not revert to placeholder mail. + authorUser.KeepEmailPrivate = false } else { authorUser = &user_model.User{ FullName: author.Name, diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index 34dde576a1..6fd240da62 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -66,6 +66,14 @@ {{end}} +
+ + +