diff --git a/api/client.go b/api/client.go index 2fc72aaa6..d7b981bee 100644 --- a/api/client.go +++ b/api/client.go @@ -37,6 +37,16 @@ func AddHeader(name, value string) ClientOption { } } +// AddHeaderFunc is an AddHeader that gets the string value from a function +func AddHeaderFunc(name string, value func() string) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + req.Header.Add(name, value()) + return tr.RoundTrip(req) + }} + } +} + // VerboseLog enables request/response logging within a RoundTripper func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption { logger := &httpretty.Logger{ @@ -63,6 +73,40 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption { } } +var issuedScopesWarning bool + +// CheckScopes checks whether an OAuth scope is present in a response +func CheckScopes(wantedScope string, cb func(string) error) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + res, err := tr.RoundTrip(req) + if err != nil || res.StatusCode > 299 || issuedScopesWarning { + return res, err + } + + appID := res.Header.Get("X-Oauth-Client-Id") + hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",") + + hasWanted := false + for _, s := range hasScopes { + if wantedScope == strings.TrimSpace(s) { + hasWanted = true + break + } + } + + if !hasWanted { + if err := cb(appID); err != nil { + return res, err + } + issuedScopesWarning = true + } + + return res, nil + }} + } +} + type funcTripper struct { roundTrip func(*http.Request) (*http.Response, error) } diff --git a/auth/oauth.go b/auth/oauth.go index dc7a9d085..3f9d9ddc4 100644 --- a/auth/oauth.go +++ b/auth/oauth.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "os" + "strings" "github.com/cli/cli/pkg/browser" ) @@ -29,6 +30,7 @@ type OAuthFlow struct { Hostname string ClientID string ClientSecret string + Scopes []string WriteSuccessHTML func(io.Writer) VerboseStream io.Writer } @@ -45,11 +47,15 @@ func (oa *OAuthFlow) ObtainAccessToken() (accessToken string, err error) { } port := listener.Addr().(*net.TCPAddr).Port + scopes := "repo" + if oa.Scopes != nil { + scopes = strings.Join(oa.Scopes, " ") + } + q := url.Values{} q.Set("client_id", oa.ClientID) q.Set("redirect_uri", fmt.Sprintf("http://127.0.0.1:%d/callback", port)) - // TODO: make scopes configurable - q.Set("scope", "repo") + q.Set("scope", scopes) q.Set("state", state) startURL := fmt.Sprintf("https://%s/login/oauth/authorize?%s", oa.Hostname, q.Encode()) diff --git a/command/root.go b/command/root.go index 605b82cad..a2d8fed95 100644 --- a/command/root.go +++ b/command/root.go @@ -132,12 +132,47 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { if err != nil { return nil, err } + var opts []api.ClientOption if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, apiVerboseLog()) } + + getAuthValue := func() string { + return fmt.Sprintf("token %s", token) + } + + checkScopesFunc := func(appID string) error { + if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { + newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") + if err != nil { + return err + } + cfg, err := ctx.Config() + if err != nil { + return err + } + _ = cfg.Set(defaultHostname, "oauth_token", newToken) + _ = cfg.Set(defaultHostname, "user", loginHandle) + // update config file on disk + err = cfg.Write() + if err != nil { + return err + } + // update configuration in memory + token = newToken + config.AuthFlowComplete() + } else { + fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") + fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`") + fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + } + return nil + } + opts = append(opts, - api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), + api.CheckScopes("read:org", checkScopesFunc), + api.AddHeaderFunc("Authorization", getAuthValue), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), diff --git a/internal/config/config_setup.go b/internal/config/config_setup.go index 9bc0fb775..4d2a929cb 100644 --- a/internal/config/config_setup.go +++ b/internal/config/config_setup.go @@ -24,9 +24,14 @@ var ( oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) -// TODO: have a conversation about whether this belongs in the "context" package -// FIXME: make testable -func setupConfigFile(filename string) (Config, error) { +// IsGitHubApp reports whether an OAuth app is "GitHub CLI" or "GitHub CLI (dev)" +func IsGitHubApp(id string) bool { + // this intentionally doesn't use `oauthClientID` because that is a variable + // that can potentially be changed at build time via GH_OAUTH_CLIENT_ID + return id == "178c6fc778ccc68e1d6a" || id == "4d747ba5675d5d66553f" +} + +func AuthFlow(notice string) (string, string, error) { var verboseStream io.Writer if strings.Contains(os.Getenv("DEBUG"), "oauth") { verboseStream = os.Stderr @@ -36,21 +41,37 @@ func setupConfigFile(filename string) (Config, error) { Hostname: oauthHost, ClientID: oauthClientID, ClientSecret: oauthClientSecret, + Scopes: []string{"repo", "read:org"}, WriteSuccessHTML: func(w io.Writer) { fmt.Fprintln(w, oauthSuccessPage) }, VerboseStream: verboseStream, } - fmt.Fprintln(os.Stderr, "Notice: authentication required") + fmt.Fprintln(os.Stderr, notice) fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname) _ = waitForEnter(os.Stdin) token, err := flow.ObtainAccessToken() if err != nil { - return nil, err + return "", "", err } userLogin, err := getViewer(token) + if err != nil { + return "", "", err + } + + return token, userLogin, nil +} + +func AuthFlowComplete() { + fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") + _ = waitForEnter(os.Stdin) +} + +// FIXME: make testable +func setupConfigFile(filename string) (Config, error) { + token, userLogin, err := AuthFlow("Notice: authentication required") if err != nil { return nil, err } @@ -61,9 +82,9 @@ func setupConfigFile(filename string) (Config, error) { } yamlHosts := map[string]map[string]string{} - yamlHosts[flow.Hostname] = map[string]string{} - yamlHosts[flow.Hostname]["user"] = userLogin - yamlHosts[flow.Hostname]["oauth_token"] = token + yamlHosts[oauthHost] = map[string]string{} + yamlHosts[oauthHost]["user"] = userLogin + yamlHosts[oauthHost]["oauth_token"] = token defaultConfig := yamlConfig{ Hosts: yamlHosts, @@ -84,14 +105,9 @@ func setupConfigFile(filename string) (Config, error) { if err != nil { return nil, err } - n, err := cfgFile.Write(yamlData) - if err == nil && n < len(yamlData) { - err = io.ErrShortWrite - } - - if err == nil { - fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") - _ = waitForEnter(os.Stdin) + _, err = cfgFile.Write(yamlData) + if err != nil { + return nil, err } // TODO cleaner error handling? this "should" always work given that we /just/ wrote the file...