From 75ababac236774a186de818d30a2c2ddca5639e9 Mon Sep 17 00:00:00 2001 From: "Wesley O. Nichols" Date: Tue, 19 May 2026 16:27:17 +0200 Subject: [PATCH] fix: request Drive scope in `gwcli configure` The live consent path (`gwcli configure` -> ConfigureAuth -> NewAuthenticator) built the OAuth consent URL from `authScopes` (oauth.go), which omitted `https://www.googleapis.com/auth/drive`. A freshly-configured OAuth token therefore never carried the Drive scope, so every `drive *` / `artifacts download` command failed with insufficient scope, and there was no consent path that requested Drive at all. The codebase had three divergent scope definitions, which masked the bug (two of them *did* list Drive but were unreachable dead code): - `authScopes` (oauth.go) - the only one that runs; no Drive - `scope` const (connection.go) - only used by dead `auth()` - inline list in `oauth2Config` - cosmetic at runtime; divergent Fix and consolidate to a single source of truth: - Add `drive.DriveScope` to `authScopes` (the actual fix) and rewrite the comment to explain installed-app OAuth is not all-or-nothing but Google won't add scopes to an already-issued token, so everything must be bundled on the consent screen. The service-account DWD path still requests Drive separately and is unaffected. - `oauth2Config` now reuses `authScopes` instead of its own hardcoded list, so the lists can't silently diverge again. - Delete the dead OAuth-configure cluster (configure.go: Configure / auth / makeConfig / readConf / readLine, the `scope` and `accessType` consts) that the real `ConfigureAuth` flow never invokes. - Update TestNewAuthenticatorScopes to assert Drive is requested. Existing OAuth users must re-run `gwcli configure` to re-consent (Google does not grant new scopes to an already-issued token.json). Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gwcli/configure.go | 183 ---------------------------------------- pkg/gwcli/connection.go | 37 ++------ pkg/gwcli/oauth.go | 16 +++- pkg/gwcli/oauth_test.go | 1 + 4 files changed, 20 insertions(+), 217 deletions(-) delete mode 100644 pkg/gwcli/configure.go diff --git a/pkg/gwcli/configure.go b/pkg/gwcli/configure.go deleted file mode 100644 index dbc423f..0000000 --- a/pkg/gwcli/configure.go +++ /dev/null @@ -1,183 +0,0 @@ -package gwcli - -import ( - "bufio" - "encoding/json" - "flag" - "fmt" - "html" - "io/ioutil" - "net" - "net/http" - "os" - "path" - "strings" - - "github.com/pkg/errors" - log "github.com/sirupsen/logrus" - "golang.org/x/oauth2" -) - -const ( - spaces = "\n\t\r " -) - -var ( - // Populate these for a binary-only release. - - // DefaultClientID is the Oauth client ID. - DefaultClientID = "" - - // DefaultClientSecret is the Oauth client secret. - DefaultClientSecret = "" -) - -var ( - // TODO: Listen to a dynamic port. - oauthListenPort = flag.Int("oauth_listen_port", 0, "Oauth port to listen to. 0 means pick dynamically.") -) - -// ConfigOAuth contains the config for the oauth. -type ConfigOAuth struct { - ClientID, ClientSecret, RefreshToken, AccessToken, APIKey string -} - -// Config is… hmm… this should probably be cleand up. -type Config struct { - OAuth ConfigOAuth -} - -func readLine(s string) (string, error) { - reader := bufio.NewReader(os.Stdin) - fmt.Print(s) - id, err := reader.ReadString('\n') - if err != nil { - return "", err - } - id = strings.Trim(id, spaces) - return id, nil -} - -func auth(cfg ConfigOAuth) (string, error) { - at := oauth2.AccessTypeOffline - if accessType == "online" { - at = oauth2.AccessTypeOnline - } - - // - // Start a webserver. - // - ln, err := net.Listen("tcp", ":0") - if err != nil { - log.Fatalf("Failed to listen to TCP port 0 (any): %v", err) - } - port := ln.Addr().(*net.TCPAddr).Port - fmt.Printf("Listening to port %d\n", port) - - codeCh := make(chan string) - go http.Serve(ln, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - r.ParseForm() - codes := r.URL.Query()["code"] - if len(codes) == 0 { - fmt.Fprintf(w, "Did not get a code. Something's wrong.") - return - } - defer close(codeCh) - fmt.Fprintf(w, "Got code %q. You can close this tab now.", html.EscapeString(codes[0])) - codeCh <- codes[0] - })) - // No need to clean up. This is run in -configure and will soon exit. - - ocfg := oauth2.Config{ - ClientID: cfg.ClientID, - ClientSecret: cfg.ClientSecret, - Endpoint: oauth2.Endpoint{ - AuthURL: "https://accounts.google.com/o/oauth2/auth", - TokenURL: "https://accounts.google.com/o/oauth2/token", - }, - Scopes: []string{scope}, - RedirectURL: fmt.Sprintf("http://localhost:%d/", port), - } - fmt.Printf("Cut and paste this URL into your browser:\n %s\n", ocfg.AuthCodeURL("", at)) - line := <-codeCh - fmt.Printf("Returned code: %s\n", line) - token, err := ocfg.Exchange(oauth2.NoContext, line) - if err != nil { - return "", err - } - return token.RefreshToken, nil -} - -// Make a config, possibly by asking the user. -// -// If there's a default client ID/secret, then scrub that before -// returning the blob. -func makeConfig(id, secret string) ([]byte, error) { - var err error - - // Use default, if available. - if id == "" { - id = DefaultClientID - } - if secret == "" { - secret = DefaultClientSecret - } - - // Else ask. - if id == "" { - id, err = readLine("ClientID: ") - if err != nil { - return nil, err - } - } - if secret == "" { - secret, err = readLine("ClientSecret: ") - if err != nil { - return nil, err - } - } - - token, err := auth(ConfigOAuth{ - ClientID: id, - ClientSecret: secret, - }) - if err != nil { - return nil, err - } - conf := &Config{ - OAuth: ConfigOAuth{ - ClientID: id, - ClientSecret: secret, - RefreshToken: token, - }, - } - // Don't store default ID/secret. - if id == DefaultClientID { - conf.OAuth.ClientID = "" - conf.OAuth.ClientSecret = "" - } - b, err := json.Marshal(conf) - if err != nil { - return nil, err - } - return b, nil -} - -// Configure sets up configuration with oauth and stuff. -func Configure(fn string) error { - conf, err := readConf(fn) - if err != nil { - log.Infof("Failed to read config %q: %v", fn, err) - } else { - log.Infof("Reusing ClientID/ClientSecret from %q", fn) - log.Infof("If you want to change ClientID/Secret then delete %q", fn) - } - b, err := makeConfig(conf.OAuth.ClientID, conf.OAuth.ClientSecret) - if err != nil { - return err - } - if err := os.MkdirAll(path.Dir(fn), 0700); err != nil { - return errors.Wrapf(err, "creating config directory %q", path.Dir(fn)) - } - return ioutil.WriteFile(fn, b, 0600) -} diff --git a/pkg/gwcli/connection.go b/pkg/gwcli/connection.go index 8d7d927..89f5532 100644 --- a/pkg/gwcli/connection.go +++ b/pkg/gwcli/connection.go @@ -30,16 +30,9 @@ import ( ) const ( - // Scope for Gmail API. The full drive scope is required to - // export/download Drive artifacts linked from email bodies (see - // drive_artifacts.go), and leaves room for future write use (e.g. - // sending Drive files as attachments). - scope = "https://www.googleapis.com/auth/gmail.modify https://www.googleapis.com/auth/gmail.settings.basic https://www.googleapis.com/auth/gmail.labels https://www.googleapis.com/auth/drive" - pageSize = 100 - accessType = "offline" - email = "me" + email = "me" ) // Different levels of detail to download. @@ -132,22 +125,6 @@ func NewFake(client *http.Client) (*CmdG, error) { return conn, conn.setupClients() } -func readConf(fn string) (Config, error) { - f, err := ioutil.ReadFile(fn) - if err != nil { - return Config{}, err - } - var conf Config - if err := json.Unmarshal(f, &conf); err != nil { - return Config{}, errors.Wrapf(err, "unmarshalling config") - } - if conf.OAuth.ClientID == "" { - conf.OAuth.ClientID = DefaultClientID - conf.OAuth.ClientSecret = DefaultClientSecret - } - return conf, nil -} - // New creates a new CmdG with OAuth/service-account authentication. // configDir should point to the directory containing credentials.json and token.json // userEmail is only required when using service account authentication (for user impersonation) @@ -345,12 +322,12 @@ func oauth2Config(credBytes []byte) (*oauth2.Config, error) { AuthURL: c.Installed.AuthURI, TokenURL: c.Installed.TokenURI, }, - Scopes: []string{ - "https://www.googleapis.com/auth/gmail.modify", - "https://www.googleapis.com/auth/gmail.settings.basic", - "https://www.googleapis.com/auth/gmail.labels", - "https://www.googleapis.com/auth/drive", - }, + // Single source of truth: the same scope set the consent screen + // requested (see authScopes in oauth.go). For an already-issued + // token this is effectively cosmetic — the refresh endpoint does + // not re-request scopes — but keeping one list prevents the lists + // from silently diverging again. + Scopes: append([]string(nil), authScopes...), }, nil } diff --git a/pkg/gwcli/oauth.go b/pkg/gwcli/oauth.go index 9bc26de..38b958f 100644 --- a/pkg/gwcli/oauth.go +++ b/pkg/gwcli/oauth.go @@ -17,16 +17,24 @@ import ( "google.golang.org/api/tasks/v1" ) -// authScopes is the OAuth/JWT scope set shared by the Gmail, Tasks, and -// Calendar clients. The Drive scope is requested separately (see -// ServiceAccountAuthenticator.DriveService) because domain-wide-delegation -// token exchange is all-or-nothing across the requested scope set. +// authScopes is the OAuth scope set requested on the installed-app consent +// screen (NewAuthenticator) and granted to the Gmail, Tasks, Calendar, and +// Drive clients. Every scope the tool will ever need must be bundled here: +// user-consent OAuth is not all-or-nothing, but Google will not add scopes +// to an already-issued token, so anything omitted here can never be +// consented to without re-running `gwcli configure`. +// +// Service accounts do NOT use this list: ServiceAccountAuthenticator +// requests the Drive scope separately (see DriveService) because +// domain-wide-delegation token exchange *is* all-or-nothing across the +// requested scope set. var authScopes = []string{ gmail.GmailModifyScope, gmail.GmailSettingsBasicScope, gmail.GmailLabelsScope, tasks.TasksScope, calendar.CalendarScope, + drive.DriveScope, } // IsServiceAccount reports whether the credentials JSON is a service-account diff --git a/pkg/gwcli/oauth_test.go b/pkg/gwcli/oauth_test.go index 7fe4217..2013da9 100644 --- a/pkg/gwcli/oauth_test.go +++ b/pkg/gwcli/oauth_test.go @@ -27,6 +27,7 @@ func TestNewAuthenticatorScopes(t *testing.T) { "https://www.googleapis.com/auth/gmail.labels": false, "https://www.googleapis.com/auth/tasks": false, "https://www.googleapis.com/auth/calendar": false, + "https://www.googleapis.com/auth/drive": false, } for _, s := range a.cfg.Scopes { if _, ok := want[s]; ok {