feat(auth): PKCE (S256) for the device login flow

internal/api/auth.go

@@ -20,10 +20,11 @@ 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. The verifier is
+// the PKCE code verifier whose S256 challenge was sent on the login URL.
+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 {
+	if err := c.do(ctx, http.MethodPost, "/auth/device/token", nil, map[string]string{"code": code, "code_verifier": verifier}, &out); err != nil {
 		return nil, err
 	}
 	return &out, nil

internal/api/auth_test.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -12,12 +13,22 @@ 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)
 		}
+		var body map[string]string
+		if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+			t.Errorf("decode body: %v", err)
+		}
+		if body["code"] != "onetimecode" {
+			t.Errorf("code = %q, want onetimecode", body["code"])
+		}
+		if body["code_verifier"] != "theverifier" {
+			t.Errorf("code_verifier = %q, want theverifier", body["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)
 	}

internal/api/keys.go

@@ -16,24 +16,6 @@ type APIKey struct {
 	ExpiresAt   int64    `json:"expires_at"`
 	Revoked     bool     `json:"revoked"`
 	TokenPrefix string   `json:"token_prefix"`
-	Token       string   `json:"token,omitempty"` // full token, present only on create
-}
-
-type CreateKeyRequest struct {
-	Name        string   `json:"name"`
-	Description string   `json:"description,omitempty"`
-	Scopes      []string `json:"scopes"`
-	ExpiresAt   string   `json:"expires_at,omitempty"` // ISO 8601 or epoch seconds
-}
-
-// CreateKey mints a new API key. Requires a device-flow (JWT) session;
-// the backend refuses key creation authenticated by another API key.
-func (c *Client) CreateKey(ctx context.Context, req CreateKeyRequest) (*APIKey, error) {
-	var out APIKey
-	if err := c.do(ctx, http.MethodPost, "/api/v1/keys", nil, req, &out); err != nil {
-		return nil, err
-	}
-	return &out, nil
 }
 
 func (c *Client) ListKeys(ctx context.Context) ([]APIKey, error) {

internal/api/keys_test.go

@@ -2,34 +2,11 @@ package api
 
 import (
 	"context"
-	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 )
 
-func TestCreateKeyReturnsTokenOnce(t *testing.T) {
-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		var req CreateKeyRequest
-		json.NewDecoder(r.Body).Decode(&req)
-		if req.Name != "CI" || len(req.Scopes) != 1 || req.Scopes[0] != "shorten:create" {
-			t.Errorf("unexpected body: %+v", req)
-		}
-		w.WriteHeader(http.StatusCreated)
-		w.Write([]byte(`{"id":"k1","name":"CI","scopes":["shorten:create"],"token_prefix":"abc12345","token":"spoo_secret"}`))
-	}))
-	defer srv.Close()
-
-	c := New(srv.URL, newTestStore(t, nil))
-	key, err := c.CreateKey(context.Background(), CreateKeyRequest{Name: "CI", Scopes: []string{"shorten:create"}})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if key.Token != "spoo_secret" {
-		t.Fatalf("token = %q", key.Token)
-	}
-}
-
 func TestListKeys(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Write([]byte(`{"keys":[{"id":"k1","name":"CI","scopes":["shorten:create"],"token_prefix":"abc12345","revoked":false}]}`))

internal/auth/device.go

@@ -3,6 +3,8 @@ package auth
 import (
 	"context"
 	"crypto/rand"
+	"crypto/sha256"
+	"encoding/base64"
 	"encoding/hex"
 	"errors"
 	"fmt"
@@ -34,16 +36,21 @@ type DeviceFlow struct {
 }
 
 // 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 and the
+// PKCE verifier that must accompany it in the token exchange.
+func (f *DeviceFlow) Run(ctx context.Context) (string, string, error) {
 	state, err := randomState()
 	if err != nil {
-		return "", err
+		return "", "", err
+	}
+	verifier, err := codeVerifier()
+	if err != nil {
+		return "", "", 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 "", "", fmt.Errorf("cannot listen on %s (is another spoo login running?): %w", CallbackAddr, err)
 	}
 
 	codeCh := make(chan string, 1)
@@ -72,9 +79,10 @@ 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,
+		codeChallengeS256(verifier))
 
 	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 +92,11 @@ func (f *DeviceFlow) Run(ctx context.Context) (string, error) {
 
 	select {
 	case code := <-codeCh:
-		return code, nil
+		return code, verifier, nil
 	case err := <-errCh:
-		return "", err
+		return "", "", err
 	case <-ctx.Done():
-		return "", fmt.Errorf("login timed out: %w", ctx.Err())
+		return "", "", fmt.Errorf("login timed out: %w", ctx.Err())
 	}
 }
 
@@ -99,3 +107,20 @@ func randomState() (string, error) {
 	}
 	return hex.EncodeToString(b), nil
 }
+
+// codeVerifier returns a PKCE code verifier: 32 random bytes encoded as
+// unpadded base64url, always 43 characters (RFC 7636 §4.1).
+func codeVerifier() (string, error) {
+	b := make([]byte, 32)
+	if _, err := rand.Read(b); err != nil {
+		return "", err
+	}
+	return base64.RawURLEncoding.EncodeToString(b), nil
+}
+
+// codeChallengeS256 derives the S256 challenge for a verifier:
+// BASE64URL(SHA256(verifier)) without padding (RFC 7636 §4.2).
+func codeChallengeS256(verifier string) string {
+	sum := sha256.Sum256([]byte(verifier))
+	return base64.RawURLEncoding.EncodeToString(sum[:])
+}

internal/auth/device_test.go

@@ -6,14 +6,18 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
 )
 
+var challengeRe = regexp.MustCompile(`^[A-Za-z0-9_-]{43}$`)
+
 // 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) {
+	challengeCh := make(chan string, 1)
 	flow := &DeviceFlow{
 		APIBase: "https://spoo.example",
 		Out:     io.Discard,
@@ -32,6 +36,14 @@ 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, want S256", q.Get("code_challenge_method"))
+				}
+				challenge := q.Get("code_challenge")
+				if !challengeRe.MatchString(challenge) {
+					t.Errorf("code_challenge = %q, want 43 base64url chars", challenge)
+				}
+				challengeCh <- challenge
 				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,13 +58,30 @@ func TestDeviceFlowReturnsCode(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
-	code, err := flow.Run(ctx)
+	code, verifier, err := flow.Run(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
 	if code != "thecode" {
 		t.Fatalf("code = %q, want thecode", code)
 	}
+	if len(verifier) != 43 {
+		t.Fatalf("verifier length = %d, want 43", len(verifier))
+	}
+	if got := codeChallengeS256(verifier); got != <-challengeCh {
+		t.Fatalf("code_challenge on auth URL does not match S256(verifier): %q", got)
+	}
+}
+
+// RFC 7636 Appendix B test vector.
+func TestCodeChallengeS256Vector(t *testing.T) {
+	const (
+		verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
+		want     = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
+	)
+	if got := codeChallengeS256(verifier); got != want {
+		t.Fatalf("codeChallengeS256 = %q, want %q", got, want)
+	}
 }
 
 func TestDeviceFlowRejectsStateMismatch(t *testing.T) {
@@ -72,7 +101,7 @@ func TestDeviceFlowRejectsStateMismatch(t *testing.T) {
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
-	if _, err := flow.Run(ctx); err == nil {
+	if _, _, err := flow.Run(ctx); err == nil {
 		t.Fatal("expected state-mismatch error, got nil")
 	}
 }

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)
+	code, verifier, err := flow.Run(ctx)
 	if err != nil {
 		return err
 	}
-	tokens, err := d.client.ExchangeDeviceCode(ctx, code)
+	tokens, err := d.client.ExchangeDeviceCode(ctx, code, verifier)
 	if err != nil {
 		return fmt.Errorf("token exchange failed: %w", err)
 	}

internal/cmd/completion.go

@@ -102,35 +102,6 @@ func completeKeyID(cmd *cobra.Command, args []string, toComplete string) ([]stri
 	return out, cobra.ShellCompDirectiveNoFileComp
 }
 
-// apiScopes are the permission scopes accepted by `keys create --scopes`,
-// mirroring the set documented in that command's help.
-var apiScopes = []string{
-	"shorten:create", "urls:read", "urls:manage", "stats:read",
-	"domains:read", "domains:manage", "admin:all",
-}
-
-// completeScopes completes the comma-separated --scopes value: it keeps the
-// scopes already typed and offers the rest. cobra hands the whole value as
-// toComplete for slice flags, so we split on the last comma ourselves and
-// use NoSpace so the user can keep appending.
-func completeScopes(_ *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-	prefix, cur := "", toComplete
-	if i := strings.LastIndex(toComplete, ","); i >= 0 {
-		prefix, cur = toComplete[:i+1], toComplete[i+1:]
-	}
-	chosen := make(map[string]bool)
-	for _, s := range strings.Split(prefix, ",") {
-		chosen[s] = true
-	}
-	var out []string
-	for _, s := range apiScopes {
-		if !chosen[s] && strings.HasPrefix(s, cur) {
-			out = append(out, prefix+s)
-		}
-	}
-	return out, cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp
-}
-
 // completeDomain completes --domain with the distinct custom domains that
 // already appear on your links. There's no domains-list endpoint, so this
 // is the practical best-effort source (a domain you've never used yet

internal/cmd/completion_test.go

@@ -110,27 +110,6 @@ func TestCompleteFixedFlags(t *testing.T) {
 	}
 }
 
-func TestCompleteScopesCommaAware(t *testing.T) {
-	pointDepsAt(t, "http://unused.invalid") // fixed list; no API call
-
-	// bare: offers the full scope set
-	out := complete(t, "keys", "create", "--scopes", "")
-	for _, want := range []string{"shorten:create", "stats:read", "admin:all"} {
-		if !strings.Contains(out, want) {
-			t.Fatalf("--scopes missing %q:\n%s", want, out)
-		}
-	}
-	// mid-list: keeps the typed prefix, drops the already-chosen scope,
-	// and narrows by the partial after the last comma
-	out = complete(t, "keys", "create", "--scopes", "shorten:create,sta")
-	if !strings.Contains(out, "shorten:create,stats:read") {
-		t.Fatalf("comma-aware completion should append stats:read:\n%s", out)
-	}
-	if strings.Contains(out, "shorten:create,shorten:create") {
-		t.Fatalf("already-chosen scope should not be re-offered:\n%s", out)
-	}
-}
-
 func TestCompleteDomainFromLinks(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.Write([]byte(`{"items":[