diff --git a/IMPLEMENTATION_EXACT.md b/IMPLEMENTATION_EXACT.md index 1a08c50..0ab8801 100644 --- a/IMPLEMENTATION_EXACT.md +++ b/IMPLEMENTATION_EXACT.md @@ -91,6 +91,8 @@ RETURNING *; **Verification**: Run `go build ./...` after adding this query to ensure sqlc generates the new function correctly. +**Database Schema Note**: This query uses existing fields (`devices.id`, `devices.auth_token`, `devices.updated_at`). **No schema change required** - the database structure remains the same. This is purely a new SQL query for existing schema. + --- ### 1.2 Middleware Enhancement @@ -103,7 +105,7 @@ RETURNING *; **Required Change**: Add fallback to check URL path parameter and query parameter -**REPLACE LINES 37-62** with: +**REPLACE LINES 37-270** with: ```go func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerFunc { @@ -123,8 +125,7 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF token := strings.TrimPrefix(authHeader, "Bearer ") device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), token) if err == nil { - // Found device via Bearer token, continue to validation - goto validateDevice + return m.validateDevice(c, device) } } @@ -134,8 +135,7 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF if urlToken != "" { device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), urlToken) if err == nil { - // Found device via URL path token, continue to validation - goto validateDevice + return m.validateDevice(c, device) } } @@ -145,8 +145,7 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF if queryToken != "" { device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), queryToken) if err == nil { - // Found device via query parameter token, continue to validation - goto validateDevice + return m.validateDevice(c, device) } } @@ -154,123 +153,68 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF return c.JSON(http.StatusUnauthorized, map[string]string{ "error": "authentication required - use Bearer token or API key", }) + } +} - validateDevice: -``` - -**Explanation**: -- `goto validateDevice` jumps to common validation code (label at line 221 in the complete function below) -- This is Go's idiomatic use of goto for error handling (allowed by PROJECT_GUIDELINES.md) -- Tries Bearer header first (for KOReader, API clients), then URL path (Kobo), then query param (OPDS) -- Each device type uses only one method: Kobo→URL path, KOReader→Bearer header -- The `validateDevice:` label is preserved in the unchanged section of the function (lines 221-270) - -**KEEP THE REST OF THE FUNCTION THE SAME** (lines 62-270 remain unchanged, including the `validateDevice:` label at line 221) - -**Complete Function After Changes** (reference for verification, shows how `goto validateDevice` connects to the label): - -```go -func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerFunc { - return func(c echo.Context) error { - var device database.Devices - var err error - - // Method 1: Try Bearer token header (KOReader, API clients, OPDS) - authHeader := c.Request().Header.Get("Authorization") - if authHeader != "" { - if !strings.HasPrefix(authHeader, "Bearer ") { - return c.JSON(http.StatusUnauthorized, map[string]string{ - "error": "invalid authorization header format", - }) - } - - token := strings.TrimPrefix(authHeader, "Bearer ") - device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), token) - if err == nil { - // Found device via Bearer token, continue to validation - goto validateDevice - } - } - - // Method 2: Try URL path parameter (Kobo sync, OPDS) - // Route format: /api/sync/kobo/:token/... - urlToken := c.Param("token") - if urlToken != "" { - device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), urlToken) - if err == nil { - // Found device via URL path token, continue to validation - goto validateDevice - } - } - - // Method 3: Try query parameter (OPDS catalog access) - // URL format: /opds/devices/:deviceId/catalog?token=... - queryToken := c.QueryParam("token") - if queryToken != "" { - device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), queryToken) - if err == nil { - // Found device via query parameter token, continue to validation - goto validateDevice - } - } - - // All authentication methods failed - return c.JSON(http.StatusUnauthorized, map[string]string{ - "error": "authentication required - use Bearer token or API key", +// validateDevice performs validation checks after successful authentication +func (m *DeviceAuthMiddleware) validateDevice(c echo.Context, device database.Devices) error { + if !device.SyncEnabled.Bool || !device.SyncEnabled.Valid { + return c.JSON(http.StatusForbidden, map[string]string{ + "error": "device sync is disabled", }) + } - validateDevice: - if !device.SyncEnabled.Bool || !device.SyncEnabled.Valid { - return c.JSON(http.StatusForbidden, map[string]string{ - "error": "device sync is disabled", - }) - } + requestType := m.getRequestType(c.Request().URL.Path) + deviceUUID := uuid.UUID(device.ID.Bytes) + deviceID := deviceUUID.String() - requestType := m.getRequestType(c.Request().URL.Path) - deviceUUID := uuid.UUID(device.ID.Bytes) - deviceID := deviceUUID.String() - - config := DeviceRateLimitConfig{ - SyncRequestsPerMinute: 60, - ProgressUpdatesPerMinute: 120, - MetadataRequestsPerMinute: 30, - } - - if !m.rateLimiter.CheckRateLimit(deviceID, requestType, config) { - remaining := m.rateLimiter.GetRemainingRequests(deviceID, requestType, config) - c.Response().Header().Set("X-RateLimit-Limit", "60") - c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) - c.Response().Header().Set("X-RateLimit-Reset", "60") - return c.JSON(http.StatusTooManyRequests, map[string]string{ - "error": "rate limit exceeded", - "message": "Too many requests", - "remaining": strconv.Itoa(remaining), - }) - } + config := DeviceRateLimitConfig{ + SyncRequestsPerMinute: 60, + ProgressUpdatesPerMinute: 120, + MetadataRequestsPerMinute: 30, + } + if !m.rateLimiter.CheckRateLimit(deviceID, requestType, config) { remaining := m.rateLimiter.GetRemainingRequests(deviceID, requestType, config) c.Response().Header().Set("X-RateLimit-Limit", "60") c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) - - ctx := DeviceContext{ - ID: device.ID.Bytes, - UserID: device.UserID.Bytes, - DeviceName: device.DeviceName, - DeviceType: device.DeviceType, - DeviceIdentifier: device.DeviceIdentifier, - SyncEnabled: device.SyncEnabled.Bool && device.SyncEnabled.Valid, - AutoSync: device.AutoSync.Bool && device.AutoSync.Valid, - } - - c.Set("device", device) - c.Set("device_ctx", ctx) - c.Set("device_id", device.ID.Bytes) - - return next(c) + c.Response().Header().Set("X-RateLimit-Reset", "60") + return c.JSON(http.StatusTooManyRequests, map[string]string{ + "error": "rate limit exceeded", + "message": "Too many requests", + "remaining": strconv.Itoa(remaining), + }) } + + remaining := m.rateLimiter.GetRemainingRequests(deviceID, requestType, config) + c.Response().Header().Set("X-RateLimit-Limit", "60") + c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) + + ctx := DeviceContext{ + ID: device.ID.Bytes, + UserID: device.UserID.Bytes, + DeviceName: device.DeviceName, + DeviceType: device.DeviceType, + DeviceIdentifier: device.DeviceIdentifier, + SyncEnabled: device.SyncEnabled.Bool && device.SyncEnabled.Valid, + AutoSync: device.AutoSync.Bool && device.AutoSync.Valid, + } + + c.Set("device", device) + c.Set("device_ctx", ctx) + c.Set("device_id", device.ID.Bytes) + + return next(c) } ``` +**Explanation**: +- Extracted validation logic into `validateDevice()` helper function +- Each auth method calls helper on success: `return m.validateDevice(c, device)` +- No goto statements - follows procedural style with clear control flow +- Helper function is testable, reusable, and maintainable +- Matches existing codebase patterns (no goto statements found) + **Verification Steps**: 1. Run `go build ./internal/middleware` 2. Run `go test ./internal/middleware/... -v` @@ -637,6 +581,13 @@ for _, device := range devices {

DEVICE SYNC CONFIGURATION

+ + +
+

+ ⚠️ Security Notice: This token is sensitive. Keep it secret. If compromised, regenerate immediately. +

+
if device.DeviceType == "kobo" { @@ -1523,101 +1474,120 @@ For remote sync, use **reverse proxy with HTTPS**: **File**: `cmd/server/tests/kobo_test.go` -**Location**: Update existing tests to use URL path token +**Location**: Add new test function for URL path authentication **Current Tests**: Use Bearer token in Authorization header -**Required Updates**: Add tests for URL path authentication +**Required Updates**: Add test for URL path authentication using table-driven pattern -**ADD THESE TESTS**: +**ADD THIS TEST**: ```go -func TestKoboSyncWithURLPathToken(t *testing.T) { - cfg := setupTestServer(t) - defer cfg.teardown() - - // Register and approve Kobo device - registrationResp := registerDevice(t, cfg, DeviceRegistration{ - DeviceName: "Test Kobo", - DeviceType: "kobo", - DeviceIdentifier: "N1234567890123", - }) - - approveResp := approveDevice(t, cfg, registrationResp.RegistrationID) - require.True(t, approveResp.Approved) - - // Extract auth token - authToken := approveResp.AuthToken - require.NotEmpty(t, authToken) - - // Test sync using URL path token (no Authorization header) - markupURL := fmt.Sprintf("%s/api/sync/kobo/%s/markup", cfg.server.URL, authToken) - syncData := map[string]interface{}{ - "ReadingSync": []map[string]interface{}{ - { - "ContentId": "test-book-uuid", - "PercentRead": 45.6, - "EntitlementId": "entitlement-123", - "RemainingTimeMinutes": 120, - }, - }, - "BookmarkSync": []interface{}{}, +func TestKoboAuthenticationMethods(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") } - body, _ := json.Marshal(syncData) - req := httptest.NewRequest("POST", markupURL, bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - // NOTE: No Authorization header - token in URL path + // ONE test setup shared across all subtests (single database pool) + setup := setupTestServer(t) + token := loginTestUser(t, setup.Server, setup.DB) + mediaItemID := createTestMediaItemID(t, setup.Server, token) - w := httptest.NewRecorder() - cfg.server.Echo.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) -} - -func TestKoboSyncRoutesSupportBearerToken(t *testing.T) { - cfg := setupTestServer(t) - defer cfg.teardown() - - // Register and approve device - registrationResp := registerDevice(t, cfg, DeviceRegistration{ - DeviceName: "Test Kobo", - DeviceType: "kobo", - DeviceIdentifier: "N1234567890123", - }) - - approveResp := approveDevice(t, cfg, registrationResp.RegistrationID) - require.True(t, approveResp.Approved) - - authToken := approveResp.AuthToken - - // Test that Kobo sync routes accept Bearer token for API clients/automation - // NOTE: Kobo devices will NEVER send Bearer tokens - they only use URL path - // This test verifies the middleware supports Bearer tokens for other clients - markupURL := fmt.Sprintf("%s/api/sync/kobo/%s/markup", cfg.server.URL, authToken) - syncData := map[string]interface{}{ - "ReadingSync": []map[string]interface{}{ - { - "ContentId": "test-book-uuid", - "PercentRead": 45.6, + // Define test cases + tests := []struct { + name string + authMethod string + setupDevice func(*testing.T, *TestServerSetup, string) *TestDeviceSetup + buildRequest func(*testing.T, *httptest.Server, string, string, *TestDeviceSetup) *http.Request + }{ + { + name: "URL Path Token (Kobo Firmware)", + authMethod: "URL path parameter", + setupDevice: func(t *testing.T, setup *TestServerSetup, mediaID string) *TestDeviceSetup { + deviceSetup := setupDeviceTest(t) + return deviceSetup.CreateDevice(t, "Test Kobo", "kobo", "kobo-url-path-test") + }, + buildRequest: func(t *testing.T, ts *httptest.Server, mediaID string, authToken string, device *TestDeviceSetup) *http.Request { + // Token in URL path: /api/sync/kobo/{token}/markup + markupURL := fmt.Sprintf("%s/api/sync/kobo/%s/markup", ts.URL, device.AuthToken) + syncData := map[string]interface{}{ + "ReadingSync": []map[string]interface{}{ + { + "ContentId": mediaID, + "PercentRead": 45.6, + "EntitlementId": "entitlement-123", + "RemainingTimeMinutes": 120, + "FirstReadTime": "2026-01-25T10:00:00Z", + "LastModified": "2026-01-30T20:00:00Z", + }, + }, + "BookmarkSync": []interface{}{}, + } + body, _ := json.Marshal(syncData) + req, _ := http.NewRequest("POST", markupURL, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + // NOTE: No Authorization header - token in URL path + return req + }, + }, + { + name: "Bearer Token (API Clients/Testing)", + authMethod: "Authorization header", + setupDevice: func(t *testing.T, setup *TestServerSetup, mediaID string) *TestDeviceSetup { + deviceSetup := setupDeviceTest(t) + return deviceSetup.CreateDevice(t, "Test Kobo API Client", "kobo", "kobo-bearer-test") + }, + buildRequest: func(t *testing.T, ts *httptest.Server, mediaID string, authToken string, device *TestDeviceSetup) *http.Request { + // Token in Authorization header (for API clients, not Kobo devices) + markupURL := fmt.Sprintf("%s/api/sync/kobo/%s/markup", ts.URL, device.AuthToken) + syncData := map[string]interface{}{ + "ReadingSync": []map[string]interface{}{ + { + "ContentId": mediaID, + "PercentRead": 45.6, + }, + }, + "BookmarkSync": []interface{}{}, + } + body, _ := json.Marshal(syncData) + req, _ := http.NewRequest("POST", markupURL, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", device.AuthToken)) + return req }, }, - "BookmarkSync": []interface{}{}, } - body, _ := json.Marshal(syncData) - req := httptest.NewRequest("POST", markupURL, bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", authToken)) + // Run all test cases sharing ONE database pool + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + koboDevice := tt.setupDevice(t, setup, mediaItemID) - w := httptest.NewRecorder() - cfg.server.Echo.ServeHTTP(w, req) + req := tt.buildRequest(t, setup.Server, mediaItemID, token, koboDevice) - assert.Equal(t, http.StatusOK, w.Code) + client := &http.Client{} + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode, "Should succeed with %s", tt.authMethod) + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + assert.Contains(t, result, "Status") + }) + } } ``` -**Verification**: Run `go test ./cmd/server/tests/... -v -run TestKobo` +**Explanation**: +- **ONE** call to `setupTestServer(t)` for entire test function +- Table-driven pattern with `t.Run()` subtests +- Each subtest shares the same database pool (1 connection max) +- Tests both authentication methods: URL path (Kobo) and Bearer (API clients) +- Follows existing test pattern in your codebase (see `TestKoboMarkupSync`) + +**Verification**: Run `go test ./cmd/server/tests/... -v -run TestKoboAuthenticationMethods` ---