diff --git a/internal/client/device.go b/internal/client/device.go index e64f26b..018086a 100644 --- a/internal/client/device.go +++ b/internal/client/device.go @@ -120,6 +120,8 @@ type DeviceAuth struct { VerificationURIComplete string `json:"verification_uri_complete"` ExpiresIn int `json:"expires_in"` Interval int `json:"interval"` + Error string `json:"error"` + ErrorDescription string `json:"error_description"` } // startDevice asks the authorization server for a device/user code pair. The @@ -155,6 +157,14 @@ func startDevice(ctx context.Context, endpoint string, creds Credentials, resour return nil, fmt.Errorf("starting authorization: unreadable response (status %d)", resp.StatusCode) } if auth.DeviceCode == "" || auth.VerificationURI == "" { + // The authorization server explains its own refusals (RFC 6749 ยง5.2); + // reporting only the status turns a fixable answer into a guess. + if auth.ErrorDescription != "" { + return nil, fmt.Errorf("starting authorization: %s", auth.ErrorDescription) + } + if auth.Error != "" { + return nil, fmt.Errorf("starting authorization: %s", auth.Error) + } return nil, fmt.Errorf("starting authorization: server returned status %d", resp.StatusCode) } return &auth, nil diff --git a/internal/client/device_test.go b/internal/client/device_test.go index 2fabb50..156837b 100644 --- a/internal/client/device_test.go +++ b/internal/client/device_test.go @@ -170,3 +170,24 @@ func TestRegisterRejectsMissingDeviceGrant(t *testing.T) { t.Errorf("error should name the missing grant, got %v", err) } } + +// An authorization server explains its refusals; passing only the status code +// through leaves the user guessing at a question the server already answered. +func TestStartDeviceReportsServerError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _ = json.NewEncoder(w).Encode(map[string]any{ + "error": "invalid_client", + "error_description": "client_secret is required for pre-registered clients", + }) + })) + defer srv.Close() + + _, err := startDevice(context.Background(), srv.URL, Credentials{ClientID: "ikc_x"}, "", nil) + if err == nil { + t.Fatal("want an error") + } + if !strings.Contains(err.Error(), "client_secret is required") { + t.Errorf("error should carry the server's description, got %v", err) + } +} diff --git a/internal/setup/setup.go b/internal/setup/setup.go index b022487..410b71b 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -226,10 +226,13 @@ func authenticate(ctx context.Context, cfg *client.Config, opts Options) error { // identically on a laptop, over SSH, in a container, and headless. eps, discErr := client.Discover(ctx, cfg.URL) if discErr == nil && eps.DeviceAuthorization != "" { - // Reuse this machine's registration when it has one; a forced login is - // about the grant, not the client identity. + // Reuse this machine's registration when it has one, so a routine + // re-login doesn't leave a trail of dead clients at the authorization + // server. A forced login starts clean instead: --force is what someone + // reaches for when the saved credentials are the suspect, and an + // identity inherited from an older lard is exactly that. var creds client.Credentials - if cfg.OAuth != nil { + if cfg.OAuth != nil && !opts.Force { creds = cfg.OAuth.Credentials } tok, creds, err := client.LoginDevice(ctx, cfg.URL, creds, client.DefaultScopes, !opts.NoBrowser)