From 665e814c5d17d21ce35dec1074081b6c14fb2136 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Fri, 18 Oct 2024 12:58:00 -0700 Subject: [PATCH 01/14] Setup acceptance testing for auth and tests for auth-token and auth-status --- acceptance/acceptance_test.go | 9 +++++++++ acceptance/testdata/auth/auth-status.txtar | 3 +++ acceptance/testdata/auth/auth-token.txtar | 3 +++ 3 files changed, 15 insertions(+) create mode 100644 acceptance/testdata/auth/auth-status.txtar create mode 100644 acceptance/testdata/auth/auth-token.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 017e6e62b..8804b1534 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -63,6 +63,15 @@ func TestAPI(t *testing.T) { testscript.Run(t, testScriptParamsFor(tsEnv, "api")) } +func TestAuth(t *testing.T) { + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + t.Fatal(err) + } + + testscript.Run(t, testScriptParamsFor(tsEnv, "auth")) +} + func TestReleases(t *testing.T) { var tsEnv testScriptEnv if err := tsEnv.fromEnv(); err != nil { diff --git a/acceptance/testdata/auth/auth-status.txtar b/acceptance/testdata/auth/auth-status.txtar new file mode 100644 index 000000000..da3011d1a --- /dev/null +++ b/acceptance/testdata/auth/auth-status.txtar @@ -0,0 +1,3 @@ +# Check the authentication status +exec gh auth status --hostname $GH_HOST +stdout $GH_HOST \ No newline at end of file diff --git a/acceptance/testdata/auth/auth-token.txtar b/acceptance/testdata/auth/auth-token.txtar new file mode 100644 index 000000000..614d11817 --- /dev/null +++ b/acceptance/testdata/auth/auth-token.txtar @@ -0,0 +1,3 @@ +# Check authentication token +exec gh auth token --hostname $GH_HOST +stdout $GH_TOKEN \ No newline at end of file From 52daa9cf7d6f446726e11061e0525377d50d3843 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Fri, 18 Oct 2024 14:59:15 -0700 Subject: [PATCH 02/14] Add --token flag to `gh auth login` to accept a PAT as a flag Additionally, this commit adds acceptance testing for `gh auth login` and `gh auth logout`. The --token flag was necessary for adding testing for `gh auth login` because the current implementation with `--with-token` appears to be broken. It hangs, waiting for user input, but user input doesn't exit it. Additionally, it appears that `--with-token` is intended to allow for TTY input of an auth token, but it isn't implemented. `--with-token` does work when used with the redirect operator `<` when the token is saved in a file. However, due to limitations of testscripts, I could not use a file for saving the token in a repeatable manner. Thus, implementing the `--token` flag seemed like a quick solution to validate that the direction I was going during testing was valid. Whether the flag stays or not is up for discussion, and I'd love to get input on that from the team. --- acceptance/acceptance_test.go | 26 ++++++++++++++++- .../testdata/auth/auth-login-logout.txtar | 29 +++++++++++++++++++ pkg/cmd/auth/login/login.go | 15 ++++++---- pkg/cmd/auth/logout/logout.go | 1 + 4 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 acceptance/testdata/auth/auth-login-logout.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 8804b1534..3c2b3358b 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -95,7 +95,7 @@ func TestRepo(t *testing.T) { if err := tsEnv.fromEnv(); err != nil { t.Fatal(err) } - + testscript.Run(t, testScriptParamsFor(tsEnv, "repo")) } @@ -240,6 +240,30 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, ts.Check(os.WriteFile(src, data, mode)) }, + // perhaps these need to be extracted to different files, soon? + // I'm prompted to ask because "deleteEnvVar" could just be + // syntactic sugar for setEnvVar with an empty value. Can't do that + // with the current implementation + "deleteEnvVar": func(ts *testscript.TestScript, neg bool, args []string) { + if neg { + ts.Fatalf("unsupported: ! deleteEnvVar") + } + if len(args) != 1 { + ts.Fatalf("usage: deleteEnvVar name") + } + + ts.Setenv(args[0], "") + }, + "setEnvVar": func(ts *testscript.TestScript, neg bool, args []string) { + if neg { + ts.Fatalf("unsupported: ! setEnvVar") + } + if len(args) != 2 { + ts.Fatalf("usage: setEnvVar name value") + } + + ts.Setenv(args[0], args[1]) + }, "stdout2env": func(ts *testscript.TestScript, neg bool, args []string) { if neg { ts.Fatalf("unsupported: ! stdout2env") diff --git a/acceptance/testdata/auth/auth-login-logout.txtar b/acceptance/testdata/auth/auth-login-logout.txtar new file mode 100644 index 000000000..3e46d4a0f --- /dev/null +++ b/acceptance/testdata/auth/auth-login-logout.txtar @@ -0,0 +1,29 @@ +# We aren't logged in at the moment, but GH_HOST will override the +# need to login. We are going to clear GH_HOST first to ensure no +# overrides are happening + +# Copy $GH_TOKEN to a new env var +setEnvVar LOGIN_TOKEN $GH_TOKEN +exec echo $LOGIN_TOKEN +stdout $GH_TOKEN + +# Remove GH_TOKEN env var so we don't fall back to it +deleteEnvVar GH_TOKEN + +# Ensure the token was deleted +exec echo $GH_TOKEN +! stdout '.' + +# Login to the host +exec gh auth login --hostname=$GH_HOST --with-token --token=$LOGIN_TOKEN --insecure-storage + +# Check that we are logged in +exec gh auth status --hostname $GH_HOST +stdout $GH_HOST + +# Logout of the host +exec gh auth logout --hostname $GH_HOST +stderr 'Logged out of' + +# Check that we are logged out +! exec gh auth status --hostname $GH_HOST diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 653c9e6c0..eb0b2e1c4 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -104,14 +104,18 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm } if tokenStdin { - defer opts.IO.In.Close() - token, err := io.ReadAll(opts.IO.In) - if err != nil { - return fmt.Errorf("failed to read token from standard input: %w", err) + if opts.Token == "" { + defer opts.IO.In.Close() + token, err := io.ReadAll(opts.IO.In) + if err != nil { + return fmt.Errorf("failed to read token from standard input: %w", err) + } + opts.Token = strings.TrimSpace(string(token)) } - opts.Token = strings.TrimSpace(string(token)) } + // I think this is dead code because the token will always be set if we make it past + // the conditional above (lines 106-115) if opts.IO.CanPrompt() && opts.Token == "" { opts.Interactive = true } @@ -139,6 +143,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes to request") cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate") + cmd.Flags().StringVarP(&opts.Token, "token", "t", "", "Authenticate using a personal access token") cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations on this host") // secure storage became the default on 2023/4/04; this flag is left as a no-op for backwards compatibility diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index ef978ebac..f1eaa4e24 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -58,6 +58,7 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co } cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of") + // This flag doesn't seem to be working with --user but it is working with -u in acceptance testing cmd.Flags().StringVarP(&opts.Username, "user", "u", "", "The account to log out of") return cmd From aaf4c4e4e3a3c91f5d7a5f87e197bf0de4b59476 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Mon, 21 Oct 2024 09:22:27 -0700 Subject: [PATCH 03/14] Clean up auth-login-logout acceptance test with native functionality The previous commit introduced two new functions, setEnvVar and deleteEnvVar that are duplicative of functionality native to testscripts. This commit switches to the native functionality and removes the duplicative functions introduced in the previous commit. Additionally, it removes the `--token` flag that was added to `gh auth login` --- acceptance/acceptance_test.go | 24 ------------------- .../testdata/auth/auth-login-logout.txtar | 10 ++++---- pkg/cmd/auth/login/login.go | 15 ++++-------- 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 3c2b3358b..dd5b8b0d1 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -240,30 +240,6 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, ts.Check(os.WriteFile(src, data, mode)) }, - // perhaps these need to be extracted to different files, soon? - // I'm prompted to ask because "deleteEnvVar" could just be - // syntactic sugar for setEnvVar with an empty value. Can't do that - // with the current implementation - "deleteEnvVar": func(ts *testscript.TestScript, neg bool, args []string) { - if neg { - ts.Fatalf("unsupported: ! deleteEnvVar") - } - if len(args) != 1 { - ts.Fatalf("usage: deleteEnvVar name") - } - - ts.Setenv(args[0], "") - }, - "setEnvVar": func(ts *testscript.TestScript, neg bool, args []string) { - if neg { - ts.Fatalf("unsupported: ! setEnvVar") - } - if len(args) != 2 { - ts.Fatalf("usage: setEnvVar name value") - } - - ts.Setenv(args[0], args[1]) - }, "stdout2env": func(ts *testscript.TestScript, neg bool, args []string) { if neg { ts.Fatalf("unsupported: ! stdout2env") diff --git a/acceptance/testdata/auth/auth-login-logout.txtar b/acceptance/testdata/auth/auth-login-logout.txtar index 3e46d4a0f..3933ce1da 100644 --- a/acceptance/testdata/auth/auth-login-logout.txtar +++ b/acceptance/testdata/auth/auth-login-logout.txtar @@ -3,19 +3,21 @@ # overrides are happening # Copy $GH_TOKEN to a new env var -setEnvVar LOGIN_TOKEN $GH_TOKEN +env LOGIN_TOKEN=$GH_TOKEN exec echo $LOGIN_TOKEN stdout $GH_TOKEN # Remove GH_TOKEN env var so we don't fall back to it -deleteEnvVar GH_TOKEN +env GH_TOKEN='' # Ensure the token was deleted exec echo $GH_TOKEN ! stdout '.' -# Login to the host -exec gh auth login --hostname=$GH_HOST --with-token --token=$LOGIN_TOKEN --insecure-storage +# Login to the host by feeding the token to stdin +exec echo $LOGIN_TOKEN +stdin stdout +exec gh auth login --hostname=$GH_HOST --with-token --insecure-storage # Check that we are logged in exec gh auth status --hostname $GH_HOST diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index eb0b2e1c4..653c9e6c0 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -104,18 +104,14 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm } if tokenStdin { - if opts.Token == "" { - defer opts.IO.In.Close() - token, err := io.ReadAll(opts.IO.In) - if err != nil { - return fmt.Errorf("failed to read token from standard input: %w", err) - } - opts.Token = strings.TrimSpace(string(token)) + defer opts.IO.In.Close() + token, err := io.ReadAll(opts.IO.In) + if err != nil { + return fmt.Errorf("failed to read token from standard input: %w", err) } + opts.Token = strings.TrimSpace(string(token)) } - // I think this is dead code because the token will always be set if we make it past - // the conditional above (lines 106-115) if opts.IO.CanPrompt() && opts.Token == "" { opts.Interactive = true } @@ -143,7 +139,6 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes to request") cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate") - cmd.Flags().StringVarP(&opts.Token, "token", "t", "", "Authenticate using a personal access token") cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations on this host") // secure storage became the default on 2023/4/04; this flag is left as a no-op for backwards compatibility From 0614d85027c0b05f2c2a37a2f88352d9c51bda8d Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Mon, 21 Oct 2024 15:21:47 -0700 Subject: [PATCH 04/14] Add acceptance tests for auth-setup-git and formattedStringToEnv helper func To test this, I decided to look into the .gitconfig used for the test and examine the credential helpers. However, the format of the git command is `git config --get credential..helper` What's awkward about this is that the depends on the host the user specified when running the tests, meaning I'd need to create a key like credential.https://github.com.helper to access what I need while setting this up. There was no functionality for string formatting before, so I added the command formattedStringToEnv which essentially wraps fmt.Sprintf() and saves the string to an environment variable. This allowed me to dynamically create the config key in the test. --- acceptance/acceptance_test.go | 19 +++++++++++++++++++ acceptance/testdata/auth/auth-setup-git.txtar | 13 +++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 acceptance/testdata/auth/auth-setup-git.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index dd5b8b0d1..ace985af6 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -203,6 +203,17 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, ts.Setenv(env[:i], strings.ToUpper(env[i+1:])) } }, + // Creates a formatted string using standard fmt.Sprintf syntax + // and sets it as an environment variable of the given name. + "formattedStringToEnv": func(ts *testscript.TestScript, neg bool, args []string) { + if neg { + ts.Fatalf("unsupported: ! formattedStringToEnv") + } + if len(args) < 3 { + ts.Fatalf("usage: formattedStringToEnv name string args...") + } + ts.Setenv(args[0], fmt.Sprintf(args[1], toAnySlice(args[2:])...)) + }, "replace": func(ts *testscript.TestScript, neg bool, args []string) { if neg { ts.Fatalf("unsupported: ! replace") @@ -270,6 +281,14 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, } } +func toAnySlice(strings []string) []any { + anys := make([]any, len(strings)) + for i, s := range strings { + anys[i] = s + } + return anys +} + var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") func randomString(n int) string { diff --git a/acceptance/testdata/auth/auth-setup-git.txtar b/acceptance/testdata/auth/auth-setup-git.txtar new file mode 100644 index 000000000..84794ae8f --- /dev/null +++ b/acceptance/testdata/auth/auth-setup-git.txtar @@ -0,0 +1,13 @@ +# Setup the credential key name string +formattedStringToEnv CREDENTIAL_HELPER_KEY 'credential.https://%s.helper' $GH_HOST + +# Check that the credential helper is unset for the host. This command is +# expected to fail before gh auth setup-git is run. +! exec git config --get $CREDENTIAL_HELPER_KEY + +# Run the setup-git command +exec gh auth setup-git + +# Check that the credential helper is set to gh +exec git config --get $CREDENTIAL_HELPER_KEY +stdout gh From 59aedc4e89c4aa359c87c55ad17b2a38233bb008 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Mon, 21 Oct 2024 16:09:58 -0700 Subject: [PATCH 05/14] Remove comment from gh auth logout --- pkg/cmd/auth/logout/logout.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index f1eaa4e24..ef978ebac 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -58,7 +58,6 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co } cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of") - // This flag doesn't seem to be working with --user but it is working with -u in acceptance testing cmd.Flags().StringVarP(&opts.Username, "user", "u", "", "The account to log out of") return cmd From 2b480daf7abba51332ef1493a6d19120bc4c9ddd Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Wed, 23 Oct 2024 13:56:09 -0700 Subject: [PATCH 06/14] Address PR feedback --- acceptance/acceptance_test.go | 19 ------------------- .../testdata/auth/auth-login-logout.txtar | 10 ++-------- acceptance/testdata/auth/auth-setup-git.txtar | 9 +++------ acceptance/testdata/auth/auth-status.txtar | 2 +- 4 files changed, 6 insertions(+), 34 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index ace985af6..dd5b8b0d1 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -203,17 +203,6 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, ts.Setenv(env[:i], strings.ToUpper(env[i+1:])) } }, - // Creates a formatted string using standard fmt.Sprintf syntax - // and sets it as an environment variable of the given name. - "formattedStringToEnv": func(ts *testscript.TestScript, neg bool, args []string) { - if neg { - ts.Fatalf("unsupported: ! formattedStringToEnv") - } - if len(args) < 3 { - ts.Fatalf("usage: formattedStringToEnv name string args...") - } - ts.Setenv(args[0], fmt.Sprintf(args[1], toAnySlice(args[2:])...)) - }, "replace": func(ts *testscript.TestScript, neg bool, args []string) { if neg { ts.Fatalf("unsupported: ! replace") @@ -281,14 +270,6 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, } } -func toAnySlice(strings []string) []any { - anys := make([]any, len(strings)) - for i, s := range strings { - anys[i] = s - } - return anys -} - var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") func randomString(n int) string { diff --git a/acceptance/testdata/auth/auth-login-logout.txtar b/acceptance/testdata/auth/auth-login-logout.txtar index 3933ce1da..fb25f9eb5 100644 --- a/acceptance/testdata/auth/auth-login-logout.txtar +++ b/acceptance/testdata/auth/auth-login-logout.txtar @@ -1,19 +1,13 @@ -# We aren't logged in at the moment, but GH_HOST will override the -# need to login. We are going to clear GH_HOST first to ensure no +# We aren't logged in at the moment, but GH_TOKEN will override the +# need to login. We are going to clear GH_TOKEN first to ensure no # overrides are happening # Copy $GH_TOKEN to a new env var env LOGIN_TOKEN=$GH_TOKEN -exec echo $LOGIN_TOKEN -stdout $GH_TOKEN # Remove GH_TOKEN env var so we don't fall back to it env GH_TOKEN='' -# Ensure the token was deleted -exec echo $GH_TOKEN -! stdout '.' - # Login to the host by feeding the token to stdin exec echo $LOGIN_TOKEN stdin stdout diff --git a/acceptance/testdata/auth/auth-setup-git.txtar b/acceptance/testdata/auth/auth-setup-git.txtar index 84794ae8f..e3be28cd5 100644 --- a/acceptance/testdata/auth/auth-setup-git.txtar +++ b/acceptance/testdata/auth/auth-setup-git.txtar @@ -1,13 +1,10 @@ -# Setup the credential key name string -formattedStringToEnv CREDENTIAL_HELPER_KEY 'credential.https://%s.helper' $GH_HOST - # Check that the credential helper is unset for the host. This command is # expected to fail before gh auth setup-git is run. -! exec git config --get $CREDENTIAL_HELPER_KEY +! exec git config --get credential.https://${GH_HOST}.helper # Run the setup-git command exec gh auth setup-git # Check that the credential helper is set to gh -exec git config --get $CREDENTIAL_HELPER_KEY -stdout gh +exec git config --get credential.https://${GH_HOST}.helper +stdout '^.*gh auth git-credential$' diff --git a/acceptance/testdata/auth/auth-status.txtar b/acceptance/testdata/auth/auth-status.txtar index da3011d1a..2afee1eb6 100644 --- a/acceptance/testdata/auth/auth-status.txtar +++ b/acceptance/testdata/auth/auth-status.txtar @@ -1,3 +1,3 @@ # Check the authentication status exec gh auth status --hostname $GH_HOST -stdout $GH_HOST \ No newline at end of file +stdout '✓ Logged in to ' \ No newline at end of file From f27cddcb9279f0d1840b221af476a87a890c7460 Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Thu, 24 Oct 2024 08:43:40 -0600 Subject: [PATCH 07/14] Add acceptance test for gpg-key --- acceptance/acceptance_test.go | 11 +++++++- acceptance/testdata/gpg-key/gpg-key.txtar | 32 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 acceptance/testdata/gpg-key/gpg-key.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index bdd631703..2d9e8aeb0 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -86,7 +86,7 @@ func TestRepo(t *testing.T) { if err := tsEnv.fromEnv(); err != nil { t.Fatal(err) } - + testscript.Run(t, testScriptParamsFor(tsEnv, "repo")) } @@ -108,6 +108,15 @@ func TestVariables(t *testing.T) { testscript.Run(t, testScriptParamsFor(tsEnv, "variable")) } +func TestGPGKeys(t *testing.T) { + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + t.Fatal(err) + } + + testscript.Run(t, testScriptParamsFor(tsEnv, "gpg-key")) +} + func testScriptParamsFor(tsEnv testScriptEnv, command string) testscript.Params { var files []string if tsEnv.script != "" { diff --git a/acceptance/testdata/gpg-key/gpg-key.txtar b/acceptance/testdata/gpg-key/gpg-key.txtar new file mode 100644 index 000000000..dff028bab --- /dev/null +++ b/acceptance/testdata/gpg-key/gpg-key.txtar @@ -0,0 +1,32 @@ +skip 'it modifies the user''s personal GitHub account GPG keys' + +# This test requires the admin:gpg_key scope to add and delete GPG keys to and +# from the user's personal GitHub account. +# This test uses a GPG key that generated for this test only. The private key +# has been deleted + +# Add the gpg key to GH account +exec gh gpg-key add gpg-key.pub + +# Defer deleting the gpg key from GH account +defer gh gpg-key delete --yes 24C30F9C9115E747 + +# Verify the gpg key was added to GH account +exec gh gpg-key list +stdout 24C30F9C9115E747 + +-- gpg-key.pub -- +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mDMEZxpWhhYJKwYBBAHaRw8BAQdAmYiobR2ai/lVWOBtlAPRG1ZEMG5Effavpt5w +n+wQ//W0R0dIIENMSSBhY2NlcHRhbmNlIHRlc3QgKGZvciBHSCBDTEkgYWNjZXB0 +YW5jZSB0ZXN0aW5nKSA8Y2xpQGdpdGh1Yi5jb20+iJkEExYKAEEWIQTEAQLLUl1x +MDSmbL0kww+ckRXnRwUCZxpWhgIbAwUJAAFRgAULCQgHAgIiAgYVCgkICwIEFgID +AQIeBwIXgAAKCRAkww+ckRXnRxkuAP9GiFi/etWxRjnkomdTaOU8Ccd6oHspuEzB +PFxOJdYslQD+MXgY5UhM/q2iEVj0tiVsfRzDqB+g2weaF5EpqIwWcQ+4OARnGlaG +EgorBgEEAZdVAQUBAQdA3D1vnVTc9URDQw/oAd1mG/zRX7vF4QrjFqFIt7uMf2gD +AQgHiH4EGBYKACYWIQTEAQLLUl1xMDSmbL0kww+ckRXnRwUCZxpWhgIbDAUJAAFR +gAAKCRAkww+ckRXnRxVuAQCngnR11jh2mob0FN0rPWce2juoJsh5gPB2d7LS4r5P +VwEA6F2FeetcP51EyKyQGTp3GpmZgk0uCGJa1G5uqT+9mgc= +=RLWi +-----END PGP PUBLIC KEY BLOCK----- \ No newline at end of file From 4100992d4004b28c507fe638a5ef3782a9a9d22e Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 24 Oct 2024 17:00:54 +0200 Subject: [PATCH 08/14] Add Acceptance test for label command --- acceptance/acceptance_test.go | 9 +++++++++ acceptance/testdata/label/label.txtar | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 acceptance/testdata/label/label.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2d9e8aeb0..70ddbe4dc 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -117,6 +117,15 @@ func TestGPGKeys(t *testing.T) { testscript.Run(t, testScriptParamsFor(tsEnv, "gpg-key")) } +func TestLabels(t *testing.T) { + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + t.Fatal(err) + } + + testscript.Run(t, testScriptParamsFor(tsEnv, "label")) +} + func testScriptParamsFor(tsEnv testScriptEnv, command string) testscript.Params { var files []string if tsEnv.script != "" { diff --git a/acceptance/testdata/label/label.txtar b/acceptance/testdata/label/label.txtar new file mode 100644 index 000000000..dd72133da --- /dev/null +++ b/acceptance/testdata/label/label.txtar @@ -0,0 +1,25 @@ +# Setup useful env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Create a repository +exec gh repo create ${ORG}/${REPO} --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Set the GH_REPO env var to reduce redunant flags +env GH_REPO=${ORG}/${REPO} + +# Create a custom label +exec gh label create 'acceptance-test' --description 'First Description' + +# List the labels and check our custom label is there +exec gh label list +stdout 'acceptance-test\tFirst Description' + +# Edit the label +exec gh label edit 'acceptance-test' --description 'Edited Description' + +# List the labels and check our custom label has been updated +exec gh label list +stdout 'acceptance-test\tEdited Description' From 76e6fbba366b2e99f7841e92d6f6cc38c7226d6c Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 24 Oct 2024 17:46:17 +0200 Subject: [PATCH 09/14] Add SSH Key Acceptance test --- acceptance/acceptance_test.go | 9 +++++++++ acceptance/testdata/ssh-key/ssh-key.txtar | 24 +++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 acceptance/testdata/ssh-key/ssh-key.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 70ddbe4dc..18a3f0cef 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -126,6 +126,15 @@ func TestLabels(t *testing.T) { testscript.Run(t, testScriptParamsFor(tsEnv, "label")) } +func TestSSHKeys(t *testing.T) { + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + t.Fatal(err) + } + + testscript.Run(t, testScriptParamsFor(tsEnv, "ssh-key")) +} + func testScriptParamsFor(tsEnv testScriptEnv, command string) testscript.Params { var files []string if tsEnv.script != "" { diff --git a/acceptance/testdata/ssh-key/ssh-key.txtar b/acceptance/testdata/ssh-key/ssh-key.txtar new file mode 100644 index 000000000..4ba8643bb --- /dev/null +++ b/acceptance/testdata/ssh-key/ssh-key.txtar @@ -0,0 +1,24 @@ +skip 'it modifies the user''s personal GitHub account SSH keys' + +# scopes admin:ssh_signing_key,admin:public_key + +# Add an SSH key to the account +exec gh ssh-key add sshKey.pub --title 'acceptance-test-key' + +# List the SSH keys +exec gh ssh-key list +stdout 'acceptance-test-key' + +# Get the ID of the key we created +exec gh api /user/keys --jq '.[] | select(.title == "acceptance-test-key") | .id' +stdout2env SSH_KEY_ID + +# Delete the SSH key +exec gh ssh-key delete --yes ${SSH_KEY_ID} + +# Check the key is deleted +exec gh ssh-key list +! stdout 'acceptance-test-key' + +-- sshKey.pub -- +ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAZmdeRNskfpvYL5YHB/YJaW8hTEXpnvPMkx5Ri+YwUr acceptance From d4c70009bffb0e7cb4521f56fa0d7d55de9ee299 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 24 Oct 2024 17:54:27 +0200 Subject: [PATCH 10/14] Adjust environment help for host and tokens (#9809) --- pkg/cmd/root/help_topic.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index e76dd2271..6d22b6ae7 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -41,17 +41,18 @@ var HelpTopics = []helpTopic{ { name: "environment", short: "Environment variables that can be used with gh", - long: heredoc.Docf(` - %[1]sGH_TOKEN%[1]s, %[1]sGITHUB_TOKEN%[1]s (in order of precedence): an authentication token for github.com - API requests. Setting this avoids being prompted to authenticate and takes precedence over - previously stored credentials. + long: heredoc.Docf(` + %[1]sGH_TOKEN%[1]s, %[1]sGITHUB_TOKEN%[1]s (in order of precedence): an authentication token that will be used when + a command targets either github.com or a subdomain of ghe.com. Setting this avoids being prompted to + authenticate and takes precedence over previously stored credentials. - %[1]sGH_ENTERPRISE_TOKEN%[1]s, %[1]sGITHUB_ENTERPRISE_TOKEN%[1]s (in order of precedence): an authentication - token for API requests to GitHub Enterprise. When setting this, also set %[1]sGH_HOST%[1]s. + %[1]sGH_ENTERPRISE_TOKEN%[1]s, %[1]sGITHUB_ENTERPRISE_TOKEN%[1]s (in order of precedence): an authentication + token that will be used when a command targets a GitHub Enterprise Server host. - %[1]sGH_HOST%[1]s: specify the GitHub hostname for commands that would otherwise assume the - "github.com" host when not in a context of an existing repository. When setting this, - also set %[1]sGH_ENTERPRISE_TOKEN%[1]s. + %[1]sGH_HOST%[1]s: specify the GitHub hostname for commands where a hostname has not been provided, or + cannot be inferred from the context of a local Git repository. If this host was previously + authenticated with, the stored credentials will be used. Otherwise, setting %[1]sGH_TOKEN%[1]s or + %[1]sGH_ENTERPRISE_TOKEN%[1]s is required, depending on the targeted host. %[1]sGH_REPO%[1]s: specify the GitHub repository in the %[1]s[HOST/]OWNER/REPO%[1]s format for commands that otherwise operate on a local repository. From 2f849f03ff426233e2847e5e7d55c1925078ed73 Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Thu, 24 Oct 2024 09:58:06 -0600 Subject: [PATCH 11/14] Add acceptance tests for org command --- acceptance/acceptance_test.go | 9 +++++++++ acceptance/testdata/org/org-list.txtar | 6 ++++++ 2 files changed, 15 insertions(+) create mode 100644 acceptance/testdata/org/org-list.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 18a3f0cef..56d7bf455 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -135,6 +135,15 @@ func TestSSHKeys(t *testing.T) { testscript.Run(t, testScriptParamsFor(tsEnv, "ssh-key")) } +func TestOrg(t *testing.T) { + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + t.Fatal(err) + } + + testscript.Run(t, testScriptParamsFor(tsEnv, "org")) +} + func testScriptParamsFor(tsEnv testScriptEnv, command string) testscript.Params { var files []string if tsEnv.script != "" { diff --git a/acceptance/testdata/org/org-list.txtar b/acceptance/testdata/org/org-list.txtar new file mode 100644 index 000000000..ca114babe --- /dev/null +++ b/acceptance/testdata/org/org-list.txtar @@ -0,0 +1,6 @@ +# This test could fail if the user is a member of more than 30 organizations because +# the `gh org list` command only returns the first 30 organizations the user is a member of + +# List organizations the user is a member of +exec gh org list +stdout ${GH_ACCEPTANCE_ORG} \ No newline at end of file From f72a82db99909239b127cadcd3c39d67caec90ff Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Thu, 24 Oct 2024 09:59:02 -0600 Subject: [PATCH 12/14] Refactor gpg-key delete to align with ssh-key delete --- acceptance/testdata/gpg-key/gpg-key.txtar | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/acceptance/testdata/gpg-key/gpg-key.txtar b/acceptance/testdata/gpg-key/gpg-key.txtar index dff028bab..8f0d71545 100644 --- a/acceptance/testdata/gpg-key/gpg-key.txtar +++ b/acceptance/testdata/gpg-key/gpg-key.txtar @@ -8,12 +8,16 @@ skip 'it modifies the user''s personal GitHub account GPG keys' # Add the gpg key to GH account exec gh gpg-key add gpg-key.pub -# Defer deleting the gpg key from GH account -defer gh gpg-key delete --yes 24C30F9C9115E747 - # Verify the gpg key was added to GH account exec gh gpg-key list -stdout 24C30F9C9115E747 +stdout '24C30F9C9115E747' + +# Delete the gpg key from GH account +exec gh gpg-key delete --yes '24C30F9C9115E747' + +# Check the key is deleted +exec gh gpg-key list +! stdout '24C30F9C9115E747' -- gpg-key.pub -- -----BEGIN PGP PUBLIC KEY BLOCK----- From 366aea95ebb4e76c1bc4b32dc4ed245492e83340 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 24 Oct 2024 18:35:30 +0200 Subject: [PATCH 13/14] Note token redaction in Acceptance test README --- acceptance/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/acceptance/README.md b/acceptance/README.md index 750cb75d1..8e8d7838e 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -159,7 +159,9 @@ When tests fail they fail like this: This is generally enough information to understand why a test has failed. However, we can get more information by providing the `-v` flag to `go test`, which turns on verbose mode and shows each command and any associated `stdio`. > [!WARNING] -> Verbose mode dumps the `testscript` environment variables, including the `GH_TOKEN`, so be careful. +> Verbose mode dumps the `testscript` environment variables, so make sure there is nothing sensitive in there. +> We have taken steps to [redact tokens](https://github.com/cli/cli/pull/9804) in log output but there's no +> guarantee it's comprehensive. By default `testscript` removes the directory in which it was running the script, and if you've been a conscientious engineer, you should be cleaning up resources using the `defer` statement. However, this can be an impediment to debugging. As such you can set `GH_ACCEPTANCE_PRESERVE_WORK_DIR=true` and `GH_ACCEPTANCE_SKIP_DEFER=true` to skip these cleanup steps. From 30d9fc53d1c4e2262e840dab0d9743bcc6931f81 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 25 Oct 2024 16:29:23 +0200 Subject: [PATCH 14/14] Update testscript to use hard fork --- acceptance/acceptance_test.go | 2 +- go.mod | 4 +--- go.sum | 6 ++++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index e764abbbc..6efa4864e 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -14,7 +14,7 @@ import ( "math/rand" "github.com/cli/cli/v2/internal/ghcmd" - "github.com/rogpeppe/go-internal/testscript" + "github.com/cli/go-internal/testscript" ) func ghMain() int { diff --git a/go.mod b/go.mod index 138ef8b60..77be0c19d 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/charmbracelet/glamour v0.7.0 github.com/charmbracelet/lipgloss v0.10.1-0.20240413172830-d0be07ea6b9c github.com/cli/go-gh/v2 v2.11.0 + github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 github.com/cli/oauth v1.1.1 github.com/cli/safeexec v1.0.1 github.com/cpuguy83/go-md2man/v2 v2.0.5 @@ -125,7 +126,6 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/rodaine/table v1.0.1 // indirect - github.com/rogpeppe/go-internal v1.13.1 github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/sagikazarmark/locafero v0.4.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect @@ -167,5 +167,3 @@ require ( gopkg.in/ini.v1 v1.67.0 // indirect k8s.io/klog/v2 v2.120.1 // indirect ) - -replace github.com/rogpeppe/go-internal => github.com/cli/go-internal v0.0.0-20241024130215-fa3c22e38b9b diff --git a/go.sum b/go.sum index d23e604f0..fe7adc3e7 100644 --- a/go.sum +++ b/go.sum @@ -97,8 +97,8 @@ github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo= github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk= github.com/cli/go-gh/v2 v2.11.0 h1:TERLYMMWderKBO3lBff/JIu2+eSly2oFRgN2WvO+3eA= github.com/cli/go-gh/v2 v2.11.0/go.mod h1:MeRoKzXff3ygHu7zP+NVTT+imcHW6p3tpuxHAzRM2xE= -github.com/cli/go-internal v0.0.0-20241024130215-fa3c22e38b9b h1:ogNz+pm9aI2Zn58V+WTMzPViN2fRVq/h2H9gwdvM09k= -github.com/cli/go-internal v0.0.0-20241024130215-fa3c22e38b9b/go.mod h1:S8kfXMp+yh77OxPD4fdM6YUknrZpQxLhvxzS4gDHENY= +github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 h1:QDrhR4JA2n3ij9YQN0u5ZeuvRIIvsUGmf5yPlTS0w8E= +github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24/go.mod h1:rr9GNING0onuVw8MnracQHn7PcchnFlP882Y0II2KZk= github.com/cli/oauth v1.1.1 h1:459gD3hSjlKX9B1uXBuiAMdpXBUQ9QGf/NDcCpoQxPs= github.com/cli/oauth v1.1.1/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4= github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q= @@ -368,6 +368,8 @@ github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/rodaine/table v1.0.1 h1:U/VwCnUxlVYxw8+NJiLIuCxA/xa6jL38MY3FYysVWWQ= github.com/rodaine/table v1.0.1/go.mod h1:UVEtfBsflpeEcD56nF4F5AocNFta0ZuolpSVdPtlmP4= +github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= +github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk=