From c196ca628eb7043a590e47f822333a11c0977c9f Mon Sep 17 00:00:00 2001 From: Benjamin Werner Date: Sun, 4 Oct 2026 14:34:40 -0700 Subject: [PATCH 1/2] auth login: say when credentials are being saved, not still awaiting approval After the browser approves the device, login saves the token to the system keyring. That can block on an unlock prompt, such as GNOME Keyring's password dialog, which may open on another screen. The spinner kept saying "Waiting for confirmation..." the whole time, so login looked stuck on the browser step even though access had been approved. Once the token arrives, the spinner now says access was approved and that credentials are being saved, and that the keyring may ask to be unlocked. See #1132. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/cmd/auth/login.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index b3d64036..99ef831d 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) From f5984b8cea8ce7e5cf69aeee93abe9743f0bc189 Mon Sep 17 00:00:00 2001 From: Benjamin Werner Date: Sun, 4 Oct 2026 14:34:40 -0700 Subject: [PATCH 2/2] auth: describe expired and denied device logins in plain terms The token endpoint can return the same generic error_description for every device-flow error, including expired_token: "The authorization server encountered an unexpected condition which prevented it from fulfilling the request." When a confirmation code expired before it was approved, login reported what read like a server failure. Describe RFC 8628's terminal device-flow errors from their error codes: an expired code tells the user to run `pscale auth login` again, and a denied request says it was denied. Other errors still use the server's description. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/auth/authenticator.go | 9 +++++ internal/auth/authenticator_test.go | 54 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/internal/auth/authenticator.go b/internal/auth/authenticator.go index ba9ee6d9..b5800c3b 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 888b3457..d5a5ade2 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...)) }