From 3e98fadb9f669877f49b907d11f81e64524400f7 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Mon, 2 Nov 2026 16:11:25 -0500 Subject: [PATCH] test(devices): integration coverage for the pairing-code flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the reject-route test (behavior removed) with coverage of the replacement flow, run against the shared dev DB per the doctrine: fresh per-run regular user for deterministic claims, uuid-suffixed identifiers so UNIQUE(device_identifier) stays clear across runs, and device rows cascade away with the fresh user — testuser is untouched. - TestPairingClaimHappyPath: lookup normalizes spaced digits, does not bind; claim binds to the claimer only; the device's status poll then delivers a token matching the stored row. - TestPairingWrongCodeIsGenericAndNonDestructive: a miss answers the generic invalid-code error and leaves the real code claimable. - TestPairingClaimBindsClaimerOnly: the admin account has no device row for another user's claim, and the used code answers 409. - TestPairingReRegisterRetiresPriorCode: the old code and its registration UUID both die; the fresh code claims. - TestPairingCancelByRegistrationUUID: cancel is idempotent, kills only its own registration (code and poll), and leaves a sibling registration untouched. - TestPairingAttemptRateLimit: 13th lookup/claim attempt in a minute is refused with 429 (uses the fresh regular user — the limiter is process-global, so sibling tests sharing a token would otherwise pre-spend the budget). - TestListPendingRegistrations now pins the invisibility invariant: a live registration appears in no account's pending list, total 0. - TestRejectDeviceRegistrationRemoved pins the reject route's removal (404/405). --- cmd/server/tests/device_pairing_test.go | 265 ++++++++++++++++++++++++ cmd/server/tests/device_test.go | 47 +++-- 2 files changed, 298 insertions(+), 14 deletions(-) create mode 100644 cmd/server/tests/device_pairing_test.go diff --git a/cmd/server/tests/device_pairing_test.go b/cmd/server/tests/device_pairing_test.go new file mode 100644 index 00000000..bfa1afb3 --- /dev/null +++ b/cmd/server/tests/device_pairing_test.go @@ -0,0 +1,265 @@ +package main + +// Pairing-code flow integration tests. The register response mints a +// 6-digit code shown on the registering device's screen; the person +// holding the device types it into THEIR account (POST /api/devices/claim). +// This replaces the retired broadcast-list approve, where +// first-click-wins let any account claim (or reject) a device it could +// see but not name. +// +// Devices claimed here land on the fresh per-run regular user, so the +// harness cascade removes them; identifiers are uuid-suffixed to stay +// clear of the UNIQUE(device_identifier) index across runs. + +import ( + "bookhoard/internal/database" + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// registerPairingDevice initiates a registration and returns its decoded +// response (registration_id, pairing_code, ...). +func registerPairingDevice(t *testing.T, setup *TestServerSetup, identifier string) map[string]interface{} { + t.Helper() + regRequest := map[string]interface{}{ + "device_name": "Test Pairing Device", + "device_type": "koreader", + "device_identifier": identifier, + } + regBody, _ := json.Marshal(regRequest) + req := httptest.NewRequest("POST", "/api/devices/register", bytes.NewReader(regBody)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + setup.Server.Config.Handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusCreated, rec.Code, "Should initiate registration") + + var regResponse map[string]interface{} + json.Unmarshal(rec.Body.Bytes(), ®Response) + return regResponse +} + +// pollRegistrationStatus hits the device-side status endpoint (no auth — +// the registration UUID is the bearer). +func pollRegistrationStatus(t *testing.T, setup *TestServerSetup, registrationID string) (int, map[string]interface{}) { + t.Helper() + body, _ := json.Marshal(map[string]string{"registration_id": registrationID}) + req := httptest.NewRequest("POST", "/api/devices/register/status", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + setup.Server.Config.Handler.ServeHTTP(rec, req) + var resp map[string]interface{} + json.Unmarshal(rec.Body.Bytes(), &resp) + return rec.Code, resp +} + +func pairingCodeBody(code string) []byte { + body, _ := json.Marshal(map[string]string{"pairing_code": code}) + return body +} + +// pairingLookupOrClaim POSTs to the given claim-family endpoint with the +// given bearer token and returns status + decoded body. +func pairingLookupOrClaim(t *testing.T, setup *TestServerSetup, path, token, code string) (int, map[string]interface{}) { + t.Helper() + req := httptest.NewRequest("POST", path, bytes.NewReader(pairingCodeBody(code))) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + rec := httptest.NewRecorder() + setup.Server.Config.Handler.ServeHTTP(rec, req) + var resp map[string]interface{} + json.Unmarshal(rec.Body.Bytes(), &resp) + return rec.Code, resp +} + +// wrongPairingCode derives a code guaranteed to differ from the target's +// digits (rotate each digit +1 mod 10). +func wrongPairingCode(code string) string { + rotated := make([]byte, len(code)) + for i := 0; i < len(code); i++ { + rotated[i] = byte('0' + (int(code[i]-'0')+1)%10) + } + return string(rotated) +} + +func TestPairingClaimHappyPath(t *testing.T) { + setup := setupTestServer(t) + identifier := fmt.Sprintf("pairing-happy-%s", uuid.New().String()) + + reg := registerPairingDevice(t, setup, identifier) + code, ok := reg["pairing_code"].(string) + require.True(t, ok, "register response must carry pairing_code") + require.Len(t, code, 6, "pairing code must be 6 digits") + for _, d := range code { + require.GreaterOrEqual(t, d, '0') + require.LessOrEqual(t, d, '9') + } + + // Lookup first — the confirm card's data, without binding. + status, resp := pairingLookupOrClaim(t, setup, "/api/devices/claim/lookup", setup.RegularToken, " "+code[:3]+" "+code[3:]) + require.Equal(t, http.StatusOK, status, "lookup should resolve the live code (spaces normalized)") + assert.Equal(t, "Test Pairing Device", resp["device_name"]) + assert.NotEmpty(t, resp["expires_in"]) + + // Still pending — lookup must not have bound anything. + pollStatus, pollResp := pollRegistrationStatus(t, setup, reg["registration_id"].(string)) + require.Equal(t, http.StatusOK, pollStatus) + assert.Equal(t, "pending", pollResp["status"]) + + // Claim as the fresh regular user (deterministic account per doctrine). + status, resp = pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code) + require.Equal(t, http.StatusOK, status, "claim should bind the device") + assert.Equal(t, true, resp["approved"]) + + // The device's own poll now delivers credentials. + pollStatus, pollResp = pollRegistrationStatus(t, setup, reg["registration_id"].(string)) + require.Equal(t, http.StatusOK, pollStatus) + assert.Equal(t, "approved", pollResp["status"]) + token, ok := pollResp["auth_token"].(string) + require.True(t, ok, "approved poll must deliver the auth token") + require.NotEmpty(t, token) + + // The device row belongs to the claimer, and the poll-delivered token + // matches the stored row. + devices, err := setup.DB.ListDevicesByUser(context.Background(), + pgtype.UUID{Bytes: [16]byte(getRegularUserID(t, setup.DB)), Valid: true}) + require.NoError(t, err) + var found *database.Devices + for i := range devices { + if devices[i].DeviceIdentifier == identifier { + found = &devices[i] + } + } + require.NotNil(t, found, "device row must exist under the claimer") + assert.Equal(t, token, found.AuthToken, "poll-delivered token must match the stored row") +} + +func TestPairingWrongCodeIsGenericAndNonDestructive(t *testing.T) { + setup := setupTestServer(t) + reg := registerPairingDevice(t, setup, fmt.Sprintf("pairing-miss-%s", uuid.New().String())) + code := reg["pairing_code"].(string) + + // A wrong code fails with the generic message — no hint whether any + // registration is waiting. + status, resp := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, wrongPairingCode(code)) + require.Equal(t, http.StatusBadRequest, status) + assert.Equal(t, "invalid pairing code", resp["error"]) + + // The miss must not have killed or claimed the real registration. + status, resp = pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code) + require.Equal(t, http.StatusOK, status, "the correct code must still claim after a miss") + assert.Equal(t, true, resp["approved"]) +} + +func TestPairingClaimBindsClaimerOnly(t *testing.T) { + setup := setupTestServer(t) + identifier := fmt.Sprintf("pairing-scope-%s", uuid.New().String()) + + reg := registerPairingDevice(t, setup, identifier) + code := reg["pairing_code"].(string) + + // The regular user claims it. + status, _ := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code) + require.Equal(t, http.StatusOK, status) + + // The admin (a different account) has no such device — and cannot + // re-claim the now-used code either. + adminDevices, err := setup.DB.ListDevicesByUser(context.Background(), + pgtype.UUID{Bytes: [16]byte(getTestUserID(t, setup.DB)), Valid: true}) + require.NoError(t, err) + for _, d := range adminDevices { + assert.NotEqual(t, identifier, d.DeviceIdentifier, + "device must be bound to the claimer, not to every account") + } + + status, resp := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.Token, code) + assert.Equal(t, http.StatusConflict, status, "an already-used code must not bind again") + assert.Contains(t, resp["error"], "already used") +} + +func TestPairingReRegisterRetiresPriorCode(t *testing.T) { + setup := setupTestServer(t) + identifier := fmt.Sprintf("pairing-retry-%s", uuid.New().String()) + + reg1 := registerPairingDevice(t, setup, identifier) + reg2 := registerPairingDevice(t, setup, identifier) + + code1, ok := reg1["pairing_code"].(string) + require.True(t, ok) + code2, ok := reg2["pairing_code"].(string) + require.True(t, ok) + require.NotEqual(t, code1, code2, "each registration mints its own code") + + // The retired attempt is gone from both the code and UUID channels. + status, _ := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code1) + assert.Equal(t, http.StatusBadRequest, status, "retired code must not claim") + pollStatus, _ := pollRegistrationStatus(t, setup, reg1["registration_id"].(string)) + assert.Equal(t, http.StatusNotFound, pollStatus, "retired registration must not poll") + + // The fresh code claims fine. + status, resp := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code2) + require.Equal(t, http.StatusOK, status) + assert.Equal(t, true, resp["approved"]) +} + +func TestPairingCancelByRegistrationUUID(t *testing.T) { + setup := setupTestServer(t) + cancelled := registerPairingDevice(t, setup, fmt.Sprintf("pairing-cancel-%s", uuid.New().String())) + kept := registerPairingDevice(t, setup, fmt.Sprintf("pairing-keep-%s", uuid.New().String())) + + // Cancel only the first attempt — bearer is its own registration UUID. + body, _ := json.Marshal(map[string]string{"registration_id": cancelled["registration_id"].(string)}) + req := httptest.NewRequest("POST", "/api/devices/register/cancel", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + setup.Server.Config.Handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code, "cancel should succeed") + + // Cancelled: code dead, status poll gone. + code1 := cancelled["pairing_code"].(string) + status, _ := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code1) + assert.Equal(t, http.StatusBadRequest, status, "cancelled code must not claim") + pollStatus, _ := pollRegistrationStatus(t, setup, cancelled["registration_id"].(string)) + assert.Equal(t, http.StatusNotFound, pollStatus, "cancelled registration must not poll") + + // Idempotent: cancelling again is still a 200. + req = httptest.NewRequest("POST", "/api/devices/register/cancel", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec = httptest.NewRecorder() + setup.Server.Config.Handler.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code, "cancel must be idempotent") + + // The other registration is untouched by its neighbour's cancel. + code2 := kept["pairing_code"].(string) + status, resp := pairingLookupOrClaim(t, setup, "/api/devices/claim", setup.RegularToken, code2) + require.Equal(t, http.StatusOK, status, "cancel must scope to its own registration only") + assert.Equal(t, true, resp["approved"]) +} + +func TestPairingAttemptRateLimit(t *testing.T) { + setup := setupTestServer(t) + + // The regular user is fresh per setup, so the limiter key + // (userID|ip) is this test's alone — the budget cannot have been + // pre-spent by sibling tests sharing the process-global limiter. + // Burn the budget with bad lookups (12/min); the 13th attempt must + // be refused before the code check. + for i := 0; i < 12; i++ { + code := fmt.Sprintf("%06d", i*7919+13) + status, _ := pairingLookupOrClaim(t, setup, "/api/devices/claim/lookup", setup.RegularToken, code) + require.Equal(t, http.StatusBadRequest, status, "attempt %d should fail as a plain miss", i+1) + } + + status, resp := pairingLookupOrClaim(t, setup, "/api/devices/claim/lookup", setup.RegularToken, "999999") + assert.Equal(t, http.StatusTooManyRequests, status, "13th attempt must hit the budget") + assert.Contains(t, resp["error"], "too many") +} diff --git a/cmd/server/tests/device_test.go b/cmd/server/tests/device_test.go index 481c630b..f251429f 100644 --- a/cmd/server/tests/device_test.go +++ b/cmd/server/tests/device_test.go @@ -237,10 +237,25 @@ func TestDeviceAuthentication(t *testing.T) { func TestListPendingRegistrations(t *testing.T) { setup := setupTestServer(t) - req := httptest.NewRequest("GET", "/api/devices/pending", nil) - req.Header.Set("Authorization", "Bearer "+setup.Token) + // A live registration must NOT appear in any account's pending list: + // the broadcast is what let any user claim (or reject) someone else's + // device. Pairing codes replaced it — see device_pairing_test.go. + regRequest := map[string]interface{}{ + "device_name": "Broadcast Test Device", + "device_type": "koreader", + "device_identifier": fmt.Sprintf("broadcast-test-%s", uuid.New().String()), + } + regBody, _ := json.Marshal(regRequest) + req := httptest.NewRequest("POST", "/api/devices/register", bytes.NewReader(regBody)) + req.Header.Set("Content-Type", "application/json") rec := httptest.NewRecorder() setup.Server.Config.Handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusCreated, rec.Code, "Should initiate registration") + + req = httptest.NewRequest("GET", "/api/devices/pending", nil) + req.Header.Set("Authorization", "Bearer "+setup.Token) + rec = httptest.NewRecorder() + setup.Server.Config.Handler.ServeHTTP(rec, req) assert.Equal(t, http.StatusOK, rec.Code, "Should list pending registrations") @@ -248,8 +263,9 @@ func TestListPendingRegistrations(t *testing.T) { json.Unmarshal(rec.Body.Bytes(), &response) pending, ok := response["pending_registrations"].([]interface{}) - assert.True(t, ok, "Should have pending_registrations array") - assert.NotNil(t, pending, "Pending registrations should not be nil") + require.True(t, ok, "Should have pending_registrations array") + assert.Empty(t, pending, "Unclaimed registrations must be invisible to every account") + assert.Equal(t, float64(0), response["total"], "Total must stay zero while a registration is live") } // Regression test for the "approve returns 500 after app reinstall" bug: @@ -370,13 +386,18 @@ func TestApproveDeviceRegistration(t *testing.T) { assert.True(t, approveResponse["approved"].(bool), "Should confirm approval") } -func TestRejectDeviceRegistration(t *testing.T) { +// TestRejectDeviceRegistrationRemoved pins the removal: the reject route is +// gone because unclaimed registrations are invisible since pairing codes +// took over — there is nothing left for anyone to reject. See +// device_pairing_test.go for the replacement flow (cancel by the +// registering device itself). +func TestRejectDeviceRegistrationRemoved(t *testing.T) { setup := setupTestServer(t) regRequest := map[string]interface{}{ "device_name": "Test Device for Rejection", "device_type": "koreader", - "device_identifier": "test-rejection-12345", + "device_identifier": fmt.Sprintf("test-rejection-%s", uuid.New().String()), } regBody, _ := json.Marshal(regRequest) @@ -385,25 +406,23 @@ func TestRejectDeviceRegistration(t *testing.T) { rec := httptest.NewRecorder() setup.Server.Config.Handler.ServeHTTP(rec, req) - assert.Equal(t, http.StatusCreated, rec.Code, "Should initiate device registration") + require.Equal(t, http.StatusCreated, rec.Code, "Should initiate device registration") var regResponse map[string]interface{} json.Unmarshal(rec.Body.Bytes(), ®Response) registrationID, ok := regResponse["registration_id"].(string) - assert.True(t, ok, "Should have registration_id") + require.True(t, ok, "Should have registration_id") req = httptest.NewRequest("POST", fmt.Sprintf("/api/devices/reject/%s", registrationID), nil) req.Header.Set("Authorization", "Bearer "+setup.Token) rec = httptest.NewRecorder() setup.Server.Config.Handler.ServeHTTP(rec, req) - assert.Equal(t, http.StatusOK, rec.Code, "Should reject device registration") - - var rejectResponse map[string]interface{} - json.Unmarshal(rec.Body.Bytes(), &rejectResponse) - - assert.Equal(t, "device registration rejected", rejectResponse["message"], "Should confirm rejection message") + // 404 = no route, 405 = path-shaped collision without POST — either + // proves the reject surface is gone. + assert.True(t, rec.Code == http.StatusNotFound || rec.Code == http.StatusMethodNotAllowed, + "Reject route must not exist (registration invisible, nothing to reject); got %d", rec.Code) } func TestRegenerateDeviceToken_Success(t *testing.T) {