Skip to content

Commit 22be3e9

Browse files
davidsvantessondavidsvantesson
committed
Possibility to not use whitelist but allow anyone with write access
1 parent 8f09aff commit 22be3e9

File tree

11 files changed

+156
-45
lines changed

11 files changed

+156
-45
lines changed

‎models/branches.go

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ const (
2626

2727
// ProtectedBranch struct
2828
type ProtectedBranch struct {
29-
ID int64 `xorm:"pk autoincr"`
30-
RepoID int64 `xorm:"UNIQUE(s)"`
31-
BranchName string `xorm:"UNIQUE(s)"`
32-
CanPush bool `xorm:"NOT NULL DEFAULT false"`
33-
EnableWhitelist bool
29+
ID int64 `xorm:"pk autoincr"`
30+
RepoID int64 `xorm:"UNIQUE(s)"`
31+
BranchName string `xorm:"UNIQUE(s)"`
32+
CanPush bool `xorm:"NOT NULL DEFAULT false"`
33+
EnableWhitelist bool `xorm:"NOT NULL DEFAULT false"`
3434
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
3535
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
3636
EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"`
@@ -39,6 +39,7 @@ type ProtectedBranch struct {
3939
MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
4040
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
4141
StatusCheckContexts []string `xorm:"JSON TEXT"`
42+
EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"`
4243
ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
4344
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
4445
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
@@ -53,10 +54,25 @@ func (protectBranch *ProtectedBranch) IsProtected() bool {
5354

5455
// CanUserPush returns if some user could push to this protected branch
5556
func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
56-
if !protectBranch.EnableWhitelist {
57+
if !protectBranch.CanPush {
5758
return false
5859
}
5960

61+
if !protectBranch.EnableWhitelist {
62+
if user, err := GetUserByID(userID); err != nil {
63+
log.Error("GetUserByID: %v", err)
64+
return false
65+
} else if repo, err := GetRepositoryByID(protectBranch.RepoID); err != nil {
66+
log.Error("GetRepositoryByID: %v", err)
67+
return false
68+
} else if writeAccess, err := HasAccessUnit(user, repo, UnitTypeCode, AccessModeWrite); err != nil {
69+
log.Error("HasAccessUnit: %v", err)
70+
return false
71+
} else {
72+
return writeAccess
73+
}
74+
}
75+
6076
if base.Int64sContains(protectBranch.WhitelistUserIDs, userID) {
6177
return true
6278
}
@@ -111,12 +127,28 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest)
111127
return 0
112128
}
113129

130+
repo, err := GetRepositoryByID(protectBranch.RepoID)
131+
if err != nil {
132+
log.Error("GetRepositoryByID: %v", err)
133+
return 0
134+
}
135+
114136
approvals := int64(0)
115137
userIDs := make([]int64, 0)
116138
for _, review := range reviews {
117139
if review.Type != ReviewTypeApprove {
118140
continue
119141
}
142+
if !protectBranch.EnableApprovalsWhitelist {
143+
if user, err := GetUserByID(review.ID); err != nil {
144+
log.Error("GetUserByID: %v", err)
145+
} else if writeAccess, err := HasAccessUnit(user, repo, UnitTypeCode, AccessModeWrite); err != nil {
146+
log.Error("HasAccessUnit: %v", err)
147+
} else if writeAccess {
148+
approvals++
149+
}
150+
continue
151+
}
120152
if base.Int64sContains(protectBranch.ApprovalsWhitelistUserIDs, review.ID) {
121153
approvals++
122154
continue

‎models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,8 @@ var migrations = []Migration{
272272
NewMigration("Add template options to repository", addTemplateToRepo),
273273
// v108 -> v109
274274
NewMigration("Add comment_id on table notification", addCommentIDOnNotification),
275+
// v109 -> v110
276+
NewMigration("update branch protection whitelist enable", BranchProtectionAddEnableWhitelist),
275277
}
276278

277279
// Migrate database to current version

‎models/migrations/v109.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"xorm.io/xorm"
9+
)
10+
11+
// BranchProtectionAddEnableWhitelist migrates can push vs whitelist push and enabling of approvals whitelist
12+
func BranchProtectionAddEnableWhitelist(x *xorm.Engine) error {
13+
14+
type ProtectedBranch struct {
15+
CanPush bool `xorm:"NOT NULL DEFAULT false"`
16+
EnableWhitelist bool `xorm:"NOT NULL DEFAULT false"`
17+
EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"`
18+
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
19+
}
20+
21+
sess := x.NewSession()
22+
defer sess.Close()
23+
24+
if err := x.Sync2(new(ProtectedBranch)); err != nil {
25+
return err
26+
}
27+
28+
if _, err := x.Exec("UPDATE `protected_branch` SET `can_push` = `enable_whitelist`"); err != nil {
29+
return err
30+
}
31+
_, err := x.Exec("UPDATE `protected_branch` SET `enable_approvals_whitelist` = ? WHERE `required_approvals` > ?", true, 0)
32+
return err
33+
}

‎modules/auth/repo_form.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,19 +153,20 @@ func (f *RepoSettingForm) Validate(ctx *macaron.Context, errs binding.Errors) bi
153153

154154
// ProtectBranchForm form for changing protected branch settings
155155
type ProtectBranchForm struct {
156-
Protected bool
157-
EnableWhitelist bool
158-
WhitelistUsers string
159-
WhitelistTeams string
160-
WhitelistDeployKeys bool
161-
EnableMergeWhitelist bool
162-
MergeWhitelistUsers string
163-
MergeWhitelistTeams string
164-
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
165-
StatusCheckContexts []string
166-
RequiredApprovals int64
167-
ApprovalsWhitelistUsers string
168-
ApprovalsWhitelistTeams string
156+
Protected bool
157+
EnablePush string
158+
WhitelistUsers string
159+
WhitelistTeams string
160+
WhitelistDeployKeys bool
161+
EnableMergeWhitelist bool
162+
MergeWhitelistUsers string
163+
MergeWhitelistTeams string
164+
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
165+
StatusCheckContexts []string
166+
RequiredApprovals int64
167+
EnableApprovalsWhitelist bool
168+
ApprovalsWhitelistUsers string
169+
ApprovalsWhitelistTeams string
169170
}
170171

171172
// Validate validates the fields

‎options/locale/locale_en-US.ini

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,9 +1361,13 @@ settings.protected_branch_can_push_yes = You can push
13611361
settings.protected_branch_can_push_no = You can not push
13621362
settings.branch_protection = Branch Protection for Branch '<b>%s</b>'
13631363
settings.protect_this_branch = Enable Branch Protection
1364-
settings.protect_this_branch_desc = Prevent deletion and disable any Git pushing to the branch.
1365-
settings.protect_whitelist_committers = Enable Push Whitelist
1366-
settings.protect_whitelist_committers_desc = Allow whitelisted users or teams to push to this branch (but not force push).
1364+
settings.protect_this_branch_desc = Prevents deletion and restricts Git pushing and merging to the branch.
1365+
settings.protect_disable_push = Disable Push
1366+
settings.protect_disable_push_desc = No pushing will be allowed to this branch.
1367+
settings.protect_enable_push = Enable Push
1368+
settings.protect_enable_push_desc = Anyone with write access will be allowed to push to this branch (but not force push).
1369+
settings.protect_whitelist_committers = Whitelist Restricted Push
1370+
settings.protect_whitelist_committers_desc = Only whitelisted users or teams will be allowed to push to this branch (but not force push).
13671371
settings.protect_whitelist_deploy_keys = Whitelist deploy keys with write access to push
13681372
settings.protect_whitelist_users = Whitelisted users for pushing:
13691373
settings.protect_whitelist_search_users = Search users…
@@ -1377,7 +1381,9 @@ settings.protect_check_status_contexts = Enable Status Check
13771381
settings.protect_check_status_contexts_desc = Require status checks to pass before merging Choose which status checks must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed. If no contexts are selected, the last commit must be successful regardless of context.
13781382
settings.protect_check_status_contexts_list = Status checks found in the last week for this repository
13791383
settings.protect_required_approvals = Required approvals:
1380-
settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews of whitelisted users or teams.
1384+
settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews.
1385+
settings.protect_approvals_whitelist_enabled = Restrict approvals to whitelisted users or teams
1386+
settings.protect_approvals_whitelist_enabled_desc = Only reviews from whitelisted users or teams will count to the required approvals. Without approval whitelist, reviews from anyone with write access count to the required approvals.
13811387
settings.protect_approvals_whitelist_users = Whitelisted reviewers:
13821388
settings.protect_approvals_whitelist_teams = Whitelisted teams for reviews:
13831389
settings.add_protected_branch = Enable protection

‎public/js/index.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎public/js/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎routers/private/hook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func HookPreReceive(ctx *macaron.Context) {
9898

9999
canPush := false
100100
if isDeployKey {
101-
canPush = protectBranch.WhitelistDeployKeys
101+
canPush = protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys)
102102
} else {
103103
canPush = protectBranch.CanUserPush(userID)
104104
}

‎routers/repo/setting_protected_branch.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,17 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm)
196196
}
197197

198198
var whitelistUsers, whitelistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams []int64
199-
protectBranch.EnableWhitelist = f.EnableWhitelist
199+
switch f.EnablePush {
200+
case "all":
201+
protectBranch.CanPush = true
202+
protectBranch.EnableWhitelist = false
203+
case "whitelist":
204+
protectBranch.CanPush = true
205+
protectBranch.EnableWhitelist = true
206+
default:
207+
protectBranch.CanPush = false
208+
protectBranch.EnableWhitelist = false
209+
}
200210
if strings.TrimSpace(f.WhitelistUsers) != "" {
201211
whitelistUsers, _ = base.StringsToInt64s(strings.Split(f.WhitelistUsers, ","))
202212
}
@@ -216,6 +226,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm)
216226
protectBranch.WhitelistDeployKeys = f.WhitelistDeployKeys
217227

218228
protectBranch.RequiredApprovals = f.RequiredApprovals
229+
protectBranch.EnableApprovalsWhitelist = f.EnableApprovalsWhitelist
219230
if strings.TrimSpace(f.ApprovalsWhitelistUsers) != "" {
220231
approvalsWhitelistUsers, _ = base.StringsToInt64s(strings.Split(f.ApprovalsWhitelistUsers, ","))
221232
}

‎templates/repo/settings/protected_branch.tmpl

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,22 @@
1919
</div>
2020
<div id="protection_box" class="fields {{if not .Branch.IsProtected}}disabled{{end}}">
2121
<div class="field">
22-
<div class="ui checkbox">
23-
<input class="enable-whitelist" name="enable_whitelist" type="checkbox" data-target="#whitelist_box" {{if .Branch.EnableWhitelist}}checked{{end}}>
22+
<div class="ui radio checkbox">
23+
<input name="enable_push" type="radio" value="none" class="disable-whitelist" data-target="#whitelist_box" {{if not .Branch.CanPush}}checked{{end}}>
24+
<label>{{.i18n.Tr "repo.settings.protect_disable_push"}}</label>
25+
<p class="help">{{.i18n.Tr "repo.settings.protect_disable_push_desc"}}</p>
26+
</div>
27+
</div>
28+
<div class="field">
29+
<div class="ui radio checkbox">
30+
<input name="enable_push" type="radio" value="all" class="disable-whitelist" data-target="#whitelist_box" {{if and (.Branch.CanPush) (not .Branch.EnableWhitelist)}}checked{{end}}>
31+
<label>{{.i18n.Tr "repo.settings.protect_enable_push"}}</label>
32+
<p class="help">{{.i18n.Tr "repo.settings.protect_enable_push_desc"}}</p>
33+
</div>
34+
</div>
35+
<div class="field">
36+
<div class="ui radio checkbox">
37+
<input name="enable_push" type="radio" value="whitelist" class="enable-whitelist" data-target="#whitelist_box" {{if and (.Branch.CanPush) (.Branch.EnableWhitelist)}}checked{{end}}>
2438
<label>{{.i18n.Tr "repo.settings.protect_whitelist_committers"}}</label>
2539
<p class="help">{{.i18n.Tr "repo.settings.protect_whitelist_committers_desc"}}</p>
2640
</div>
@@ -148,7 +162,14 @@
148162
<input name="required_approvals" id="required-approvals" type="number" value="{{.Branch.RequiredApprovals}}">
149163
<p class="help">{{.i18n.Tr "repo.settings.protect_required_approvals_desc"}}</p>
150164
</div>
151-
<div class="fields">
165+
<div class="field">
166+
<div class="ui checkbox">
167+
<input class="enable-whitelist" name="enable_approvals_whitelist" type="checkbox" data-target="#approvals_whitelist_box" {{if .Branch.EnableApprovalsWhitelist}}checked{{end}}>
168+
<label>{{.i18n.Tr "repo.settings.protect_approvals_whitelist_enabled"}}</label>
169+
<p class="help">{{.i18n.Tr "repo.settings.protect_approvals_whitelist_enabled_desc"}}</p>
170+
</div>
171+
</div>
172+
<div id="approvals_whitelist_box" class="fields {{if not .Branch.EnableApprovalsWhitelist}}disabled{{end}}">
152173
<div class="whitelist field">
153174
<label>{{.i18n.Tr "repo.settings.protect_approvals_whitelist_users"}}</label>
154175
<div class="ui multiple search selection dropdown">
@@ -164,24 +185,24 @@
164185
</div>
165186
</div>
166187
</div>
167-
{{if .Owner.IsOrganization}}
168-
<br>
169-
<div class="whitelist field">
170-
<label>{{.i18n.Tr "repo.settings.protect_approvals_whitelist_teams"}}</label>
171-
<div class="ui multiple search selection dropdown">
172-
<input type="hidden" name="approvals_whitelist_teams" value="{{.approvals_whitelist_teams}}">
173-
<div class="default text">{{.i18n.Tr "repo.settings.protect_whitelist_search_teams"}}</div>
174-
<div class="menu">
175-
{{range .Teams}}
176-
<div class="item" data-value="{{.ID}}">
177-
<i class="octicon octicon-jersey"></i>
178-
{{.Name}}
188+
{{if .Owner.IsOrganization}}
189+
<br>
190+
<div class="whitelist field">
191+
<label>{{.i18n.Tr "repo.settings.protect_approvals_whitelist_teams"}}</label>
192+
<div class="ui multiple search selection dropdown">
193+
<input type="hidden" name="approvals_whitelist_teams" value="{{.approvals_whitelist_teams}}">
194+
<div class="default text">{{.i18n.Tr "repo.settings.protect_whitelist_search_teams"}}</div>
195+
<div class="menu">
196+
{{range .Teams}}
197+
<div class="item" data-value="{{.ID}}">
198+
<i class="octicon octicon-jersey"></i>
199+
{{.Name}}
200+
</div>
201+
{{end}}
179202
</div>
180-
{{end}}
181203
</div>
182204
</div>
183-
</div>
184-
{{end}}
205+
{{end}}
185206
</div>
186207
</div>
187208

‎web_src/js/index.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,11 @@ function initRepository() {
984984
$($(this).data('target')).addClass('disabled');
985985
}
986986
});
987+
$('.disable-whitelist').change(function () {
988+
if (this.checked) {
989+
$($(this).data('target')).addClass('disabled');
990+
}
991+
});
987992
}
988993
}
989994

0 commit comments

Comments
 (0)