From 0f791220532c39aaa258b68a7c70238e18893054 Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 26 Mar 2024 01:39:15 +0100 Subject: [PATCH] [BUG] Detect protected branch on branch rename - If a branch cannot be renamed due to a protected branch rule, show this error in the UI instead of throwing an internal server error. - Add integration test (also simplify the existing one). - Resolves #2751 --- options/locale/locale_en-US.ini | 1 + routers/web/repo/setting/protected_branch.go | 8 ++- tests/integration/rename_branch_test.go | 75 ++++++++++++++------ 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index cc0fa143c..86975748b 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2513,6 +2513,7 @@ settings.lfs_pointers.inRepo=In Repo settings.lfs_pointers.exists=Exists in store settings.lfs_pointers.accessible=Accessible to User settings.lfs_pointers.associateAccessible=Associate accessible %d OIDs +settings.rename_branch_failed_protected=Cannot rename branch %s because it is a protected branch. settings.rename_branch_failed_exist=Cannot rename branch because target branch %s exists. settings.rename_branch_failed_not_exist=Cannot rename branch %s because it does not exist. settings.rename_branch_success =Branch %s was successfully renamed to %s. diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index b37520a39..7ee67e592 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -4,6 +4,7 @@ package setting import ( + "errors" "fmt" "net/http" "net/url" @@ -316,7 +317,12 @@ func RenameBranchPost(ctx *context.Context) { msg, err := repository.RenameBranch(ctx, ctx.Repo.Repository, ctx.Doer, ctx.Repo.GitRepo, form.From, form.To) if err != nil { - ctx.ServerError("RenameBranch", err) + if errors.Is(err, git_model.ErrBranchIsProtected) { + ctx.Flash.Error(ctx.Tr("repo.settings.rename_branch_failed_protected", form.To)) + ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink)) + } else { + ctx.ServerError("RenameBranch", err) + } return } diff --git a/tests/integration/rename_branch_test.go b/tests/integration/rename_branch_test.go index 703fc243a..b037178f9 100644 --- a/tests/integration/rename_branch_test.go +++ b/tests/integration/rename_branch_test.go @@ -10,6 +10,7 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + gitea_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -18,33 +19,63 @@ import ( func TestRenameBranch(t *testing.T) { defer tests.PrepareTestEnv(t)() - unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: 1, Name: "master"}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: "master"}) // get branch setting page session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user2/repo1/settings/branches") - resp := session.MakeRequest(t, req, http.StatusOK) - htmlDoc := NewHTMLParser(t, resp.Body) + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - postData := map[string]string{ - "_csrf": htmlDoc.GetCSRF(), - "from": "master", - "to": "main", - } - req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", postData) - session.MakeRequest(t, req, http.StatusSeeOther) + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"), + "from": "master", + "to": "main", + }) + session.MakeRequest(t, req, http.StatusSeeOther) - // check new branch link - req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", postData) - session.MakeRequest(t, req, http.StatusOK) + // check new branch link + req = NewRequest(t, "GET", "/user2/repo1/src/branch/main/README.md") + session.MakeRequest(t, req, http.StatusOK) - // check old branch link - req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/master/README.md", postData) - resp = session.MakeRequest(t, req, http.StatusSeeOther) - location := resp.Header().Get("Location") - assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location) + // check old branch link + req = NewRequest(t, "GET", "/user2/repo1/src/branch/master/README.md") + resp := session.MakeRequest(t, req, http.StatusSeeOther) + location := resp.Header().Get("Location") + assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location) - // check db - repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - assert.Equal(t, "main", repo1.DefaultBranch) + // check db + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.Equal(t, "main", repo1.DefaultBranch) + }) + + t.Run("Protected branch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Add protected branch + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches/edit"), + "rule_name": "*", + "enable_push": "true", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + // Verify it was added. + unittest.AssertExistsIf(t, true, &git_model.ProtectedBranch{RuleName: "*", RepoID: repo.ID}) + + req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"), + "from": "main", + "to": "main2", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.EqualValues(t, "error%3DCannot%2Brename%2Bbranch%2Bmain2%2Bbecause%2Bit%2Bis%2Ba%2Bprotected%2Bbranch.", flashCookie.Value) + + // Verify it didn't change. + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.Equal(t, "main", repo1.DefaultBranch) + }) }