feat(server): ghost-session guard — JWTs for deleted users stop authenticating
The JWT middleware validated signature + expiry and fabricated the request user purely from token claims, so a signature-valid token for a DELETED user kept full API access until token expiry — observed live on hardware (a phone session survived the test suite's user-table recreation and kept serving its true empty sets; open handoff bug 1). An ensureUserExistsMiddleware already existed but was only wired on the frontend SSR group (frontend.go) — every /api group used bare createJWTMiddleware. The existence check now lives INSIDE createJWTMiddleware's SuccessHandler so every protected group (SSR and API alike) gets it: - After claim validation, GetUser by the token's user_id; a missing row clears the token cookie and answers 401 user_not_found for API calls (the app's TokenAuthenticator then re-logins with stored credentials) or redirects to /login?session=invalid for page requests — same shape-aware split the error handler already uses. - Cost: one primary-key lookup per authenticated request (~0.2ms local); instant revocation, no TTL semantics. The redundant SSR-only middleware is deleted (its redirect behavior is preserved inside the guard). The guard exposed two processing-issues tests that were unknowingly RELYING on ghost semantics: they nested a second setupTestServer whose bootstrap deleted the first setup's regular user, then asserted 403 from the dead token. De-nested: a createLibraryDirect helper inserts the library under the live setup's user, no second bootstrap. New TestDeletedUserTokenRejected pins the contract: token works while the user exists -> 401 user_not_found after deletion. Full suite: 219 groups, zero failures.
This commit is contained in:
1 parent
4f05a232fe
commit
9cd6356892
5 files changed
+125
-57
No files matched your search
@@ -16,8 +16,7 @@ func TestProcessingIssuesListAuthentication(t *testing.T) {
|
|||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
// Create a test library
|
// Create a test library
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test Auth Library", "manga")
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test Auth Library", "manga")
|
|
||||||
|
|
||||||
t.Run("GET /api/libraries/:id/issues/list - No authentication", func(t *testing.T) {
|
t.Run("GET /api/libraries/:id/issues/list - No authentication", func(t *testing.T) {
|
||||||
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/list", nil)
|
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/list", nil)
|
||||||
@@ -55,8 +54,7 @@ func TestProcessingIssuesListResponseStructure(t *testing.T) {
|
|||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
// Create a test library
|
// Create a test library
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test Structure Library", "comics")
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test Structure Library", "comics")
|
|
||||||
|
|
||||||
t.Run("Returns JSON array for empty library", func(t *testing.T) {
|
t.Run("Returns JSON array for empty library", func(t *testing.T) {
|
||||||
token := setup.Token
|
token := setup.Token
|
||||||
@@ -163,8 +161,7 @@ func TestProcessingIssueStatsAuthentication(t *testing.T) {
|
|||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
// Create a test library
|
// Create a test library
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test Stats Auth Library", "manga")
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test Stats Auth Library", "manga")
|
|
||||||
|
|
||||||
t.Run("GET /api/libraries/:id/issues/stats - No authentication", func(t *testing.T) {
|
t.Run("GET /api/libraries/:id/issues/stats - No authentication", func(t *testing.T) {
|
||||||
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/stats", nil)
|
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/stats", nil)
|
||||||
@@ -202,8 +199,7 @@ func TestProcessingIssueStatsResponseStructure(t *testing.T) {
|
|||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
// Create a test library
|
// Create a test library
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test Stats Structure Library", "comics")
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test Stats Structure Library", "comics")
|
|
||||||
|
|
||||||
t.Run("Returns correct structure for empty library", func(t *testing.T) {
|
t.Run("Returns correct structure for empty library", func(t *testing.T) {
|
||||||
token := setup.Token
|
token := setup.Token
|
||||||
@@ -343,11 +339,9 @@ func TestProcessingIssuesCrossLibraryIsolation(t *testing.T) {
|
|||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
// Create two different libraries with different types
|
// Create two different libraries with different types
|
||||||
deviceSetup1 := setupDeviceTest(t)
|
library1ID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Isolation Test Library 1", "manga")
|
||||||
library1ID := deviceSetup1.CreateLibrary(t, "Isolation Test Library 1", "manga")
|
|
||||||
|
|
||||||
deviceSetup2 := setupDeviceTest(t)
|
library2ID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Isolation Test Library 2", "comics")
|
||||||
library2ID := deviceSetup2.CreateLibrary(t, "Isolation Test Library 2", "comics")
|
|
||||||
|
|
||||||
token := setup.Token
|
token := setup.Token
|
||||||
|
|
||||||
@@ -431,8 +425,7 @@ func TestProcessingIssuesDifferentLibraryTypes(t *testing.T) {
|
|||||||
|
|
||||||
for _, lt := range libraryTypes {
|
for _, lt := range libraryTypes {
|
||||||
t.Run(lt.name+" - list endpoint works", func(t *testing.T) {
|
t.Run(lt.name+" - list endpoint works", func(t *testing.T) {
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test "+lt.name, lt.libraryType)
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test "+lt.name, lt.libraryType)
|
|
||||||
|
|
||||||
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/list", nil)
|
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/list", nil)
|
||||||
req.Header.Set("Authorization", "Bearer "+token)
|
req.Header.Set("Authorization", "Bearer "+token)
|
||||||
@@ -448,8 +441,7 @@ func TestProcessingIssuesDifferentLibraryTypes(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run(lt.name+" - stats endpoint works", func(t *testing.T) {
|
t.Run(lt.name+" - stats endpoint works", func(t *testing.T) {
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test "+lt.name, lt.libraryType)
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test "+lt.name, lt.libraryType)
|
|
||||||
|
|
||||||
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/stats", nil)
|
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/stats", nil)
|
||||||
req.Header.Set("Authorization", "Bearer "+token)
|
req.Header.Set("Authorization", "Bearer "+token)
|
||||||
@@ -474,8 +466,7 @@ func TestProcessingIssuesInvalidTokens(t *testing.T) {
|
|||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
// Create a test library
|
// Create a test library
|
||||||
deviceSetup := setupDeviceTest(t)
|
libraryID := createLibraryDirect(t, setup.DB, getTestUserID(t, setup.DB), "Test Token Library", "ebooks")
|
||||||
libraryID := deviceSetup.CreateLibrary(t, "Test Token Library", "ebooks")
|
|
||||||
|
|
||||||
t.Run("List endpoint rejects invalid token", func(t *testing.T) {
|
t.Run("List endpoint rejects invalid token", func(t *testing.T) {
|
||||||
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/list", nil)
|
req := httptest.NewRequest("GET", "/api/libraries/"+libraryID+"/issues/list", nil)
|
||||||
|
|||||||
@@ -657,6 +657,69 @@ func TestProgressWeb_EdgeCases(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestDeletedUserTokenRejected pins the ghost-session guard: a
|
||||||
|
// signature-valid JWT for a user that has been DELETED must stop
|
||||||
|
// authenticating (401 user_not_found), not serve a phantom session.
|
||||||
|
// Before the guard, deleted users kept full API access until token
|
||||||
|
// expiry (observed live on hardware 2026-10-09).
|
||||||
|
func TestDeletedUserTokenRejected(t *testing.T) {
|
||||||
|
setup := setupTestServer(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create a dedicated user, then log in as it to get a fresh token.
|
||||||
|
createResp := doReq(t, "POST", setup.Server.URL+"/api/auth/register", map[string]interface{}{
|
||||||
|
"email": "ghost-test@tests.bookhoard.internal",
|
||||||
|
"username": "ghosttest",
|
||||||
|
"password": "Ghost@Pass123!",
|
||||||
|
}, setup.Token)
|
||||||
|
defer createResp.Body.Close()
|
||||||
|
if createResp.StatusCode != http.StatusCreated {
|
||||||
|
t.Skipf("registration unavailable (status %d)", createResp.StatusCode)
|
||||||
|
}
|
||||||
|
|
||||||
|
loginResp := doReq(t, "POST", setup.Server.URL+"/api/auth/login", map[string]interface{}{
|
||||||
|
"login": "ghost-test@tests.bookhoard.internal",
|
||||||
|
"password": "Ghost@Pass123!",
|
||||||
|
}, "")
|
||||||
|
defer loginResp.Body.Close()
|
||||||
|
require.Equal(t, http.StatusOK, loginResp.StatusCode)
|
||||||
|
var loginBody struct {
|
||||||
|
AccessToken string `json:"access_token"`
|
||||||
|
}
|
||||||
|
require.NoError(t, json.NewDecoder(loginResp.Body).Decode(&loginBody))
|
||||||
|
require.NotEmpty(t, loginBody.AccessToken)
|
||||||
|
|
||||||
|
ghostToken := loginBody.AccessToken
|
||||||
|
|
||||||
|
// Sanity: the token works while the user exists (user-scoped endpoint
|
||||||
|
// a freshly-registered regular user can access).
|
||||||
|
okResp := doReq(t, "GET", setup.Server.URL+"/api/collections", nil, ghostToken)
|
||||||
|
defer okResp.Body.Close()
|
||||||
|
require.Equal(t, http.StatusOK, okResp.StatusCode, "token must work while the user exists")
|
||||||
|
|
||||||
|
// Delete the user directly via the DB for determinism (same query the
|
||||||
|
// suite's own cleanup uses).
|
||||||
|
ghostID := ""
|
||||||
|
users, err := setup.DB.ListUsers(ctx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
for _, u := range users {
|
||||||
|
if u.Email == "ghost-test@tests.bookhoard.internal" {
|
||||||
|
ghostID = uuid.UUID(u.ID.Bytes).String()
|
||||||
|
require.NoError(t, setup.DB.DeleteUser(ctx, u.ID))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
require.NotEmpty(t, ghostID, "ghost user row must exist before deletion")
|
||||||
|
|
||||||
|
// The same token must now be rejected with 401 user_not_found.
|
||||||
|
resp := doReq(t, "GET", setup.Server.URL+"/api/collections", nil, ghostToken)
|
||||||
|
defer resp.Body.Close()
|
||||||
|
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "ghost token must be rejected")
|
||||||
|
|
||||||
|
var errBody map[string]string
|
||||||
|
require.NoError(t, json.NewDecoder(resp.Body).Decode(&errBody))
|
||||||
|
assert.Equal(t, "user_not_found", errBody["error"])
|
||||||
|
}
|
||||||
|
|
||||||
// TestProgressWeb_LocatorCoherence pins the locator coherence rule: a
|
// TestProgressWeb_LocatorCoherence pins the locator coherence rule: a
|
||||||
// percentage-only write that DECLARES no locator (ClearLocator) must clear
|
// percentage-only write that DECLARES no locator (ClearLocator) must clear
|
||||||
// any inherited stale anchor instead of re-pairing the new percentage with
|
// any inherited stale anchor instead of re-pairing the new percentage with
|
||||||
|
|||||||
@@ -794,6 +794,26 @@ func getTestUserID(t *testing.T, db *database.Queries) uuid.UUID {
|
|||||||
return userUUID
|
return userUUID
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// createLibraryDirect inserts a library owned by the given user WITHOUT a
|
||||||
|
// nested test-server setup: a second setupTestServer bootstraps away the
|
||||||
|
// first setup's users, and the ghost-session guard rightly rejects the
|
||||||
|
// outer test's tokens from then on.
|
||||||
|
func createLibraryDirect(t *testing.T, db *database.Queries, ownerID uuid.UUID, name, libraryType string) string {
|
||||||
|
ctx := context.Background()
|
||||||
|
libraryTypeRow, err := db.GetLibraryTypeByName(ctx, libraryType)
|
||||||
|
require.NoError(t, err, "Should find library type")
|
||||||
|
library, err := db.CreateLibrary(ctx, database.CreateLibraryParams{
|
||||||
|
Name: name,
|
||||||
|
Description: pgtype.Text{String: "Test library description", Valid: true},
|
||||||
|
LibraryTypeID: libraryTypeRow.ID,
|
||||||
|
CreatedByAdminID: pgtype.UUID{Bytes: [16]byte(ownerID), Valid: true},
|
||||||
|
})
|
||||||
|
require.NoError(t, err, "Should create library")
|
||||||
|
libraryUUID, err := uuid.FromBytes(library.ID.Bytes[0:16])
|
||||||
|
require.NoError(t, err, "Should parse library ID")
|
||||||
|
return libraryUUID.String()
|
||||||
|
}
|
||||||
|
|
||||||
func getRegularUserID(t *testing.T, db *database.Queries) uuid.UUID {
|
func getRegularUserID(t *testing.T, db *database.Queries) uuid.UUID {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
user, err := db.GetUserByEmail(ctx, "testregularuser@tests.bookhoard.internal")
|
user, err := db.GetUserByEmail(ctx, "testregularuser@tests.bookhoard.internal")
|
||||||
|
|||||||
@@ -120,7 +120,7 @@ func registerFrontendRoutes(cfg *Config) {
|
|||||||
// ============================================================================
|
// ============================================================================
|
||||||
|
|
||||||
// Protected frontend routes (no /api prefix)
|
// Protected frontend routes (no /api prefix)
|
||||||
frontendProtected := e.Group("", jwtMiddleware, ensureUserExistsMiddleware(cfg))
|
frontendProtected := e.Group("", jwtMiddleware)
|
||||||
|
|
||||||
// Series browse page
|
// Series browse page
|
||||||
frontendProtected.GET("/series", func(c *echo.Context) error {
|
frontendProtected.GET("/series", func(c *echo.Context) error {
|
||||||
|
|||||||
+32
-38
@@ -69,7 +69,7 @@ type Config struct {
|
|||||||
LoginTracker *ratelimit.LoginAttemptTracker
|
LoginTracker *ratelimit.LoginAttemptTracker
|
||||||
ScannerHandler *handlers.Handler
|
ScannerHandler *handlers.Handler
|
||||||
JobsHandler *handlers.JobsHandler
|
JobsHandler *handlers.JobsHandler
|
||||||
SidecarHandler *handlers.SidecarHandler
|
SidecarHandler *handlers.SidecarHandler
|
||||||
ReaderHandler *handlers.ReaderHandler
|
ReaderHandler *handlers.ReaderHandler
|
||||||
LibraryService *services.LibraryService
|
LibraryService *services.LibraryService
|
||||||
}
|
}
|
||||||
@@ -113,6 +113,31 @@ func createJWTMiddleware(cfg *Config) echo.MiddlewareFunc {
|
|||||||
return echo.NewHTTPError(http.StatusBadRequest, "invalid user ID in token")
|
return echo.NewHTTPError(http.StatusBadRequest, "invalid user ID in token")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Ghost-session guard: a signature-valid token for a user that
|
||||||
|
// no longer exists must not authenticate (deleted users kept
|
||||||
|
// full access until token expiry before this check). A
|
||||||
|
// primary-key lookup per request — instant revocation, no TTL
|
||||||
|
// semantics; the guard also covers the SSR groups that used to
|
||||||
|
// chain ensureUserExistsMiddleware.
|
||||||
|
if !userExists(cfg, userUUID) {
|
||||||
|
log.Printf("Ghost session rejected: user %s no longer exists", userIDStr)
|
||||||
|
c.SetCookie(&http.Cookie{
|
||||||
|
Name: "token",
|
||||||
|
Value: "",
|
||||||
|
Expires: time.Now().Add(-24 * time.Hour),
|
||||||
|
Path: "/",
|
||||||
|
HttpOnly: true,
|
||||||
|
SameSite: http.SameSiteLaxMode,
|
||||||
|
})
|
||||||
|
if wantsHTML(c.Request().Header) {
|
||||||
|
return c.Redirect(http.StatusFound, "/login?session=invalid")
|
||||||
|
}
|
||||||
|
return c.JSON(http.StatusUnauthorized, map[string]string{
|
||||||
|
"error": "user_not_found",
|
||||||
|
"message": "This account no longer exists. Please log in again.",
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
c.Set("user", database.Users{
|
c.Set("user", database.Users{
|
||||||
ID: pgtype.UUID{Bytes: userUUID, Valid: true},
|
ID: pgtype.UUID{Bytes: userUUID, Valid: true},
|
||||||
Email: claims["user_email"].(string),
|
Email: claims["user_email"].(string),
|
||||||
@@ -138,43 +163,12 @@ func createJWTMiddleware(cfg *Config) echo.MiddlewareFunc {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
// ensureUserExistsMiddleware checks if the authenticated user still exists in the database
|
// userExists checks the users table for the token's claimed user. Runs on
|
||||||
func ensureUserExistsMiddleware(cfg *Config) echo.MiddlewareFunc {
|
// every authenticated request: a primary-key lookup, cheap at this scale,
|
||||||
return func(next echo.HandlerFunc) echo.HandlerFunc {
|
// and it makes deletion revocable the moment it happens.
|
||||||
return func(c *echo.Context) error {
|
func userExists(cfg *Config, id uuid.UUID) bool {
|
||||||
userIDStr, ok := c.Get("user_id").(string)
|
_, err := cfg.Queries.GetUser(context.Background(), uuidToPGType(id))
|
||||||
if !ok {
|
return err == nil
|
||||||
return next(c)
|
|
||||||
}
|
|
||||||
|
|
||||||
userUUID, err := uuid.Parse(userIDStr)
|
|
||||||
if err != nil {
|
|
||||||
log.Printf("Invalid UUID in user existence check: %v", err)
|
|
||||||
return next(c)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check if user exists in database
|
|
||||||
_, err = cfg.Queries.GetUser(c.Request().Context(), uuidToPGType(userUUID))
|
|
||||||
if err != nil {
|
|
||||||
log.Printf("User not found in database: %s", userIDStr)
|
|
||||||
|
|
||||||
// Clear invalid cookie
|
|
||||||
c.SetCookie(&http.Cookie{
|
|
||||||
Name: "token",
|
|
||||||
Value: "",
|
|
||||||
Expires: time.Now().Add(-24 * time.Hour),
|
|
||||||
Path: "/",
|
|
||||||
HttpOnly: true,
|
|
||||||
SameSite: http.SameSiteLaxMode,
|
|
||||||
})
|
|
||||||
|
|
||||||
// Redirect to login with session=invalid
|
|
||||||
return c.Redirect(http.StatusFound, "/login?session=invalid")
|
|
||||||
}
|
|
||||||
|
|
||||||
return next(c)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// wantsHTML determines if the request expects HTML response
|
// wantsHTML determines if the request expects HTML response
|
||||||
|
|||||||
Reference in new issue
Block a user