lib/oauthutil: add PKCE support

lib/oauthutil/oauthutil.go

@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -34,10 +35,12 @@ var (
 
 	// oauthCancelFn stores the cancel function for the currently active OAuth flow
 	oauthCancelFn context.CancelFunc
-	// oauthCancelMu protects oauthCancelFn and oauthURL
+	// oauthCancelMu protects oauthCancelFn, oauthURL and oauthVerifier
 	oauthCancelMu sync.Mutex
 	// oauthURL stores the URL for the currently active OAuth flow
 	oauthURL string
+	// oauthVerifier stores the PKCE verifier for the remote setup path
+	oauthVerifier string
 )
 
 const (
@@ -109,6 +112,7 @@ type Config struct {
 	RedirectURL          string
 	ClientCredentialFlow bool
 	AuthStyle            oauth2.AuthStyle
+	UsePKCE              bool // If set, use PKCE (RFC 7636) for the authorization code flow
 }
 
 // MakeOauth2Config makes an oauth2.Config from our config
@@ -167,6 +171,11 @@ var SharedOptions = []fs.Option{{
 	Advanced: true,
 }}
 
+// SharedOptionsPKCE are shared between backends that use PKCE OAuth2 flow, without Client Secret.
+var SharedOptionsPKCE = slices.DeleteFunc(slices.Clone(SharedOptions), func(o fs.Option) bool {
+	return o.Name == config.ConfigClientSecret || o.Name == config.ConfigClientCredentials
+})
+
 // oldToken contains an end-user's tokens.
 // This is the data you must store to persist authentication.
 //
@@ -696,10 +705,14 @@ func ConfigOAuth(ctx context.Context, name string, m configmap.Mapper, ri *fs.Re
 			return nil, err
 		}
 		if noWebserverNeeded(opt.OAuth2Config) {
-			authURL, _, err := getAuthURL(name, m, opt.OAuth2Config, opt)
+			authURL, _, pkceVerifier, err := getAuthURL(name, m, opt.OAuth2Config, opt)
 			if err != nil {
 				return nil, err
 			}
+			// Remember the verifier for the exchange in *oauth-do
+			oauthCancelMu.Lock()
+			oauthVerifier = pkceVerifier
+			oauthCancelMu.Unlock()
 			return fs.ConfigInput(newState("*oauth-do"), "config_verification_code", fmt.Sprintf("Verification code\n\nGo to this URL, authenticate then paste the code here.\n\n%s\n", authURL))
 		}
 		var out strings.Builder
@@ -784,6 +797,7 @@ version recommended):
 		if changed {
 			fs.Logf(nil, "Make sure your Redirect URL is set to %q in your custom config.\n", oauthConfig.RedirectURL)
 		}
+		var pkceVerifier string
 		if oauthConfig.ClientCredentialFlow {
 			err = clientCredentialsFlowGetToken(ctx, name, m, oauthConfig, opt)
 			if err != nil {
@@ -792,12 +806,18 @@ version recommended):
 		} else {
 			if code == "" {
 				oauthConfig = fixRedirect(oauthConfig)
-				code, err = configSetup(ctx, ri.Name, name, m, oauthConfig, opt)
+				code, pkceVerifier, err = configSetup(ctx, ri.Name, name, m, oauthConfig, opt)
 				if err != nil {
 					return nil, fmt.Errorf("config failed to refresh token: %w", err)
 				}
+			} else {
+				// The code was pasted in: pick up the verifier from *oauth-remote
+				oauthCancelMu.Lock()
+				pkceVerifier = oauthVerifier
+				oauthVerifier = ""
+				oauthCancelMu.Unlock()
 			}
-			err = configExchange(ctx, name, m, oauthConfig, code)
+			err = configExchange(ctx, name, m, oauthConfig, code, pkceVerifier)
 			if err != nil {
 				return nil, err
 			}
@@ -882,13 +902,13 @@ func noWebserverNeeded(oauthConfig *Config) bool {
 }
 
 // get the URL we need to send the user to
-func getAuthURL(name string, m configmap.Mapper, oauthConfig *Config, opt *Options) (authURL string, state string, err error) {
+func getAuthURL(name string, m configmap.Mapper, oauthConfig *Config, opt *Options) (authURL string, state string, pkceVerifier string, err error) {
 	oauthConfig, _ = OverrideCredentials(name, m, oauthConfig)
 
 	// Make random state
 	state, err = random.Password(128)
 	if err != nil {
-		return "", "", err
+		return "", "", "", err
 	}
 
 	// Create the configuration required for the OAuth flow
@@ -899,8 +919,15 @@ func getAuthURL(name string, m configmap.Mapper, oauthConfig *Config, opt *Optio
 	if !opt.NoOffline {
 		opts = append(opts, oauth2.AccessTypeOffline)
 	}
+
+	// Generate PKCE challenge if needed
+	if oauthConfig.UsePKCE {
+		pkceVerifier = oauth2.GenerateVerifier()
+		opts = append(opts, oauth2.S256ChallengeOption(pkceVerifier))
+	}
+
 	authURL = oauth2Conf.AuthCodeURL(state, opts...)
-	return authURL, state, nil
+	return authURL, state, pkceVerifier, nil
 }
 
 // If TitleBarRedirect is set but we are doing a real oauth, then
@@ -942,23 +969,23 @@ func clientCredentialsFlowGetToken(ctx context.Context, name string, m configmap
 // If opt is nil it will use the default Options.
 //
 // It will run an internal webserver to receive the results
-func configSetup(ctx context.Context, id, name string, m configmap.Mapper, oauthConfig *Config, opt *Options) (string, error) {
+func configSetup(ctx context.Context, id, name string, m configmap.Mapper, oauthConfig *Config, opt *Options) (code string, pkceVerifier string, err error) {
 	if opt == nil {
 		opt = &Options{}
 	}
 	authorizeNoAutoBrowserValue, ok := m.Get(config.ConfigAuthNoBrowser)
 	authorizeNoAutoBrowser := ok && authorizeNoAutoBrowserValue != ""
 
-	authURL, state, err := getAuthURL(name, m, oauthConfig, opt)
+	authURL, state, pkceVerifier, err := getAuthURL(name, m, oauthConfig, opt)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 
 	// Prepare webserver
 	server := newAuthServer(opt, bindAddress, state, authURL)
 	err = server.Init()
 	if err != nil {
-		return "", fmt.Errorf("failed to start auth webserver: %w", err)
+		return "", "", fmt.Errorf("failed to start auth webserver: %w", err)
 	}
 	authURL = "http://" + bindAddress + "/auth?state=" + state
 
@@ -997,29 +1024,36 @@ func configSetup(ctx context.Context, id, name string, m configmap.Mapper, oauth
 	select {
 	case auth = <-server.result:
 	case <-oauthCtx.Done():
-		return "", errors.New("oauth authentication was cancelled")
+		return "", "", errors.New("oauth authentication was cancelled")
 	}
 	if !auth.OK || auth.Code == "" {
-		return "", auth
+		return "", "", auth
 	}
 	fs.Logf(nil, "Got code\n")
 	if opt.CheckAuth != nil {
 		err = opt.CheckAuth(oauthConfig, auth)
 		if err != nil {
-			return "", err
+			return "", "", err
 		}
 	}
-	return auth.Code, nil
+	return auth.Code, pkceVerifier, nil
 }
 
 // Exchange the code for a token
-func configExchange(ctx context.Context, name string, m configmap.Mapper, oauthConfig *Config, code string) error {
+func configExchange(ctx context.Context, name string, m configmap.Mapper, oauthConfig *Config, code string, pkceVerifier string) error {
 	ctx = Context(ctx, fshttp.NewClient(ctx))
 
 	// Create the configuration required for the OAuth flow
 	oauth2Conf := oauthConfig.MakeOauth2Config()
 
-	token, err := oauth2Conf.Exchange(ctx, code)
+	var token *oauth2.Token
+	var err error
+	if pkceVerifier != "" {
+		token, err = oauth2Conf.Exchange(ctx, code, oauth2.VerifierOption(pkceVerifier))
+	} else {
+		token, err = oauth2Conf.Exchange(ctx, code)
+	}
+
 	if err != nil {
 		return fmt.Errorf("failed to get token: %w", err)
 	}

lib/oauthutil/oauthutil_test.go

@@ -1,11 +1,22 @@
 package oauthutil
 
 import (
+	"context"
+	"crypto/sha256"
+	"encoding/base64"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
+	"regexp"
+	"strings"
+	"sync"
 	"testing"
 
+	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/config/configmap"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestAuthServerHandleAuth checks that only a callback carrying the
@@ -40,3 +51,98 @@ func TestAuthServerHandleAuth(t *testing.T) {
 		})
 	}
 }
+
+// TestConfigOauthPKCE verifies that the PKCE verifier generated with
+// the authorization URL on the remote setup path (title bar redirect)
+// is used when the pasted verification code is exchanged for a token
+func TestConfigOauthPKCE(t *testing.T) {
+	for _, usePKCE := range []bool{true, false} {
+		t.Run(fmt.Sprintf("usePKCE=%v", usePKCE), func(t *testing.T) {
+			// Fake token server which captures the code_verifier of the exchange
+			var mu sync.Mutex
+			var codeVerifier string
+			tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				_ = r.ParseForm()
+				mu.Lock()
+				codeVerifier = r.PostFormValue("code_verifier")
+				mu.Unlock()
+				w.Header().Set("Content-Type", "application/json")
+				_, _ = fmt.Fprint(w, `{"access_token":"access_token","token_type":"Bearer"}`)
+			}))
+			defer tokenServer.Close()
+
+			testConfig := &Config{
+				ClientID:     "client_id",
+				ClientSecret: "client_secret",
+				AuthURL:      tokenServer.URL + "/auth",
+				TokenURL:     tokenServer.URL + "/token",
+				RedirectURL:  TitleBarRedirectURL,
+				UsePKCE:      usePKCE,
+			}
+			ri := &fs.RegInfo{
+				Name: "testpkce",
+				Config: func(ctx context.Context, name string, m configmap.Mapper, in fs.ConfigIn) (*fs.ConfigOut, error) {
+					return ConfigOut("", &Options{OAuth2Config: testConfig})
+				},
+			}
+			m := configmap.Simple{}
+			ctx := context.Background()
+
+			// Start the oauth flow as if the backend had returned OAuth
+			// options from a state "oauth" with an empty result
+			in := fs.ConfigIn{State: fs.StatePush("", "*oauth", "", "oauth", "")}
+
+			// *oauth: no token yet so goto *oauth-confirm
+			out, err := ConfigOAuth(ctx, "remote", m, ri, in)
+			require.NoError(t, err)
+			require.True(t, strings.HasPrefix(out.State, "*oauth-confirm"), out.State)
+
+			// *oauth-confirm: replace the token -> *oauth-islocal
+			out, err = ConfigOAuth(ctx, "remote", m, ri, fs.ConfigIn{State: out.State, Result: "true"})
+			require.NoError(t, err)
+			require.True(t, strings.HasPrefix(out.State, "*oauth-islocal"), out.State)
+
+			// *oauth-islocal: no browser on this machine -> *oauth-remote
+			out, err = ConfigOAuth(ctx, "remote", m, ri, fs.ConfigIn{State: out.State, Result: "false"})
+			require.NoError(t, err)
+			require.True(t, strings.HasPrefix(out.State, "*oauth-remote"), out.State)
+
+			// *oauth-remote: title bar redirect so no webserver is
+			// needed: show the authorization URL and ask for the code
+			out, err = ConfigOAuth(ctx, "remote", m, ri, fs.ConfigIn{State: out.State})
+			require.NoError(t, err)
+			require.True(t, strings.HasPrefix(out.State, "*oauth-do"), out.State)
+			require.NotNil(t, out.Option)
+			authURL := regexp.MustCompile(`https?://\S+`).FindString(out.Option.Help)
+			require.NotEmpty(t, authURL)
+			u, err := url.Parse(authURL)
+			require.NoError(t, err)
+			challenge := u.Query().Get("code_challenge")
+			if usePKCE {
+				require.Equal(t, "S256", u.Query().Get("code_challenge_method"))
+				require.NotEmpty(t, challenge)
+			}
+
+			// *oauth-do: exchange the pasted code for a token
+			out, err = ConfigOAuth(ctx, "remote", m, ri, fs.ConfigIn{State: out.State, Result: "verification-code"})
+			require.NoError(t, err)
+			require.True(t, strings.HasPrefix(out.State, "*oauth-done"), out.State)
+
+			// The token must have been stored
+			token, ok := m.Get("token")
+			require.True(t, ok)
+			require.Contains(t, token, "access_token")
+
+			// The exchange must have been made with the verifier which
+			// matches the challenge sent in the authorization URL
+			mu.Lock()
+			defer mu.Unlock()
+			if usePKCE {
+				sum := sha256.Sum256([]byte(codeVerifier))
+				assert.Equal(t, challenge, base64.RawURLEncoding.EncodeToString(sum[:]))
+			} else {
+				assert.Empty(t, codeVerifier)
+			}
+		})
+	}
+}