Guide user through re-authorization flow if read:org scope is missing

How this works for people with existing OAuth tokens:

    $ gh issue list -L1
    Notice: additional authorization required
    Press Enter to open github.com in your browser...
    [auth flow in the browser...]
    Authentication complete. Press Enter to continue...

    Showing 1 of 132 issues in cli/cli
    ...

Users of Personal Access Tokens get a different notice:

    Warning: gh now requires the `read:org` OAuth scope.
    Visit https://github.com/settings/tokens and edit your token to enable `read:org`
    or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`
This commit is contained in:
Mislav Marohnić 2020-04-23 18:15:20 +02:00
parent 8ed9a0324e
commit 3aaa231cc5
3 changed files with 81 additions and 30 deletions

View file

@ -7,7 +7,6 @@ import (
"io"
"io/ioutil"
"net/http"
"os"
"regexp"
"strings"
@ -38,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{
@ -67,15 +76,15 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption {
var issuedScopesWarning bool
// CheckScopes checks whether an OAuth scope is present in a response
func CheckScopes(wantedScope string) ClientOption {
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 || issuedScopesWarning {
if err != nil || res.StatusCode > 299 || issuedScopesWarning {
return res, err
}
isApp := res.Header.Get("X-Oauth-Client-Id") != ""
appID := res.Header.Get("X-Oauth-Client-Id")
hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",")
hasWanted := false
@ -87,14 +96,8 @@ func CheckScopes(wantedScope string) ClientOption {
}
if !hasWanted {
fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.")
// TODO: offer to take the person through the authentication flow again?
// TODO: retry the original request if it was a read?
if isApp {
fmt.Fprintln(os.Stderr, "To re-authenticate, please `rm ~/.config/gh/config.yml` and try again.")
} else {
// the person has pasted a Personal Access Token
fmt.Fprintln(os.Stderr, "Re-generate your token in `rm ~/.config/gh/config.yml` and try again.")
if err := cb(appID); err != nil {
return res, err
}
issuedScopesWarning = true
}

View file

@ -142,13 +142,46 @@ 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.CheckScopes("read:org"),
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"),

View file

@ -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
@ -43,15 +48,30 @@ func setupConfigFile(filename string) (Config, error) {
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
}
@ -62,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,
@ -85,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...