diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index f336605066b..239e98c1b70 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2663,7 +2663,7 @@ "repo.branch.new_branch_from": "Create new branch from \"%s\"", "repo.branch.renamed": "Branch %s was renamed to %s.", "repo.branch.rename_default_or_protected_branch_error": "Only admins can rename default or protected branches.", - "repo.branch.rename_protected_branch_failed": "This branch is protected by glob-based protection rules.", + "repo.branch.rename_protected_branch_failed": "Failed to rename branch due to branch protection rules.", "repo.branch.commits_divergence_from": "Commit divergence: %[1]d behind and %[2]d ahead of %[3]s", "repo.branch.commits_no_divergence": "The same as branch %[1]s", "repo.tag.create_tag": "Create tag %s", diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 4624d7e738c..9bdc0c76b8b 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -515,7 +515,7 @@ func RenameBranch(ctx *context.APIContext) { case repo_model.IsErrUserDoesNotHaveAccessToRepo(err): ctx.APIError(http.StatusForbidden, "User must be a repo or site admin to rename default or protected branches.") case errors.Is(err, git_model.ErrBranchIsProtected): - ctx.APIError(http.StatusForbidden, "Branch is protected by glob-based protection rules.") + ctx.APIError(http.StatusForbidden, "Failed to rename branch due to branch protection rules.") default: ctx.APIErrorInternal(err) } diff --git a/services/repository/branch.go b/services/repository/branch.go index b076514590d..a580208af6a 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -442,6 +442,15 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m } } + // We also need to check if "to" matches with a protected branch rule. + rule, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, to) + if err != nil { + return "", err + } + if rule != nil && !rule.CanUserPush(ctx, doer) { + return "", git_model.ErrBranchIsProtected + } + if err := git_model.RenameBranch(ctx, repo, from, to, func(ctx context.Context, isDefault bool) error { err2 := gitrepo.RenameBranch(ctx, repo, from, to) if err2 != nil { diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 043aa10c7fb..1163c1e8c9c 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -237,7 +237,40 @@ func TestAPIRenameBranch(t *testing.T) { MakeRequest(t, req, http.StatusCreated) resp := testAPIRenameBranch(t, "user2", "user2", "repo1", from, "new-branch-name", http.StatusForbidden) - assert.Contains(t, resp.Body.String(), "Branch is protected by glob-based protection rules.") + assert.Contains(t, resp.Body.String(), "Failed to rename branch due to branch protection rules.") + }) + t.Run("RenameBranchToMatchProtectionRulesWithAllowedUser", func(t *testing.T) { + // allow an admin (the owner in this case) to rename a regular branch to one that matches a branch protection rule + repoName := "repo1" + ownerName := "user2" + from := "regular-branch-1" + ctx := NewAPITestContext(t, ownerName, repoName, auth_model.AccessTokenScopeWriteRepository) + testAPICreateBranch(t, ctx.Session, ownerName, repoName, "", from, http.StatusCreated) + + // NOTE: The protected/** branch protection rule was created in a previous test, with push enabled. + testAPIRenameBranch(t, ownerName, ownerName, repoName, from, "protected/2", http.StatusNoContent) + }) + t.Run("RenameBranchToMatchProtectionRulesWithUnauthorizedUser", func(t *testing.T) { + // don't allow renaming a regular branch to a protected branch if the doer is not in the push whitelist + repoName := "repo1" + ownerName := "user2" + pushWhitelist := []string{ownerName} + token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository) + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/branch_protections", ownerName, repoName), + &api.BranchProtection{ + RuleName: "owner-protected/**", + PushWhitelistUsernames: pushWhitelist, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + from := "regular-branch-2" + ctx := NewAPITestContext(t, ownerName, repoName, auth_model.AccessTokenScopeWriteRepository) + testAPICreateBranch(t, ctx.Session, ownerName, repoName, "", from, http.StatusCreated) + + unprivilegedUser := "user40" + resp := testAPIRenameBranch(t, unprivilegedUser, ownerName, repoName, from, "owner-protected/1", http.StatusForbidden) + + assert.Contains(t, resp.Body.String(), "Failed to rename branch due to branch protection rules.") }) t.Run("RenameBranchNormalScenario", func(t *testing.T) { testAPIRenameBranch(t, "user2", "user2", "repo1", "branch2", "new-branch-name", http.StatusNoContent)