From 1b2f6c1801e944e122427f6d79fdb725664afcff Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Wed, 11 Feb 2026 18:40:59 -0500 Subject: [PATCH] docs: add OPDS device authentication fix documentation --- fix-opds-device-authentication.md | 175 ++++++++++++++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100644 fix-opds-device-authentication.md diff --git a/fix-opds-device-authentication.md b/fix-opds-device-authentication.md new file mode 100644 index 0000000..3dd39a2 --- /dev/null +++ b/fix-opds-device-authentication.md @@ -0,0 +1,175 @@ +# Fix OPDS Device Authentication + +## Problem Summary + +**Current State:** +- OPDS routes (`/opds/devices/*`) are **public** - no authentication +- Device sync API (`/api/sync/kobo/*`, `/api/sync/koreader/*`) uses `DeviceAuthMiddleware` → validates `devices.auth_token` +- Anyone can enumerate/access device UUIDs without authentication +- Test `GetDeviceCatalog_WithoutDeviceAuth` expects **401** (auth required) but gets **404** (device not found) + +**Design Principle:** Devices belong to users; OPDS access should require device authentication via `devices.auth_token` (same model as sync API). + +--- + +## Step-by-Step Implementation Plan + +### Step 1: Apply `DeviceAuthMiddleware` to OPDS Routes + +**File:** `internal/router/opds.go` + +**Changes:** +```go +func registerOPDSRoutes(cfg *Config) { + e := cfg.Echo + + // Apply device authentication middleware + opds := e.Group("/opds/devices") + opds.Use(cfg.DeviceAuthMiddleware.Authenticate) // ← ADD THIS + + // OPDS routes now require valid devices.auth_token + opds.GET("/:deviceId/catalog", cfg.OPDSHandler.GetDeviceCatalog) + opds.GET("/:deviceId/search", cfg.OPDSHandler.SearchDeviceCatalog) + opds.GET("/:deviceId/nav", cfg.OPDSHandler.GetDeviceNavigation) + opds.GET("/:deviceId/download/:bookId", cfg.OPDSHandler.DownloadBook) + opds.GET("/:deviceId/cover/:bookId", cfg.OPDSHandler.GetCoverImage) + opds.GET("/:deviceId/formats/:bookId", cfg.OPDSHandler.ListFormats) +} +``` + +**Rationale:** +- Reuses existing authentication pattern (proven on sync endpoints) +- Middleware validates `devices.auth_token` from `Authorization: Bearer {token}` header +- Returns 401 if missing/invalid/expired token +- Device must belong to user (already enforced in device table) + +--- + +### Step 2: Update OPDS Handler Comments + +**File:** `internal/router/opds.go` + +**Change:** +```go +// Register OPDS routes with device authentication +// Devices must use their devices.auth_token (generated during device registration/approval) +// Kobo devices store this token for both sync and OPDS catalog access +// Returns 401 Unauthorized if device token is missing, invalid, or device sync is disabled +func registerOPDSRoutes(cfg *Config) { + e := cfg.Echo + + // Require device authentication for all OPDS endpoints + opds := e.Group("/opds/devices") + opds.Use(cfg.DeviceAuthMiddleware.Authenticate) + ... +} +``` + +**Remove misleading comment:** +```go +- // OPDS routes (public - device authentication optional) +- // Note: OPDSHandler implements its own device authentication +``` + +--- + +### Step 3: Update Test Expectations + +**File:** `cmd/server/tests/opds_test.go` + +**Change 1 - Remove invalid 200 option:** +```go +// Line 18-29: GetDeviceCatalog_WithoutDeviceAuth +t.Run("GetDeviceCatalog_WithoutDeviceAuth", func(t *testing.T) { + deviceID := uuid.New() + httpReq, _ := http.NewRequest("GET", setup.Server.URL+"/opds/devices/"+deviceID.String()+"/catalog", nil) + + resp, err := client.Do(httpReq) + require.NoError(t, err) + defer resp.Body.Close() + + // OPDS endpoints require device authentication + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) // ← Expect 401 only +}) +``` + +**Change 2 - Update comment:** +```go +// Test: OPDS endpoints require device authentication via devices.auth_token +``` + +--- + +### Step 4: Update Additional OPDS Edge Case Test + +**File:** `cmd/server/tests/opds_test.go` + +**Change:** Update `TestOPDSEdgeCases/Search_SpecialCharacters` to use auth token: +```go +t.Run("Search_SpecialCharacters", func(t *testing.T) { + deviceID := uuid.New() + + // Search with special characters (requires valid device auth token) + httpReq, _ := http.NewRequest("GET", setup.Server.URL+"/opds/devices/"+deviceID.String()+"/search?q=test%20%26%20more", nil) + httpReq.Header.Set("Authorization", "Bearer "+setup.DeviceToken) // ← ADD THIS +``` + +--- + +### Step 5: Update Router Comment + +**File:** `internal/router/opds.go` + +**Add documentation:** +```go +// Register OPDS routes with device authentication +// Devices must use their devices.auth_token (generated during device registration/approval) +// Kobo devices store this token for both sync and OPDS catalog access +// Returns 401 Unauthorized if device token is missing, invalid, or device sync is disabled +func registerOPDSRoutes(cfg *Config) { + e := cfg.Echo + + // Require device authentication for all OPDS endpoints + opds := e.Group("/opds/devices") + opds.Use(cfg.DeviceAuthMiddleware.Authenticate) + ... +} +``` + +--- + +## Test Verification Plan + +After implementing changes, verify: + +1. **`GetDeviceCatalog_WithoutDeviceAuth`** - Should return **401** (no auth token provided) +2. **`GetDeviceCatalog_ValidDevice`** - With `setup.DeviceToken` should return **200** (valid device) +3. **`SearchDeviceCatalog_ValidDevice`** - With device token should return **200** or **404** (no results) +4. **`TestOPDSEdgeCases`** - All should use `setup.DeviceToken` for authentication + +--- + +## Git Commit Structure + +**Commit 1:** Apply DeviceAuthMiddleware to OPDS routes +``` +fix(opds): Require device authentication for OPDS catalog endpoints + +- Apply DeviceAuthMiddleware.Authenticate to /opds/devices/* routes +- OPDS now uses same authentication model as sync API (devices.auth_token) +- Removes security vulnerability allowing unauthorized device enumeration +- Updates router comments to clarify authentication requirements + +Refs: #PROJECT_GUIDELINES.md +``` + +**Commit 2:** Update OPDS test expectations +``` +test(opds): Update test expectations for device authentication + +- GetDeviceCatalog_WithoutDeviceAuth: Expect 401 (auth required) +- TestOPDSEdgeCases: Use setup.DeviceToken for authenticated requests +- Remove misleading "public or 401" test expectations + +Refs: #PROJECT_GUIDELINES.md +```