diff --git a/internal/api/auth.go b/internal/api/auth.go index 65d0157..ffc29b0 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -20,10 +20,12 @@ type DeviceTokens struct { } // ExchangeDeviceCode trades a one-time device-auth code for a JWT pair. -// The code is the credential — no prior auth is required. -func (c *Client) ExchangeDeviceCode(ctx context.Context, code string) (*DeviceTokens, error) { +// The code is the credential — no prior auth is required — but it must be +// redeemed with the PKCE verifier whose S256 challenge was sent at login. +func (c *Client) ExchangeDeviceCode(ctx context.Context, code, verifier string) (*DeviceTokens, error) { var out DeviceTokens - if err := c.do(ctx, http.MethodPost, "/auth/device/token", nil, map[string]string{"code": code}, &out); err != nil { + body := map[string]string{"code": code, "code_verifier": verifier} + if err := c.do(ctx, http.MethodPost, "/auth/device/token", nil, body, &out); err != nil { return nil, err } return &out, nil diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 54b6d95..3daf9ba 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -2,6 +2,8 @@ package api import ( "context" + "encoding/json" + "io" "net/http" "net/http/httptest" "testing" @@ -12,12 +14,23 @@ func TestExchangeDeviceCode(t *testing.T) { if r.URL.Path != "/auth/device/token" || r.Method != http.MethodPost { t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) } + body, _ := io.ReadAll(r.Body) + var got map[string]string + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("bad body: %v", err) + } + if got["code"] != "onetimecode" { + t.Errorf("code = %q", got["code"]) + } + if got["code_verifier"] != "theverifier" { + t.Errorf("code_verifier = %q", got["code_verifier"]) + } w.Write([]byte(`{"access_token":"at","refresh_token":"rt","user":{"id":"1","email":"a@b.c","email_verified":true,"name":"A","plan":"free"}}`)) })) defer srv.Close() c := New(srv.URL, newTestStore(t, nil)) - tok, err := c.ExchangeDeviceCode(context.Background(), "onetimecode") + tok, err := c.ExchangeDeviceCode(context.Background(), "onetimecode", "theverifier") if err != nil { t.Fatal(err) } diff --git a/internal/auth/device.go b/internal/auth/device.go index b0b06df..2e6535a 100644 --- a/internal/auth/device.go +++ b/internal/auth/device.go @@ -33,17 +33,30 @@ type DeviceFlow struct { Out io.Writer // progress messages (stderr) } +// Result carries what the flow proves back to the caller: the one-time +// code and the PKCE verifier it must be redeemed with. +type Result struct { + Code string + Verifier string +} + // Run blocks until the consent callback delivers a code, the context -// expires, or the callback is invalid. Returns the one-time code. -func (f *DeviceFlow) Run(ctx context.Context) (string, error) { +// expires, or the callback is invalid. Returns the one-time code plus the +// PKCE verifier the caller must present at token exchange. +func (f *DeviceFlow) Run(ctx context.Context) (Result, error) { state, err := randomState() if err != nil { - return "", err + return Result{}, err + } + + pkce, err := newPKCE() + if err != nil { + return Result{}, err } ln, err := net.Listen("tcp", CallbackAddr) if err != nil { - return "", fmt.Errorf("cannot listen on %s (is another spoo login running?): %w", CallbackAddr, err) + return Result{}, fmt.Errorf("cannot listen on %s (is another spoo login running?): %w", CallbackAddr, err) } codeCh := make(chan string, 1) @@ -72,9 +85,9 @@ func (f *DeviceFlow) Run(ctx context.Context) (string, error) { go srv.Serve(ln) defer srv.Shutdown(context.Background()) - authURL := fmt.Sprintf("%s/auth/device/login?app_id=%s&redirect_uri=%s&state=%s", + authURL := fmt.Sprintf("%s/auth/device/login?app_id=%s&redirect_uri=%s&state=%s&code_challenge=%s&code_challenge_method=S256", f.APIBase, AppID, - url.QueryEscape("http://"+CallbackAddr+CallbackPath), state) + url.QueryEscape("http://"+CallbackAddr+CallbackPath), state, pkce.Challenge) fmt.Fprintln(f.Out, "Opening your browser to authorize spoo CLI…") fmt.Fprintf(f.Out, "If it doesn't open automatically, visit:\n\n %s\n\n", authURL) @@ -84,11 +97,11 @@ func (f *DeviceFlow) Run(ctx context.Context) (string, error) { select { case code := <-codeCh: - return code, nil + return Result{Code: code, Verifier: pkce.Verifier}, nil case err := <-errCh: - return "", err + return Result{}, err case <-ctx.Done(): - return "", fmt.Errorf("login timed out: %w", ctx.Err()) + return Result{}, fmt.Errorf("login timed out: %w", ctx.Err()) } } diff --git a/internal/auth/device_test.go b/internal/auth/device_test.go index 684537b..00e81ec 100644 --- a/internal/auth/device_test.go +++ b/internal/auth/device_test.go @@ -2,6 +2,8 @@ package auth import ( "context" + "crypto/sha256" + "encoding/base64" "fmt" "io" "net/http" @@ -14,6 +16,7 @@ import ( // Simulates the browser leg: the flow opens a URL; we parse state and // redirect_uri out of it and hit the loopback callback like spoo.me would. func TestDeviceFlowReturnsCode(t *testing.T) { + var sawChallenge string flow := &DeviceFlow{ APIBase: "https://spoo.example", Out: io.Discard, @@ -32,6 +35,13 @@ func TestDeviceFlowReturnsCode(t *testing.T) { if !strings.HasPrefix(cb, "http://127.0.0.1:53682/callback") { t.Errorf("redirect_uri = %q", cb) } + if q.Get("code_challenge_method") != "S256" { + t.Errorf("code_challenge_method = %q", q.Get("code_challenge_method")) + } + sawChallenge = q.Get("code_challenge") + if len(sawChallenge) != 43 { + t.Errorf("code_challenge = %q (len %d, want 43)", sawChallenge, len(sawChallenge)) + } time.Sleep(50 * time.Millisecond) // let the server start resp, err := http.Get(fmt.Sprintf("%s?code=thecode&state=%s", cb, q.Get("state"))) if err != nil { @@ -46,12 +56,17 @@ func TestDeviceFlowReturnsCode(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - code, err := flow.Run(ctx) + res, err := flow.Run(ctx) if err != nil { t.Fatal(err) } - if code != "thecode" { - t.Fatalf("code = %q, want thecode", code) + if res.Code != "thecode" { + t.Fatalf("code = %q, want thecode", res.Code) + } + // The verifier must S256-hash to the challenge the server saw. + sum := sha256.Sum256([]byte(res.Verifier)) + if got := base64.RawURLEncoding.EncodeToString(sum[:]); got != sawChallenge { + t.Fatalf("verifier hashes to %q, challenge was %q", got, sawChallenge) } } diff --git a/internal/auth/pkce.go b/internal/auth/pkce.go new file mode 100644 index 0000000..5ce80bf --- /dev/null +++ b/internal/auth/pkce.go @@ -0,0 +1,29 @@ +package auth + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" +) + +// PKCE holds a proof-key pair for the device authorization flow (RFC 7636). +// The verifier stays on the client; the S256 challenge travels to the +// server at login and is proven by sending the verifier at token exchange. +type PKCE struct { + Verifier string + Challenge string +} + +// newPKCE mints a fresh verifier and its S256 challenge. The verifier is a +// 43-character base64url string (32 random bytes), the minimum RFC 7636 +// length, which the backend also enforces. +func newPKCE() (PKCE, error) { + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + return PKCE{}, err + } + verifier := base64.RawURLEncoding.EncodeToString(b) + sum := sha256.Sum256([]byte(verifier)) + challenge := base64.RawURLEncoding.EncodeToString(sum[:]) + return PKCE{Verifier: verifier, Challenge: challenge}, nil +} diff --git a/internal/auth/pkce_test.go b/internal/auth/pkce_test.go new file mode 100644 index 0000000..ef62a5e --- /dev/null +++ b/internal/auth/pkce_test.go @@ -0,0 +1,42 @@ +package auth + +import ( + "crypto/sha256" + "encoding/base64" + "testing" +) + +func TestNewPKCE(t *testing.T) { + p, err := newPKCE() + if err != nil { + t.Fatal(err) + } + if len(p.Verifier) != 43 { + t.Errorf("verifier len = %d, want 43", len(p.Verifier)) + } + if len(p.Challenge) != 43 { + t.Errorf("challenge len = %d, want 43", len(p.Challenge)) + } + sum := sha256.Sum256([]byte(p.Verifier)) + if want := base64.RawURLEncoding.EncodeToString(sum[:]); want != p.Challenge { + t.Fatalf("challenge = %q, want S256(verifier) = %q", p.Challenge, want) + } +} + +func TestNewPKCEUnique(t *testing.T) { + a, _ := newPKCE() + b, _ := newPKCE() + if a.Verifier == b.Verifier { + t.Fatal("two verifiers collided") + } +} + +// RFC 7636 Appendix B fixed vector — proves our S256 derivation matches. +func TestS256MatchesRFCVector(t *testing.T) { + verifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + sum := sha256.Sum256([]byte(verifier)) + got := base64.RawURLEncoding.EncodeToString(sum[:]) + if want := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"; got != want { + t.Fatalf("challenge = %q, want %q", got, want) + } +} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 2ae6690..d671325 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -61,11 +61,11 @@ func loginWithBrowser(cmd *cobra.Command, d *deps) error { ctx, cancel := context.WithTimeout(cmd.Context(), 5*time.Minute) defer cancel() - code, err := flow.Run(ctx) + res, err := flow.Run(ctx) if err != nil { return err } - tokens, err := d.client.ExchangeDeviceCode(ctx, code) + tokens, err := d.client.ExchangeDeviceCode(ctx, res.Code, res.Verifier) if err != nil { return fmt.Errorf("token exchange failed: %w", err) }