fix(admin): owner gate answers 503 on a role read fault, not 403 (OC-0345)

ownerOnlyMiddleware collapsed `err != nil || role == nil` into 403 "role not
found", so a transient GetRoleByID failure told the Owner they lack the
Owner role. Split the outcomes: a store error logs and answers 503
SERVICE_UNAVAILABLE (the perimeter's contract); a genuinely missing role
still answers 403. The existing whitebox tests, which inject only the user
into the context, are unchanged.

Test: TestOwnerOnlyMiddleware_RoleLookupFailureIs503 (roles table renamed,
whitebox — through the full stack the perimeter would answer its own 503).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo
This commit is contained in:
J3vb
2026-08-30 10:19:05 +02:00
co-authored by Claude Fable 5
parent 775eba50ae
commit fb1afb8a8d
2 changed files with 58 additions and 1 deletions
+9 -1
View File
@@ -128,7 +128,15 @@ func ownerOnlyMiddleware(database *db.DB, next http.Handler) http.Handler {
}
role, err := database.GetRoleByID(r.Context(), user.RoleID)
if err != nil || role == nil {
if err != nil {
// A read fault is an outage, not a missing role: answering 403
// would tell the Owner they lack the Owner role. Mirror the
// perimeter's contract above — log it, report 503 (OC-0345).
slog.ErrorContext(r.Context(), "admin: owner role lookup failed", "error", err)
writeErr(w, http.StatusServiceUnavailable, "SERVICE_UNAVAILABLE", "authorization service temporarily unavailable")
return
}
if role == nil {
writeErr(w, http.StatusForbidden, "FORBIDDEN", "role not found")
return
}
+49
View File
@@ -522,3 +522,52 @@ func TestSpawnDetached_CommandConstruction(t *testing.T) {
t.Error("cmd.Stderr should not be nil")
}
}
// TestOwnerOnlyMiddleware_RoleLookupFailureIs503 pins OC-0345: a database
// fault on the owner gate's role read is an outage, not a missing role, so the
// Owner must get 503 SERVICE_UNAVAILABLE — never the 403 "role not found" a
// genuinely absent role earns. Whitebox on purpose: through the full stack
// adminAuthMiddleware reads the role first and would answer its own 503, so
// the branch under test would never run.
func TestOwnerOnlyMiddleware_RoleLookupFailureIs503(t *testing.T) {
database := openWhiteboxTestDB(t)
uid, err := database.CreateUser(context.Background(), "ownerfault", "$2a$12$x", 1)
if err != nil {
t.Fatalf("CreateUser: %v", err)
}
user, err := database.GetUserByID(context.Background(), uid)
if err != nil || user == nil {
t.Fatalf("GetUserByID: %v", err)
}
// Every query against roles now fails with a non-sentinel error.
if _, err := database.ExecContext(context.Background(), `ALTER TABLE roles RENAME TO roles_gone`); err != nil {
t.Fatalf("hide roles: %v", err)
}
reached := false
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
reached = true
w.WriteHeader(http.StatusOK)
})
handler := ownerOnlyMiddleware(database, next)
ctx := context.WithValue(context.Background(), adminUserKey, user)
req := httptest.NewRequest(http.MethodPost, "/backup", nil).WithContext(ctx)
w := httptest.NewRecorder()
handler.ServeHTTP(w, req)
if reached {
t.Error("next handler was reached although the role could not be read")
}
if w.Code != http.StatusServiceUnavailable {
t.Errorf("status = %d, want 503 (a role read fault is not a missing role)", w.Code)
}
var resp map[string]string
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("unmarshal: %v", err)
}
if resp["error"] != "SERVICE_UNAVAILABLE" {
t.Errorf("error = %q, want SERVICE_UNAVAILABLE", resp["error"])
}
}