docs: add OPDS device authentication fix documentation
This commit is contained in:
@@ -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
|
||||
```
|
||||
Reference in New Issue
Block a user