diff --git a/internal/auth/authenticator.go b/internal/auth/authenticator.go index ba9ee6d9e..b5800c3b1 100644 --- a/internal/auth/authenticator.go +++ b/internal/auth/authenticator.go @@ -85,6 +85,15 @@ type ErrorResponse struct { } func (e ErrorResponse) Error() string { + // The device flow's terminal errors (RFC 8628, section 3.5) describe an + // outcome the user can act on, so describe them here rather than relying + // on the server's error_description. + switch e.ErrorCode { + case "expired_token": + return "the confirmation code expired before it was approved; run 'pscale auth login' again" + case "access_denied": + return "the login request was denied in the browser" + } return e.Description } diff --git a/internal/auth/authenticator_test.go b/internal/auth/authenticator_test.go index 888b34574..d5a5ade26 100644 --- a/internal/auth/authenticator_test.go +++ b/internal/auth/authenticator_test.go @@ -2,6 +2,7 @@ package auth import ( "context" + "errors" "fmt" "io" "net/http" @@ -168,6 +169,59 @@ func TestGetAccessTokenForDevice(t *testing.T) { } } +func TestGetAccessTokenForDeviceTerminalErrors(t *testing.T) { + // The server can attach a generic error_description to every device-flow + // error, so the message must come from the error code. + const genericDescription = "The authorization server encountered an unexpected condition which prevented it from fulfilling the request." + + tests := []struct { + code string + want string + }{ + { + code: "expired_token", + want: "the confirmation code expired before it was approved; run 'pscale auth login' again", + }, + { + code: "access_denied", + want: "the login request was denied in the browser", + }, + } + + for _, tt := range tests { + t.Run(tt.code, func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + if _, err := fmt.Fprintf(w, `{"error": %q, "error_description": %q}`, tt.code, genericDescription); err != nil { + panicf("failed to write response bytes: %v", err) + } + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + authenticator, err := New(cleanhttp.DefaultClient(), testClientID, testClientSecret, SetBaseURL(srv.URL), WithMockClock(clock.NewMock())) + if err != nil { + t.Fatalf("error creating client: %v", err) + } + + _, err = authenticator.GetAccessTokenForDevice(context.TODO(), DeviceVerification{ + DeviceCode: "deadbeef", + CheckInterval: 10 * time.Millisecond, + }) + + var errRes *ErrorResponse + if !errors.As(err, &errRes) { + t.Fatalf("expected an ErrorResponse, got %v", err) + } + assert.Equal(t, tt.code, errRes.ErrorCode) + assert.EqualError(t, err, tt.want) + }) + } +} + func panicf(format string, a ...any) { panic(fmt.Sprintf(format, a...)) } diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index b3d64036b..99ef831df 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -17,6 +17,8 @@ import ( "github.com/spf13/cobra" ) +const savingCredentialsMessage = "Access approved. Saving credentials to your system keyring; if it asks to be unlocked, enter your keyring password..." + // LoginCmd is the command for logging into a PlanetScale account. func LoginCmd(ch *cmdutil.Helper) *cobra.Command { var clientID string @@ -82,10 +84,10 @@ func LoginCmd(ch *cmdutil.Helper) *cobra.Command { ch.Printer.Printf("\nIf something goes wrong, copy and paste this URL into your browser: %s\n\n", printer.Bold(deviceVerification.VerificationCompleteURL)) } - var end func() + var progress *printer.ProgressHandle if !jsonMode { - end = ch.Printer.PrintProgress("Waiting for confirmation...") - defer end() + progress = ch.Printer.StartProgress("Waiting for confirmation...") + defer progress.Stop() } accessToken, err := authenticator.GetAccessTokenForDevice(ctx, *deviceVerification) @@ -96,6 +98,12 @@ func LoginCmd(ch *cmdutil.Helper) *cobra.Command { return err } + // Saving to the system keyring can block on an unlock prompt, + // such as GNOME Keyring's password dialog, which may open on + // another screen. Without this, login looks stuck waiting for the + // browser even though access was already approved. + progress.Update(savingCredentialsMessage) + err = config.WriteAccessToken(accessToken) if err != nil { if jsonMode { @@ -108,9 +116,7 @@ func LoginCmd(ch *cmdutil.Helper) *cobra.Command { return fmt.Errorf("error logging in: %w\n\nPlease ensure you have write permissions to the configuration directory: %s", err, configDir) } - if end != nil { - end() - } + progress.Stop() orgSetupErr := writeDefaultOrganizationIfNeeded(ctx, ch, accessToken, authURL)