Skip to content

Commit fcaf912

Browse files
committed
Merge branch 'pr-commands-isolate-2' into pr-commands-isolate-3
2 parents 20e3045 + ce5b5ca commit fcaf912

File tree

20 files changed

+1389
-49
lines changed

20 files changed

+1389
-49
lines changed

.github/workflows/releases.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ jobs:
3232
if: "!contains(github.ref, '-')" # skip prereleases
3333
with:
3434
formula-name: gh
35+
download-url: https://github.com/cli/cli.git
3536
env:
3637
COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }}
3738
- name: Checkout documentation site

api/client.go

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package api
33
import (
44
"bytes"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"io"
89
"io/ioutil"
@@ -195,19 +196,18 @@ func (err HTTPError) Error() string {
195196
return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL)
196197
}
197198

198-
// Returns whether or not scopes are present, appID, and error
199-
func (c Client) HasScopes(wantedScopes ...string) (bool, string, error) {
200-
url := "https://api.github.com/user"
201-
req, err := http.NewRequest("GET", url, nil)
199+
func (c Client) HasMinimumScopes(hostname string) (bool, error) {
200+
apiEndpoint := ghinstance.RESTPrefix(hostname)
201+
202+
req, err := http.NewRequest("GET", apiEndpoint, nil)
202203
if err != nil {
203-
return false, "", err
204+
return false, err
204205
}
205206

206207
req.Header.Set("Content-Type", "application/json; charset=utf-8")
207-
208208
res, err := c.http.Do(req)
209209
if err != nil {
210-
return false, "", err
210+
return false, err
211211
}
212212

213213
defer func() {
@@ -218,26 +218,36 @@ func (c Client) HasScopes(wantedScopes ...string) (bool, string, error) {
218218
}()
219219

220220
if res.StatusCode != 200 {
221-
return false, "", handleHTTPError(res)
221+
return false, handleHTTPError(res)
222222
}
223223

224-
appID := res.Header.Get("X-Oauth-Client-Id")
225224
hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",")
226225

227-
found := 0
226+
search := map[string]bool{
227+
"repo": false,
228+
"read:org": false,
229+
"admin:org": false,
230+
}
231+
228232
for _, s := range hasScopes {
229-
for _, w := range wantedScopes {
230-
if w == strings.TrimSpace(s) {
231-
found++
232-
}
233-
}
233+
search[strings.TrimSpace(s)] = true
234234
}
235235

236-
if found == len(wantedScopes) {
237-
return true, appID, nil
236+
errorMsgs := []string{}
237+
if !search["repo"] {
238+
errorMsgs = append(errorMsgs, "missing required scope 'repo'")
238239
}
239240

240-
return false, appID, nil
241+
if !search["read:org"] && !search["admin:org"] {
242+
errorMsgs = append(errorMsgs, "missing required scope 'read:org'")
243+
}
244+
245+
if len(errorMsgs) > 0 {
246+
return false, errors.New(strings.Join(errorMsgs, ";"))
247+
}
248+
249+
return true, nil
250+
241251
}
242252

243253
// GraphQL performs a GraphQL request and parses the response

command/alias.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func aliasList(cmd *cobra.Command, args []string) error {
194194

195195
var aliasDeleteCmd = &cobra.Command{
196196
Use: "delete <alias>",
197-
Short: "Delete an alias.",
197+
Short: "Delete an alias",
198198
Args: cobra.ExactArgs(1),
199199
RunE: aliasDelete,
200200
}

command/root.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import (
2222
"github.com/cli/cli/internal/ghrepo"
2323
"github.com/cli/cli/internal/run"
2424
apiCmd "github.com/cli/cli/pkg/cmd/api"
25+
authCmd "github.com/cli/cli/pkg/cmd/auth"
26+
authLoginCmd "github.com/cli/cli/pkg/cmd/auth/login"
27+
authLogoutCmd "github.com/cli/cli/pkg/cmd/auth/logout"
2528
gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create"
2629
prCmd "github.com/cli/cli/pkg/cmd/pr"
2730
repoCmd "github.com/cli/cli/pkg/cmd/repo"
@@ -132,6 +135,10 @@ func init() {
132135
RootCmd.AddCommand(gistCmd)
133136
gistCmd.AddCommand(gistCreateCmd.NewCmdCreate(cmdFactory, nil))
134137

138+
RootCmd.AddCommand(authCmd.Cmd)
139+
authCmd.Cmd.AddCommand(authLoginCmd.NewCmdLogin(cmdFactory, nil))
140+
authCmd.Cmd.AddCommand(authLogoutCmd.NewCmdLogout(cmdFactory, nil))
141+
135142
resolvedBaseRepo := func() (ghrepo.Interface, error) {
136143
httpClient, err := cmdFactory.HttpClient()
137144
if err != nil {

internal/config/config_setup.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/cli/cli/api"
1111
"github.com/cli/cli/auth"
12+
"github.com/cli/cli/utils"
1213
)
1314

1415
var (
@@ -67,7 +68,7 @@ func authFlow(oauthHost, notice string) (string, string, error) {
6768
}
6869

6970
fmt.Fprintln(os.Stderr, notice)
70-
fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname)
71+
fmt.Fprintf(os.Stderr, "- %s to open %s in your browser... ", utils.Bold("Press Enter"), flow.Hostname)
7172
_ = waitForEnter(os.Stdin)
7273
token, err := flow.ObtainAccessToken()
7374
if err != nil {
@@ -83,7 +84,8 @@ func authFlow(oauthHost, notice string) (string, string, error) {
8384
}
8485

8586
func AuthFlowComplete() {
86-
fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ")
87+
fmt.Fprintf(os.Stderr, "%s Authentication complete. %s to continue...\n",
88+
utils.GreenCheck(), utils.Bold("Press Enter"))
8789
_ = waitForEnter(os.Stdin)
8890
}
8991

internal/config/config_type.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import (
44
"bytes"
55
"errors"
66
"fmt"
7+
"sort"
78

9+
"github.com/cli/cli/internal/ghinstance"
810
"gopkg.in/yaml.v3"
911
)
1012

@@ -14,6 +16,8 @@ const defaultGitProtocol = "https"
1416
type Config interface {
1517
Get(string, string) (string, error)
1618
Set(string, string, string) error
19+
UnsetHost(string)
20+
Hosts() ([]string, error)
1721
Aliases() (*AliasConfig, error)
1822
Write() error
1923
}
@@ -29,7 +33,7 @@ type HostConfig struct {
2933

3034
// This type implements a low-level get/set config that is backed by an in-memory tree of Yaml
3135
// nodes. It allows us to interact with a yaml-based config programmatically, preserving any
32-
// comments that were present when the yaml waas parsed.
36+
// comments that were present when the yaml was parsed.
3337
type ConfigMap struct {
3438
Root *yaml.Node
3539
}
@@ -236,6 +240,20 @@ func (c *fileConfig) Set(hostname, key, value string) error {
236240
}
237241
}
238242

243+
func (c *fileConfig) UnsetHost(hostname string) {
244+
if hostname == "" {
245+
return
246+
}
247+
248+
hostsEntry, err := c.FindEntry("hosts")
249+
if err != nil {
250+
return
251+
}
252+
253+
cm := ConfigMap{hostsEntry.ValueNode}
254+
cm.RemoveEntry(hostname)
255+
}
256+
239257
func (c *fileConfig) configForHost(hostname string) (*HostConfig, error) {
240258
hosts, err := c.hostEntries()
241259
if err != nil {
@@ -357,6 +375,23 @@ func (c *fileConfig) hostEntries() ([]*HostConfig, error) {
357375
return hostConfigs, nil
358376
}
359377

378+
// Hosts returns a list of all known hostnames configred in hosts.yml
379+
func (c *fileConfig) Hosts() ([]string, error) {
380+
entries, err := c.hostEntries()
381+
if err != nil {
382+
return nil, err
383+
}
384+
385+
hostnames := []string{}
386+
for _, entry := range entries {
387+
hostnames = append(hostnames, entry.Host)
388+
}
389+
390+
sort.SliceStable(hostnames, func(i, j int) bool { return hostnames[i] == ghinstance.Default() })
391+
392+
return hostnames, nil
393+
}
394+
360395
func (c *fileConfig) makeConfigForHost(hostname string) *HostConfig {
361396
hostRoot := &yaml.Node{Kind: yaml.MappingNode}
362397
hostCfg := &HostConfig{

pkg/cmd/api/api.go

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -426,23 +426,43 @@ func parseErrorResponse(r io.Reader, statusCode int) (io.Reader, string, error)
426426

427427
var parsedBody struct {
428428
Message string
429-
Errors []struct {
430-
Message string
431-
}
429+
Errors []json.RawMessage
432430
}
433431
err = json.Unmarshal(b, &parsedBody)
434432
if err != nil {
435433
return r, "", err
436434
}
437-
438435
if parsedBody.Message != "" {
439436
return bodyCopy, fmt.Sprintf("%s (HTTP %d)", parsedBody.Message, statusCode), nil
440-
} else if len(parsedBody.Errors) > 0 {
441-
msgs := make([]string, len(parsedBody.Errors))
442-
for i, e := range parsedBody.Errors {
443-
msgs[i] = e.Message
437+
}
438+
439+
type errorMessage struct {
440+
Message string
441+
}
442+
var errors []string
443+
for _, rawErr := range parsedBody.Errors {
444+
if len(rawErr) == 0 {
445+
continue
446+
}
447+
if rawErr[0] == '{' {
448+
var objectError errorMessage
449+
err := json.Unmarshal(rawErr, &objectError)
450+
if err != nil {
451+
return r, "", err
452+
}
453+
errors = append(errors, objectError.Message)
454+
} else if rawErr[0] == '"' {
455+
var stringError string
456+
err := json.Unmarshal(rawErr, &stringError)
457+
if err != nil {
458+
return r, "", err
459+
}
460+
errors = append(errors, stringError)
444461
}
445-
return bodyCopy, strings.Join(msgs, "\n"), nil
462+
}
463+
464+
if len(errors) > 0 {
465+
return bodyCopy, strings.Join(errors, "\n"), nil
446466
}
447467

448468
return bodyCopy, "", nil

pkg/cmd/api/api_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,17 @@ func Test_apiRun(t *testing.T) {
264264
stdout: `{"message": "THIS IS FINE"}`,
265265
stderr: "gh: THIS IS FINE (HTTP 400)\n",
266266
},
267+
{
268+
name: "REST string errors",
269+
httpResponse: &http.Response{
270+
StatusCode: 400,
271+
Body: ioutil.NopCloser(bytes.NewBufferString(`{"errors": ["ALSO", "FINE"]}`)),
272+
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
273+
},
274+
err: cmdutil.SilentError,
275+
stdout: `{"errors": ["ALSO", "FINE"]}`,
276+
stderr: "gh: ALSO\nFINE\n",
277+
},
267278
{
268279
name: "GraphQL error",
269280
options: ApiOptions{

pkg/cmd/auth/auth.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package auth
2+
3+
import (
4+
"github.com/spf13/cobra"
5+
)
6+
7+
var Cmd = &cobra.Command{
8+
Use: "auth <command>",
9+
Short: "Login, logout, and refresh your authentication",
10+
Long: `Manage gh's authentication state.`,
11+
// TODO this all doesn't exist yet
12+
//Example: heredoc.Doc(`
13+
// $ gh auth login
14+
// $ gh auth status
15+
// $ gh auth refresh --scopes gist
16+
// $ gh auth logout
17+
//`),
18+
}

pkg/cmd/auth/login/client.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package login
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
7+
"github.com/cli/cli/api"
8+
"github.com/cli/cli/internal/config"
9+
)
10+
11+
func validateHostCfg(hostname string, cfg config.Config) error {
12+
apiClient, err := clientFromCfg(hostname, cfg)
13+
if err != nil {
14+
return err
15+
}
16+
17+
_, err = apiClient.HasMinimumScopes(hostname)
18+
if err != nil {
19+
return fmt.Errorf("could not validate token: %w", err)
20+
}
21+
22+
return nil
23+
}
24+
25+
var clientFromCfg = func(hostname string, cfg config.Config) (*api.Client, error) {
26+
var opts []api.ClientOption
27+
28+
token, err := cfg.Get(hostname, "oauth_token")
29+
if err != nil {
30+
return nil, err
31+
}
32+
33+
if token == "" {
34+
return nil, fmt.Errorf("no token found in config for %s", hostname)
35+
}
36+
37+
opts = append(opts,
38+
// no access to Version so the user agent is more generic here.
39+
api.AddHeader("User-Agent", "GitHub CLI"),
40+
api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) {
41+
return fmt.Sprintf("token %s", token), nil
42+
}),
43+
)
44+
45+
httpClient := api.NewHTTPClient(opts...)
46+
47+
return api.NewClientFromHTTP(httpClient), nil
48+
}

0 commit comments

Comments
 (0)