check parent annotatiosn for auth skip

vilmibm · vilmibm · commit cba401deb055 · 2020-08-19T10:21:19.000-05:00
diff --git a/cmd/gh/main.go b/cmd/gh/main.go
@@ -93,15 +93,15 @@ func main() {
 		}
 	}
 
-	_, skipAuthCheck := cmd.Annotations["skipAuthCheck"]
+	authCheckEnabled := cmdutil.IsAuthCheckEnabled(cmd)
 
 	// TODO support other names
 	ghtoken := os.Getenv("GITHUB_TOKEN")
 	if ghtoken != "" {
-		skipAuthCheck = true
+		authCheckEnabled = false
 	}
 
-	if !skipAuthCheck {
+	if authCheckEnabled {
 		hasAuth := false
 
 		cfg, err := cmdFactory.Config()
diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go
@@ -95,8 +95,6 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
 		},
 	}
 
-	cmdutil.DisableAuthCheck(cmd)
-
 	cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to authenticate with")
 	cmd.Flags().Bool("with-token", false, "Read token from standard input")
 
diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go
@@ -57,8 +57,6 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co
 		},
 	}
 
-	cmdutil.DisableAuthCheck(cmd)
-
 	cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of")
 
 	return cmd
diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go
@@ -57,8 +57,6 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
 		},
 	}
 
-	cmdutil.DisableAuthCheck(cmd)
-
 	cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication")
 	cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes for gh to have")
 
diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go
@@ -57,8 +57,6 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co
 		},
 	}
 
-	cmdutil.DisableAuthCheck(cmd)
-
 	cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "Check a specific hostname's auth status")
 
 	return cmd
diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go
@@ -58,8 +58,6 @@ func NewCmdConfigGet(f *cmdutil.Factory) *cobra.Command {
 		},
 	}
 
-	cmdutil.DisableAuthCheck(cmd)
-
 	cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting")
 
 	return cmd
@@ -96,8 +94,6 @@ func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command {
 		},
 	}
 
-	cmdutil.DisableAuthCheck(cmd)
-
 	cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting")
 
 	return cmd
diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go
@@ -31,7 +31,7 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, s
 			token, err := cfg.Get(hostname, "oauth_token")
 			if err != nil || token == "" {
 				// Users shouldn't see this because of the pre-execute auth check on commands
-				return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s", hostname, hostname)
+				return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s`", hostname, hostname)
 			}
 
 			return fmt.Sprintf("token %s", token), nil
diff --git a/pkg/cmdutil/auth_check.go b/pkg/cmdutil/auth_check.go
@@ -31,3 +31,16 @@ func CheckAuth(cfg config.Config) bool {
 
 	return false
 }
+
+func IsAuthCheckEnabled(cmd *cobra.Command) bool {
+	if !cmd.Runnable() {
+		return false
+	}
+	for c := cmd; c.Parent() != nil; c = c.Parent() {
+		if c.Annotations != nil && c.Annotations["skipAuthCheck"] == "true" {
+			return false
+		}
+	}
+
+	return true
+}

Original file line number	Diff line number	Diff line change
`@@ -93,15 +93,15 @@ func main() {`
`93`	`93`	`}`
`94`	`94`	`}`
`95`	`95`
`96`		`- _, skipAuthCheck := cmd.Annotations["skipAuthCheck"]`
	`96`	`+ authCheckEnabled := cmdutil.IsAuthCheckEnabled(cmd)`
`97`	`97`
`98`	`98`	`// TODO support other names`
`99`	`99`	`ghtoken := os.Getenv("GITHUB_TOKEN")`
`100`	`100`	`if ghtoken != "" {`
`101`		`- skipAuthCheck = true`
	`101`	`+ authCheckEnabled = false`
`102`	`102`	`}`
`103`	`103`
`104`		`- if !skipAuthCheck {`
	`104`	`+ if authCheckEnabled {`
`105`	`105`	`hasAuth := false`
`106`	`106`
`107`	`107`	`cfg, err := cmdFactory.Config()`
Original file line number	Diff line number	Diff line change
`@@ -95,8 +95,6 @@ func NewCmdLogin(f cmdutil.Factory, runF func(LoginOptions) error) *cobra.Comm`
`95`	`95`	`},`
`96`	`96`	`}`
`97`	`97`
`98`		`- cmdutil.DisableAuthCheck(cmd)`
`99`		`-`
`100`	`98`	`cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to authenticate with")`
`101`	`99`	`cmd.Flags().Bool("with-token", false, "Read token from standard input")`
`102`	`100`
Original file line number	Diff line number	Diff line change
`@@ -57,8 +57,6 @@ func NewCmdLogout(f cmdutil.Factory, runF func(LogoutOptions) error) *cobra.Co`
`57`	`57`	`},`
`58`	`58`	`}`
`59`	`59`
`60`		`- cmdutil.DisableAuthCheck(cmd)`
`61`		`-`
`62`	`60`	`cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of")`
`63`	`61`
`64`	`62`	`return cmd`
Original file line number	Diff line number	Diff line change
`@@ -57,8 +57,6 @@ func NewCmdRefresh(f cmdutil.Factory, runF func(RefreshOptions) error) *cobra.`
`57`	`57`	`},`
`58`	`58`	`}`
`59`	`59`
`60`		`- cmdutil.DisableAuthCheck(cmd)`
`61`		`-`
`62`	`60`	`cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication")`
`63`	`61`	`cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes for gh to have")`
`64`	`62`
Original file line number	Diff line number	Diff line change
`@@ -57,8 +57,6 @@ func NewCmdStatus(f cmdutil.Factory, runF func(StatusOptions) error) *cobra.Co`
`57`	`57`	`},`
`58`	`58`	`}`
`59`	`59`
`60`		`- cmdutil.DisableAuthCheck(cmd)`
`61`		`-`
`62`	`60`	`cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "Check a specific hostname's auth status")`
`63`	`61`
`64`	`62`	`return cmd`
Original file line number	Diff line number	Diff line change
`@@ -58,8 +58,6 @@ func NewCmdConfigGet(f cmdutil.Factory) cobra.Command {`
`58`	`58`	`},`
`59`	`59`	`}`
`60`	`60`
`61`		`- cmdutil.DisableAuthCheck(cmd)`
`62`		`-`
`63`	`61`	`cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting")`
`64`	`62`
`65`	`63`	`return cmd`
`@@ -96,8 +94,6 @@ func NewCmdConfigSet(f cmdutil.Factory) cobra.Command {`
`96`	`94`	`},`
`97`	`95`	`}`
`98`	`96`
`99`		`- cmdutil.DisableAuthCheck(cmd)`
`100`		`-`
`101`	`97`	`cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting")`
`102`	`98`
`103`	`99`	`return cmd`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, s`
`31`	`31`	`token, err := cfg.Get(hostname, "oauth_token")`
`32`	`32`	`if err != nil \|\| token == "" {`
`33`	`33`	`// Users shouldn't see this because of the pre-execute auth check on commands`
`34`		- return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s", hostname, hostname)
	`34`	+ return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s`", hostname, hostname)
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`return fmt.Sprintf("token %s", token), nil`