Merge pull request #288 from dropbox/json-api-error-codes

Add Dropbox API error classification and contract audits

Author: AndreyVMarkelov

README.md

@@ -165,11 +165,33 @@ $ dbxcli rm --output=json /old-file.txt
 $ dbxcli restore --output=json /Reports/old.pdf 015f...
 ```
 
-Structured success output is rolling out command by command. Currently migrated commands are `version`, `account`, `du`, `ls`, `search`, `revs`, `cp`, `mv`, `put`, `get`, `share-link create`, `share-link list`, `share-link info`, `share-link update`, `share-link revoke`, `share-link download`, `share list folder`, `team info`, `team list-members`, `team list-groups`, `team add-member`, `team remove-member`, `mkdir`, `rm`, and `restore`. Commands that have not been migrated return a JSON error whose `error.message` is `structured output is not supported for this command yet` when used with `--output=json`.
+Structured success output is rolling out command by command. Currently migrated commands are `version`, `account`, `du`, `ls`, `search`, `revs`, `cp`, `mv`, `put`, `get`, `share-link create`, `share-link list`, `share-link info`, `share-link update`, `share-link revoke`, `share-link download`, `share list folder`, `share list link`, `team info`, `team list-members`, `team list-groups`, `team add-member`, `team remove-member`, `mkdir`, `rm`, and `restore`. Commands that have not been migrated return a JSON error whose `error.message` is `structured output is not supported for this command yet` when used with `--output=json`.
 
 Command results and JSON errors are written to stdout. Status, progress, human-facing warnings, diagnostics, and verbose logs are written to stderr. JSON errors include a `warnings` array for machine-actionable warnings; it is `[]` when no warnings are present. Successful JSON payloads use the same `warnings` field.
 Current warning codes include `deprecated_command` for deprecated command paths and `skipped_symlink` for symlinks skipped by recursive upload.
 
+Commands that intentionally do not support JSON output yet include `login`, `logout`, and `completion`. Cobra help output and shell-completion protocol commands are also text-only.
+
+JSON error responses use stable `error.code` values:
+
+| Code                            | Meaning                                                                           |
+|---------------------------------|-----------------------------------------------------------------------------------|
+| `invalid_arguments`             | The command arguments or flags are invalid.                                       |
+| `path_conflict`                 | A local or Dropbox path conflicts with the requested operation.                   |
+| `auth_required`                 | No usable saved credentials were found, or Dropbox rejected the saved token.      |
+| `auth_refresh_failed`           | Saved refreshable credentials could not be refreshed.                             |
+| `app_key_required`              | Login or token refresh needs a Dropbox app key.                                   |
+| `auth_exchange_failed`          | The OAuth authorization-code exchange failed or returned unusable tokens.         |
+| `not_found`                     | Dropbox reported that the requested object was not found.                         |
+| `permission_denied`             | Dropbox denied access because of permissions, scope, member selection, or state.  |
+| `rate_limited`                  | Dropbox rate limited the request.                                                 |
+| `dropbox_api_error`             | Dropbox returned an API error that does not map to a more specific code yet.      |
+| `structured_output_unsupported` | The command does not support `--output=json` yet.                                 |
+| `unsupported_output_format`     | `--output` was not `text` or `json`.                                              |
+| `unknown_command`               | Cobra could not resolve the command.                                              |
+| `unknown_flag`                  | Cobra could not resolve a flag.                                                   |
+| `command_failed`                | Fallback for failures without a more specific stable code.                        |
+
 Successful JSON responses for migrated commands return an `input` object, a `results` array, and a `warnings` array. Result payloads are command-specific. For commands such as `mkdir`, each result reports what happened to the requested path:
 
 ```json
@@ -466,7 +488,7 @@ In JSON mode, command errors are written to stdout as JSON, including errors fro
 }
 ```
 
-Error `code` values are stable identifiers intended for scripts. Current codes are `structured_output_unsupported`, `unsupported_output_format`, `unknown_command`, `unknown_flag`, `path_conflict`, `invalid_arguments`, and `command_failed`.
+Error `code` values are stable identifiers intended for scripts. The current stable codes are listed in the table above.
 
 ### Authentication
 

cmd/auth.go

@@ -17,7 +17,6 @@ package cmd
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -239,7 +238,10 @@ func getAccessToken(tokType string, domain string, force bool) (string, string,
 	if !force && credential.shouldRefresh(time.Now()) {
 		credential, err = refreshStoredCredential(tokType, domain, credential)
 		if err != nil {
-			return "", "", fmt.Errorf("refresh saved Dropbox credentials: %w; run %q again", err, loginCommand(tokType))
+			if jsonErrorCode(err) == jsonErrorCodeAppKeyRequired {
+				return "", "", appKeyRequiredErrorf("refresh saved Dropbox credentials: %w; run %q again", err, loginCommand(tokType))
+			}
+			return "", "", authRefreshFailedErrorf("refresh saved Dropbox credentials: %w; run %q again", err, loginCommand(tokType))
 		}
 		tokens[tokType] = credential
 		if err = writeTokens(filePath, tokenMap); err != nil {
@@ -262,7 +264,7 @@ func loginCommand(tokType string) string {
 }
 
 func missingAccessTokenError(tokType string) error {
-	return fmt.Errorf("no saved Dropbox credentials; run %q first or set %s", loginCommand(tokType), envAccessToken)
+	return authRequiredErrorf("no saved Dropbox credentials; run %q first or set %s", loginCommand(tokType), envAccessToken)
 }
 
 func appCredentialsName(tokType string) string {
@@ -287,7 +289,7 @@ func ensureOAuthAppCredentials(tokType string) error {
 	}
 	creds.Key = strings.TrimSpace(creds.Key)
 	if creds.Key == "" {
-		return errors.New("Dropbox app key is required")
+		return appKeyRequiredError("Dropbox app key is required")
 	}
 
 	setOAuthCredentials(tokType, creds.Key)
@@ -325,13 +327,13 @@ func requestAccessCredential(tokType string, domain string) (storedCredential, e
 	}
 	token, err := exchangeAuthorizationCode(context.Background(), conf, code, verifier)
 	if err != nil {
-		return storedCredential{}, err
+		return storedCredential{}, authExchangeFailedErrorf("exchange authorization code: %w", err)
 	}
 	if token == nil || token.AccessToken == "" {
-		return storedCredential{}, errors.New("authorization did not return an access token")
+		return storedCredential{}, authExchangeFailedError("authorization did not return an access token")
 	}
 	if token.RefreshToken == "" {
-		return storedCredential{}, errors.New("authorization did not return a refresh token")
+		return storedCredential{}, authExchangeFailedError("authorization did not return a refresh token")
 	}
 	return storedCredentialFromOAuthToken(token, conf.ClientID), nil
 }
@@ -342,15 +344,15 @@ func refreshStoredCredential(tokType string, domain string, credential storedCre
 		appKey = oauthCredentials(tokType)
 	}
 	if strings.TrimSpace(appKey) == "" {
-		return storedCredential{}, errors.New("saved credentials cannot be refreshed without a Dropbox app key")
+		return storedCredential{}, appKeyRequiredError("saved credentials cannot be refreshed without a Dropbox app key")
 	}
 
 	token, err := refreshOAuthToken(context.Background(), oauthConfigWithAppKey(appKey, domain), credential.oauthToken())
 	if err != nil {
 		return storedCredential{}, err
 	}
 	if token == nil || token.AccessToken == "" {
-		return storedCredential{}, errors.New("token refresh did not return an access token")
+		return storedCredential{}, authRefreshFailedErrorf("token refresh did not return an access token")
 	}
 
 	refreshed := storedCredentialFromOAuthToken(token, appKey)

cmd/auth_test.go

@@ -371,6 +371,9 @@ func TestGetAccessTokenRefreshFailureLeavesAuthFileUnchanged(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected refresh failure")
 	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeAuthRefreshFailed; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
 	if !strings.Contains(err.Error(), "dbxcli login") {
 		t.Fatalf("expected login hint, got %q", err)
 	}
@@ -384,6 +387,43 @@ func TestGetAccessTokenRefreshFailureLeavesAuthFileUnchanged(t *testing.T) {
 	}
 }
 
+func TestGetAccessTokenRefreshWithoutAppKeyReturnsAppKeyRequired(t *testing.T) {
+	expired := time.Now().Add(-time.Hour).UTC()
+	authFile := filepath.Join(t.TempDir(), "auth.json")
+	tokens := TokenMap{
+		"": {
+			tokenPersonal: {
+				AccessToken:  "old-access",
+				RefreshToken: "old-refresh",
+				TokenType:    "Bearer",
+				Expiry:       &expired,
+			},
+		},
+	}
+	if err := writeTokens(authFile, tokens); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv(envAuthFile, authFile)
+
+	restoreOAuthCredentials(t)
+	setOAuthCredentials(tokenPersonal, "")
+	refreshOAuthToken = func(ctx context.Context, conf *oauth2.Config, token *oauth2.Token) (*oauth2.Token, error) {
+		t.Fatal("refresh should not run without an app key")
+		return nil, nil
+	}
+
+	_, _, err := getAccessToken(tokenPersonal, "", false)
+	if err == nil {
+		t.Fatal("expected missing app key error")
+	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeAppKeyRequired; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
+	if !strings.Contains(err.Error(), "dbxcli login") {
+		t.Fatalf("expected login hint, got %q", err)
+	}
+}
+
 func TestGetAccessTokenMissingTokenWithDefaultPersonalCredentialsReturnsLoginError(t *testing.T) {
 	restoreOAuthCredentials(t)
 	setOAuthCredentials(tokenPersonal, defaultPersonalAppKey)
@@ -415,6 +455,9 @@ func TestGetAccessTokenMissingTokenWithDefaultPersonalCredentialsReturnsLoginErr
 	if err == nil {
 		t.Fatal("expected missing credentials error")
 	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeAuthRequired; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
 	if !strings.Contains(err.Error(), "dbxcli login") {
 		t.Fatalf("expected login hint, got %q", err)
 	}
@@ -454,6 +497,9 @@ func TestGetAccessTokenMissingTokenWithConfiguredAppKeyReturnsLoginError(t *test
 	if err == nil {
 		t.Fatal("expected missing credentials error")
 	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeAuthRequired; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
 	if !strings.Contains(err.Error(), "dbxcli login") {
 		t.Fatalf("expected login hint, got %q", err)
 	}
@@ -495,6 +541,8 @@ func TestRequestAccessTokenRejectsEmptyToken(t *testing.T) {
 
 	if _, err := requestAccessToken(tokenPersonal, ""); err == nil {
 		t.Fatal("expected empty access token to return an error")
+	} else if got, want := jsonErrorCode(err), jsonErrorCodeAuthExchangeFailed; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
 	}
 }
 
@@ -517,6 +565,8 @@ func TestRequestAccessTokenRejectsMissingRefreshToken(t *testing.T) {
 
 	if _, err := requestAccessToken(tokenPersonal, ""); err == nil {
 		t.Fatal("expected missing refresh token to return an error")
+	} else if got, want := jsonErrorCode(err), jsonErrorCodeAuthExchangeFailed; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
 	}
 }
 
@@ -742,5 +792,7 @@ func TestRequestAccessTokenRejectsEmptyAppCredentials(t *testing.T) {
 
 	if _, err := requestAccessToken(tokenTeamManage, ""); err == nil {
 		t.Fatal("expected empty app credentials to fail")
+	} else if got, want := jsonErrorCode(err), jsonErrorCodeAppKeyRequired; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
 	}
 }