[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

(cherry picked from commit 564e701f40)
(cherry picked from commit de8f2e03cc)
(cherry picked from commit 0182cff12e)
(cherry picked from commit 9c74254d46)
(cherry picked from commit 2f0b68f821)
(cherry picked from commit 079b995d49)
(cherry picked from commit 6952ea6ee3)
(cherry picked from commit 6c7d5a5d14)
(cherry picked from commit 49c39f0ed5)
(cherry picked from commit a8f9727388)

models/user/email_address.go

@@ -231,6 +231,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

models/user/email_address_test.go

@@ -4,6 +4,7 @@
 package user_test
 
 import (
+	"fmt"
 	"testing"
 
 	"code.gitea.io/gitea/models/db"
@@ -219,3 +220,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)
+		})
+	}
+}

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) {

services/forms/repo_form.go

@@ -760,6 +760,7 @@ type EditRepoFileForm struct {
 	CommitChoice  string `binding:"Required;MaxSize(50)"`
 	NewBranchName string `binding:"GitRefName;MaxSize(100)"`
 	LastCommit    string
+	CommitMailID  int64 `binding:"Required"`
 	Signoff       bool
 }
 

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,

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}}

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)
 

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)
+		})
+	})
+}

tests/integration/empty_repo_test.go

@@ -50,10 +50,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)