[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)
This commit is contained in:
Gusted 2023-11-17 00:17:40 +01:00 committed by Earl Warren
parent abc129c762
commit 079b995d49
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
10 changed files with 303 additions and 25 deletions

View file

@ -189,6 +189,25 @@ func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error)
return emails, nil 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 // GetEmailAddressByID gets a user's email address by ID
func GetEmailAddressByID(ctx context.Context, uid, id int64) (*EmailAddress, error) { func GetEmailAddressByID(ctx context.Context, uid, id int64) (*EmailAddress, error) {
// User ID is required for security reasons // User ID is required for security reasons

View file

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

View file

@ -1244,6 +1244,7 @@ editor.new_branch_name_desc = New branch name…
editor.cancel = Cancel editor.cancel = Cancel
editor.filename_cannot_be_empty = The filename cannot be empty. editor.filename_cannot_be_empty = The filename cannot be empty.
editor.filename_is_invalid = The filename is invalid: "%s". 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_does_not_exist = Branch "%s" does not exist in this repository.
editor.branch_already_exists = Branch "%s" already exists 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. editor.directory_is_a_file = Directory name "%s" is already used as a filename in this repository.

View file

@ -14,6 +14,7 @@ import (
git_model "code.gitea.io/gitea/models/git" git_model "code.gitea.io/gitea/models/git"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit" "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/base"
"code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/charset"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
@ -99,6 +100,27 @@ func getParentTreeFields(treePath string) (treeNames, treePaths []string) {
return treeNames, treePaths 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) { func editFile(ctx *context.Context, isNewFile bool) {
ctx.Data["PageIsEdit"] = true ctx.Data["PageIsEdit"] = true
ctx.Data["IsNewFile"] = isNewFile ctx.Data["IsNewFile"] = isNewFile
@ -177,6 +199,12 @@ func editFile(ctx *context.Context, isNewFile bool) {
treeNames = append(treeNames, fileName) treeNames = append(treeNames, fileName)
} }
commitEmails, err := getSelectableEmailAddresses(ctx)
if err != nil {
ctx.ServerError("getSelectableEmailAddresses", err)
return
}
ctx.Data["TreeNames"] = treeNames ctx.Data["TreeNames"] = treeNames
ctx.Data["TreePaths"] = treePaths ctx.Data["TreePaths"] = treePaths
ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() 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["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",")
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath) ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath)
ctx.Data["CommitMails"] = commitEmails
ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail()
ctx.HTML(http.StatusOK, tplEditFile) ctx.HTML(http.StatusOK, tplEditFile)
} }
@ -227,6 +257,12 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
branchName = form.NewBranchName branchName = form.NewBranchName
} }
commitEmails, err := getSelectableEmailAddresses(ctx)
if err != nil {
ctx.ServerError("getSelectableEmailAddresses", err)
return
}
ctx.Data["PageIsEdit"] = true ctx.Data["PageIsEdit"] = true
ctx.Data["PageHasPosted"] = true ctx.Data["PageHasPosted"] = true
ctx.Data["IsNewFile"] = isNewFile 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["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",")
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, form.TreePath) ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, form.TreePath)
ctx.Data["CommitMails"] = commitEmails
ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail()
if ctx.HasError() { if ctx.HasError() {
ctx.HTML(http.StatusOK, tplEditFile) ctx.HTML(http.StatusOK, tplEditFile)
@ -277,6 +315,30 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
operation = "create" 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{ if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{
LastCommitID: form.LastCommit, LastCommitID: form.LastCommit,
OldBranch: ctx.Repo.BranchName, OldBranch: ctx.Repo.BranchName,
@ -291,6 +353,8 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
}, },
}, },
Signoff: form.Signoff, Signoff: form.Signoff,
Author: gitIdentity,
Committer: gitIdentity,
}); err != nil { }); err != nil {
// This is where we handle all the errors thrown by files_service.ChangeRepoFiles // This is where we handle all the errors thrown by files_service.ChangeRepoFiles
if git.IsErrNotExist(err) { if git.IsErrNotExist(err) {

View file

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

View file

@ -123,6 +123,8 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m
if committer.Name != "" { if committer.Name != "" {
committerUser.FullName = committer.Name committerUser.FullName = committer.Name
} }
// Use the provided email and not revert to placeholder mail.
committerUser.KeepEmailPrivate = false
} else { } else {
committerUser = &user_model.User{ committerUser = &user_model.User{
FullName: committer.Name, FullName: committer.Name,
@ -136,6 +138,8 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m
if authorUser.Name != "" { if authorUser.Name != "" {
authorUser.FullName = author.Name authorUser.FullName = author.Name
} }
// Use the provided email and not revert to placeholder mail.
authorUser.KeepEmailPrivate = false
} else { } else {
authorUser = &user_model.User{ authorUser = &user_model.User{
FullName: author.Name, FullName: author.Name,

View file

@ -66,6 +66,14 @@
</div> </div>
{{end}} {{end}}
</div> </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> </div>
<button id="commit-button" type="submit" class="ui primary button"> <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}} {{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}}

View file

@ -31,6 +31,7 @@ func TestRepoLanguages(t *testing.T) {
"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) session.MakeRequest(t, req, http.StatusSeeOther)

View file

@ -4,14 +4,21 @@
package integration package integration
import ( import (
"fmt"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
"path" "path"
"testing" "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" 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/json"
"code.gitea.io/gitea/modules/translation"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -35,6 +42,7 @@ func TestCreateFile(t *testing.T) {
"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) session.MakeRequest(t, req, http.StatusSeeOther)
}) })
@ -72,6 +80,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
"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) resp = session.MakeRequest(t, req, http.StatusOK)
@ -116,6 +125,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa
"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) session.MakeRequest(t, req, http.StatusSeeOther)
@ -146,6 +156,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra
"content": newContent, "content": newContent,
"commit_choice": "commit-to-new-branch", "commit_choice": "commit-to-new-branch",
"new_branch_name": targetBranch, "new_branch_name": targetBranch,
"commit_mail_id": "-1",
}, },
) )
session.MakeRequest(t, req, http.StatusSeeOther) 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") 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)
})
})
}

View file

@ -58,6 +58,7 @@ func TestEmptyRepoAddFile(t *testing.T) {
"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) resp = session.MakeRequest(t, req, http.StatusSeeOther)