diff --git a/models/user/email_address.go b/models/user/email_address.go index 2af2621f5f..b795a7e94b 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 71fdbf8b20..6207b531bd 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1244,6 +1244,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 @@ </div> {{end}} </div> + <div class="field {{if .Err_CommitMailID}}error{{end}}"> + <label for="commit_mail_id">Commit email</label> + <select class="ui selection dropdown" name="commit_mail_id"> + {{range .CommitMails}} + <option{{if eq $.DefaultCommitMail .Email}} selected="selected"{{end}} value="{{.ID}}">{{.Email}}</option> + {{end}} + </select> + </div> </div> <button id="commit-button" type="submit" class="ui primary button"> {{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}} diff --git a/tests/integration/api_repo_languages_test.go b/tests/integration/api_repo_languages_test.go index 1045aef57d..3572e2a8e1 100644 --- a/tests/integration/api_repo_languages_test.go +++ b/tests/integration/api_repo_languages_test.go @@ -26,11 +26,12 @@ func TestRepoLanguages(t *testing.T) { // Save new file to master branch req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ - "_csrf": doc.GetCSRF(), - "last_commit": lastCommit, - "tree_path": "test.go", - "content": "package main", - "commit_choice": "direct", + "_csrf": doc.GetCSRF(), + "last_commit": lastCommit, + "tree_path": "test.go", + "content": "package main", + "commit_choice": "direct", + "commit_mail_id": "3", }) session.MakeRequest(t, req, http.StatusSeeOther) diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index de2a9d7d23..5f005a7320 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -4,14 +4,21 @@ package integration import ( + "fmt" "net/http" "net/http/httptest" "net/url" "path" "testing" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" gitea_context "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) @@ -30,11 +37,12 @@ func TestCreateFile(t *testing.T) { // Save new file to master branch req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ - "_csrf": doc.GetCSRF(), - "last_commit": lastCommit, - "tree_path": "test.txt", - "content": "Content", - "commit_choice": "direct", + "_csrf": doc.GetCSRF(), + "last_commit": lastCommit, + "tree_path": "test.txt", + "content": "Content", + "commit_choice": "direct", + "commit_mail_id": "3", }) session.MakeRequest(t, req, http.StatusSeeOther) }) @@ -67,11 +75,12 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { // Save new file to master branch req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ - "_csrf": doc.GetCSRF(), - "last_commit": lastCommit, - "tree_path": "test.txt", - "content": "Content", - "commit_choice": "direct", + "_csrf": doc.GetCSRF(), + "last_commit": lastCommit, + "tree_path": "test.txt", + "content": "Content", + "commit_choice": "direct", + "commit_mail_id": "3", }) resp = session.MakeRequest(t, req, http.StatusOK) @@ -111,11 +120,12 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa // Submit the edits req = NewRequestWithValues(t, "POST", path.Join(user, repo, "_edit", branch, filePath), map[string]string{ - "_csrf": htmlDoc.GetCSRF(), - "last_commit": lastCommit, - "tree_path": filePath, - "content": newContent, - "commit_choice": "direct", + "_csrf": htmlDoc.GetCSRF(), + "last_commit": lastCommit, + "tree_path": filePath, + "content": newContent, + "commit_choice": "direct", + "commit_mail_id": "-1", }, ) session.MakeRequest(t, req, http.StatusSeeOther) @@ -146,6 +156,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra "content": newContent, "commit_choice": "commit-to-new-branch", "new_branch_name": targetBranch, + "commit_mail_id": "-1", }, ) session.MakeRequest(t, req, http.StatusSeeOther) @@ -171,3 +182,136 @@ func TestEditFileToNewBranch(t *testing.T) { testEditFileToNewBranch(t, session, "user2", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n") }) } + +func TestEditFileCommitMail(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + session := loginUser(t, user.Name) + link := path.Join("user2", "repo1", "_edit", "master", "README.md") + + lastCommitAndCSRF := func() (string, string) { + req := NewRequest(t, "GET", link) + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + lastCommit := htmlDoc.GetInputValueByName("last_commit") + assert.NotEmpty(t, lastCommit) + + return lastCommit, htmlDoc.GetCSRF() + } + + t.Run("Not activated", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 35, UID: user.ID}) + assert.False(t, email.IsActivated) + + lastCommit, csrf := lastCommitAndCSRF() + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": csrf, + "last_commit": lastCommit, + "tree_path": "README.md", + "content": "new_content", + "commit_choice": "direct", + "commit_mail_id": fmt.Sprintf("%d", email.ID), + }) + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + assert.Contains(t, + htmlDoc.doc.Find(".ui.negative.message").Text(), + translation.NewLocale("en-US").Tr("repo.editor.invalid_commit_mail"), + ) + }) + + t.Run("Not belong to user", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 1, IsActivated: true}) + assert.NotEqualValues(t, email.UID, user.ID) + + lastCommit, csrf := lastCommitAndCSRF() + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": csrf, + "last_commit": lastCommit, + "tree_path": "README.md", + "content": "new_content", + "commit_choice": "direct", + "commit_mail_id": fmt.Sprintf("%d", email.ID), + }) + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + assert.Contains(t, + htmlDoc.doc.Find(".ui.negative.message").Text(), + translation.NewLocale("en-US").Tr("repo.editor.invalid_commit_mail"), + ) + }) + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + gitRepo, _ := git.OpenRepository(git.DefaultContext, repo1.RepoPath()) + defer gitRepo.Close() + + t.Run("Placeholder mail", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + lastCommit, csrf := lastCommitAndCSRF() + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": csrf, + "last_commit": lastCommit, + "tree_path": "README.md", + "content": "authored by placeholder mail", + "commit_choice": "direct", + "commit_mail_id": "-1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + commit, err := gitRepo.GetCommitByPath("README.md") + assert.NoError(t, err) + + fileContent, err := commit.GetFileContent("README.md", 64) + assert.NoError(t, err) + assert.EqualValues(t, "authored by placeholder mail", fileContent) + + assert.EqualValues(t, "user2", commit.Author.Name) + assert.EqualValues(t, "user2@noreply.example.org", commit.Author.Email) + assert.EqualValues(t, "user2", commit.Committer.Name) + assert.EqualValues(t, "user2@noreply.example.org", commit.Committer.Email) + }) + + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Require that the user has KeepEmailPrivate enabled, because it needs + // to be tested that even with this setting enabled, it will use the + // provided mail and not revert to the placeholder one. + assert.True(t, user.KeepEmailPrivate) + + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 3, UID: user.ID, IsActivated: true}) + + lastCommit, csrf := lastCommitAndCSRF() + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": csrf, + "last_commit": lastCommit, + "tree_path": "README.md", + "content": "authored by activated mail", + "commit_choice": "direct", + "commit_mail_id": fmt.Sprintf("%d", email.ID), + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + commit, err := gitRepo.GetCommitByPath("README.md") + assert.NoError(t, err) + + fileContent, err := commit.GetFileContent("README.md", 64) + assert.NoError(t, err) + assert.EqualValues(t, "authored by activated mail", fileContent) + + assert.EqualValues(t, "user2", commit.Author.Name) + assert.EqualValues(t, email.Email, commit.Author.Email) + assert.EqualValues(t, "user2", commit.Committer.Name) + assert.EqualValues(t, email.Email, commit.Committer.Email) + }) + }) +} diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go index 78453f28a5..0e8fb8d087 100644 --- a/tests/integration/empty_repo_test.go +++ b/tests/integration/empty_repo_test.go @@ -55,10 +55,11 @@ func TestEmptyRepoAddFile(t *testing.T) { doc := NewHTMLParser(t, resp.Body).Find(`input[name="commit_choice"]`) assert.Empty(t, doc.AttrOr("checked", "_no_")) req = NewRequestWithValues(t, "POST", "/user30/empty/_new/"+setting.Repository.DefaultBranch, map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), - "commit_choice": "direct", - "tree_path": "test-file.md", - "content": "newly-added-test-file", + "_csrf": GetCSRF(t, session, "/user/settings"), + "commit_choice": "direct", + "tree_path": "test-file.md", + "content": "newly-added-test-file", + "commit_mail_id": "32", }) resp = session.MakeRequest(t, req, http.StatusSeeOther)