Bookhoard/bookhoard
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 12 Wiki Activity

docs: refactor middleware authentication to use helper function pattern

Browse Source 
- Extract device validation logic into separate validateDevice() method
- Replace goto statements with direct function returns for clearer control flow
- Add security warning banner in device configuration UI
- Refactor Kobo tests to use table-driven pattern
- Add database schema note clarifying no schema changes required
- Improve code maintainability and testability
This commit is contained in:
john-okeefe
2026-02-12 20:35:20 -05:00
parent fa09850611
commit 5644e15eda
1 changed files with 159 additions and 189 deletions
Download Patch File Download Diff File Expand all files Collapse all files
IMPLEMENTATION_EXACT.md
+159 -189
View File
@@ -91,6 +91,8 @@ RETURNING *;
**Verification**: Run `go build ./...` after adding this query to ensure sqlc generates the new function correctly. **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 ### 1.2 Middleware Enhancement
@@ -103,7 +105,7 @@ RETURNING *;
**Required Change**: Add fallback to check URL path parameter and query parameter **Required Change**: Add fallback to check URL path parameter and query parameter
**REPLACE LINES 37-62** with: **REPLACE LINES 37-270** with:
```go ```go
func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerFunc { 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 ") token := strings.TrimPrefix(authHeader, "Bearer ")
device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), token) device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), token)
if err == nil { if err == nil {
// Found device via Bearer token, continue to validation return m.validateDevice(c, device)
goto validateDevice
} }
} }
@@ -134,8 +135,7 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF
if urlToken != "" { if urlToken != "" {
device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), urlToken) device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), urlToken)
if err == nil { if err == nil {
// Found device via URL path token, continue to validation return m.validateDevice(c, device)
goto validateDevice
} }
} }
@@ -145,8 +145,7 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF
if queryToken != "" { if queryToken != "" {
device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), queryToken) device, err = m.db.GetDeviceByAuthToken(c.Request().Context(), queryToken)
if err == nil { if err == nil {
// Found device via query parameter token, continue to validation return m.validateDevice(c, device)
goto validateDevice
} }
} }
@@ -154,123 +153,68 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF
return c.JSON(http.StatusUnauthorized, map[string]string{ return c.JSON(http.StatusUnauthorized, map[string]string{
"error": "authentication required - use Bearer token or API key", "error": "authentication required - use Bearer token or API key",
}) })
}
}
validateDevice: // 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 {
**Explanation**: return c.JSON(http.StatusForbidden, map[string]string{
- `goto validateDevice` jumps to common validation code (label at line 221 in the complete function below) "error": "device sync is disabled",
- 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: requestType := m.getRequestType(c.Request().URL.Path)
if !device.SyncEnabled.Bool || !device.SyncEnabled.Valid { deviceUUID := uuid.UUID(device.ID.Bytes)
return c.JSON(http.StatusForbidden, map[string]string{ deviceID := deviceUUID.String()
"error": "device sync is disabled",
})
}
requestType := m.getRequestType(c.Request().URL.Path) config := DeviceRateLimitConfig{
deviceUUID := uuid.UUID(device.ID.Bytes) SyncRequestsPerMinute: 60,
deviceID := deviceUUID.String() ProgressUpdatesPerMinute: 120,
MetadataRequestsPerMinute: 30,
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),
})
}
if !m.rateLimiter.CheckRateLimit(deviceID, requestType, config) {
remaining := m.rateLimiter.GetRemainingRequests(deviceID, requestType, config) remaining := m.rateLimiter.GetRemainingRequests(deviceID, requestType, config)
c.Response().Header().Set("X-RateLimit-Limit", "60") c.Response().Header().Set("X-RateLimit-Limit", "60")
c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining))
c.Response().Header().Set("X-RateLimit-Reset", "60")
ctx := DeviceContext{ return c.JSON(http.StatusTooManyRequests, map[string]string{
ID: device.ID.Bytes, "error": "rate limit exceeded",
UserID: device.UserID.Bytes, "message": "Too many requests",
DeviceName: device.DeviceName, "remaining": strconv.Itoa(remaining),
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)
} }
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**: **Verification Steps**:
1. Run `go build ./internal/middleware` 1. Run `go build ./internal/middleware`
2. Run `go test ./internal/middleware/... -v` 2. Run `go test ./internal/middleware/... -v`
@@ -637,6 +581,13 @@ for _, device := range devices {
<!-- NEW: Sync URL & Token Management --> <!-- NEW: Sync URL & Token Management -->
<div class="border-t pt-4" style="border-color: var(--border);"> <div class="border-t pt-4" style="border-color: var(--border);">
<p class="text-xs font-semibold mb-2" style="color: var(--text-secondary)">DEVICE SYNC CONFIGURATION</p> <p class="text-xs font-semibold mb-2" style="color: var(--text-secondary)">DEVICE SYNC CONFIGURATION</p>
<!-- Security Warning Banner -->
<div class="mb-3 p-2 rounded" style="background-color: var(--bg-tertiary); border-left: 3px solid var(--accent);">
<p class="text-xs" style="color: var(--text-primary);">
<span class="font-semibold">⚠️ Security Notice:</span> This token is sensitive. Keep it secret. If compromised, regenerate immediately.
</p>
</div>
if device.DeviceType == "kobo" { if device.DeviceType == "kobo" {
<!-- Kobo: Copy Full Sync URL --> <!-- Kobo: Copy Full Sync URL -->
@@ -1523,101 +1474,120 @@ For remote sync, use **reverse proxy with HTTPS**:
**File**: `cmd/server/tests/kobo_test.go` **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 **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 ```go
func TestKoboSyncWithURLPathToken(t *testing.T) { func TestKoboAuthenticationMethods(t *testing.T) {
cfg := setupTestServer(t) if testing.Short() {
defer cfg.teardown() t.Skip("Skipping integration test in short mode")
// 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{}{},
} }
body, _ := json.Marshal(syncData) // ONE test setup shared across all subtests (single database pool)
req := httptest.NewRequest("POST", markupURL, bytes.NewReader(body)) setup := setupTestServer(t)
req.Header.Set("Content-Type", "application/json") token := loginTestUser(t, setup.Server, setup.DB)
// NOTE: No Authorization header - token in URL path mediaItemID := createTestMediaItemID(t, setup.Server, token)
w := httptest.NewRecorder() // Define test cases
cfg.server.Echo.ServeHTTP(w, req) tests := []struct {
name string
assert.Equal(t, http.StatusOK, w.Code) authMethod string
} setupDevice func(*testing.T, *TestServerSetup, string) *TestDeviceSetup
buildRequest func(*testing.T, *httptest.Server, string, string, *TestDeviceSetup) *http.Request
func TestKoboSyncRoutesSupportBearerToken(t *testing.T) { }{
cfg := setupTestServer(t) {
defer cfg.teardown() name: "URL Path Token (Kobo Firmware)",
authMethod: "URL path parameter",
// Register and approve device setupDevice: func(t *testing.T, setup *TestServerSetup, mediaID string) *TestDeviceSetup {
registrationResp := registerDevice(t, cfg, DeviceRegistration{ deviceSetup := setupDeviceTest(t)
DeviceName: "Test Kobo", return deviceSetup.CreateDevice(t, "Test Kobo", "kobo", "kobo-url-path-test")
DeviceType: "kobo", },
DeviceIdentifier: "N1234567890123", 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)
approveResp := approveDevice(t, cfg, registrationResp.RegistrationID) syncData := map[string]interface{}{
require.True(t, approveResp.Approved) "ReadingSync": []map[string]interface{}{
{
authToken := approveResp.AuthToken "ContentId": mediaID,
"PercentRead": 45.6,
// Test that Kobo sync routes accept Bearer token for API clients/automation "EntitlementId": "entitlement-123",
// NOTE: Kobo devices will NEVER send Bearer tokens - they only use URL path "RemainingTimeMinutes": 120,
// This test verifies the middleware supports Bearer tokens for other clients "FirstReadTime": "2026-01-25T10:00:00Z",
markupURL := fmt.Sprintf("%s/api/sync/kobo/%s/markup", cfg.server.URL, authToken) "LastModified": "2026-01-30T20:00:00Z",
syncData := map[string]interface{}{ },
"ReadingSync": []map[string]interface{}{ },
{ "BookmarkSync": []interface{}{},
"ContentId": "test-book-uuid", }
"PercentRead": 45.6, 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) // Run all test cases sharing ONE database pool
req := httptest.NewRequest("POST", markupURL, bytes.NewReader(body)) for _, tt := range tests {
req.Header.Set("Content-Type", "application/json") t.Run(tt.name, func(t *testing.T) {
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", authToken)) koboDevice := tt.setupDevice(t, setup, mediaItemID)
w := httptest.NewRecorder() req := tt.buildRequest(t, setup.Server, mediaItemID, token, koboDevice)
cfg.server.Echo.ServeHTTP(w, req)
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`
--- ---