From 81591a09b8a1a356ae9b6e8bfc130b64293a922e Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Fri, 11 Oct 2024 15:06:11 -0700 Subject: [PATCH] Use go-gh/auth package for IsEnterprise, IsTenancy, and NormalizeHostname --- api/client.go | 4 +- api/http_client.go | 4 +- internal/authflow/flow.go | 4 +- internal/config/config.go | 10 +- .../featuredetection/feature_detection.go | 7 +- internal/ghinstance/host.go | 42 +---- internal/ghinstance/host_test.go | 145 ------------------ internal/ghrepo/repo.go | 4 +- pkg/cmd/attestation/auth/host.go | 4 +- pkg/cmd/attestation/inspect/inspect.go | 2 +- .../attestation/trustedroot/trustedroot.go | 3 +- pkg/cmd/attestation/verify/verify.go | 2 +- pkg/cmd/auth/login/login.go | 4 +- pkg/cmd/gist/clone/clone.go | 5 +- pkg/cmd/pr/status/http.go | 5 +- pkg/cmd/repo/delete/delete.go | 4 +- pkg/cmd/ruleset/list/list.go | 4 +- pkg/cmd/ruleset/view/view.go | 4 +- 18 files changed, 45 insertions(+), 212 deletions(-) diff --git a/api/client.go b/api/client.go index e32856554..b30f5f164 100644 --- a/api/client.go +++ b/api/client.go @@ -10,8 +10,8 @@ import ( "regexp" "strings" - "github.com/cli/cli/v2/internal/ghinstance" ghAPI "github.com/cli/go-gh/v2/pkg/api" + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) const ( @@ -249,7 +249,7 @@ func generateScopesSuggestion(statusCode int, endpointNeedsScopes, tokenHasScope return fmt.Sprintf( "This API operation needs the %[1]q scope. To request it, run: gh auth refresh -h %[2]s -s %[1]s", s, - ghinstance.NormalizeHostname(hostname), + ghauth.NormalizeHostname(hostname), ) } diff --git a/api/http_client.go b/api/http_client.go index f6e133f1f..e9381fa85 100644 --- a/api/http_client.go +++ b/api/http_client.go @@ -7,9 +7,9 @@ import ( "strings" "time" - "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/utils" ghAPI "github.com/cli/go-gh/v2/pkg/api" + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) type tokenGetter interface { @@ -98,7 +98,7 @@ func AddAuthTokenHeader(rt http.RoundTripper, cfg tokenGetter) http.RoundTripper // Only set header if an initial request or redirect request to the same host as the initial request. // If the host has changed during a redirect do not add the authentication token header. if !redirectHostnameChange { - hostname := ghinstance.NormalizeHostname(getHost(req)) + hostname := ghauth.NormalizeHostname(getHost(req)) if token, _ := cfg.ActiveToken(hostname); token != "" { req.Header.Set(authorization, fmt.Sprintf("token %s", token)) } diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index e52f8dc0a..1511f6e67 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -16,6 +16,8 @@ import ( "github.com/cli/cli/v2/utils" "github.com/cli/oauth" "github.com/henvic/httpretty" + + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) var ( @@ -105,7 +107,7 @@ func AuthFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition func getCallbackURI(oauthHost string) string { callbackURI := "http://127.0.0.1/callback" - if ghinstance.IsEnterprise(oauthHost) { + if ghauth.IsEnterprise(oauthHost) { // the OAuth app on Enterprise hosts is still registered with a legacy callback URL // see https://github.com/cli/cli/pull/222, https://github.com/cli/cli/pull/650 callbackURI = "http://localhost/" diff --git a/internal/config/config.go b/internal/config/config.go index 1b56d30b2..476154d66 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,7 +10,7 @@ import ( "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/keyring" o "github.com/cli/cli/v2/pkg/option" - ghAuth "github.com/cli/go-gh/v2/pkg/auth" + ghauth "github.com/cli/go-gh/v2/pkg/auth" ghConfig "github.com/cli/go-gh/v2/pkg/config" ) @@ -206,7 +206,7 @@ func (c *AuthConfig) ActiveToken(hostname string) (string, string) { if c.tokenOverride != nil { return c.tokenOverride(hostname) } - token, source := ghAuth.TokenFromEnvOrConfig(hostname) + token, source := ghauth.TokenFromEnvOrConfig(hostname) if token == "" { var err error token, err = c.TokenFromKeyring(hostname) @@ -240,7 +240,7 @@ func (c *AuthConfig) HasEnvToken() bool { // It has to use a hostname that is not going to be found in the hosts so that it // can guarantee that tokens will only be returned from a set env var. // Discussed here, but maybe worth revisiting: https://github.com/cli/cli/pull/7169#discussion_r1136979033 - token, _ := ghAuth.TokenFromEnvOrConfig(hostname) + token, _ := ghauth.TokenFromEnvOrConfig(hostname) return token != "" } @@ -282,7 +282,7 @@ func (c *AuthConfig) Hosts() []string { if c.hostsOverride != nil { return c.hostsOverride() } - return ghAuth.KnownHosts() + return ghauth.KnownHosts() } // SetHosts will override any hosts resolution and return the given @@ -297,7 +297,7 @@ func (c *AuthConfig) DefaultHost() (string, string) { if c.defaultHostOverride != nil { return c.defaultHostOverride() } - return ghAuth.DefaultHost() + return ghauth.DefaultHost() } // SetDefaultHost will override any host resolution and return the given diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index 396d1eb5b..a9bbe25f8 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -4,8 +4,9 @@ import ( "net/http" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/internal/ghinstance" "golang.org/x/sync/errgroup" + + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) type Detector interface { @@ -62,7 +63,7 @@ func NewDetector(httpClient *http.Client, host string) Detector { } func (d *detector) IssueFeatures() (IssueFeatures, error) { - if !ghinstance.IsEnterprise(d.host) { + if !ghauth.IsEnterprise(d.host) { return allIssueFeatures, nil } @@ -163,7 +164,7 @@ func (d *detector) PullRequestFeatures() (PullRequestFeatures, error) { } func (d *detector) RepositoryFeatures() (RepositoryFeatures, error) { - if !ghinstance.IsEnterprise(d.host) { + if !ghauth.IsEnterprise(d.host) { return allRepositoryFeatures, nil } diff --git a/internal/ghinstance/host.go b/internal/ghinstance/host.go index f80bfef57..836a478c5 100644 --- a/internal/ghinstance/host.go +++ b/internal/ghinstance/host.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "strings" + + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) // DefaultHostname is the domain name of the default GitHub instance. @@ -20,22 +22,10 @@ func Default() string { return defaultHostname } -// IsEnterprise reports whether a non-normalized host name looks like a GHE instance. -func IsEnterprise(h string) bool { - normalizedHostName := NormalizeHostname(h) - return normalizedHostName != defaultHostname && normalizedHostName != localhost -} - -// IsTenancy reports whether a non-normalized host name looks like a tenancy instance. -func IsTenancy(h string) bool { - normalizedHostName := NormalizeHostname(h) - return strings.HasSuffix(normalizedHostName, "."+tenancyHost) -} - // TenantName extracts the tenant name from tenancy host name and // reports whether it found the tenant name. func TenantName(h string) (string, bool) { - normalizedHostName := NormalizeHostname(h) + normalizedHostName := ghauth.NormalizeHostname(h) return cutSuffix(normalizedHostName, "."+tenancyHost) } @@ -43,22 +33,6 @@ func isGarage(h string) bool { return strings.EqualFold(h, "garage.github.com") } -// NormalizeHostname returns the canonical host name of a GitHub instance. -func NormalizeHostname(h string) string { - hostname := strings.ToLower(h) - if strings.HasSuffix(hostname, "."+defaultHostname) { - return defaultHostname - } - if strings.HasSuffix(hostname, "."+localhost) { - return localhost - } - if before, found := cutSuffix(hostname, "."+tenancyHost); found { - idx := strings.LastIndex(before, ".") - return fmt.Sprintf("%s.%s", before[idx+1:], tenancyHost) - } - return hostname -} - func HostnameValidator(hostname string) error { if len(strings.TrimSpace(hostname)) < 1 { return errors.New("a value is required") @@ -77,10 +51,10 @@ func GraphQLEndpoint(hostname string) string { // conditional can be removed as the flow will fall through to the bottom. // However, we can't do that until we've investigated all places in which // Tenancy is currently treated as Enterprise. - if IsTenancy(hostname) { + if ghauth.IsTenancy(hostname) { return fmt.Sprintf("https://api.%s/graphql", hostname) } - if IsEnterprise(hostname) { + if ghauth.IsEnterprise(hostname) { return fmt.Sprintf("https://%s/api/graphql", hostname) } if strings.EqualFold(hostname, localhost) { @@ -97,10 +71,10 @@ func RESTPrefix(hostname string) string { // conditional can be removed as the flow will fall through to the bottom. // However, we can't do that until we've investigated all places in which // Tenancy is currently treated as Enterprise. - if IsTenancy(hostname) { + if ghauth.IsTenancy(hostname) { return fmt.Sprintf("https://api.%s/", hostname) } - if IsEnterprise(hostname) { + if ghauth.IsEnterprise(hostname) { return fmt.Sprintf("https://%s/api/v3/", hostname) } if strings.EqualFold(hostname, localhost) { @@ -121,7 +95,7 @@ func GistHost(hostname string) string { if isGarage(hostname) { return fmt.Sprintf("%s/gist/", hostname) } - if IsEnterprise(hostname) { + if ghauth.IsEnterprise(hostname) { return fmt.Sprintf("%s/gist/", hostname) } if strings.EqualFold(hostname, localhost) { diff --git a/internal/ghinstance/host_test.go b/internal/ghinstance/host_test.go index e9fb97008..1b7e0146d 100644 --- a/internal/ghinstance/host_test.go +++ b/internal/ghinstance/host_test.go @@ -6,88 +6,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestIsEnterprise(t *testing.T) { - tests := []struct { - host string - want bool - }{ - { - host: "github.com", - want: false, - }, - { - host: "api.github.com", - want: false, - }, - { - host: "github.localhost", - want: false, - }, - { - host: "api.github.localhost", - want: false, - }, - { - host: "garage.github.com", - want: false, - }, - { - host: "ghe.io", - want: true, - }, - { - host: "example.com", - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.host, func(t *testing.T) { - if got := IsEnterprise(tt.host); got != tt.want { - t.Errorf("IsEnterprise() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestIsTenancy(t *testing.T) { - tests := []struct { - host string - want bool - }{ - { - host: "github.com", - want: false, - }, - { - host: "github.localhost", - want: false, - }, - { - host: "garage.github.com", - want: false, - }, - { - host: "ghe.com", - want: false, - }, - { - host: "tenant.ghe.com", - want: true, - }, - { - host: "api.tenant.ghe.com", - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.host, func(t *testing.T) { - if got := IsTenancy(tt.host); got != tt.want { - t.Errorf("IsTenancy() = %v, want %v", got, tt.want) - } - }) - } -} - func TestTenantName(t *testing.T) { tests := []struct { host string @@ -130,69 +48,6 @@ func TestTenantName(t *testing.T) { } } -func TestNormalizeHostname(t *testing.T) { - tests := []struct { - host string - want string - }{ - { - host: "GitHub.com", - want: "github.com", - }, - { - host: "api.github.com", - want: "github.com", - }, - { - host: "ssh.github.com", - want: "github.com", - }, - { - host: "upload.github.com", - want: "github.com", - }, - { - host: "GitHub.localhost", - want: "github.localhost", - }, - { - host: "api.github.localhost", - want: "github.localhost", - }, - { - host: "garage.github.com", - want: "github.com", - }, - { - host: "GHE.IO", - want: "ghe.io", - }, - { - host: "git.my.org", - want: "git.my.org", - }, - { - host: "ghe.com", - want: "ghe.com", - }, - { - host: "tenant.ghe.com", - want: "tenant.ghe.com", - }, - { - host: "api.tenant.ghe.com", - want: "tenant.ghe.com", - }, - } - for _, tt := range tests { - t.Run(tt.host, func(t *testing.T) { - if got := NormalizeHostname(tt.host); got != tt.want { - t.Errorf("NormalizeHostname() = %v, want %v", got, tt.want) - } - }) - } -} - func TestHostnameValidator(t *testing.T) { tests := []struct { name string diff --git a/internal/ghrepo/repo.go b/internal/ghrepo/repo.go index 4f0328e99..a31d354b5 100644 --- a/internal/ghrepo/repo.go +++ b/internal/ghrepo/repo.go @@ -6,7 +6,7 @@ import ( "strings" "github.com/cli/cli/v2/internal/ghinstance" - ghAuth "github.com/cli/go-gh/v2/pkg/auth" + ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/cli/go-gh/v2/pkg/repository" ) @@ -37,7 +37,7 @@ func FullName(r Interface) string { } func defaultHost() string { - host, _ := ghAuth.DefaultHost() + host, _ := ghauth.DefaultHost() return host } diff --git a/pkg/cmd/attestation/auth/host.go b/pkg/cmd/attestation/auth/host.go index a40df8335..a8997c403 100644 --- a/pkg/cmd/attestation/auth/host.go +++ b/pkg/cmd/attestation/auth/host.go @@ -3,7 +3,7 @@ package auth import ( "errors" - "github.com/cli/cli/v2/internal/ghinstance" + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) var ErrUnsupportedHost = errors.New("An unsupported host was detected. Note that gh attestation does not currently support GHES") @@ -11,7 +11,7 @@ var ErrUnsupportedHost = errors.New("An unsupported host was detected. Note that func IsHostSupported(host string) error { // Note that this check is slightly redundant as Tenancy should not be considered Enterprise // but the ghinstance package has not been updated to reflect this yet. - if ghinstance.IsEnterprise(host) && !ghinstance.IsTenancy(host) { + if ghauth.IsEnterprise(host) && !ghauth.IsTenancy(host) { return ErrUnsupportedHost } return nil diff --git a/pkg/cmd/attestation/inspect/inspect.go b/pkg/cmd/attestation/inspect/inspect.go index 867ff8c1b..c548b2ec7 100644 --- a/pkg/cmd/attestation/inspect/inspect.go +++ b/pkg/cmd/attestation/inspect/inspect.go @@ -85,7 +85,7 @@ func NewInspectCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command Logger: opts.Logger, } // Prepare for tenancy if detected - if ghinstance.IsTenancy(opts.Hostname) { + if ghauth.IsTenancy(opts.Hostname) { hc, err := f.HttpClient() if err != nil { return err diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index 5473adc0c..c092b81ec 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -6,7 +6,6 @@ import ( "fmt" "os" - "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/auth" "github.com/cli/cli/v2/pkg/cmd/attestation/io" @@ -68,7 +67,7 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com return err } - if ghinstance.IsTenancy(opts.Hostname) { + if ghauth.IsTenancy(opts.Hostname) { c, err := f.Config() if err != nil { return err diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 17b8a84d1..d14081dd8 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -138,7 +138,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } // Prepare for tenancy if detected - if ghinstance.IsTenancy(opts.Hostname) { + if ghauth.IsTenancy(opts.Hostname) { td, err := opts.APIClient.GetTrustDomain() if err != nil { return fmt.Errorf("error getting trust domain, make sure you are authenticated against the host: %w", err) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 1480a29a0..653c9e6c0 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -15,7 +15,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - ghAuth "github.com/cli/go-gh/v2/pkg/auth" + ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" ) @@ -123,7 +123,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm } if opts.Hostname == "" && (!opts.Interactive || opts.Web) { - opts.Hostname, _ = ghAuth.DefaultHost() + opts.Hostname, _ = ghauth.DefaultHost() } opts.MainExecutable = f.Executable() diff --git a/pkg/cmd/gist/clone/clone.go b/pkg/cmd/gist/clone/clone.go index 6fa6f226a..9c9ec57e8 100644 --- a/pkg/cmd/gist/clone/clone.go +++ b/pkg/cmd/gist/clone/clone.go @@ -8,11 +8,12 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/gh" - "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" "github.com/spf13/pflag" + + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) type CloneOptions struct { @@ -93,7 +94,7 @@ func cloneRun(opts *CloneOptions) error { } func formatRemoteURL(hostname string, gistID string, protocol string) string { - if ghinstance.IsEnterprise(hostname) { + if ghauth.IsEnterprise(hostname) || ghauth.IsTenancy(hostname) { if protocol == "ssh" { return fmt.Sprintf("git@%s:gist/%s.git", hostname, gistID) } diff --git a/pkg/cmd/pr/status/http.go b/pkg/cmd/pr/status/http.go index a800a24ac..b3fbbac26 100644 --- a/pkg/cmd/pr/status/http.go +++ b/pkg/cmd/pr/status/http.go @@ -6,9 +6,10 @@ import ( "strings" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/set" + + ghauth "github.com/cli/go-gh/v2/pkg/auth" ) type requestOptions struct { @@ -186,7 +187,7 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r } func getCurrentUsername(username string, hostname string, apiClient *api.Client) (string, error) { - if username == "@me" && ghinstance.IsEnterprise(hostname) { + if username == "@me" && ghauth.IsEnterprise(hostname) { var err error username, err = api.CurrentLoginName(apiClient, hostname) if err != nil { diff --git a/pkg/cmd/repo/delete/delete.go b/pkg/cmd/repo/delete/delete.go index 1e96e404d..722d748b9 100644 --- a/pkg/cmd/repo/delete/delete.go +++ b/pkg/cmd/repo/delete/delete.go @@ -10,7 +10,7 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - ghAuth "github.com/cli/go-gh/v2/pkg/auth" + ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" ) @@ -85,7 +85,7 @@ func deleteRun(opts *DeleteOptions) error { } else { repoSelector := opts.RepoArg if !strings.Contains(repoSelector, "/") { - defaultHost, _ := ghAuth.DefaultHost() + defaultHost, _ := ghauth.DefaultHost() currentUser, err := api.CurrentLoginName(apiClient, defaultHost) if err != nil { return err diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 7cc533e13..2b945c906 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -15,7 +15,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - ghAuth "github.com/cli/go-gh/v2/pkg/auth" + ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" ) @@ -109,7 +109,7 @@ func listRun(opts *ListOptions) error { } } - hostname, _ := ghAuth.DefaultHost() + hostname, _ := ghauth.DefaultHost() if opts.WebMode { var rulesetURL string diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index d272d273e..cefd926ab 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -15,7 +15,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - ghAuth "github.com/cli/go-gh/v2/pkg/auth" + ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" ) @@ -124,7 +124,7 @@ func viewRun(opts *ViewOptions) error { } } - hostname, _ := ghAuth.DefaultHost() + hostname, _ := ghauth.DefaultHost() cs := opts.IO.ColorScheme() if opts.InteractiveMode {