From 67a34657cc641400ad0750685015c22b20fb3a09 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Sun, 5 Jul 2026 22:07:04 -0700 Subject: [PATCH 1/4] Migrate PKCE OAuth and refresh helpers to Dropbox SDK Replace the hand-rolled oauth2.Config-based PKCE flow and refresh logic with the SDK's dropbox/oauth helpers (NewPKCEFlow, Exchange, Refresh). Introduce an oauthFlow interface so tests can stub the flow. --- CHANGELOG.md | 5 ++ cmd/auth.go | 66 +++++++-------- cmd/auth_test.go | 201 +++++++++++++++++++--------------------------- cmd/login_test.go | 35 +------- cmd/root_test.go | 2 +- 5 files changed, 120 insertions(+), 189 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 903a7e7e..5abacd78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ - `put` now preserves the source file's modification time as `ClientModified` instead of the upload time (stdin uploads use the spool file mtime). - The retry loop now respects context cancellation during backoff and never retries context errors. +**Changed:** + +- Upgraded Dropbox SDK to v6.4.0 and migrated PKCE OAuth and refresh-token + protocol helpers to the SDK. + **Infrastructure:** - Added scheduled/manual OSSF Scorecard scanning without public Scorecard API publishing. diff --git a/cmd/auth.go b/cmd/auth.go index 22d4317b..4d0af503 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -23,18 +23,16 @@ import ( "strings" "time" - "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox" + dropboxoauth "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/oauth" "github.com/mitchellh/go-homedir" "golang.org/x/oauth2" ) const ( - configFileName = "auth.json" - envAccessToken = "DBXCLI_ACCESS_TOKEN" - envAuthFile = "DBXCLI_AUTH_FILE" - tokenAccessTypeParam = "token_access_type" - tokenAccessTypeOffline = "offline" - tokenRefreshWindow = 5 * time.Minute + configFileName = "auth.json" + envAccessToken = "DBXCLI_ACCESS_TOKEN" + envAuthFile = "DBXCLI_AUTH_FILE" + tokenRefreshWindow = 5 * time.Minute authSourceEnv = "env" authSourceSaved = "saved" @@ -67,6 +65,11 @@ type authContext struct { AuthFile string } +type oauthFlow interface { + AuthCodeURL() string + Exchange(context.Context, string) (*oauth2.Token, error) +} + var currentAuthContext *authContext var readAppKey = func(prompt string) (string, error) { @@ -98,29 +101,22 @@ var readAuthorizationCode = func() (string, error) { var generateOAuthVerifier = oauth2.GenerateVerifier var generateOAuthState = oauth2.GenerateVerifier -var exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - return conf.Exchange(ctx, code, oauth2.VerifierOption(verifier)) +var newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { + return dropboxoauth.NewPKCEFlow( + appKey, + dropboxoauth.WithDomain(domain), + dropboxoauth.WithState(state), + dropboxoauth.WithVerifier(verifier), + dropboxoauth.WithTokenAccessType(dropboxoauth.TokenAccessTypeOffline), + ) } -var refreshOAuthToken = func(ctx context.Context, conf *oauth2.Config, token *oauth2.Token) (*oauth2.Token, error) { - expired := *token - expired.Expiry = time.Now().Add(-time.Second) - return conf.TokenSource(ctx, &expired).Token() +var exchangeAuthorizationCode = func(ctx context.Context, flow oauthFlow, code string) (*oauth2.Token, error) { + return flow.Exchange(ctx, code) } -func oauthConfig(tokenType string, domain string) *oauth2.Config { - appKey := oauthCredentials(tokenType) - return oauthConfigWithAppKey(appKey, domain) -} - -func oauthConfigWithAppKey(appKey string, domain string) *oauth2.Config { - endpoint := dropbox.OAuthEndpoint(domain) - endpoint.AuthStyle = oauth2.AuthStyleInParams - - return &oauth2.Config{ - ClientID: appKey, - Endpoint: endpoint, - } +var refreshOAuthToken = func(ctx context.Context, appKey string, domain string, token *oauth2.Token) (*oauth2.Token, error) { + return dropboxoauth.Refresh(ctx, appKey, token, dropboxoauth.WithDomain(domain)) } func (c *storedCredential) UnmarshalJSON(b []byte) error { @@ -362,15 +358,15 @@ func requestAccessCredential(tokType string, domain string) (storedCredential, e return storedCredential{}, err } - conf := oauthConfig(tokType, domain) + appKey := oauthCredentials(tokType) verifier := generateOAuthVerifier() state := generateOAuthState() - authCodeURL := conf.AuthCodeURL(state, - oauth2.S256ChallengeOption(verifier), - oauth2.SetAuthURLParam(tokenAccessTypeParam, tokenAccessTypeOffline), - ) + flow, err := newOAuthPKCEFlow(appKey, domain, state, verifier) + if err != nil { + return storedCredential{}, err + } - fmt.Printf("1. Go to %v\n", authCodeURL) + fmt.Printf("1. Go to %v\n", flow.AuthCodeURL()) fmt.Printf("2. Click \"Allow\" (you might have to log in first).\n") fmt.Printf("3. Copy the authorization code.\n") fmt.Printf("Enter the authorization code here: ") @@ -379,7 +375,7 @@ func requestAccessCredential(tokType string, domain string) (storedCredential, e if err != nil { return storedCredential{}, err } - token, err := exchangeAuthorizationCode(currentContext(), conf, code, verifier) + token, err := exchangeAuthorizationCode(currentContext(), flow, code) if err != nil { return storedCredential{}, authExchangeFailedErrorfWithDetails("exchange authorization code: %w", map[string]any{ "token_type": authTokenTypeName(tokType), @@ -395,7 +391,7 @@ func requestAccessCredential(tokType string, domain string) (storedCredential, e "token_type": authTokenTypeName(tokType), }) } - return storedCredentialFromOAuthToken(token, conf.ClientID), nil + return storedCredentialFromOAuthToken(token, appKey), nil } func refreshStoredCredential(tokType string, domain string, credential storedCredential) (storedCredential, error) { @@ -409,7 +405,7 @@ func refreshStoredCredential(tokType string, domain string, credential storedCre }) } - token, err := refreshOAuthToken(currentContext(), oauthConfigWithAppKey(appKey, domain), credential.oauthToken()) + token, err := refreshOAuthToken(currentContext(), appKey, domain, credential.oauthToken()) if err != nil { return storedCredential{}, err } diff --git a/cmd/auth_test.go b/cmd/auth_test.go index 429afa35..2e37bfe0 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -160,6 +160,8 @@ func restoreOAuthCredentials(t *testing.T) { origReadAppCredentials := readAppCredentials origGenerateOAuthVerifier := generateOAuthVerifier origGenerateOAuthState := generateOAuthState + origNewOAuthPKCEFlow := newOAuthPKCEFlow + origExchangeAuthorizationCode := exchangeAuthorizationCode origRefreshOAuthToken := refreshOAuthToken t.Cleanup(func() { personalAppKey = origPersonalAppKey @@ -169,10 +171,25 @@ func restoreOAuthCredentials(t *testing.T) { readAppCredentials = origReadAppCredentials generateOAuthVerifier = origGenerateOAuthVerifier generateOAuthState = origGenerateOAuthState + newOAuthPKCEFlow = origNewOAuthPKCEFlow + exchangeAuthorizationCode = origExchangeAuthorizationCode refreshOAuthToken = origRefreshOAuthToken }) } +type testOAuthFlow struct { + authCodeURL string + exchange func(context.Context, string) (*oauth2.Token, error) +} + +func (f testOAuthFlow) AuthCodeURL() string { + return f.authCodeURL +} + +func (f testOAuthFlow) Exchange(ctx context.Context, code string) (*oauth2.Token, error) { + return f.exchange(ctx, code) +} + func mockOAuthAppCredentials(t *testing.T) { t.Helper() @@ -191,28 +208,42 @@ func mockAuthorization(t *testing.T, code string, accessToken string) { mockOAuthAppCredentials(t) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAuthorizationCode = func() (string, error) { return code, nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, gotCode string, verifier string) (*oauth2.Token, error) { - if gotCode != code { - t.Fatalf("expected authorization code %q, got %q", code, gotCode) - } + newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { if verifier == "" { t.Fatal("expected PKCE verifier") } - return &oauth2.Token{ - AccessToken: accessToken, - RefreshToken: "refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(time.Hour), + return testOAuthFlow{ + authCodeURL: "https://example.com/oauth", + exchange: func(ctx context.Context, gotCode string) (*oauth2.Token, error) { + if gotCode != code { + t.Fatalf("expected authorization code %q, got %q", code, gotCode) + } + return &oauth2.Token{ + AccessToken: accessToken, + RefreshToken: "refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + }, nil + }, + }, nil + } +} + +func stubOAuthFlowToken(t *testing.T, wantAppKey string, token *oauth2.Token) { + t.Helper() + + newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { + if appKey != wantAppKey { + t.Fatalf("expected app key %q, got %q", wantAppKey, appKey) + } + return testOAuthFlow{ + authCodeURL: "https://example.com/oauth", + exchange: func(ctx context.Context, code string) (*oauth2.Token, error) { + return token, nil + }, }, nil } } @@ -224,18 +255,13 @@ func TestGetAccessTokenUsesExistingToken(t *testing.T) { } t.Setenv(envAuthFile, authFile) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) + restoreOAuthCredentials(t) readAuthorizationCode = func() (string, error) { t.Fatal("authorization prompt should not be used for existing token") return "", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - t.Fatal("authorization exchange should not be used for existing token") + newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { + t.Fatal("authorization flow should not be created for existing token") return nil, nil } @@ -303,9 +329,9 @@ func TestGetAccessTokenRefreshesExpiredCredential(t *testing.T) { restoreOAuthCredentials(t) refreshExpiry := time.Now().Add(time.Hour).UTC() - refreshOAuthToken = func(ctx context.Context, conf *oauth2.Config, token *oauth2.Token) (*oauth2.Token, error) { - if conf.ClientID != "stored-app-key" { - t.Fatalf("expected stored app key for refresh, got %q", conf.ClientID) + refreshOAuthToken = func(ctx context.Context, appKey string, domain string, token *oauth2.Token) (*oauth2.Token, error) { + if appKey != "stored-app-key" { + t.Fatalf("expected stored app key for refresh, got %q", appKey) } if token.RefreshToken != "old-refresh" { t.Fatalf("expected old refresh token, got %q", token.RefreshToken) @@ -365,7 +391,7 @@ func TestGetAccessTokenRefreshFailureLeavesAuthFileUnchanged(t *testing.T) { t.Setenv(envAuthFile, authFile) restoreOAuthCredentials(t) - refreshOAuthToken = func(ctx context.Context, conf *oauth2.Config, token *oauth2.Token) (*oauth2.Token, error) { + refreshOAuthToken = func(ctx context.Context, appKey string, domain string, token *oauth2.Token) (*oauth2.Token, error) { return nil, errors.New("refresh failed") } @@ -409,7 +435,7 @@ func TestGetAccessTokenRefreshWithoutAppKeyReturnsAppKeyRequired(t *testing.T) { restoreOAuthCredentials(t) setOAuthCredentials(tokenPersonal, "") - refreshOAuthToken = func(ctx context.Context, conf *oauth2.Config, token *oauth2.Token) (*oauth2.Token, error) { + refreshOAuthToken = func(ctx context.Context, appKey string, domain string, token *oauth2.Token) (*oauth2.Token, error) { t.Fatal("refresh should not run without an app key") return nil, nil } @@ -433,13 +459,6 @@ func TestGetAccessTokenMissingTokenWithDefaultPersonalCredentialsReturnsLoginErr authFile := filepath.Join(t.TempDir(), "auth.json") t.Setenv(envAuthFile, authFile) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("app credential prompt should not run for command lazy auth") return appCredentials{}, nil @@ -448,8 +467,8 @@ func TestGetAccessTokenMissingTokenWithDefaultPersonalCredentialsReturnsLoginErr t.Fatal("authorization prompt should not run when app credentials are missing") return "", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - t.Fatal("authorization exchange should not run when app credentials are missing") + newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { + t.Fatal("authorization flow should not run when app credentials are missing") return nil, nil } @@ -475,13 +494,6 @@ func TestGetAccessTokenMissingTokenWithConfiguredAppKeyReturnsLoginError(t *test authFile := filepath.Join(t.TempDir(), "auth.json") t.Setenv(envAuthFile, authFile) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("app credential prompt should not run for command lazy auth") return appCredentials{}, nil @@ -490,8 +502,8 @@ func TestGetAccessTokenMissingTokenWithConfiguredAppKeyReturnsLoginError(t *test t.Fatal("authorization prompt should not run for command lazy auth") return "", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - t.Fatal("authorization exchange should not run for command lazy auth") + newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { + t.Fatal("authorization flow should not run for command lazy auth") return nil, nil } @@ -537,7 +549,7 @@ func TestRequestAccessTokenRejectsEmptyToken(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { + exchangeAuthorizationCode = func(ctx context.Context, flow oauthFlow, code string) (*oauth2.Token, error) { return &oauth2.Token{}, nil } @@ -561,7 +573,7 @@ func TestRequestAccessTokenRejectsMissingRefreshToken(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { + exchangeAuthorizationCode = func(ctx context.Context, flow oauthFlow, code string) (*oauth2.Token, error) { return &oauth2.Token{AccessToken: "access-token"}, nil } @@ -585,7 +597,7 @@ func TestRequestAccessTokenReturnsReadError(t *testing.T) { readAuthorizationCode = func() (string, error) { return "", errors.New("read failed") } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { + exchangeAuthorizationCode = func(ctx context.Context, flow oauthFlow, code string) (*oauth2.Token, error) { t.Fatal("authorization exchange should not run when reading code fails") return nil, nil } @@ -599,13 +611,6 @@ func TestRequestAccessTokenUsesDefaultTeamManageAppKey(t *testing.T) { restoreOAuthCredentials(t) setOAuthCredentials(tokenTeamManage, defaultTeamManageAppKey) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("app credential prompt should not be used for the default team manage app key") return appCredentials{}, nil @@ -613,15 +618,7 @@ func TestRequestAccessTokenUsesDefaultTeamManageAppKey(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - if conf.ClientID != defaultTeamManageAppKey { - t.Fatalf("expected default team manage app key, got %q", conf.ClientID) - } - if conf.ClientSecret != "" { - t.Fatalf("expected no client secret for PKCE, got %q", conf.ClientSecret) - } - return &oauth2.Token{AccessToken: "access-token", RefreshToken: "refresh-token", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}, nil - } + stubOAuthFlowToken(t, defaultTeamManageAppKey, &oauth2.Token{AccessToken: "access-token", RefreshToken: "refresh-token", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}) token, err := requestAccessToken(tokenTeamManage, "") if err != nil { @@ -636,13 +633,6 @@ func TestRequestAccessTokenUsesDefaultPersonalAppKey(t *testing.T) { restoreOAuthCredentials(t) setOAuthCredentials(tokenPersonal, defaultPersonalAppKey) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("app credential prompt should not be used for the default personal app key") return appCredentials{}, nil @@ -650,15 +640,7 @@ func TestRequestAccessTokenUsesDefaultPersonalAppKey(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - if conf.ClientID != defaultPersonalAppKey { - t.Fatalf("expected default personal app key, got %q", conf.ClientID) - } - if conf.ClientSecret != "" { - t.Fatalf("expected no client secret for PKCE, got %q", conf.ClientSecret) - } - return &oauth2.Token{AccessToken: "access-token", RefreshToken: "refresh-token", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}, nil - } + stubOAuthFlowToken(t, defaultPersonalAppKey, &oauth2.Token{AccessToken: "access-token", RefreshToken: "refresh-token", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}) if _, err := requestAccessToken(tokenPersonal, ""); err != nil { t.Fatal(err) @@ -668,13 +650,6 @@ func TestRequestAccessTokenUsesDefaultPersonalAppKey(t *testing.T) { func TestRequestAccessTokenUsesPKCEOfflineAuthURL(t *testing.T) { mockOAuthAppCredentials(t) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - const verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" const state = "test-oauth-state" generateOAuthVerifier = func() string { @@ -686,15 +661,23 @@ func TestRequestAccessTokenUsesPKCEOfflineAuthURL(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, gotVerifier string) (*oauth2.Token, error) { + newOAuthPKCEFlow = func(appKey string, domain string, gotState string, gotVerifier string) (oauthFlow, error) { + if gotState != state { + t.Fatalf("expected state %q, got %q", state, gotState) + } if gotVerifier != verifier { t.Fatalf("expected verifier %q, got %q", verifier, gotVerifier) } - return &oauth2.Token{ - AccessToken: "access-token", - RefreshToken: "refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(time.Hour), + return testOAuthFlow{ + authCodeURL: "https://example.com/oauth?token_access_type=offline&state=test-oauth-state&code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM&code_challenge_method=S256", + exchange: func(ctx context.Context, code string) (*oauth2.Token, error) { + return &oauth2.Token{ + AccessToken: "access-token", + RefreshToken: "refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + }, nil + }, }, nil } @@ -747,13 +730,6 @@ func TestRequestAccessTokenUsesConfiguredAppCredentials(t *testing.T) { restoreOAuthCredentials(t) setOAuthCredentials(tokenPersonal, "configured-key") - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("app credential prompt should not be used") return appCredentials{}, nil @@ -761,15 +737,7 @@ func TestRequestAccessTokenUsesConfiguredAppCredentials(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - if conf.ClientID != "configured-key" { - t.Fatalf("expected configured app key, got %q", conf.ClientID) - } - if conf.ClientSecret != "" { - t.Fatalf("expected no client secret for PKCE, got %q", conf.ClientSecret) - } - return &oauth2.Token{AccessToken: "access-token", RefreshToken: "refresh-token", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}, nil - } + stubOAuthFlowToken(t, "configured-key", &oauth2.Token{AccessToken: "access-token", RefreshToken: "refresh-token", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}) if _, err := requestAccessToken(tokenPersonal, ""); err != nil { t.Fatal(err) @@ -780,13 +748,6 @@ func TestRequestAccessTokenRejectsEmptyAppCredentials(t *testing.T) { restoreOAuthCredentials(t) setOAuthCredentials(tokenTeamManage, "") - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { return appCredentials{Key: " "}, nil } @@ -794,8 +755,8 @@ func TestRequestAccessTokenRejectsEmptyAppCredentials(t *testing.T) { t.Fatal("authorization code prompt should not run when app credentials are invalid") return "", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - t.Fatal("authorization exchange should not run when app credentials are invalid") + newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { + t.Fatal("authorization flow should not run when app credentials are invalid") return nil, nil } diff --git a/cmd/login_test.go b/cmd/login_test.go index c139963a..e40e329b 100644 --- a/cmd/login_test.go +++ b/cmd/login_test.go @@ -2,7 +2,6 @@ package cmd import ( "bytes" - "context" "path/filepath" "testing" @@ -105,13 +104,6 @@ func TestLoginUsesAppKeyFlag(t *testing.T) { authFile := filepath.Join(t.TempDir(), "auth.json") t.Setenv(envAuthFile, authFile) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("app credential prompt should not be used when app key flag is set") return appCredentials{}, nil @@ -119,15 +111,7 @@ func TestLoginUsesAppKeyFlag(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - if conf.ClientID != "flag-key" { - t.Fatalf("expected flag app key, got %q", conf.ClientID) - } - if conf.ClientSecret != "" { - t.Fatalf("expected no client secret for PKCE, got %q", conf.ClientSecret) - } - return &oauth2.Token{AccessToken: "flag-token", RefreshToken: "refresh-token"}, nil - } + stubOAuthFlowToken(t, "flag-key", &oauth2.Token{AccessToken: "flag-token", RefreshToken: "refresh-token"}) cmd := newLoginTestCommand() if err := cmd.Flags().Set("app-key", "flag-key"); err != nil { @@ -157,13 +141,6 @@ func TestLoginAppKeyFlagUsesBundledDefaultKey(t *testing.T) { authFile := filepath.Join(t.TempDir(), "auth.json") t.Setenv(envAuthFile, authFile) - origReadAuthorizationCode := readAuthorizationCode - origExchangeAuthorizationCode := exchangeAuthorizationCode - t.Cleanup(func() { - readAuthorizationCode = origReadAuthorizationCode - exchangeAuthorizationCode = origExchangeAuthorizationCode - }) - readAppCredentials = func(tokType string) (appCredentials, error) { t.Fatal("full app credential prompt should not be used when app key flag is set") return appCredentials{}, nil @@ -171,15 +148,7 @@ func TestLoginAppKeyFlagUsesBundledDefaultKey(t *testing.T) { readAuthorizationCode = func() (string, error) { return "auth-code", nil } - exchangeAuthorizationCode = func(ctx context.Context, conf *oauth2.Config, code string, verifier string) (*oauth2.Token, error) { - if conf.ClientID != "flag-key" { - t.Fatalf("expected flag app key, got %q", conf.ClientID) - } - if conf.ClientSecret != "" { - t.Fatalf("expected no client secret for PKCE, got %q", conf.ClientSecret) - } - return &oauth2.Token{AccessToken: "flag-token", RefreshToken: "refresh-token"}, nil - } + stubOAuthFlowToken(t, "flag-key", &oauth2.Token{AccessToken: "flag-token", RefreshToken: "refresh-token"}) cmd := newLoginTestCommand() if err := cmd.Flags().Set("app-key", "flag-key"); err != nil { diff --git a/cmd/root_test.go b/cmd/root_test.go index b7603305..cc407c74 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -548,7 +548,7 @@ func TestInitDbxAccessTokenEnvBypassesRefresh(t *testing.T) { t.Fatal(err) } - refreshOAuthToken = func(ctx context.Context, conf *oauth2.Config, token *oauth2.Token) (*oauth2.Token, error) { + refreshOAuthToken = func(ctx context.Context, appKey string, domain string, token *oauth2.Token) (*oauth2.Token, error) { t.Fatal("refresh should not run when DBXCLI_ACCESS_TOKEN is set") return nil, nil } From bd376562e2fd6b2cf7fef14b971ef1c9fc01e90f Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Tue, 4 Aug 2026 08:57:34 -0700 Subject: [PATCH 2/4] add test --- cmd/auth_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/cmd/auth_test.go b/cmd/auth_test.go index 2e37bfe0..ec53e255 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "io" + "net/url" "os" "path/filepath" "strings" @@ -702,6 +703,39 @@ func TestRequestAccessTokenUsesPKCEOfflineAuthURL(t *testing.T) { } } +func TestNewOAuthPKCEFlowUsesOfflinePKCEOptions(t *testing.T) { + const ( + appKey = "test-app-key" + state = "test-oauth-state" + verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + ) + + flow, err := newOAuthPKCEFlow(appKey, "", state, verifier) + if err != nil { + t.Fatal(err) + } + authURL, err := url.Parse(flow.AuthCodeURL()) + if err != nil { + t.Fatal(err) + } + query := authURL.Query() + if got := query.Get("client_id"); got != appKey { + t.Errorf("expected client_id %q, got %q", appKey, got) + } + if got := query.Get("state"); got != state { + t.Errorf("expected state %q, got %q", state, got) + } + if got := query.Get("token_access_type"); got != "offline" { + t.Errorf("expected token_access_type offline, got %q", got) + } + if got := query.Get("code_challenge_method"); got != "S256" { + t.Errorf("expected code_challenge_method S256, got %q", got) + } + if got := query.Get("code_challenge"); got == "" { + t.Error("expected non-empty PKCE code_challenge") + } +} + func TestReadAppCredentialsReadsVisibleKey(t *testing.T) { restoreOAuthCredentials(t) From 61c987b59dd989ccf537a2190fd1d825a3cf0be4 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Tue, 4 Aug 2026 09:02:03 -0700 Subject: [PATCH 3/4] refactoring --- cmd/auth.go | 9 ++++++++- cmd/auth_test.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index 4d0af503..0b70b2bb 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -101,7 +101,12 @@ var readAuthorizationCode = func() (string, error) { var generateOAuthVerifier = oauth2.GenerateVerifier var generateOAuthState = oauth2.GenerateVerifier -var newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier string) (oauthFlow, error) { +func createOAuthPKCEFlow( + appKey string, + domain string, + state string, + verifier string, +) (oauthFlow, error) { return dropboxoauth.NewPKCEFlow( appKey, dropboxoauth.WithDomain(domain), @@ -111,6 +116,8 @@ var newOAuthPKCEFlow = func(appKey string, domain string, state string, verifier ) } +var newOAuthPKCEFlow = createOAuthPKCEFlow + var exchangeAuthorizationCode = func(ctx context.Context, flow oauthFlow, code string) (*oauth2.Token, error) { return flow.Exchange(ctx, code) } diff --git a/cmd/auth_test.go b/cmd/auth_test.go index ec53e255..8ada9992 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -710,7 +710,7 @@ func TestNewOAuthPKCEFlowUsesOfflinePKCEOptions(t *testing.T) { verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" ) - flow, err := newOAuthPKCEFlow(appKey, "", state, verifier) + flow, err := createOAuthPKCEFlow(appKey, "", state, verifier) if err != nil { t.Fatal(err) } From 5ce236222c41a26bc3f5f8ec3d914d0d64dc35e7 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Tue, 4 Aug 2026 09:08:56 -0700 Subject: [PATCH 4/4] improve tests --- CHANGELOG.md | 3 +-- cmd/auth_test.go | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5abacd78..b92cb469 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,7 @@ **Changed:** -- Upgraded Dropbox SDK to v6.4.0 and migrated PKCE OAuth and refresh-token - protocol helpers to the SDK. +- Migrated PKCE OAuth and refresh-token protocol helpers to the Dropbox SDK. **Infrastructure:** diff --git a/cmd/auth_test.go b/cmd/auth_test.go index 8ada9992..f5d8ad6e 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -159,6 +159,7 @@ func restoreOAuthCredentials(t *testing.T) { origTeamManageAppKey := teamManageAppKey origReadAppKey := readAppKey origReadAppCredentials := readAppCredentials + origReadAuthorizationCode := readAuthorizationCode origGenerateOAuthVerifier := generateOAuthVerifier origGenerateOAuthState := generateOAuthState origNewOAuthPKCEFlow := newOAuthPKCEFlow @@ -170,6 +171,7 @@ func restoreOAuthCredentials(t *testing.T) { teamManageAppKey = origTeamManageAppKey readAppKey = origReadAppKey readAppCredentials = origReadAppCredentials + readAuthorizationCode = origReadAuthorizationCode generateOAuthVerifier = origGenerateOAuthVerifier generateOAuthState = origGenerateOAuthState newOAuthPKCEFlow = origNewOAuthPKCEFlow