diff --git a/pkg/cmd/gpg-key/add/add.go b/pkg/cmd/gpg-key/add/add.go index c5b8c2abe..d50529479 100644 --- a/pkg/cmd/gpg-key/add/add.go +++ b/pkg/cmd/gpg-key/add/add.go @@ -7,6 +7,7 @@ import ( "net/http" "os" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -80,18 +81,30 @@ func runAdd(opts *AddOptions) error { err = gpgKeyUpload(httpClient, hostname, keyReader) if err != nil { - if errors.Is(err, scopesError) { - cs := opts.IO.ColorScheme() + cs := opts.IO.ColorScheme() + if errors.Is(err, errScopesMissing) { fmt.Fprint(opts.IO.ErrOut, "Error: insufficient OAuth scopes to list GPG keys\n") fmt.Fprintf(opts.IO.ErrOut, "Run the following to grant scopes: %s\n", cs.Bold("gh auth refresh -s write:gpg_key")) return cmdutil.SilentError } + if errors.Is(err, errDuplicateKey) { + fmt.Fprintf(opts.IO.ErrOut, "%s Error: the key already exists in your account\n", cs.FailureIcon()) + return cmdutil.SilentError + } + if errors.Is(err, errWrongFormat) { + fmt.Fprint(opts.IO.ErrOut, heredoc.Docf(` + %s Error: the GPG key you are trying to upload might not be in ASCII-armored format. + Find your GPG key ID with: %s + Then add it to your account: %s + `, cs.FailureIcon(), cs.Bold("gpg --list-keys"), cs.Bold("gpg --armor --export | gh gpg-key add -"))) + return cmdutil.SilentError + } return err } if opts.IO.IsStdoutTTY() { cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.ErrOut, "%s GPG key added to your account\n", cs.SuccessIcon()) + fmt.Fprintf(opts.IO.Out, "%s GPG key added to your account\n", cs.SuccessIcon()) } return nil } diff --git a/pkg/cmd/gpg-key/add/add_test.go b/pkg/cmd/gpg-key/add/add_test.go index bbb7a01da..01261beae 100644 --- a/pkg/cmd/gpg-key/add/add_test.go +++ b/pkg/cmd/gpg-key/add/add_test.go @@ -4,6 +4,7 @@ import ( "net/http" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -11,32 +12,114 @@ import ( ) func Test_runAdd(t *testing.T) { - ios, stdin, stdout, stderr := iostreams.Test() - ios.SetStdinTTY(false) - ios.SetStdoutTTY(true) - ios.SetStderrTTY(true) + tests := []struct { + name string + stdin string + httpStubs func(*httpmock.Registry) + wantStdout string + wantStderr string + wantErrMsg string + opts AddOptions + }{ + { + name: "valid key", + stdin: "-----BEGIN PGP PUBLIC KEY BLOCK-----", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "user/gpg_keys"), + httpmock.StatusStringResponse(200, ``)) + }, + wantStdout: "✓ GPG key added to your account\n", + wantStderr: "", + wantErrMsg: "", + opts: AddOptions{KeyFile: "-"}, + }, + { + name: "binary format fails", + stdin: "gCAAAAA7H7MHTZWFLJKD3vP4F7v", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "user/gpg_keys"), + httpmock.StatusStringResponse(422, `{ + "message": "Validation Failed", + "errors": [{ + "resource": "GpgKey", + "code": "custom", + "message": "We got an error doing that." + }], + "documentation_url": "https://docs.github.com/v3/users/gpg_keys" + }`), + ) + }, + wantStdout: "", + wantStderr: heredoc.Doc(` + X Error: the GPG key you are trying to upload might not be in ASCII-armored format. + Find your GPG key ID with: gpg --list-keys + Then add it to your account: gpg --armor --export | gh gpg-key add - + `), + wantErrMsg: "SilentError", + opts: AddOptions{KeyFile: "-"}, + }, + { + name: "duplicate key", + stdin: "-----BEGIN PGP PUBLIC KEY BLOCK-----", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("POST", "user/gpg_keys"), + httpmock.WithHeader(httpmock.StatusStringResponse(422, `{ + "message": "Validation Failed", + "errors": [{ + "resource": "GpgKey", + "code": "custom", + "field": "key_id", + "message": "key_id already exists" + }, { + "resource": "GpgKey", + "code": "custom", + "field": "public_key", + "message": "public_key already exists" + }], + "documentation_url": "https://docs.github.com/v3/users/gpg_keys" + }`), "Content-type", "application/json"), + ) + }, + wantStdout: "", + wantStderr: "X Error: the key already exists in your account\n", + wantErrMsg: "SilentError", + opts: AddOptions{KeyFile: "-"}, + }, + } - stdin.WriteString("PUBKEY") + for _, tt := range tests { + ios, stdin, stdout, stderr := iostreams.Test() + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + stdin.WriteString(tt.stdin) - tr := httpmock.Registry{} - defer tr.Verify(t) + reg := &httpmock.Registry{} - tr.Register( - httpmock.REST("POST", "user/gpg_keys"), - httpmock.StringResponse(`{}`)) - - err := runAdd(&AddOptions{ - IO: ios, - Config: func() (config.Config, error) { + tt.opts.IO = ios + tt.opts.HTTPClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + tt.opts.Config = func() (config.Config, error) { return config.NewBlankConfig(), nil - }, - HTTPClient: func() (*http.Client, error) { - return &http.Client{Transport: &tr}, nil - }, - KeyFile: "-", - }) - assert.NoError(t, err) + } - assert.Equal(t, "", stdout.String()) - assert.Equal(t, "✓ GPG key added to your account\n", stderr.String()) + t.Run(tt.name, func(t *testing.T) { + defer reg.Verify(t) + err := runAdd(&tt.opts) + if tt.wantErrMsg != "" { + assert.Equal(t, tt.wantErrMsg, err.Error()) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } } diff --git a/pkg/cmd/gpg-key/add/http.go b/pkg/cmd/gpg-key/add/http.go index 6d0dbd026..c0f6144aa 100644 --- a/pkg/cmd/gpg-key/add/http.go +++ b/pkg/cmd/gpg-key/add/http.go @@ -11,7 +11,9 @@ import ( "github.com/cli/cli/v2/internal/ghinstance" ) -var scopesError = errors.New("insufficient OAuth scopes") +var errScopesMissing = errors.New("insufficient OAuth scopes") +var errDuplicateKey = errors.New("key already exists") +var errWrongFormat = errors.New("key in wrong format") func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader) error { url := ghinstance.RESTPrefix(hostname) + "user/gpg_keys" @@ -42,25 +44,27 @@ func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader) e defer resp.Body.Close() if resp.StatusCode == 404 { - return scopesError + return errScopesMissing } else if resp.StatusCode > 299 { - var httpError api.HTTPError err := api.HandleHTTPError(resp) - if errors.As(err, &httpError) && isDuplicateError(&httpError) { - return nil + var httpError api.HTTPError + if errors.As(err, &httpError) { + for _, e := range httpError.Errors { + if resp.StatusCode == 422 && e.Field == "key_id" && e.Message == "key_id already exists" { + return errDuplicateKey + } + } + } + if resp.StatusCode == 422 && !isGpgKeyArmored(keyBytes) { + return errWrongFormat } return err } - _, err = io.Copy(io.Discard, resp.Body) - if err != nil { - return err - } - + _, _ = io.Copy(io.Discard, resp.Body) return nil } -func isDuplicateError(err *api.HTTPError) bool { - return err.StatusCode == 422 && len(err.Errors) == 1 && - err.Errors[0].Field == "key" && err.Errors[0].Message == "key is already in use" +func isGpgKeyArmored(keyBytes []byte) bool { + return bytes.Contains(keyBytes, []byte("-----BEGIN ")) }