From 53602619635b6f77e855764b3d7853a8e8c36424 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 17 Apr 2020 20:26:43 +0200 Subject: [PATCH 1/4] use same process as in routers/repo/branch.go/deleteBranch --- routers/api/v1/api.go | 1 + routers/api/v1/repo/branch.go | 96 ++++++++++++++++++++++++++++++++++ templates/swagger/v1_json.tmpl | 41 +++++++++++++++ 3 files changed, 138 insertions(+) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index bce3bf245283..9e4b434010d7 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -664,6 +664,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/branches", func() { m.Get("", repo.ListBranches) m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch) + m.Delete("/*", context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch) }, reqRepoReader(models.UnitTypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 07c61595019f..731403c89cba 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -6,12 +6,15 @@ package repo import ( + "fmt" "net/http" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/repofiles" repo_module "code.gitea.io/gitea/modules/repository" api "code.gitea.io/gitea/modules/structs" ) @@ -81,6 +84,99 @@ func GetBranch(ctx *context.APIContext) { ctx.JSON(http.StatusOK, br) } +// DeleteBranch get a branch of a repository +func DeleteBranch(ctx *context.APIContext) { + // swagger:operation DELETE /repos/{owner}/{repo}/branches/{branch} repository repoDeleteBranch + // --- + // summary: Delete a specific branch from a repository + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: branch + // in: path + // description: branch to delete + // type: string + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + // "403": + // "$ref": "#/responses/error" + + if ctx.Repo.TreePath != "" { + // if TreePath != "", then URL contained extra slashes + // (i.e. "master/subbranch" instead of "master"), so branch does + // not exist + ctx.NotFound() + return + } + + isProtected, err := ctx.Repo.Repository.IsProtectedBranch(ctx.Repo.BranchName, ctx.User) + if err != nil { + ctx.InternalServerError(err) + return + } + if isProtected { + ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected")) + return + } + + branch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.BranchName) + if err != nil { + if git.IsErrBranchNotExist(err) { + ctx.NotFound(err) + } else { + ctx.Error(http.StatusInternalServerError, "GetBranch", err) + } + return + } + + c, err := branch.GetCommit() + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetCommit", err) + return + } + + if err := ctx.Repo.GitRepo.DeleteBranch(ctx.Repo.BranchName, git.DeleteBranchOptions{ + Force: true, + }); err != nil { + ctx.Error(http.StatusInternalServerError, "DeleteBranch", err) + return + } + + // Don't return error below this + if err := repofiles.PushUpdate( + ctx.Repo.Repository, + ctx.Repo.BranchName, + repofiles.PushUpdateOptions{ + RefFullName: git.BranchPrefix + ctx.Repo.BranchName, + OldCommitID: c.ID.String(), + NewCommitID: git.EmptySHA, + PusherID: ctx.User.ID, + PusherName: ctx.User.Name, + RepoUserName: ctx.Repo.Owner.Name, + RepoName: ctx.Repo.Repository.Name, + }); err != nil { + log.Error("Update: %v", err) + } + + if err := ctx.Repo.Repository.AddDeletedBranch(ctx.Repo.BranchName, c.ID.String(), ctx.User.ID); err != nil { + log.Warn("AddDeletedBranch: %v", err) + } + + ctx.Status(http.StatusNoContent) +} + // ListBranches list all the branches of a repository func ListBranches(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/branches repository repoListBranches diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index e9368a7d2ae0..3b74902199db 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2269,6 +2269,47 @@ "$ref": "#/responses/Branch" } } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Delete a specific branch from a repository", + "operationId": "repoDeleteBranch", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "branch to delete", + "name": "branch", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "403": { + "$ref": "#/responses/error" + } + } } }, "/repos/{owner}/{repo}/collaborators": { From 4dca94b34a9b7912cca51183e44aa0a14aba1a43 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 17 Apr 2020 21:01:44 +0200 Subject: [PATCH 2/4] make sure default branch can not be deleted --- integrations/api_branch_test.go | 14 ++++++++++++++ options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/branch.go | 5 +++++ routers/repo/branch.go | 7 ++++++- 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index b6452a6ab4ed..8417ab36c58e 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -80,6 +80,13 @@ func testAPIDeleteBranchProtection(t *testing.T, branchName string, expectedHTTP session.MakeRequest(t, req, expectedHTTPStatus) } +func testAPIDeleteBranch(t *testing.T, branchName string, expectedHTTPStatus int) { + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session) + req := NewRequestf(t, "DELETE", "/api/v1/repos/user2/repo1/branches/%s?token=%s", branchName, token) + session.MakeRequest(t, req, expectedHTTPStatus) +} + func TestAPIGetBranch(t *testing.T) { for _, test := range []struct { BranchName string @@ -106,10 +113,17 @@ func TestAPIBranchProtection(t *testing.T) { // Can only create once testAPICreateBranchProtection(t, "master", http.StatusForbidden) + // Can't delete a protected branch + testAPIDeleteBranch(t, "master", http.StatusForbidden) + testAPIGetBranchProtection(t, "master", http.StatusOK) testAPIEditBranchProtection(t, "master", &api.BranchProtection{ EnablePush: true, }, http.StatusOK) testAPIDeleteBranchProtection(t, "master", http.StatusNoContent) + + // Test branch deletion + testAPIDeleteBranch(t, "master", http.StatusForbidden) + testAPIDeleteBranch(t, "branch2", http.StatusNoContent) } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9653784f91fc..aaeb1274804e 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1671,6 +1671,7 @@ branch.deleted_by = Deleted by %s branch.restore_success = Branch '%s' has been restored. branch.restore_failed = Failed to restore branch '%s'. branch.protected_deletion_failed = Branch '%s' is protected. It cannot be deleted. +branch.dfault_deletion_failed = Branch '%s' is default branch. It cannot be deleted. branch.restore = Restore Branch '%s' branch.download = Download Branch '%s' branch.included_desc = This branch is part of the default branch diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 731403c89cba..57c74d7dab86 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -121,6 +121,11 @@ func DeleteBranch(ctx *context.APIContext) { return } + if ctx.Repo.Repository.DefaultBranch == ctx.Repo.BranchName { + ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch")) + return + } + isProtected, err := ctx.Repo.Repository.IsProtectedBranch(ctx.Repo.BranchName, ctx.User) if err != nil { ctx.InternalServerError(err) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 615450b368e9..7a3864885bae 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -56,8 +56,13 @@ func Branches(ctx *context.Context) { // DeleteBranchPost responses for delete merged branch func DeleteBranchPost(ctx *context.Context) { defer redirect(ctx) - branchName := ctx.Query("name") + + if ctx.Repo.Repository.DefaultBranch == ctx.Repo.BranchName { + ctx.Flash.Error(ctx.Tr("repo.branch.dfault_deletion_failed", branchName)) + return + } + isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User) if err != nil { log.Error("DeleteBranch: %v", err) From 95a6b84b513209c248d628196514dbcc642509b9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 18 Apr 2020 00:01:21 +0200 Subject: [PATCH 3/4] remove IsDefaultBranch from UI process - it is worth its own pull --- options/locale/locale_en-US.ini | 1 - routers/repo/branch.go | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index aaeb1274804e..9653784f91fc 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1671,7 +1671,6 @@ branch.deleted_by = Deleted by %s branch.restore_success = Branch '%s' has been restored. branch.restore_failed = Failed to restore branch '%s'. branch.protected_deletion_failed = Branch '%s' is protected. It cannot be deleted. -branch.dfault_deletion_failed = Branch '%s' is default branch. It cannot be deleted. branch.restore = Restore Branch '%s' branch.download = Download Branch '%s' branch.included_desc = This branch is part of the default branch diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 7a3864885bae..615450b368e9 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -56,13 +56,8 @@ func Branches(ctx *context.Context) { // DeleteBranchPost responses for delete merged branch func DeleteBranchPost(ctx *context.Context) { defer redirect(ctx) - branchName := ctx.Query("name") - - if ctx.Repo.Repository.DefaultBranch == ctx.Repo.BranchName { - ctx.Flash.Error(ctx.Tr("repo.branch.dfault_deletion_failed", branchName)) - return - } + branchName := ctx.Query("name") isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User) if err != nil { log.Error("DeleteBranch: %v", err) From ecc360aa896b9a51af1c4913a5e6312f0a10d375 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 18 Apr 2020 12:52:08 +0200 Subject: [PATCH 4/4] permissions --- routers/api/v1/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 9e4b434010d7..225f6a5325cd 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -664,7 +664,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/branches", func() { m.Get("", repo.ListBranches) m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch) - m.Delete("/*", context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch) + m.Delete("/*", reqRepoWriter(models.UnitTypeCode), context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch) }, reqRepoReader(models.UnitTypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections)