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

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

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

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.

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

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

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(),
+			"_csrf":          doc.GetCSRF(),
-			"last_commit":   lastCommit,
+			"last_commit":    lastCommit,
-			"tree_path":     "test.go",
+			"tree_path":      "test.go",
-			"content":       "package main",
+			"content":        "package main",
-			"commit_choice": "direct",
+			"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(),
+			"_csrf":          doc.GetCSRF(),
-			"last_commit":   lastCommit,
+			"last_commit":    lastCommit,
-			"tree_path":     "test.txt",
+			"tree_path":      "test.txt",
-			"content":       "Content",
+			"content":        "Content",
-			"commit_choice": "direct",
+			"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(),
+			"_csrf":          doc.GetCSRF(),
-			"last_commit":   lastCommit,
+			"last_commit":    lastCommit,
-			"tree_path":     "test.txt",
+			"tree_path":      "test.txt",
-			"content":       "Content",
+			"content":        "Content",
-			"commit_choice": "direct",
+			"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(),
+			"_csrf":          htmlDoc.GetCSRF(),
-			"last_commit":   lastCommit,
+			"last_commit":    lastCommit,
-			"tree_path":     filePath,
+			"tree_path":      filePath,
-			"content":       newContent,
+			"content":        newContent,
-			"commit_choice": "direct",
+			"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

@@ -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"),
+		"_csrf":          GetCSRF(t, session, "/user/settings"),
-		"commit_choice": "direct",
+		"commit_choice":  "direct",
-		"tree_path":     "test-file.md",
+		"tree_path":      "test-file.md",
-		"content":       "newly-added-test-file",
+		"content":        "newly-added-test-file",
+		"commit_mail_id": "32",
 	})
 
 	resp = session.MakeRequest(t, req, http.StatusSeeOther)