Skip to content

Commit ff85cbe

Browse files
lkysowlkysow
committed
Require approval from someone other than PR author
In Bitbucket cloud, the pull request author can approve their own pull request. Ignore this approval. Fixes #201
1 parent fc6b4a1 commit ff85cbe

File tree

8 files changed

+728
-2
lines changed

8 files changed

+728
-2
lines changed

‎runatlantis.io/docs/apply-requirements.md

+6-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ projects:
1515
A pull request approval might not be as secure as you'd expect:
1616
* In GitHub **any user with read permissions** to the repo can approve a pull request.
1717
* In GitLab, you [can set](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#editing-approvals) who is allowed to approve.
18-
* In Bitbucket, a user can **approve their own pull request**.
18+
:::
19+
20+
::: tip Note
21+
In Bitbucket Cloud (bitbucket.org), a user can approve their own pull request.
22+
Atlantis does not count that as an approval and requires at least one user that
23+
is not the author of the pull request to approve it.
1924
:::
2025
2126
## Next Steps

‎server/events/vcs/bitbucketcloud/client.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool
110110
return false, errors.Wrapf(err, "API response %q was missing fields", string(resp))
111111
}
112112
for _, participant := range pullResp.Participants {
113-
if *participant.Approved {
113+
// Bitbucket allows the author to approve their own pull request. This
114+
// defeats the purpose of approvals so we don't count that approval.
115+
if *participant.Approved && *participant.User.Username != pull.Author {
114116
return true, nil
115117
}
116118
}

‎server/events/vcs/bitbucketcloud/client_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package bitbucketcloud_test
22

33
import (
44
"fmt"
5+
"io/ioutil"
56
"net/http"
67
"net/http/httptest"
8+
"path/filepath"
79
"testing"
810

911
"github.com/runatlantis/atlantis/server/events/models"
@@ -148,3 +150,66 @@ func TestClient_GetModifiedFilesOldNil(t *testing.T) {
148150
Ok(t, err)
149151
Equals(t, []string{"parent/child/file1.txt"}, files)
150152
}
153+
154+
func TestClient_PullIsApproved(t *testing.T) {
155+
cases := []struct {
156+
description string
157+
testdata string
158+
exp bool
159+
}{
160+
{
161+
"no approvers",
162+
"pull-unapproved.json",
163+
false,
164+
},
165+
{
166+
"approver is the author",
167+
"pull-approved-by-author.json",
168+
false,
169+
},
170+
{
171+
"single approver",
172+
"pull-approved.json",
173+
true,
174+
},
175+
{
176+
"two approvers one author",
177+
"pull-approved-multiple.json",
178+
true,
179+
},
180+
}
181+
182+
for _, c := range cases {
183+
t.Run(c.description, func(t *testing.T) {
184+
json, err := ioutil.ReadFile(filepath.Join("testdata", c.testdata))
185+
Ok(t, err)
186+
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
187+
switch r.RequestURI {
188+
// The first request should hit this URL.
189+
case "/2.0/repositories/owner/repo/pullrequests/1":
190+
w.Write([]byte(json)) // nolint: errcheck
191+
return
192+
default:
193+
t.Errorf("got unexpected request at %q", r.RequestURI)
194+
http.Error(w, "not found", http.StatusNotFound)
195+
return
196+
}
197+
}))
198+
defer testServer.Close()
199+
200+
client := bitbucketcloud.NewClient(http.DefaultClient, "user", "pass", "runatlantis.io")
201+
client.BaseURL = testServer.URL
202+
203+
repo, err := models.NewRepo(models.BitbucketServer, "owner/repo", "https://bitbucket.org/owner/repo.git", "user", "token")
204+
Ok(t, err)
205+
approved, err := client.PullIsApproved(repo, models.PullRequest{
206+
Num: 1,
207+
Branch: "branch",
208+
Author: "author",
209+
BaseRepo: repo,
210+
})
211+
Ok(t, err)
212+
Equals(t, c.exp, approved)
213+
})
214+
}
215+
}

‎server/events/vcs/bitbucketcloud/models.go

+3
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ type Link struct {
5959
}
6060
type Participant struct {
6161
Approved *bool `json:"approved,omitempty" validate:"required"`
62+
User *struct {
63+
Username *string `json:"username,omitempty" validate:"required"`
64+
} `json:"user,omitempty" validate:"required"`
6265
}
6366
type Source struct {
6467
Repository *Repository `json:"repository,omitempty" validate:"required"`

‎server/events/vcs/bitbucketcloud/testdata/pull-approved-by-author.json

+166
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
{
2+
"type": "pullrequest",
3+
"description": "main.tf edited online with Bitbucket",
4+
"links": {
5+
"decline": {
6+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/decline"
7+
},
8+
"commits": {
9+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/commits"
10+
},
11+
"self": {
12+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5"
13+
},
14+
"comments": {
15+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/comments"
16+
},
17+
"merge": {
18+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/merge"
19+
},
20+
"html": {
21+
"href": "https://bitbucket.org/lkysow/atlantis-example/pull-requests/5"
22+
},
23+
"activity": {
24+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/activity"
25+
},
26+
"diff": {
27+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/diff"
28+
},
29+
"approve": {
30+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/approve"
31+
},
32+
"statuses": {
33+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/pullrequests/5/statuses"
34+
}
35+
},
36+
"title": "main.tf edited online with Bitbucket",
37+
"close_source_branch": true,
38+
"reviewers": [],
39+
"id": 5,
40+
"destination": {
41+
"commit": {
42+
"hash": "fe607a7f5172",
43+
"type": "commit",
44+
"links": {
45+
"self": {
46+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/commit/fe607a7f5172"
47+
},
48+
"html": {
49+
"href": "https://bitbucket.org/lkysow/atlantis-example/commits/fe607a7f5172"
50+
}
51+
}
52+
},
53+
"repository": {
54+
"links": {
55+
"self": {
56+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example"
57+
},
58+
"html": {
59+
"href": "https://bitbucket.org/lkysow/atlantis-example"
60+
},
61+
"avatar": {
62+
"href": "https://bytebucket.org/ravatar/%7B94189367-116b-436a-9f77-2314b97a6067%7D?ts=default"
63+
}
64+
},
65+
"type": "repository",
66+
"name": "atlantis-example",
67+
"full_name": "lkysow/atlantis-example",
68+
"uuid": "{94189367-116b-436a-9f77-2314b97a6067}"
69+
},
70+
"branch": {
71+
"name": "master"
72+
}
73+
},
74+
"created_on": "2018-07-25T12:23:21.100810+00:00",
75+
"summary": {
76+
"raw": "main.tf edited online with Bitbucket",
77+
"markup": "markdown",
78+
"html": "<p>main.tf edited online with Bitbucket</p>",
79+
"type": "rendered"
80+
},
81+
"source": {
82+
"commit": {
83+
"hash": "3428957ade18",
84+
"type": "commit",
85+
"links": {
86+
"self": {
87+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/commit/3428957ade18"
88+
},
89+
"html": {
90+
"href": "https://bitbucket.org/lkysow/atlantis-example/commits/3428957ade18"
91+
}
92+
}
93+
},
94+
"repository": {
95+
"links": {
96+
"self": {
97+
"href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example"
98+
},
99+
"html": {
100+
"href": "https://bitbucket.org/lkysow/atlantis-example"
101+
},
102+
"avatar": {
103+
"href": "https://bytebucket.org/ravatar/%7B94189367-116b-436a-9f77-2314b97a6067%7D?ts=default"
104+
}
105+
},
106+
"type": "repository",
107+
"name": "atlantis-example",
108+
"full_name": "lkysow/atlantis-example",
109+
"uuid": "{94189367-116b-436a-9f77-2314b97a6067}"
110+
},
111+
"branch": {
112+
"name": "lkysow/maintf-edited-online-with-bitbucket-1532521398289"
113+
}
114+
},
115+
"comment_count": 3,
116+
"state": "OPEN",
117+
"task_count": 0,
118+
"participants": [
119+
{
120+
"role": "PARTICIPANT",
121+
"participated_on": "2018-07-28T00:06:42.255492+00:00",
122+
"type": "participant",
123+
"approved": true,
124+
"user": {
125+
"username": "author",
126+
"display_name": "Luke",
127+
"account_id": "557058:dc3817de-68b5-45cd-b81c-5c39d2560090",
128+
"links": {
129+
"self": {
130+
"href": "https://api.bitbucket.org/2.0/users/lkysow"
131+
},
132+
"html": {
133+
"href": "https://bitbucket.org/lkysow/"
134+
},
135+
"avatar": {
136+
"href": "https://bitbucket.org/account/lkysow/avatar/"
137+
}
138+
},
139+
"type": "user",
140+
"uuid": "{bf34a99b-8a11-452c-8fbc-bdffc340e584}"
141+
}
142+
}
143+
],
144+
"reason": "",
145+
"updated_on": "2018-07-28T00:06:42.257659+00:00",
146+
"author": {
147+
"username": "lkysow",
148+
"display_name": "Luke",
149+
"account_id": "557058:dc3817de-68b5-45cd-b81c-5c39d2560090",
150+
"links": {
151+
"self": {
152+
"href": "https://api.bitbucket.org/2.0/users/lkysow"
153+
},
154+
"html": {
155+
"href": "https://bitbucket.org/lkysow/"
156+
},
157+
"avatar": {
158+
"href": "https://bitbucket.org/account/lkysow/avatar/"
159+
}
160+
},
161+
"type": "user",
162+
"uuid": "{bf34a99b-8a11-452c-8fbc-bdffc340e584}"
163+
},
164+
"merge_commit": null,
165+
"closed_by": null
166+
}

0 commit comments

Comments
 (0)