Skip to content

Commit 1739e84

Browse files
authored
improve protected branch to add whitelist support (#2451)
* improve protected branch to add whitelist support * fix lint * fix style check * fix tests * fix description on UI and import * fix test * bug fixed * fix tests and languages * move isSliceInt64Eq to util pkg; improve function names & typo
1 parent be3319b commit 1739e84

File tree

29 files changed

+736
-303
lines changed

29 files changed

+736
-303
lines changed

cmd/hook.go

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,10 @@ func runHookPreReceive(c *cli.Context) error {
8484
// the environment setted on serv command
8585
repoID, _ := strconv.ParseInt(os.Getenv(models.ProtectedBranchRepoID), 10, 64)
8686
isUncyclo := (os.Getenv(models.EnvRepoIsUncyclo) == "true")
87-
//username := os.Getenv(models.EnvRepoUsername)
88-
//reponame := os.Getenv(models.EnvRepoName)
89-
//repoPath := models.RepoPath(username, reponame)
87+
username := os.Getenv(models.EnvRepoUsername)
88+
reponame := os.Getenv(models.EnvRepoName)
89+
userIDStr := os.Getenv(models.EnvPusherID)
90+
repoPath := models.RepoPath(username, reponame)
9091

9192
buf := bytes.NewBuffer(nil)
9293
scanner := bufio.NewScanner(os.Stdin)
@@ -104,36 +105,37 @@ func runHookPreReceive(c *cli.Context) error {
104105
continue
105106
}
106107

107-
//oldCommitID := string(fields[0])
108+
oldCommitID := string(fields[0])
108109
newCommitID := string(fields[1])
109110
refFullName := string(fields[2])
110111

111-
// FIXME: when we add feature to protected branch to deny force push, then uncomment below
112-
/*var isForce bool
113-
// detect force push
114-
if git.EmptySHA != oldCommitID {
115-
output, err := git.NewCommand("rev-list", oldCommitID, "^"+newCommitID).RunInDir(repoPath)
116-
if err != nil {
117-
fail("Internal error", "Fail to detect force push: %v", err)
118-
} else if len(output) > 0 {
119-
isForce = true
120-
}
121-
}*/
122-
123112
branchName := strings.TrimPrefix(refFullName, git.BranchPrefix)
124113
protectBranch, err := private.GetProtectedBranchBy(repoID, branchName)
125114
if err != nil {
126115
log.GitLogger.Fatal(2, "retrieve protected branches information failed")
127116
}
128117

129-
if protectBranch != nil {
130-
if !protectBranch.CanPush {
131-
// check and deletion
132-
if newCommitID == git.EmptySHA {
133-
fail(fmt.Sprintf("branch %s is protected from deletion", branchName), "")
134-
} else {
118+
if protectBranch != nil && protectBranch.IsProtected() {
119+
// detect force push
120+
if git.EmptySHA != oldCommitID {
121+
output, err := git.NewCommand("rev-list", oldCommitID, "^"+newCommitID).RunInDir(repoPath)
122+
if err != nil {
123+
fail("Internal error", "Fail to detect force push: %v", err)
124+
} else if len(output) > 0 {
125+
fail(fmt.Sprintf("branch %s is protected from force push", branchName), "")
126+
}
127+
}
128+
129+
// check and deletion
130+
if newCommitID == git.EmptySHA {
131+
fail(fmt.Sprintf("branch %s is protected from deletion", branchName), "")
132+
} else {
133+
userID, _ := strconv.ParseInt(userIDStr, 10, 64)
134+
canPush, err := private.CanUserPush(protectBranch.ID, userID)
135+
if err != nil {
136+
fail("Internal error", "Fail to detect user can push: %v", err)
137+
} else if !canPush {
135138
fail(fmt.Sprintf("protected branch %s can not be pushed to", branchName), "")
136-
//fail(fmt.Sprintf("branch %s is protected from force push", branchName), "")
137139
}
138140
}
139141
}

integrations/editor_test.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,15 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
4343

4444
csrf := GetCSRF(t, session, "/user2/repo1/settings/branches")
4545
// Change master branch to protected
46-
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches?action=protected_branch", map[string]string{
47-
"_csrf": csrf,
48-
"branchName": "master",
49-
"canPush": "true",
46+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/master", map[string]string{
47+
"_csrf": csrf,
48+
"protected": "on",
5049
})
51-
resp := session.MakeRequest(t, req, http.StatusOK)
50+
resp := session.MakeRequest(t, req, http.StatusFound)
5251
// Check if master branch has been locked successfully
5352
flashCookie := session.GetCookie("macaron_flash")
5453
assert.NotNil(t, flashCookie)
55-
assert.EqualValues(t, flashCookie.Value, "success%3Dmaster%2BLocked%2Bsuccessfully")
54+
assert.EqualValues(t, "success%3DBranch%2Bmaster%2Bprotect%2Boptions%2Bchanged%2Bsuccessfully.", flashCookie.Value)
5655

5756
// Request editor page
5857
req = NewRequest(t, "GET", "/user2/repo1/_new/master/")
@@ -74,6 +73,20 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
7473
resp = session.MakeRequest(t, req, http.StatusOK)
7574
// Check body for error message
7675
assert.Contains(t, string(resp.Body), "Can not commit to protected branch 'master'.")
76+
77+
// remove the protected branch
78+
csrf = GetCSRF(t, session, "/user2/repo1/settings/branches")
79+
// Change master branch to protected
80+
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/master", map[string]string{
81+
"_csrf": csrf,
82+
"protected": "off",
83+
})
84+
resp = session.MakeRequest(t, req, http.StatusFound)
85+
// Check if master branch has been locked successfully
86+
flashCookie = session.GetCookie("macaron_flash")
87+
assert.NotNil(t, flashCookie)
88+
assert.EqualValues(t, "success%3DBranch%2Bmaster%2Bprotect%2Boptions%2Bremoved%2Bsuccessfully", flashCookie.Value)
89+
7790
}
7891

7992
func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePath string) *TestResponse {

integrations/integration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func MakeRequest(t testing.TB, req *http.Request, expectedStatus int) *TestRespo
269269
mac.ServeHTTP(respWriter, req)
270270
if expectedStatus != NoExpectedStatus {
271271
assert.EqualValues(t, expectedStatus, respWriter.HeaderCode,
272-
"Request URL: %s", req.URL.String())
272+
"Request URL: %s %s", req.URL.String(), buffer.String())
273273
}
274274
return &TestResponse{
275275
HeaderCode: respWriter.HeaderCode,

integrations/internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func assertProtectedBranch(t *testing.T, repoID int64, branchName string, isErr,
3131
var branch models.ProtectedBranch
3232
t.Log(string(resp.Body))
3333
assert.NoError(t, json.Unmarshal(resp.Body, &branch))
34-
assert.Equal(t, canPush, branch.CanPush)
34+
assert.Equal(t, canPush, !branch.IsProtected())
3535
}
3636
}
3737

models/branches.go

Lines changed: 112 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import (
88
"fmt"
99
"strings"
1010
"time"
11+
12+
"code.gitea.io/gitea/modules/base"
13+
"code.gitea.io/gitea/modules/log"
14+
"code.gitea.io/gitea/modules/util"
15+
16+
"github.com/Unknwon/com"
1117
)
1218

1319
const (
@@ -17,14 +23,43 @@ const (
1723

1824
// ProtectedBranch struct
1925
type ProtectedBranch struct {
20-
ID int64 `xorm:"pk autoincr"`
21-
RepoID int64 `xorm:"UNIQUE(s)"`
22-
BranchName string `xorm:"UNIQUE(s)"`
23-
CanPush bool
24-
Created time.Time `xorm:"-"`
25-
CreatedUnix int64 `xorm:"created"`
26-
Updated time.Time `xorm:"-"`
27-
UpdatedUnix int64 `xorm:"updated"`
26+
ID int64 `xorm:"pk autoincr"`
27+
RepoID int64 `xorm:"UNIQUE(s)"`
28+
BranchName string `xorm:"UNIQUE(s)"`
29+
EnableWhitelist bool
30+
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
31+
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
32+
Created time.Time `xorm:"-"`
33+
CreatedUnix int64 `xorm:"created"`
34+
Updated time.Time `xorm:"-"`
35+
UpdatedUnix int64 `xorm:"updated"`
36+
}
37+
38+
// IsProtected returns if the branch is protected
39+
func (protectBranch *ProtectedBranch) IsProtected() bool {
40+
return protectBranch.ID > 0
41+
}
42+
43+
// CanUserPush returns if some user could push to this protected branch
44+
func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
45+
if !protectBranch.EnableWhitelist {
46+
return false
47+
}
48+
49+
if base.Int64sContains(protectBranch.WhitelistUserIDs, userID) {
50+
return true
51+
}
52+
53+
if len(protectBranch.WhitelistTeamIDs) == 0 {
54+
return false
55+
}
56+
57+
in, err := IsUserInTeams(userID, protectBranch.WhitelistTeamIDs)
58+
if err != nil {
59+
log.Error(1, "IsUserInTeams:", err)
60+
return false
61+
}
62+
return in
2863
}
2964

3065
// GetProtectedBranchByRepoID getting protected branch by repo ID
@@ -46,14 +81,81 @@ func GetProtectedBranchBy(repoID int64, BranchName string) (*ProtectedBranch, er
4681
return rel, nil
4782
}
4883

84+
// GetProtectedBranchByID getting protected branch by ID
85+
func GetProtectedBranchByID(id int64) (*ProtectedBranch, error) {
86+
rel := &ProtectedBranch{ID: id}
87+
has, err := x.Get(rel)
88+
if err != nil {
89+
return nil, err
90+
}
91+
if !has {
92+
return nil, nil
93+
}
94+
return rel, nil
95+
}
96+
97+
// UpdateProtectBranch saves branch protection options of repository.
98+
// If ID is 0, it creates a new record. Otherwise, updates existing record.
99+
// This function also performs check if whitelist user and team's IDs have been changed
100+
// to avoid unnecessary whitelist delete and regenerate.
101+
func UpdateProtectBranch(repo *Repository, protectBranch *ProtectedBranch, whitelistUserIDs, whitelistTeamIDs []int64) (err error) {
102+
if err = repo.GetOwner(); err != nil {
103+
return fmt.Errorf("GetOwner: %v", err)
104+
}
105+
106+
hasUsersChanged := !util.IsSliceInt64Eq(protectBranch.WhitelistUserIDs, whitelistUserIDs)
107+
if hasUsersChanged {
108+
protectBranch.WhitelistUserIDs = make([]int64, 0, len(whitelistUserIDs))
109+
for _, userID := range whitelistUserIDs {
110+
has, err := hasAccess(x, userID, repo, AccessModeWrite)
111+
if err != nil {
112+
return fmt.Errorf("HasAccess [user_id: %d, repo_id: %d]: %v", userID, protectBranch.RepoID, err)
113+
} else if !has {
114+
continue // Drop invalid user ID
115+
}
116+
117+
protectBranch.WhitelistUserIDs = append(protectBranch.WhitelistUserIDs, userID)
118+
}
119+
}
120+
121+
// if the repo is in an orgniziation
122+
hasTeamsChanged := !util.IsSliceInt64Eq(protectBranch.WhitelistTeamIDs, whitelistTeamIDs)
123+
if hasTeamsChanged {
124+
teams, err := GetTeamsWithAccessToRepo(repo.OwnerID, repo.ID, AccessModeWrite)
125+
if err != nil {
126+
return fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err)
127+
}
128+
protectBranch.WhitelistTeamIDs = make([]int64, 0, len(teams))
129+
for i := range teams {
130+
if teams[i].HasWriteAccess() && com.IsSliceContainsInt64(whitelistTeamIDs, teams[i].ID) {
131+
protectBranch.WhitelistTeamIDs = append(protectBranch.WhitelistTeamIDs, teams[i].ID)
132+
}
133+
}
134+
}
135+
136+
// Make sure protectBranch.ID is not 0 for whitelists
137+
if protectBranch.ID == 0 {
138+
if _, err = x.Insert(protectBranch); err != nil {
139+
return fmt.Errorf("Insert: %v", err)
140+
}
141+
return nil
142+
}
143+
144+
if _, err = x.Id(protectBranch.ID).AllCols().Update(protectBranch); err != nil {
145+
return fmt.Errorf("Update: %v", err)
146+
}
147+
148+
return nil
149+
}
150+
49151
// GetProtectedBranches get all protected branches
50152
func (repo *Repository) GetProtectedBranches() ([]*ProtectedBranch, error) {
51153
protectedBranches := make([]*ProtectedBranch, 0)
52154
return protectedBranches, x.Find(&protectedBranches, &ProtectedBranch{RepoID: repo.ID})
53155
}
54156

55157
// IsProtectedBranch checks if branch is protected
56-
func (repo *Repository) IsProtectedBranch(branchName string) (bool, error) {
158+
func (repo *Repository) IsProtectedBranch(branchName string, doer *User) (bool, error) {
57159
protectedBranch := &ProtectedBranch{
58160
RepoID: repo.ID,
59161
BranchName: branchName,
@@ -63,70 +165,12 @@ func (repo *Repository) IsProtectedBranch(branchName string) (bool, error) {
63165
if err != nil {
64166
return true, err
65167
} else if has {
66-
return true, nil
168+
return !protectedBranch.CanUserPush(doer.ID), nil
67169
}
68170

69171
return false, nil
70172
}
71173

72-
// AddProtectedBranch add protection to branch
73-
func (repo *Repository) AddProtectedBranch(branchName string, canPush bool) error {
74-
protectedBranch := &ProtectedBranch{
75-
RepoID: repo.ID,
76-
BranchName: branchName,
77-
}
78-
79-
has, err := x.Get(protectedBranch)
80-
if err != nil {
81-
return err
82-
} else if has {
83-
return nil
84-
}
85-
86-
sess := x.NewSession()
87-
defer sess.Close()
88-
if err = sess.Begin(); err != nil {
89-
return err
90-
}
91-
protectedBranch.CanPush = canPush
92-
if _, err = sess.InsertOne(protectedBranch); err != nil {
93-
return err
94-
}
95-
96-
return sess.Commit()
97-
}
98-
99-
// ChangeProtectedBranch access mode sets new access mode for the ProtectedBranch.
100-
func (repo *Repository) ChangeProtectedBranch(id int64, canPush bool) error {
101-
ProtectedBranch := &ProtectedBranch{
102-
RepoID: repo.ID,
103-
ID: id,
104-
}
105-
has, err := x.Get(ProtectedBranch)
106-
if err != nil {
107-
return fmt.Errorf("get ProtectedBranch: %v", err)
108-
} else if !has {
109-
return nil
110-
}
111-
112-
if ProtectedBranch.CanPush == canPush {
113-
return nil
114-
}
115-
ProtectedBranch.CanPush = canPush
116-
117-
sess := x.NewSession()
118-
defer sess.Close()
119-
if err = sess.Begin(); err != nil {
120-
return err
121-
}
122-
123-
if _, err = sess.Id(ProtectedBranch.ID).AllCols().Update(ProtectedBranch); err != nil {
124-
return fmt.Errorf("update ProtectedBranch: %v", err)
125-
}
126-
127-
return sess.Commit()
128-
}
129-
130174
// DeleteProtectedBranch removes ProtectedBranch relation between the user and repository.
131175
func (repo *Repository) DeleteProtectedBranch(id int64) (err error) {
132176
protectedBranch := &ProtectedBranch{
@@ -148,15 +192,3 @@ func (repo *Repository) DeleteProtectedBranch(id int64) (err error) {
148192

149193
return sess.Commit()
150194
}
151-
152-
// newProtectedBranch insert one queue
153-
func newProtectedBranch(protectedBranch *ProtectedBranch) error {
154-
_, err := x.InsertOne(protectedBranch)
155-
return err
156-
}
157-
158-
// UpdateProtectedBranch update queue
159-
func UpdateProtectedBranch(protectedBranch *ProtectedBranch) error {
160-
_, err := x.Update(protectedBranch)
161-
return err
162-
}

models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ var migrations = []Migration{
128128
NewMigration("remove commits and settings unit types", removeCommitsUnitType),
129129
// v39 -> v40
130130
NewMigration("adds time tracking and stopwatches", addTimetracking),
131+
// v40 -> v41
132+
NewMigration("migrate protected branch struct", migrateProtectedBranchStruct),
131133
}
132134

133135
// Migrate database to current version

0 commit comments

Comments
 (0)