Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip keyring for auth check #7169

Merged
merged 11 commits into from Mar 17, 2023
17 changes: 9 additions & 8 deletions internal/config/config.go
Expand Up @@ -126,7 +126,10 @@ type AuthConfig struct {
// searching environment variables, plain text config, and
// lastly encypted storage.
func (c *AuthConfig) Token(hostname string) (string, string) {
token, source := c.TokenFromEnvOrConfig(hostname)
if c.tokenOverride != nil {
return c.tokenOverride(hostname)
}
token, source := ghAuth.TokenFromEnvOrConfig(hostname)
if token == "" {
var err error
token, err = c.TokenFromKeyring(hostname)
Expand All @@ -137,13 +140,11 @@ func (c *AuthConfig) Token(hostname string) (string, string) {
return token, source
}

// TokenFromEnvOrConfig will retrieve the auth token for the given hostname,
// searching environment variables, and plain text config.
func (c *AuthConfig) TokenFromEnvOrConfig(hostname string) (string, string) {
if c.tokenOverride != nil {
return c.tokenOverride(hostname)
}
return ghAuth.TokenFromEnvOrConfig(hostname)
// HasEnterpriseEnvToken checks whether the current env or config contains an enterprise token
func (c *AuthConfig) HasEnterpriseEnvToken() bool {
token, source := ghAuth.TokenFromEnvOrConfig("")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works right now, but only because of an accident. We just so happen to interpret an empty string passed to TokenFromEnvOrConfig as an "enterprise" hostname because it technically doesn't match github.com. We should add a test to go-gh to make note that we shouldn't accidentally break this behavior. But this shouldn't prevent this PR from shipping 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you prefer I use example.com as before?

isEnterpriseSource := source == "GH_ENTERPRISE_TOKEN" || source == "GITHUB_ENTERPRISE_TOKEN"
return token != "" && isEnterpriseSource
}

// SetToken will override any token resolution and return the given
Expand Down
4 changes: 1 addition & 3 deletions pkg/cmd/factory/remote_resolver.go
Expand Up @@ -82,11 +82,9 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) {
}

if len(cachedRemotes) == 0 {
// Any non-github.com hostname is fine here
dummyHostname := "example.com"
if isHostEnv(src) {
return nil, fmt.Errorf("none of the git remotes configured for this repository correspond to the %s environment variable. Try adding a matching remote or unsetting the variable.", src)
} else if v, _ := cfg.Authentication().TokenFromEnvOrConfig(dummyHostname); v != "" {
} else if cfg.Authentication().HasEnterpriseEnvToken() {
return nil, errors.New("set the GH_HOST environment variable to specify which GitHub host to use")
}
return nil, errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`")
Expand Down
7 changes: 1 addition & 6 deletions pkg/cmdutil/auth_check.go
Expand Up @@ -14,12 +14,7 @@ func DisableAuthCheck(cmd *cobra.Command) {
}

func CheckAuth(cfg config.Config) bool {
// This will check if there are any environment variable
// authentication tokens set for enterprise hosts.
// Any non-github.com hostname is fine here
dummyHostname := "example.com"
token, _ := cfg.Authentication().TokenFromEnvOrConfig(dummyHostname)
if token != "" {
if cfg.Authentication().HasEnterpriseEnvToken() {
return true
}

Expand Down