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

fix: OPDS base_url placeholder bug + setup gate requires base_url

Browse Source 
Three bugs fixed:

1. Schema seeded base_url with fake placeholder 'bookhoard.example.com'.
   Removed seed; startup now seeds from BASE_URL env var only if DB row
   is empty (admin changes persist across restarts). One-time UPDATE
   clears the placeholder in existing installs.

2. config.GetBaseURL() had a broken type assertion (local SystemConfigRow
   vs database.SystemConfig) that always failed, returning . Admin panel
   showed env var fallback instead of actual DB value. Fixed with a
   function-type getter that properly wraps the DB query.

3. OPDS handler read base_url only from DB with no fallback. When DB had
   the placeholder, all feed links pointed to an unreachable domain,
   breaking KOReader search/download. Added deriveBaseURL() helper that
   falls back to the request Host/scheme when DB value is empty.

Setup gate improvements:
- isSetupComplete now requires both admin user AND non-empty base_url
- Setup middleware no longer exempts all /api/ routes; only allows
  /api/auth/register, /api/auth/login, /api/system/config before setup
  is complete. All other API routes get 503.
- Cache invalidated when base_url is saved via admin settings

Dev workflow:
- New bruno/NewDevDBSetup/SetBaseUrl.yml for dev DB setup
- NewDB.sh runs SetBaseUrl between RegisterUser and CreateEbookLibrary
This commit is contained in:
john-okeefe
2026-08-06 13:02:35 -04:00
parent 8e2c1a4b3a
commit 4716790564
12 changed files with 234 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/config/config.go
+8 -19
View File
@@ -45,30 +45,19 @@ func (c *Config) DatabaseURL() string {
c.DatabaseUser, c.DatabasePassword, c.DatabaseHost, c.DatabasePort, c.DatabaseName)
}
// GetBaseURL returns the base URL from system configuration database with fallback to config/env var
func GetBaseURL(ctx context.Context, db interface{}) string {
// Try to get from database first
type SystemConfigQuerier interface {
GetSystemConfig(ctx context.Context, key string) (SystemConfigRow, error)
}
// SystemConfigGetter returns the value for a system config key, or an error.
type SystemConfigGetter func(ctx context.Context, key string) (string, error)
if querier, ok := db.(SystemConfigQuerier); ok {
config, err := querier.GetSystemConfig(ctx, "base_url")
if err == nil && config.Value != "" {
return config.Value
}
// GetBaseURL returns the base URL from system configuration database, or empty
// string if not set. The getter abstraction avoids importing the database package.
func GetBaseURL(ctx context.Context, getter SystemConfigGetter) string {
val, err := getter(ctx, "base_url")
if err == nil && val != "" {
return val
}
// Fallback: return empty string - caller should use their own fallback
return ""
}
// SystemConfigRow represents a system configuration row
type SystemConfigRow struct {
Key string
Value string
}
func getEnv(key, defaultValue string) string {
if value := os.Getenv(key); value != "" {
return value
internal/handlers/opds.go
+7 -7
View File
@@ -38,15 +38,15 @@ func NewOPDSHandler(db *database.Queries, libraryService *services.LibraryServic
}
}
// Helper function to get base URL from system config
// Helper function to get base URL from system config with request-derived fallback
func (h *OPDSHandler) getBaseURLs(c *echo.Context) (string, string, error) {
baseURL, err := h.db.GetSystemConfig(c.Request().Context(), "base_url")
if err != nil {
return "", "", fmt.Errorf("failed to get base_url from config: %w", err)
var dbBaseURL string
if config, err := h.db.GetSystemConfig(c.Request().Context(), "base_url"); err == nil {
dbBaseURL = config.Value
}
opdsBaseURL := baseURL.Value + "/opds"
return baseURL.Value, opdsBaseURL, nil
baseURL := deriveBaseURL(c, dbBaseURL)
opdsBaseURL := baseURL + "/opds"
return baseURL, opdsBaseURL, nil
}
func (h *OPDSHandler) getAuthToken(c *echo.Context) string {
internal/handlers/sidecar.go
+19 -18
View File
@@ -3,6 +3,7 @@ package handlers
import (
"bookhoard/internal/config"
"bookhoard/internal/database"
"bookhoard/internal/setupstatus"
"encoding/json"
"fmt"
"net/http"
@@ -81,15 +82,13 @@ func (h *SidecarHandler) GetSidecarConfig(c *echo.Context) error {
userID := device.UserID.Bytes
pgUserID := pgtype.UUID{Bytes: userID, Valid: true}
// Get base URL and compute paths
baseURL, _ := h.db.GetSystemConfig(ctx, "base_url")
if baseURL.Value == "" {
baseURL.Value = h.cfg.BaseURL
}
// Get base URL and compute paths (with request-derived fallback)
dbBaseURL, _ := h.db.GetSystemConfig(ctx, "base_url")
baseURL := deriveBaseURL(c, dbBaseURL.Value)
// Generate URLs
opdsCatalogURL := fmt.Sprintf("%s/opds/devices/%s/catalog", baseURL.Value, deviceID.String())
syncAPIURL := fmt.Sprintf("%s/api/sync/kobo", baseURL.Value)
opdsCatalogURL := fmt.Sprintf("%s/opds/devices/%s/catalog", baseURL, deviceID.String())
syncAPIURL := fmt.Sprintf("%s/api/sync/kobo", baseURL)
// Get user's visible libraries with media items
mediaItems, err := h.db.GetUserMediaItemsForSync(ctx, pgUserID)
@@ -176,8 +175,8 @@ func (h *SidecarHandler) GetSidecarConfig(c *echo.Context) error {
Bookhoard: SidecarBookhoardConfig{
OPDSCatalog: opdsCatalogURL,
SyncAPI: syncAPIURL,
OPDSBaseURL: baseURL.Value + "/opds",
APIBaseURL: baseURL.Value + "/api",
OPDSBaseURL: baseURL + "/opds",
APIBaseURL: baseURL + "/api",
DeviceID: deviceID.String(),
DeviceToken: device.AuthToken,
},
@@ -219,15 +218,13 @@ func (h *SidecarHandler) DownloadSidecarConfig(c *echo.Context) error {
userID := device.UserID.Bytes
pgUserID := pgtype.UUID{Bytes: userID, Valid: true}
// Get base URL and compute paths
baseURL, _ := h.db.GetSystemConfig(ctx, "base_url")
if baseURL.Value == "" {
baseURL.Value = h.cfg.BaseURL
}
// Get base URL and compute paths (with request-derived fallback)
dbBaseURL, _ := h.db.GetSystemConfig(ctx, "base_url")
baseURL := deriveBaseURL(c, dbBaseURL.Value)
// Generate URLs
opdsCatalogURL := fmt.Sprintf("%s/opds/devices/%s/catalog", baseURL.Value, deviceID.String())
syncAPIURL := fmt.Sprintf("%s/api/sync/kobo", baseURL.Value)
opdsCatalogURL := fmt.Sprintf("%s/opds/devices/%s/catalog", baseURL, deviceID.String())
syncAPIURL := fmt.Sprintf("%s/api/sync/kobo", baseURL)
// Get user's visible libraries with media items
mediaItems, err := h.db.GetUserMediaItemsForSync(ctx, pgUserID)
@@ -309,8 +306,8 @@ func (h *SidecarHandler) DownloadSidecarConfig(c *echo.Context) error {
Bookhoard: SidecarBookhoardConfig{
OPDSCatalog: opdsCatalogURL,
SyncAPI: syncAPIURL,
OPDSBaseURL: baseURL.Value + "/opds",
APIBaseURL: baseURL.Value + "/api",
OPDSBaseURL: baseURL + "/opds",
APIBaseURL: baseURL + "/api",
DeviceID: deviceID.String(),
DeviceToken: device.AuthToken,
},
@@ -434,6 +431,10 @@ func (h *SidecarHandler) UpdateSystemConfiguration(c *echo.Context) error {
})
}
}
// Invalidate setup status cache so the middleware picks up the new
// base_url immediately (setup is not complete until base_url is set).
setupstatus.Invalidate()
}
// Check for HTMX request
internal/handlers/url.go
+36
View File
@@ -0,0 +1,36 @@
package handlers
import (
"strings"
"github.com/labstack/echo/v5"
)
// deriveBaseURL returns the base URL to use for constructing self-referential
// links (OPDS feeds, sidecar config, etc.). It prefers the database-configured
// base_url when available, and falls back to deriving the URL from the incoming
// HTTP request (Host header + scheme), which is always reachable by the client.
//
// Proxy header support: X-Forwarded-Proto and X-Forwarded-Host are respected so
// that deployments behind TLS-terminating reverse proxies advertise the correct
// external URL.
func deriveBaseURL(c *echo.Context, dbBaseURL string) string {
if dbBaseURL != "" {
return strings.TrimRight(dbBaseURL, "/")
}
scheme := "http"
if c.Request().TLS != nil {
scheme = "https"
}
if proto := c.Request().Header.Get("X-Forwarded-Proto"); proto != "" {
scheme = proto
}
host := c.Request().Host
if forwarded := c.Request().Header.Get("X-Forwarded-Host"); forwarded != "" {
host = forwarded
}
return scheme + "://" + host
}
internal/router/frontend.go
+3 -13
View File
@@ -9,7 +9,6 @@ import (
"strconv"
"time"
"bookhoard/internal/config"
"bookhoard/internal/database"
"bookhoard/internal/handlers"
"bookhoard/internal/services"
@@ -734,10 +733,7 @@ func registerFrontendRoutes(cfg *Config) {
}
// Get base URL from database config with fallback to config/env var
baseURL := config.GetBaseURL(c.Request().Context(), cfg.Queries)
if baseURL == "" {
baseURL = cfg.Cfg.BaseURL
}
baseURL := cfg.getBaseURL(c.Request().Context())
var buf bytes.Buffer
err = templates.Devices(user, devices, pendingList, errorMsg, baseURL).Render(c.Request().Context(), &buf)
@@ -995,10 +991,7 @@ func registerFrontendRoutes(cfg *Config) {
}
// Fetch current system configuration - just base_url
baseURL := config.GetBaseURL(c.Request().Context(), cfg.Queries)
if baseURL == "" {
baseURL = cfg.Cfg.BaseURL
}
baseURL := cfg.getBaseURL(c.Request().Context())
systemConfig := map[string]string{
"base_url": baseURL,
@@ -1086,10 +1079,7 @@ func registerFrontendRoutes(cfg *Config) {
}
// Get base URL from database config with fallback to config/env var
baseURL := config.GetBaseURL(c.Request().Context(), cfg.Queries)
if baseURL == "" {
baseURL = cfg.Cfg.BaseURL
}
baseURL := cfg.getBaseURL(c.Request().Context())
var buf bytes.Buffer
err = templates.Devices(user, devices, pendingList, errorMsg, baseURL).Render(c.Request().Context(), &buf)
internal/router/router.go
+18
View File
@@ -72,6 +72,24 @@ type Config struct {
LibraryService *services.LibraryService
}
// getBaseURL returns the configured base URL from the database, falling back to
// the env var / config default. Uses a closure to adapt the database query to
// config.SystemConfigGetter.
func (cfg *Config) getBaseURL(ctx context.Context) string {
getter := func(ctx context.Context, key string) (string, error) {
row, err := cfg.Queries.GetSystemConfig(ctx, key)
if err != nil {
return "", err
}
return row.Value, nil
}
baseURL := config.GetBaseURL(ctx, getter)
if baseURL == "" {
baseURL = cfg.Cfg.BaseURL
}
return baseURL
}
// createJWTMiddleware creates a JWT middleware with proper user context setup
func createJWTMiddleware(cfg *Config) echo.MiddlewareFunc {
return echojwt.WithConfig(echojwt.Config{
internal/router/setup.go
+40 -10
View File
@@ -14,7 +14,40 @@ import (
)
func isSetupComplete(cfg *Config) bool {
return setupstatus.IsSetupComplete(context.Background(), cfg.Queries)
getter := func(ctx context.Context) (string, error) {
row, err := cfg.Queries.GetSystemConfig(ctx, "base_url")
if err != nil {
return "", err
}
return row.Value, nil
}
return setupstatus.IsSetupComplete(context.Background(), cfg.Queries, getter)
}
// setupAllowedAPIRoutes lists API endpoints that remain accessible before
// initial setup is complete so the server can be configured via API.
var setupAllowedAPIRoutes = []string{
"/api/auth/register",
"/api/auth/login",
"/api/system/config",
}
// isAllowedDuringSetup reports whether a request path should bypass the setup
// gate. This includes the setup page itself, static assets, health checks, and
// the minimal set of API routes needed to perform initial configuration.
func isAllowedDuringSetup(path string) bool {
if path == "/setup" || path == "/setup/" {
return true
}
if strings.HasPrefix(path, "/static/") || path == "/health" || path == "/favicon.ico" {
return true
}
for _, route := range setupAllowedAPIRoutes {
if path == route || strings.HasPrefix(path, route+"/") {
return true
}
}
return false
}
func setupRedirectMiddleware(cfg *Config) echo.MiddlewareFunc {
@@ -22,19 +55,16 @@ func setupRedirectMiddleware(cfg *Config) echo.MiddlewareFunc {
return func(c *echo.Context) error {
path := c.Request().URL.Path
if path == "/setup" || path == "/setup/" {
return next(c)
}
if strings.HasPrefix(path, "/api/") {
return next(c)
}
if strings.HasPrefix(path, "/static/") || path == "/health" || path == "/favicon.ico" {
if isAllowedDuringSetup(path) {
return next(c)
}
if !isSetupComplete(cfg) {
if strings.HasPrefix(path, "/api/") {
return c.JSON(http.StatusServiceUnavailable, map[string]string{
"error": "Server setup is not complete. Configure an admin account and base_url via the setup wizard or API.",
})
}
return c.Redirect(http.StatusFound, "/setup")
}
internal/setupstatus/status.go
+27 -10
View File
@@ -1,8 +1,9 @@
// Package setupstatus reports whether the application's initial setup has been
// completed. Setup is considered complete as soon as at least one admin user
// exists, regardless of how that user was created (setup wizard, API, or a
// future CLI). This keeps the setup gate a derived property of real data
// rather than a manually-flipped flag that can drift out of sync.
// completed. Setup is considered complete when at least one admin user exists
// AND a non-empty base_url has been configured, regardless of how those were
// created (setup wizard, API, or a future CLI). This keeps the setup gate a
// derived property of real data rather than a manually-flipped flag that can
// drift out of sync.
package setupstatus
import (
@@ -18,6 +19,12 @@ type AdminCounter interface {
CountAdmins(ctx context.Context) (int64, error)
}
// BaseURLGetter returns the configured base_url value from the database, or an
// error if it cannot be read. Defined as a function type (not an interface) so
// it can be satisfied by a closure wrapping *database.Queries.GetSystemConfig
// without importing the database package.
type BaseURLGetter func(ctx context.Context) (string, error)
var (
cacheMu sync.RWMutex
cacheComplete bool = true
@@ -26,10 +33,11 @@ var (
)
// IsSetupComplete reports whether setup is complete. Setup is complete when at
// least one admin user exists. A short in-memory cache avoids hammering the
// database on every request. On a database error the function fails open
// (returns true) so a transient outage does not lock users out of the app.
func IsSetupComplete(ctx context.Context, q AdminCounter) bool {
// least one admin user exists AND base_url is configured. A short in-memory
// cache avoids hammering the database on every request. On a database error the
// function fails open (returns true) so a transient outage does not lock users
// out of the app.
func IsSetupComplete(ctx context.Context, q AdminCounter, baseURLGetter BaseURLGetter) bool {
cacheMu.RLock()
if time.Now().Before(cacheExpiry) {
complete := cacheComplete
@@ -38,12 +46,20 @@ func IsSetupComplete(ctx context.Context, q AdminCounter) bool {
}
cacheMu.RUnlock()
count, err := q.CountAdmins(ctx)
complete := true
count, err := q.CountAdmins(ctx)
if err == nil {
complete = count > 0
}
if complete && baseURLGetter != nil {
baseURL, err := baseURLGetter(ctx)
if err == nil {
complete = baseURL != ""
}
}
cacheMu.Lock()
cacheComplete = complete
cacheExpiry = time.Now().Add(cacheTTL)
@@ -53,7 +69,8 @@ func IsSetupComplete(ctx context.Context, q AdminCounter) bool {
// Invalidate clears the cached setup status so the next call to IsSetupComplete
// re-reads from the database. Call this after any write that could change the
// admin user count (user creation, role promotion/demotion, user deletion).
// admin user count (user creation, role promotion/demotion, user deletion) or
// the base_url configuration.
func Invalidate() {
cacheMu.Lock()
cacheComplete = true