From 0144817971012bed2b00784064c37b1e7e5acff3 Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Tue, 2 May 2017 03:49:55 +0300 Subject: [PATCH] Do not allow commiting to protected branch from online editor (#1502) * Do not allow commiting to protected branch from online editor * Add editor integration tests for adding new file and not allowing to add new file to protected branch --- Makefile | 6 +- integrations/editor_test.go | 106 ++++++++++++++++++++++++ integrations/html_helper.go | 110 +++++++++++++++++++++++++ integrations/integration_test.go | 83 ++++++++++++++++++- models/branches.go | 17 ++++ models/fixtures/user.yml | 5 +- modules/context/repo.go | 10 +++ options/locale/locale_en-US.ini | 1 + routers/repo/editor.go | 59 +++++++++++-- templates/repo/editor/commit_form.tmpl | 4 +- 10 files changed, 386 insertions(+), 15 deletions(-) create mode 100644 integrations/editor_test.go create mode 100644 integrations/html_helper.go diff --git a/Makefile b/Makefile index 28a2d8ff8..b0ca71e21 100644 --- a/Makefile +++ b/Makefile @@ -86,15 +86,15 @@ test-vendor: .PHONY: test-sqlite test-sqlite: integrations.test - GITEA_CONF=integrations/sqlite.ini ./integrations.test + GITEA_ROOT=${CURDIR} GITEA_CONF=integrations/sqlite.ini ./integrations.test .PHONY: test-mysql test-mysql: integrations.test - GITEA_CONF=integrations/mysql.ini ./integrations.test + GITEA_ROOT=${CURDIR} GITEA_CONF=integrations/mysql.ini ./integrations.test .PHONY: test-pgsql test-pgsql: integrations.test - GITEA_CONF=integrations/pgsql.ini ./integrations.test + GITEA_ROOT=${CURDIR} GITEA_CONF=integrations/pgsql.ini ./integrations.test integrations.test: $(SOURCES) go test -c code.gitea.io/gitea/integrations -tags 'sqlite' diff --git a/integrations/editor_test.go b/integrations/editor_test.go new file mode 100644 index 000000000..df0cfaa29 --- /dev/null +++ b/integrations/editor_test.go @@ -0,0 +1,106 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "bytes" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCreateFile(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user2", "password") + + // Request editor page + req, err := http.NewRequest("GET", "/user2/repo1/_new/master/", nil) + assert.NoError(t, err) + resp := session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + doc, err := NewHtmlParser(resp.Body) + assert.NoError(t, err) + lastCommit := doc.GetInputValueByName("last_commit") + assert.NotEmpty(t, lastCommit) + + // Save new file to master branch + req, err = http.NewRequest("POST", "/user2/repo1/_new/master/", + bytes.NewBufferString(url.Values{ + "_csrf": []string{doc.GetInputValueByName("_csrf")}, + "last_commit": []string{lastCommit}, + "tree_path": []string{"test.txt"}, + "content": []string{"Content"}, + "commit_choice": []string{"direct"}, + }.Encode()), + ) + assert.NoError(t, err) + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + resp = session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusFound, resp.HeaderCode) +} + +func TestCreateFileOnProtectedBranch(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user2", "password") + + // Open repository branch settings + req, err := http.NewRequest("GET", "/user2/repo1/settings/branches", nil) + assert.NoError(t, err) + resp := session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + doc, err := NewHtmlParser(resp.Body) + assert.NoError(t, err) + + // Change master branch to protected + req, err = http.NewRequest("POST", "/user2/repo1/settings/branches?action=protected_branch", + bytes.NewBufferString(url.Values{ + "_csrf": []string{doc.GetInputValueByName("_csrf")}, + "branchName": []string{"master"}, + "canPush": []string{"true"}, + }.Encode()), + ) + assert.NoError(t, err) + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + resp = session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + // Check if master branch has been locked successfully + flashCookie := session.GetCookie("macaron_flash") + assert.NotNil(t, flashCookie) + assert.EqualValues(t, flashCookie.Value, "success%3Dmaster%2BLocked%2Bsuccessfully") + + // Request editor page + req, err = http.NewRequest("GET", "/user2/repo1/_new/master/", nil) + assert.NoError(t, err) + resp = session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + doc, err = NewHtmlParser(resp.Body) + assert.NoError(t, err) + lastCommit := doc.GetInputValueByName("last_commit") + assert.NotEmpty(t, lastCommit) + + // Save new file to master branch + req, err = http.NewRequest("POST", "/user2/repo1/_new/master/", + bytes.NewBufferString(url.Values{ + "_csrf": []string{doc.GetInputValueByName("_csrf")}, + "last_commit": []string{lastCommit}, + "tree_path": []string{"test.txt"}, + "content": []string{"Content"}, + "commit_choice": []string{"direct"}, + }.Encode()), + ) + assert.NoError(t, err) + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + resp = session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + // Check body for error message + assert.Contains(t, string(resp.Body), "Can not commit to protected branch 'master'.") +} diff --git a/integrations/html_helper.go b/integrations/html_helper.go new file mode 100644 index 000000000..db4e2953e --- /dev/null +++ b/integrations/html_helper.go @@ -0,0 +1,110 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "bytes" + + "golang.org/x/net/html" +) + +type HtmlDoc struct { + doc *html.Node + body *html.Node +} + +func NewHtmlParser(content []byte) (*HtmlDoc, error) { + doc, err := html.Parse(bytes.NewReader(content)) + if err != nil { + return nil, err + } + + return &HtmlDoc{doc: doc}, nil +} + +func (doc *HtmlDoc) GetBody() *html.Node { + if doc.body == nil { + var b *html.Node + var f func(*html.Node) + f = func(n *html.Node) { + if n.Type == html.ElementNode && n.Data == "body" { + b = n + return + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + f(c) + } + } + f(doc.doc) + if b != nil { + doc.body = b + } else { + doc.body = doc.doc + } + } + return doc.body +} + +func (doc *HtmlDoc) GetAttribute(n *html.Node, key string) (string, bool) { + for _, attr := range n.Attr { + if attr.Key == key { + return attr.Val, true + } + } + return "", false +} + +func (doc *HtmlDoc) checkAttr(n *html.Node, attr, val string) bool { + if n.Type == html.ElementNode { + s, ok := doc.GetAttribute(n, attr) + if ok && s == val { + return true + } + } + return false +} + +func (doc *HtmlDoc) traverse(n *html.Node, attr, val string) *html.Node { + if doc.checkAttr(n, attr, val) { + return n + } + + for c := n.FirstChild; c != nil; c = c.NextSibling { + result := doc.traverse(c, attr, val) + if result != nil { + return result + } + } + + return nil +} + +func (doc *HtmlDoc) GetElementById(id string) *html.Node { + return doc.traverse(doc.GetBody(), "id", id) +} + +func (doc *HtmlDoc) GetInputValueById(id string) string { + inp := doc.GetElementById(id) + if inp == nil { + return "" + } + + val, _ := doc.GetAttribute(inp, "value") + return val +} + +func (doc *HtmlDoc) GetElementByName(name string) *html.Node { + return doc.traverse(doc.GetBody(), "name", name) +} + +func (doc *HtmlDoc) GetInputValueByName(name string) string { + inp := doc.GetElementByName(name) + if inp == nil { + return "" + } + + val, _ := doc.GetAttribute(inp, "value") + return val +} diff --git a/integrations/integration_test.go b/integrations/integration_test.go index e13c3b512..6696ff65f 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -11,7 +11,10 @@ import ( "io" "log" "net/http" + "net/http/cookiejar" + "net/url" "os" + "strings" "testing" "code.gitea.io/gitea/models" @@ -60,6 +63,10 @@ func initIntegrationTest() { fmt.Println("Environment variable $GITEA_CONF not set") os.Exit(1) } + if os.Getenv("GITEA_ROOT") == "" { + fmt.Println("Environment variable $GITEA_ROOT not set") + os.Exit(1) + } setting.NewContext() models.LoadConfigs() @@ -103,13 +110,82 @@ func prepareTestEnv(t *testing.T) { assert.NoError(t, com.CopyDir("integrations/gitea-integration-meta", "integrations/gitea-integration")) } +type TestSession struct { + jar http.CookieJar +} + +func (s *TestSession) GetCookie(name string) *http.Cookie { + baseURL, err := url.Parse(setting.AppURL) + if err != nil { + return nil + } + + for _, c := range s.jar.Cookies(baseURL) { + if c.Name == name { + return c + } + } + return nil +} + +func (s *TestSession) MakeRequest(t *testing.T, req *http.Request) *TestResponse { + baseURL, err := url.Parse(setting.AppURL) + assert.NoError(t, err) + for _, c := range s.jar.Cookies(baseURL) { + req.AddCookie(c) + } + resp := MakeRequest(req) + + ch := http.Header{} + ch.Add("Cookie", strings.Join(resp.Headers["Set-Cookie"], ";")) + cr := http.Request{Header: ch} + s.jar.SetCookies(baseURL, cr.Cookies()) + + return resp +} + +func loginUser(t *testing.T, userName, password string) *TestSession { + req, err := http.NewRequest("GET", "/user/login", nil) + assert.NoError(t, err) + resp := MakeRequest(req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + doc, err := NewHtmlParser(resp.Body) + assert.NoError(t, err) + + req, err = http.NewRequest("POST", "/user/login", + bytes.NewBufferString(url.Values{ + "_csrf": []string{doc.GetInputValueByName("_csrf")}, + "user_name": []string{userName}, + "password": []string{password}, + }.Encode()), + ) + assert.NoError(t, err) + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + resp = MakeRequest(req) + assert.EqualValues(t, http.StatusFound, resp.HeaderCode) + + ch := http.Header{} + ch.Add("Cookie", strings.Join(resp.Headers["Set-Cookie"], ";")) + cr := http.Request{Header: ch} + + jar, err := cookiejar.New(nil) + assert.NoError(t, err) + baseURL, err := url.Parse(setting.AppURL) + assert.NoError(t, err) + jar.SetCookies(baseURL, cr.Cookies()) + + return &TestSession{jar: jar} +} + type TestResponseWriter struct { HeaderCode int Writer io.Writer + Headers http.Header } func (w *TestResponseWriter) Header() http.Header { - return make(map[string][]string) + return w.Headers } func (w *TestResponseWriter) Write(b []byte) (int, error) { @@ -123,16 +199,19 @@ func (w *TestResponseWriter) WriteHeader(n int) { type TestResponse struct { HeaderCode int Body []byte + Headers http.Header } func MakeRequest(req *http.Request) *TestResponse { buffer := bytes.NewBuffer(nil) respWriter := &TestResponseWriter{ - Writer: buffer, + Writer: buffer, + Headers: make(map[string][]string), } mac.ServeHTTP(respWriter, req) return &TestResponse{ HeaderCode: respWriter.HeaderCode, Body: buffer.Bytes(), + Headers: respWriter.Headers, } } diff --git a/models/branches.go b/models/branches.go index 322d33daa..a71c0e962 100644 --- a/models/branches.go +++ b/models/branches.go @@ -63,6 +63,23 @@ func (repo *Repository) GetProtectedBranches() ([]*ProtectedBranch, error) { return protectedBranches, x.Find(&protectedBranches, &ProtectedBranch{RepoID: repo.ID}) } +// IsProtectedBranch checks if branch is protected +func (repo *Repository) IsProtectedBranch(branchName string) (bool, error) { + protectedBranch := &ProtectedBranch{ + RepoID: repo.ID, + BranchName: branchName, + } + + has, err := x.Get(protectedBranch) + if err != nil { + return true, err + } else if has { + return true, nil + } + + return false, nil +} + // AddProtectedBranch add protection to branch func (repo *Repository) AddProtectedBranch(branchName string, canPush bool) error { protectedBranch := &ProtectedBranch{ diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 409747aa1..dd8554c58 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -18,15 +18,16 @@ name: user2 full_name: User Two email: user2@example.com - passwd: password + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual - salt: salt + salt: ZogKvWdyEx is_admin: false avatar: avatar2 avatar_email: user2@example.com num_repos: 2 num_stars: 2 num_followers: 1 + is_active: true - id: 3 diff --git a/modules/context/repo.go b/modules/context/repo.go index 4deae2ebb..d2e5e0079 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -75,6 +75,16 @@ func (r *Repository) CanEnableEditor() bool { return r.Repository.CanEnableEditor() && r.IsViewBranch && r.IsWriter() } +// CanCommitToBranch returns true if repository is editable and user has proper access level +// and branch is not protected +func (r *Repository) CanCommitToBranch() (bool, error) { + protectedBranch, err := r.Repository.IsProtectedBranch(r.BranchName) + if err != nil { + return false, err + } + return r.CanEnableEditor() && !protectedBranch, nil +} + // GetEditorconfig returns the .editorconfig definition if found in the // HEAD of the default repo branch. func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4ffed1dda..515ce72a2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -560,6 +560,7 @@ editor.fail_to_update_file = Failed to update/create file '%s' with error: %v editor.add_subdir = Add subdirectory... editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v editor.upload_files_to_dir = Upload files to '%s' +editor.cannot_commit_to_protected_branch = Can not commit to protected branch '%s'. commits.commits = Commits commits.search = Search commits diff --git a/routers/repo/editor.go b/routers/repo/editor.go index 0c4220472..e84039064 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -26,13 +26,26 @@ const ( tplEditDiffPreview base.TplName = "repo/editor/diff_preview" tplDeleteFile base.TplName = "repo/editor/delete" tplUploadFile base.TplName = "repo/editor/upload" + + frmCommitChoiceDirect string = "direct" + frmCommitChoiceNewBranch string = "commit-to-new-branch" ) +func renderCommitRights(ctx *context.Context) bool { + canCommit, err := ctx.Repo.CanCommitToBranch() + if err != nil { + log.Error(4, "CanCommitToBranch: %v", err) + } + ctx.Data["CanCommitToBranch"] = canCommit + return canCommit +} + func editFile(ctx *context.Context, isNewFile bool) { ctx.Data["PageIsEdit"] = true ctx.Data["IsNewFile"] = isNewFile ctx.Data["RequireHighlightJS"] = true ctx.Data["RequireSimpleMDE"] = true + canCommit := renderCommitRights(ctx) var treeNames []string if len(ctx.Repo.TreePath) > 0 { @@ -90,7 +103,11 @@ func editFile(ctx *context.Context, isNewFile bool) { ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName ctx.Data["commit_summary"] = "" ctx.Data["commit_message"] = "" - ctx.Data["commit_choice"] = "direct" + if canCommit { + ctx.Data["commit_choice"] = frmCommitChoiceDirect + } else { + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + } ctx.Data["new_branch_name"] = "" ctx.Data["last_commit"] = ctx.Repo.Commit.ID ctx.Data["MarkdownFileExts"] = strings.Join(setting.Markdown.FileExtensions, ",") @@ -116,6 +133,7 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo ctx.Data["IsNewFile"] = isNewFile ctx.Data["RequireHighlightJS"] = true ctx.Data["RequireSimpleMDE"] = true + canCommit := renderCommitRights(ctx) oldBranchName := ctx.Repo.BranchName branchName := oldBranchName @@ -123,7 +141,7 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo lastCommit := form.LastCommit form.LastCommit = ctx.Repo.Commit.ID.String() - if form.CommitChoice == "commit-to-new-branch" { + if form.CommitChoice == frmCommitChoiceNewBranch { branchName = form.NewBranchName } @@ -164,6 +182,11 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchName), tplEditFile, &form) return } + } else if !canCommit { + ctx.Data["Err_NewBranchName"] = true + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + ctx.RenderWithErr(ctx.Tr("repo.editor.cannot_commit_to_protected_branch", branchName), tplEditFile, &form) + return } var newTreePath string @@ -317,10 +340,17 @@ func DeleteFile(ctx *context.Context) { ctx.Data["PageIsDelete"] = true ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName ctx.Data["TreePath"] = ctx.Repo.TreePath + canCommit := renderCommitRights(ctx) + ctx.Data["commit_summary"] = "" ctx.Data["commit_message"] = "" - ctx.Data["commit_choice"] = "direct" + if canCommit { + ctx.Data["commit_choice"] = frmCommitChoiceDirect + } else { + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + } ctx.Data["new_branch_name"] = "" + ctx.HTML(200, tplDeleteFile) } @@ -329,11 +359,12 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) { ctx.Data["PageIsDelete"] = true ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName ctx.Data["TreePath"] = ctx.Repo.TreePath + canCommit := renderCommitRights(ctx) oldBranchName := ctx.Repo.BranchName branchName := oldBranchName - if form.CommitChoice == "commit-to-new-branch" { + if form.CommitChoice == frmCommitChoiceNewBranch { branchName = form.NewBranchName } ctx.Data["commit_summary"] = form.CommitSummary @@ -352,6 +383,11 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) { ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchName), tplDeleteFile, &form) return } + } else if !canCommit { + ctx.Data["Err_NewBranchName"] = true + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + ctx.RenderWithErr(ctx.Tr("repo.editor.cannot_commit_to_protected_branch", branchName), tplDeleteFile, &form) + return } message := strings.TrimSpace(form.CommitSummary) @@ -390,6 +426,7 @@ func renderUploadSettings(ctx *context.Context) { func UploadFile(ctx *context.Context) { ctx.Data["PageIsUpload"] = true renderUploadSettings(ctx) + canCommit := renderCommitRights(ctx) // We must at least have one element for user to input. treeNames := []string{""} @@ -401,7 +438,11 @@ func UploadFile(ctx *context.Context) { ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName ctx.Data["commit_summary"] = "" ctx.Data["commit_message"] = "" - ctx.Data["commit_choice"] = "direct" + if canCommit { + ctx.Data["commit_choice"] = frmCommitChoiceDirect + } else { + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + } ctx.Data["new_branch_name"] = "" ctx.HTML(200, tplUploadFile) @@ -411,11 +452,12 @@ func UploadFile(ctx *context.Context) { func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) { ctx.Data["PageIsUpload"] = true renderUploadSettings(ctx) + canCommit := renderCommitRights(ctx) oldBranchName := ctx.Repo.BranchName branchName := oldBranchName - if form.CommitChoice == "commit-to-new-branch" { + if form.CommitChoice == frmCommitChoiceNewBranch { branchName = form.NewBranchName } @@ -446,6 +488,11 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) { ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchName), tplUploadFile, &form) return } + } else if !canCommit { + ctx.Data["Err_NewBranchName"] = true + ctx.Data["commit_choice"] = frmCommitChoiceNewBranch + ctx.RenderWithErr(ctx.Tr("repo.editor.cannot_commit_to_protected_branch", branchName), tplUploadFile, &form) + return } var newTreePath string diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index 96b0dcbd3..43021ca59 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -10,7 +10,7 @@
-
+
\ No newline at end of file +