Skip to content

Commit 4a49e35

Browse files
mislavmislav
committed
Merge remote-tracking branch 'origin' into writeorg-oauth-scope
2 parents 0be2033 + 3a224b7 commit 4a49e35

File tree

23 files changed

+1337
-594
lines changed

23 files changed

+1337
-594
lines changed

‎api/client.go‎

Lines changed: 11 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,14 @@ type HTTPError struct {
142142
RequestURL *url.URL
143143
Message string
144144
OAuthScopes string
145+
Errors []HTTPErrorItem
146+
}
147+
148+
type HTTPErrorItem struct {
149+
Message string
150+
Resource string
151+
Field string
152+
Code string
145153
}
146154

147155
func (err HTTPError) Error() string {
@@ -153,79 +161,6 @@ func (err HTTPError) Error() string {
153161
return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL)
154162
}
155163

156-
type MissingScopesError struct {
157-
MissingScopes []string
158-
}
159-
160-
func (e MissingScopesError) Error() string {
161-
var missing []string
162-
for _, s := range e.MissingScopes {
163-
missing = append(missing, fmt.Sprintf("'%s'", s))
164-
}
165-
scopes := strings.Join(missing, ", ")
166-
167-
if len(e.MissingScopes) == 1 {
168-
return "missing required scope " + scopes
169-
}
170-
return "missing required scopes " + scopes
171-
}
172-
173-
func (c Client) HasMinimumScopes(hostname string) error {
174-
apiEndpoint := ghinstance.RESTPrefix(hostname)
175-
176-
req, err := http.NewRequest("GET", apiEndpoint, nil)
177-
if err != nil {
178-
return err
179-
}
180-
181-
req.Header.Set("Content-Type", "application/json; charset=utf-8")
182-
res, err := c.http.Do(req)
183-
if err != nil {
184-
return err
185-
}
186-
187-
defer func() {
188-
// Ensure the response body is fully read and closed
189-
// before we reconnect, so that we reuse the same TCPconnection.
190-
_, _ = io.Copy(ioutil.Discard, res.Body)
191-
res.Body.Close()
192-
}()
193-
194-
if res.StatusCode != 200 {
195-
return HandleHTTPError(res)
196-
}
197-
198-
scopesHeader := res.Header.Get("X-Oauth-Scopes")
199-
if scopesHeader == "" {
200-
// if the token reports no scopes, assume that it's an integration token and give up on
201-
// detecting its capabilities
202-
return nil
203-
}
204-
205-
search := map[string]bool{
206-
"repo": false,
207-
"read:org": false,
208-
"admin:org": false,
209-
}
210-
for _, s := range strings.Split(scopesHeader, ",") {
211-
search[strings.TrimSpace(s)] = true
212-
}
213-
214-
var missingScopes []string
215-
if !search["repo"] {
216-
missingScopes = append(missingScopes, "repo")
217-
}
218-
219-
if !search["read:org"] && !search["write:org"] && !search["admin:org"] {
220-
missingScopes = append(missingScopes, "read:org")
221-
}
222-
223-
if len(missingScopes) > 0 {
224-
return &MissingScopesError{MissingScopes: missingScopes}
225-
}
226-
return nil
227-
}
228-
229164
// GraphQL performs a GraphQL request and parses the response
230165
func (c Client) GraphQL(hostname string, query string, variables map[string]interface{}, data interface{}) error {
231166
reqBody, err := json.Marshal(map[string]interface{}{"query": query, "variables": variables})
@@ -341,22 +276,16 @@ func HandleHTTPError(resp *http.Response) error {
341276
return httpError
342277
}
343278

344-
type errorObject struct {
345-
Message string
346-
Resource string
347-
Field string
348-
Code string
349-
}
350-
351279
messages := []string{parsedBody.Message}
352280
for _, raw := range parsedBody.Errors {
353281
switch raw[0] {
354282
case '"':
355283
var errString string
356284
_ = json.Unmarshal(raw, &errString)
357285
messages = append(messages, errString)
286+
httpError.Errors = append(httpError.Errors, HTTPErrorItem{Message: errString})
358287
case '{':
359-
var errInfo errorObject
288+
var errInfo HTTPErrorItem
360289
_ = json.Unmarshal(raw, &errInfo)
361290
msg := errInfo.Message
362291
if errInfo.Code != "custom" {
@@ -365,6 +294,7 @@ func HandleHTTPError(resp *http.Response) error {
365294
if msg != "" {
366295
messages = append(messages, msg)
367296
}
297+
httpError.Errors = append(httpError.Errors, errInfo)
368298
}
369299
}
370300
httpError.Message = strings.Join(messages, "\n")

‎api/client_test.go‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -109,72 +109,3 @@ func TestRESTError(t *testing.T) {
109109

110110
}
111111
}
112-
113-
func Test_HasMinimumScopes(t *testing.T) {
114-
tests := []struct {
115-
name string
116-
header string
117-
wantErr string
118-
}{
119-
{
120-
name: "no scopes",
121-
header: "",
122-
wantErr: "",
123-
},
124-
{
125-
name: "default scopes",
126-
header: "repo, read:org",
127-
wantErr: "",
128-
},
129-
{
130-
name: "admin:org satisfies read:org",
131-
header: "repo, admin:org",
132-
wantErr: "",
133-
},
134-
{
135-
name: "write:org satisfies read:org",
136-
header: "repo, write:org",
137-
wantErr: "",
138-
},
139-
{
140-
name: "insufficient scope",
141-
header: "repo",
142-
wantErr: "missing required scope 'read:org'",
143-
},
144-
{
145-
name: "insufficient scopes",
146-
header: "gist",
147-
wantErr: "missing required scopes 'repo', 'read:org'",
148-
},
149-
}
150-
for _, tt := range tests {
151-
t.Run(tt.name, func(t *testing.T) {
152-
fakehttp := &httpmock.Registry{}
153-
client := NewClient(ReplaceTripper(fakehttp))
154-
155-
fakehttp.Register(httpmock.REST("GET", ""), func(req *http.Request) (*http.Response, error) {
156-
return &http.Response{
157-
Request: req,
158-
StatusCode: 200,
159-
Body: ioutil.NopCloser(&bytes.Buffer{}),
160-
Header: map[string][]string{
161-
"X-Oauth-Scopes": {tt.header},
162-
},
163-
}, nil
164-
})
165-
166-
err := client.HasMinimumScopes("github.com")
167-
if tt.wantErr == "" {
168-
if err != nil {
169-
t.Errorf("error: %v", err)
170-
}
171-
return
172-
}
173-
if err.Error() != tt.wantErr {
174-
t.Errorf("want %q, got %q", tt.wantErr, err.Error())
175-
176-
}
177-
})
178-
}
179-
180-
}

‎api/queries_pr.go‎

Lines changed: 36 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ import (
1616
)
1717

1818
type PullRequestsPayload struct {
19-
ViewerCreated PullRequestAndTotalCount
20-
ReviewRequested PullRequestAndTotalCount
21-
CurrentPR *PullRequest
22-
DefaultBranch string
19+
ViewerCreated PullRequestAndTotalCount
20+
ReviewRequested PullRequestAndTotalCount
21+
CurrentPR *PullRequest
22+
DefaultBranch string
23+
StrictProtection bool
2324
}
2425

2526
type PullRequestAndTotalCount struct {
@@ -28,16 +29,17 @@ type PullRequestAndTotalCount struct {
2829
}
2930

3031
type PullRequest struct {
31-
ID string
32-
Number int
33-
Title string
34-
State string
35-
Closed bool
36-
URL string
37-
BaseRefName string
38-
HeadRefName string
39-
Body string
40-
Mergeable string
32+
ID string
33+
Number int
34+
Title string
35+
State string
36+
Closed bool
37+
URL string
38+
BaseRefName string
39+
HeadRefName string
40+
Body string
41+
Mergeable string
42+
MergeStateStatus string
4143

4244
Author struct {
4345
Login string
@@ -138,14 +140,6 @@ type PullRequestReviewStatus struct {
138140
ReviewRequired bool
139141
}
140142

141-
type PullRequestMergeMethod int
142-
143-
const (
144-
PullRequestMergeMethodMerge PullRequestMergeMethod = iota
145-
PullRequestMergeMethodRebase
146-
PullRequestMergeMethodSquash
147-
)
148-
149143
func (pr *PullRequest) ReviewStatus() PullRequestReviewStatus {
150144
var status PullRequestReviewStatus
151145
switch pr.ReviewDecision {
@@ -289,7 +283,10 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
289283
type response struct {
290284
Repository struct {
291285
DefaultBranchRef struct {
292-
Name string
286+
Name string
287+
BranchProtectionRule struct {
288+
RequiresStrictStatusChecks bool
289+
}
293290
}
294291
PullRequests edges
295292
PullRequest *PullRequest
@@ -341,6 +338,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
341338
state
342339
url
343340
headRefName
341+
mergeStateStatus
344342
headRepositoryOwner {
345343
login
346344
}
@@ -357,7 +355,12 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
357355
queryPrefix := `
358356
query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
359357
repository(owner: $owner, name: $repo) {
360-
defaultBranchRef { name }
358+
defaultBranchRef {
359+
name
360+
branchProtectionRule {
361+
requiresStrictStatusChecks
362+
}
363+
}
361364
pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) {
362365
totalCount
363366
edges {
@@ -372,7 +375,12 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
372375
queryPrefix = `
373376
query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
374377
repository(owner: $owner, name: $repo) {
375-
defaultBranchRef { name }
378+
defaultBranchRef {
379+
name
380+
branchProtectionRule {
381+
requiresStrictStatusChecks
382+
}
383+
}
376384
pullRequest(number: $number) {
377385
...prWithReviews
378386
}
@@ -459,8 +467,9 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
459467
PullRequests: reviewRequested,
460468
TotalCount: resp.ReviewRequested.TotalCount,
461469
},
462-
CurrentPR: currentPR,
463-
DefaultBranch: resp.Repository.DefaultBranchRef.Name,
470+
CurrentPR: currentPR,
471+
DefaultBranch: resp.Repository.DefaultBranchRef.Name,
472+
StrictProtection: resp.Repository.DefaultBranchRef.BranchProtectionRule.RequiresStrictStatusChecks,
464473
}
465474

466475
return &payload, nil
@@ -1071,47 +1080,6 @@ func PullRequestReopen(client *Client, repo ghrepo.Interface, pr *PullRequest) e
10711080
return err
10721081
}
10731082

1074-
func PullRequestMerge(client *Client, repo ghrepo.Interface, pr *PullRequest, m PullRequestMergeMethod, body *string) error {
1075-
mergeMethod := githubv4.PullRequestMergeMethodMerge
1076-
switch m {
1077-
case PullRequestMergeMethodRebase:
1078-
mergeMethod = githubv4.PullRequestMergeMethodRebase
1079-
case PullRequestMergeMethodSquash:
1080-
mergeMethod = githubv4.PullRequestMergeMethodSquash
1081-
}
1082-
1083-
var mutation struct {
1084-
MergePullRequest struct {
1085-
PullRequest struct {
1086-
ID githubv4.ID
1087-
}
1088-
} `graphql:"mergePullRequest(input: $input)"`
1089-
}
1090-
1091-
input := githubv4.MergePullRequestInput{
1092-
PullRequestID: pr.ID,
1093-
MergeMethod: &mergeMethod,
1094-
}
1095-
1096-
if m == PullRequestMergeMethodSquash {
1097-
commitHeadline := githubv4.String(fmt.Sprintf("%s (#%d)", pr.Title, pr.Number))
1098-
input.CommitHeadline = &commitHeadline
1099-
}
1100-
if body != nil {
1101-
commitBody := githubv4.String(*body)
1102-
input.CommitBody = &commitBody
1103-
}
1104-
1105-
variables := map[string]interface{}{
1106-
"input": input,
1107-
}
1108-
1109-
gql := graphQLClient(client.http, repo.RepoHost())
1110-
err := gql.MutateNamed(context.Background(), "PullRequestMerge", &mutation, variables)
1111-
1112-
return err
1113-
}
1114-
11151083
func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) error {
11161084
var mutation struct {
11171085
MarkPullRequestReadyForReview struct {

‎internal/authflow/flow.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"strings"
1010

1111
"github.com/cli/cli/api"
12-
"github.com/cli/cli/internal/config"
1312
"github.com/cli/cli/internal/ghinstance"
1413
"github.com/cli/cli/pkg/browser"
1514
"github.com/cli/cli/pkg/iostreams"
@@ -23,7 +22,12 @@ var (
2322
oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b"
2423
)
2524

26-
func AuthFlowWithConfig(cfg config.Config, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string) (string, error) {
25+
type iconfig interface {
26+
Set(string, string, string) error
27+
Write() error
28+
}
29+
30+
func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string) (string, error) {
2731
// TODO this probably shouldn't live in this package. It should probably be in a new package that
2832
// depends on both iostreams and config.
2933
stderr := IO.ErrOut
@@ -65,7 +69,7 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
6569
httpClient.Transport = api.VerboseLog(IO.ErrOut, logTraffic, IO.ColorEnabled())(httpClient.Transport)
6670
}
6771

68-
minimumScopes := []string{"repo", "read:org", "gist", "workflow"}
72+
minimumScopes := []string{"repo", "read:org", "gist"}
6973
scopes := append(minimumScopes, additionalScopes...)
7074

7175
callbackURI := "http://127.0.0.1/callback"

0 commit comments

Comments
 (0)