diff --git a/.vscode/settings.json b/.vscode/settings.json index ec0b094d..652fc6a4 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -8,7 +8,6 @@ ] , "gopls": { - "buildFlags": ["-tags=ignore", "-mod=mod"], "env": { "GOWORK": "off", "GOFLAGS": "-mod=mod", @@ -17,8 +16,7 @@ "directoryFilters": [ "-**/pkg/mod/**", "-**/go/pkg/mod/**", - "-**/root/go/pkg/mod/**", - "-**/golang.org/toolchain@**" + "-**/root/go/pkg/mod/**" ] }, "go.buildFlags": ["-tags=ignore", "-mod=mod"], diff --git a/backend/go.mod b/backend/go.mod index 4c4b812c..cd482b42 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -80,7 +80,6 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.38.0 // indirect go.opentelemetry.io/otel/metric v1.38.0 // indirect go.opentelemetry.io/otel/trace v1.38.0 // indirect - go.uber.org/mock v0.6.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/arch v0.22.0 // indirect golang.org/x/mod v0.29.0 // indirect diff --git a/backend/go.sum b/backend/go.sum index 13146561..17e69a8f 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -10,8 +10,6 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bytedance/gopkg v0.1.3 h1:TPBSwH8RsouGCBcMBktLt1AymVo2TVsBVCY4b6TnZ/M= github.com/bytedance/gopkg v0.1.3/go.mod h1:576VvJ+eJgyCzdjS+c4+77QF3p7ubbtiKARP3TxducM= -github.com/bytedance/sonic v1.14.0 h1:/OfKt8HFw0kh2rj8N0F6C/qPGRESq0BbaNZgcNXXzQQ= -github.com/bytedance/sonic v1.14.0/go.mod h1:WoEbx8WTcFJfzCe0hbmyTGrfjt8PzNEBdxlNUO24NhA= github.com/bytedance/sonic v1.14.1 h1:FBMC0zVz5XUmE4z9wF4Jey0An5FueFvOsTKKKtwIl7w= github.com/bytedance/sonic v1.14.1/go.mod h1:gi6uhQLMbTdeP0muCnrjHLeCUPyb70ujhnNlhOylAFc= github.com/bytedance/sonic/loader v0.3.0 h1:dskwH8edlzNMctoruo8FPTJDF3vLtDT0sXZwvZJyqeA= @@ -49,8 +47,6 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/francoispqt/gojay v1.2.13/go.mod h1:ehT5mTG4ua4581f1++1WLG0vPdaA9HaiDsoyrBGkyDY= github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw= -github.com/gabriel-vasile/mimetype v1.4.8 h1:FfZ3gj38NjllZIeJAmMhr+qKL8Wu+nOoI3GqacKw1NM= -github.com/gabriel-vasile/mimetype v1.4.8/go.mod h1:ByKUIKGjh1ODkGM1asKUbQZOLGrPjydw3hYPU2YU9t8= github.com/gabriel-vasile/mimetype v1.4.10 h1:zyueNbySn/z8mJZHLt6IPw0KoZsiQNszIpU+bX4+ZK0= github.com/gabriel-vasile/mimetype v1.4.10/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= github.com/gin-contrib/gzip v1.2.5 h1:fIZs0S+l17pIu1P5XRJOo/YNqfIuPCrZZ3TWB7pjckI= @@ -70,14 +66,10 @@ github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/o github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= -github.com/go-playground/validator/v10 v10.27.0 h1:w8+XrWVMhGkxOaaowyKH35gFydVHOvC0/uWoy2Fzwn4= -github.com/go-playground/validator/v10 v10.27.0/go.mod h1:I5QpIEbmr8On7W0TktmJAumgzX4CA1XNl4ZmDuVHKKo= github.com/go-playground/validator/v10 v10.28.0 h1:Q7ibns33JjyW48gHkuFT91qX48KG0ktULL6FgHdG688= github.com/go-playground/validator/v10 v10.28.0/go.mod h1:GoI6I1SjPBh9p7ykNE/yj3fFYbyDOpwMn5KXd+m2hUU= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls= -github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU= -github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/goccy/go-json v0.10.5 h1:Fq85nIqj+gXn/S5ahsiTlK3TmC85qgirsdTP/+DeaC4= github.com/goccy/go-json v0.10.5/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M= github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw= @@ -173,8 +165,6 @@ github.com/prometheus/procfs v0.16.1 h1:hZ15bTNuirocR6u0JZ6BAHHmwS1p8B4P6MRqxtzM github.com/prometheus/procfs v0.16.1/go.mod h1:teAbpZRB1iIAJYREa1LsoWUXykVXA1KlTmWl8x/U+Is= github.com/quic-go/qpack v0.5.1 h1:giqksBPnT/HDtZ6VhtFKgoLOWmlyo9Ei6u9PqzIMbhI= github.com/quic-go/qpack v0.5.1/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg= -github.com/quic-go/quic-go v0.54.1 h1:4ZAWm0AhCb6+hE+l5Q1NAL0iRn/ZrMwqHRGQiFwj2eg= -github.com/quic-go/quic-go v0.54.1/go.mod h1:e68ZEaCdyviluZmy44P6Iey98v/Wfz6HCjQEm+l8zTY= github.com/quic-go/quic-go v0.55.0 h1:zccPQIqYCXDt5NmcEabyYvOnomjs8Tlwl7tISjJh9Mk= github.com/quic-go/quic-go v0.55.0/go.mod h1:DR51ilwU1uE164KuWXhinFcKWGlEjzys2l8zUl5Ss1U= github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= @@ -231,14 +221,10 @@ go.opentelemetry.io/proto/otlp v1.7.1 h1:gTOMpGDb0WTBOP8JaO72iL3auEZhVmAQg4ipjOV go.opentelemetry.io/proto/otlp v1.7.1/go.mod h1:b2rVh6rfI/s2pHWNlB7ILJcRALpcNDzKhACevjI+ZnE= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -go.uber.org/mock v0.5.0 h1:KAMbZvZPyBPWgD14IrIQ38QCyjwpvVVV6K/bHl1IwQU= -go.uber.org/mock v0.5.0/go.mod h1:ge71pBPLYDk7QIi1LupWxdAykm7KIEFchiOqd6z7qMM= go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y= go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU= go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI= go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU= -golang.org/x/arch v0.20.0 h1:dx1zTU0MAE98U+TQ8BLl7XsJbgze2WnNKF/8tGp/Q6c= -golang.org/x/arch v0.20.0/go.mod h1:bdwinDaKcfZUGpH09BB7ZmOfhalA8lQdzl62l8gGWsk= golang.org/x/arch v0.22.0 h1:c/Zle32i5ttqRXjdLyyHZESLD/bB90DCU1g9l/0YBDI= golang.org/x/arch v0.22.0/go.mod h1:dNHoOeKiyja7GTvF9NJS1l3Z2yntpQNzgrjh1cU103A= golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q= @@ -270,8 +256,6 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20250825161204-c5933d9347a5 h1: google.golang.org/genproto/googleapis/rpc v0.0.0-20250825161204-c5933d9347a5/go.mod h1:M4/wBTSeyLxupu3W3tJtOgB14jILAS/XWPSSa3TAlJc= google.golang.org/grpc v1.75.0 h1:+TW+dqTd2Biwe6KKfhE5JpiYIBWq865PhKGSXiivqt4= google.golang.org/grpc v1.75.0/go.mod h1:JtPAzKiq4v1xcAB2hydNlWI2RnF85XXcV0mhKXr2ecQ= -google.golang.org/protobuf v1.36.9 h1:w2gp2mA27hUeUzj9Ex9FBjsBm40zfaDtEWow293U7Iw= -google.golang.org/protobuf v1.36.9/go.mod h1:fuxRtAxBytpl4zzqUh6/eyUujkJdNiuEkXntxiD/uRU= google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE= google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/backend/internal/api/handlers/certificate_handler.go b/backend/internal/api/handlers/certificate_handler.go index c9cacc76..fbaf4c98 100644 --- a/backend/internal/api/handlers/certificate_handler.go +++ b/backend/internal/api/handlers/certificate_handler.go @@ -4,9 +4,12 @@ import ( "fmt" "net/http" "strconv" + "sync" + "time" "github.com/gin-gonic/gin" + "github.com/Wikid82/charon/backend/internal/logger" "github.com/Wikid82/charon/backend/internal/services" "github.com/Wikid82/charon/backend/internal/util" ) @@ -18,26 +21,38 @@ type BackupServiceInterface interface { DeleteBackup(filename string) error GetBackupPath(filename string) (string, error) RestoreBackup(filename string) error + GetAvailableSpace() (int64, error) } type CertificateHandler struct { service *services.CertificateService backupService BackupServiceInterface notificationService *services.NotificationService + // Rate limiting for notifications + notificationMu sync.Mutex + lastNotificationTime map[uint]time.Time } func NewCertificateHandler(service *services.CertificateService, backupService BackupServiceInterface, ns *services.NotificationService) *CertificateHandler { return &CertificateHandler{ - service: service, - backupService: backupService, - notificationService: ns, + service: service, + backupService: backupService, + notificationService: ns, + lastNotificationTime: make(map[uint]time.Time), } } func (h *CertificateHandler) List(c *gin.Context) { + // Defense in depth - verify user context exists + if _, exists := c.Get("user"); !exists { + c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + return + } + certs, err := h.service.ListCertificates() if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithError(err).Error("failed to list certificates") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list certificates"}) return } @@ -51,6 +66,12 @@ type UploadCertificateRequest struct { } func (h *CertificateHandler) Upload(c *gin.Context) { + // Defense in depth - verify user context exists + if _, exists := c.Get("user"); !exists { + c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + return + } + // Handle multipart form name := c.PostForm("name") if name == "" { @@ -98,7 +119,8 @@ func (h *CertificateHandler) Upload(c *gin.Context) { cert, err := h.service.UploadCertificate(name, certPEM, keyPEM) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + logger.Log().WithError(err).Error("failed to upload certificate") + c.JSON(http.StatusBadRequest, gin.H{"error": "failed to upload certificate"}) return } @@ -120,6 +142,12 @@ func (h *CertificateHandler) Upload(c *gin.Context) { } func (h *CertificateHandler) Delete(c *gin.Context) { + // Defense in depth - verify user context exists + if _, exists := c.Get("user"); !exists { + c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + return + } + idStr := c.Param("id") id, err := strconv.ParseUint(idStr, 10, 32) if err != nil { @@ -127,9 +155,16 @@ func (h *CertificateHandler) Delete(c *gin.Context) { return } + // Validate ID range + if id == 0 { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid id"}) + return + } + // Check if certificate is in use before proceeding inUse, err := h.service.IsCertificateInUse(uint(id)) if err != nil { + logger.Log().WithError(err).WithField("certificate_id", id).Error("failed to check certificate usage") c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check certificate usage"}) return } @@ -140,7 +175,17 @@ func (h *CertificateHandler) Delete(c *gin.Context) { // Create backup before deletion if h.backupService != nil { + // Check disk space before backup (require at least 100MB free) + if availableSpace, err := h.backupService.GetAvailableSpace(); err != nil { + logger.Log().WithError(err).Warn("unable to check disk space, proceeding with backup") + } else if availableSpace < 100*1024*1024 { + logger.Log().WithField("available_bytes", availableSpace).Warn("low disk space, skipping backup") + c.JSON(http.StatusInsufficientStorage, gin.H{"error": "insufficient disk space for backup"}) + return + } + if _, err := h.backupService.CreateBackup(); err != nil { + logger.Log().WithError(err).Error("failed to create backup before deletion") c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create backup before deletion"}) return } @@ -152,21 +197,31 @@ func (h *CertificateHandler) Delete(c *gin.Context) { c.JSON(http.StatusConflict, gin.H{"error": "certificate is in use by one or more proxy hosts"}) return } - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithError(err).WithField("certificate_id", id).Error("failed to delete certificate") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to delete certificate"}) return } - // Send Notification + // Send Notification with rate limiting (1 per cert per 10 seconds) if h.notificationService != nil { - h.notificationService.SendExternal(c.Request.Context(), - "cert", - "Certificate Deleted", - fmt.Sprintf("Certificate ID %d deleted", id), - map[string]interface{}{ - "ID": id, - "Action": "deleted", - }, - ) + h.notificationMu.Lock() + lastTime, exists := h.lastNotificationTime[uint(id)] + if !exists || time.Since(lastTime) > 10*time.Second { + h.lastNotificationTime[uint(id)] = time.Now() + h.notificationMu.Unlock() + h.notificationService.SendExternal(c.Request.Context(), + "cert", + "Certificate Deleted", + fmt.Sprintf("Certificate ID %d deleted", id), + map[string]interface{}{ + "ID": id, + "Action": "deleted", + }, + ) + } else { + h.notificationMu.Unlock() + logger.Log().WithField("certificate_id", id).Debug("notification rate limited") + } } c.JSON(http.StatusOK, gin.H{"message": "certificate deleted"}) diff --git a/backend/internal/api/handlers/certificate_handler_coverage_test.go b/backend/internal/api/handlers/certificate_handler_coverage_test.go index 21f93025..f6a00be7 100644 --- a/backend/internal/api/handlers/certificate_handler_coverage_test.go +++ b/backend/internal/api/handlers/certificate_handler_coverage_test.go @@ -21,6 +21,7 @@ func TestCertificateHandler_List_DBError(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.GET("/api/certificates", h.List) @@ -38,6 +39,7 @@ func TestCertificateHandler_Delete_InvalidID(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.DELETE("/api/certificates/:id", h.Delete) @@ -56,6 +58,7 @@ func TestCertificateHandler_Delete_NotFound(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.DELETE("/api/certificates/:id", h.Delete) @@ -78,6 +81,7 @@ func TestCertificateHandler_Delete_NoBackupService(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) // Wait for background sync goroutine to complete to avoid race with -race flag // NewCertificateService spawns a goroutine that immediately queries the DB @@ -115,6 +119,7 @@ func TestCertificateHandler_Delete_CheckUsageDBError(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.DELETE("/api/certificates/:id", h.Delete) @@ -137,6 +142,7 @@ func TestCertificateHandler_List_WithCertificates(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.GET("/api/certificates", h.List) diff --git a/backend/internal/api/handlers/certificate_handler_security_test.go b/backend/internal/api/handlers/certificate_handler_security_test.go new file mode 100644 index 00000000..f676cb57 --- /dev/null +++ b/backend/internal/api/handlers/certificate_handler_security_test.go @@ -0,0 +1,199 @@ +package handlers + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + + "github.com/Wikid82/charon/backend/internal/models" + "github.com/Wikid82/charon/backend/internal/services" +) + +// TestCertificateHandler_Delete_RequiresAuth tests that delete requires authentication +func TestCertificateHandler_Delete_RequiresAuth(t *testing.T) { + db, err := gorm.Open(sqlite.Open(fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name())), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + + if err := db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + + gin.SetMode(gin.TestMode) + r := gin.New() + // Note: NOT adding mockAuthMiddleware here to test auth requirement + svc := services.NewCertificateService("/tmp", db) + h := NewCertificateHandler(svc, nil, nil) + r.DELETE("/api/certificates/:id", h.Delete) + + req := httptest.NewRequest(http.MethodDelete, "/api/certificates/1", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 Unauthorized without auth, got %d", w.Code) + } +} + +// TestCertificateHandler_List_RequiresAuth tests that list requires authentication +func TestCertificateHandler_List_RequiresAuth(t *testing.T) { + db, err := gorm.Open(sqlite.Open(fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name())), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + + if err := db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + + gin.SetMode(gin.TestMode) + r := gin.New() + // Note: NOT adding mockAuthMiddleware here to test auth requirement + svc := services.NewCertificateService("/tmp", db) + h := NewCertificateHandler(svc, nil, nil) + r.GET("/api/certificates", h.List) + + req := httptest.NewRequest(http.MethodGet, "/api/certificates", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 Unauthorized without auth, got %d", w.Code) + } +} + +// TestCertificateHandler_Upload_RequiresAuth tests that upload requires authentication +func TestCertificateHandler_Upload_RequiresAuth(t *testing.T) { + db, err := gorm.Open(sqlite.Open(fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name())), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + + if err := db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + + gin.SetMode(gin.TestMode) + r := gin.New() + // Note: NOT adding mockAuthMiddleware here to test auth requirement + svc := services.NewCertificateService("/tmp", db) + h := NewCertificateHandler(svc, nil, nil) + r.POST("/api/certificates", h.Upload) + + req := httptest.NewRequest(http.MethodPost, "/api/certificates", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 Unauthorized without auth, got %d", w.Code) + } +} + +// TestCertificateHandler_Delete_DiskSpaceCheck tests the disk space check before backup +func TestCertificateHandler_Delete_DiskSpaceCheck(t *testing.T) { + db, err := gorm.Open(sqlite.Open(fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name())), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + + if err := db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + + // Create a certificate + cert := models.SSLCertificate{ + UUID: "test-cert", + Name: "test", + Provider: "custom", + Domains: "test.com", + } + if err := db.Create(&cert).Error; err != nil { + t.Fatalf("failed to create cert: %v", err) + } + + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(mockAuthMiddleware()) + svc := services.NewCertificateService("/tmp", db) + + // Mock backup service that reports low disk space + mockBackup := &mockBackupService{ + availableSpaceFunc: func() (int64, error) { + return 50 * 1024 * 1024, nil // 50MB (less than 100MB required) + }, + } + + h := NewCertificateHandler(svc, mockBackup, nil) + r.DELETE("/api/certificates/:id", h.Delete) + + req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/api/certificates/%d", cert.ID), nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusInsufficientStorage { + t.Fatalf("expected 507 Insufficient Storage with low disk space, got %d", w.Code) + } +} + +// TestCertificateHandler_Delete_NotificationRateLimiting tests rate limiting +func TestCertificateHandler_Delete_NotificationRateLimiting(t *testing.T) { + db, err := gorm.Open(sqlite.Open(fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name())), &gorm.Config{}) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + + if err := db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}); err != nil { + t.Fatalf("failed to migrate: %v", err) + } + + // Create certificates + cert1 := models.SSLCertificate{UUID: "test-1", Name: "test1", Provider: "custom", Domains: "test1.com"} + cert2 := models.SSLCertificate{UUID: "test-2", Name: "test2", Provider: "custom", Domains: "test2.com"} + if err := db.Create(&cert1).Error; err != nil { + t.Fatalf("failed to create cert1: %v", err) + } + if err := db.Create(&cert2).Error; err != nil { + t.Fatalf("failed to create cert2: %v", err) + } + + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(mockAuthMiddleware()) + svc := services.NewCertificateService("/tmp", db) + + mockBackup := &mockBackupService{ + createFunc: func() (string, error) { + return "backup.zip", nil + }, + } + + h := NewCertificateHandler(svc, mockBackup, nil) + r.DELETE("/api/certificates/:id", h.Delete) + + // Delete first cert + req1 := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/api/certificates/%d", cert1.ID), nil) + w1 := httptest.NewRecorder() + r.ServeHTTP(w1, req1) + + if w1.Code != http.StatusOK { + t.Fatalf("first delete failed: got %d", w1.Code) + } + + // Delete second cert (different ID, should not be rate limited) + req2 := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/api/certificates/%d", cert2.ID), nil) + w2 := httptest.NewRecorder() + r.ServeHTTP(w2, req2) + + if w2.Code != http.StatusOK { + t.Fatalf("second delete failed: got %d", w2.Code) + } + + // The test passes if both deletions succeed + // Rate limiting is per-certificate ID, so different certs should not interfere +} diff --git a/backend/internal/api/handlers/certificate_handler_test.go b/backend/internal/api/handlers/certificate_handler_test.go index 4b5f6e55..b72a4080 100644 --- a/backend/internal/api/handlers/certificate_handler_test.go +++ b/backend/internal/api/handlers/certificate_handler_test.go @@ -24,10 +24,20 @@ import ( "github.com/Wikid82/charon/backend/internal/services" ) +// mockAuthMiddleware adds a mock user to the context for testing +func mockAuthMiddleware() gin.HandlerFunc { + return func(c *gin.Context) { + c.Set("user", map[string]interface{}{"id": 1, "username": "testuser"}) + c.Next() + } +} + func setupCertTestRouter(t *testing.T, db *gorm.DB) *gin.Engine { t.Helper() gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) @@ -92,6 +102,7 @@ func TestDeleteCertificate_CreatesBackup(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) // Mock BackupService @@ -145,6 +156,7 @@ func TestDeleteCertificate_BackupFailure(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) // Mock BackupService that fails @@ -198,6 +210,7 @@ func TestDeleteCertificate_InUse_NoBackup(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) // Mock BackupService @@ -227,7 +240,8 @@ func TestDeleteCertificate_InUse_NoBackup(t *testing.T) { // Mock BackupService for testing type mockBackupService struct { - createFunc func() (string, error) + createFunc func() (string, error) + availableSpaceFunc func() (int64, error) } func (m *mockBackupService) CreateBackup() (string, error) { @@ -253,6 +267,14 @@ func (m *mockBackupService) RestoreBackup(filename string) error { return fmt.Errorf("not implemented") } +func (m *mockBackupService) GetAvailableSpace() (int64, error) { + if m.availableSpaceFunc != nil { + return m.availableSpaceFunc() + } + // Default: return 1GB available + return 1024 * 1024 * 1024, nil +} + // Test List handler func TestCertificateHandler_List(t *testing.T) { db, err := gorm.Open(sqlite.Open(fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name())), &gorm.Config{}) @@ -266,6 +288,8 @@ func TestCertificateHandler_List(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.GET("/api/certificates", h.List) @@ -292,6 +316,7 @@ func TestCertificateHandler_Upload_MissingName(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.POST("/api/certificates", h.Upload) @@ -319,6 +344,7 @@ func TestCertificateHandler_Upload_MissingCertFile(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.POST("/api/certificates", h.Upload) @@ -349,6 +375,7 @@ func TestCertificateHandler_Upload_MissingKeyFile(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) h := NewCertificateHandler(svc, nil, nil) r.POST("/api/certificates", h.Upload) @@ -376,6 +403,7 @@ func TestCertificateHandler_Upload_Success(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() + r.Use(mockAuthMiddleware()) // Create a mock CertificateService that returns a created certificate // Create a temporary services.CertificateService with a temp dir and DB diff --git a/backend/internal/api/routes/routes.go b/backend/internal/api/routes/routes.go index d850f4b8..00a6f913 100644 --- a/backend/internal/api/routes/routes.go +++ b/backend/internal/api/routes/routes.go @@ -315,9 +315,9 @@ func Register(router *gin.Engine, db *gorm.DB, cfg config.Config) error { logger.Log().WithField("caddy_data_dir", caddyDataDir).Info("Using Caddy data directory for certificates scan") certService := services.NewCertificateService(caddyDataDir, db) certHandler := handlers.NewCertificateHandler(certService, backupService, notificationService) - api.GET("/certificates", certHandler.List) - api.POST("/certificates", certHandler.Upload) - api.DELETE("/certificates/:id", certHandler.Delete) + protected.GET("/certificates", certHandler.List) + protected.POST("/certificates", certHandler.Upload) + protected.DELETE("/certificates/:id", certHandler.Delete) // Initial Caddy Config Sync go func() { diff --git a/backend/internal/services/backup_service.go b/backend/internal/services/backup_service.go index ccf803cf..c9788998 100644 --- a/backend/internal/services/backup_service.go +++ b/backend/internal/services/backup_service.go @@ -8,6 +8,7 @@ import ( "path/filepath" "sort" "strings" + "syscall" "time" "github.com/Wikid82/charon/backend/internal/config" @@ -267,3 +268,13 @@ func (s *BackupService) unzip(src, dest string) error { } return nil } + +// GetAvailableSpace returns the available disk space in bytes for the backup directory +func (s *BackupService) GetAvailableSpace() (int64, error) { + var stat syscall.Statfs_t + if err := syscall.Statfs(s.BackupDir, &stat); err != nil { + return 0, fmt.Errorf("failed to get disk space: %w", err) + } + // Available blocks * block size = available bytes + return int64(stat.Bavail) * int64(stat.Bsize), nil +} diff --git a/backend/internal/services/certificate_service_test.go b/backend/internal/services/certificate_service_test.go index 4838dfec..2d27b3c0 100644 --- a/backend/internal/services/certificate_service_test.go +++ b/backend/internal/services/certificate_service_test.go @@ -441,6 +441,36 @@ func TestCertificateService_DeleteCertificate_Errors(t *testing.T) { assert.Equal(t, gorm.ErrRecordNotFound, err) }) + t.Run("delete certificate in use returns ErrCertInUse", func(t *testing.T) { + // Create certificate + domain := "in-use.com" + expiry := time.Now().Add(24 * time.Hour) + certPEM := generateTestCert(t, domain, expiry) + cert, err := cs.UploadCertificate("In Use", string(certPEM), "FAKE KEY") + require.NoError(t, err) + + // Create proxy host using this certificate + ph := models.ProxyHost{ + UUID: "test-ph", + Name: "Test Host", + DomainNames: "in-use.com", + ForwardHost: "localhost", + ForwardPort: 8080, + CertificateID: &cert.ID, + } + require.NoError(t, db.Create(&ph).Error) + + // Attempt to delete certificate - should fail with ErrCertInUse + err = cs.DeleteCertificate(cert.ID) + assert.Error(t, err) + assert.Equal(t, ErrCertInUse, err) + + // Verify certificate still exists + var dbCert models.SSLCertificate + err = db.First(&dbCert, "id = ?", cert.ID).Error + assert.NoError(t, err) + }) + t.Run("delete certificate when file already removed", func(t *testing.T) { // Create and upload cert domain := "to-delete.com" @@ -741,6 +771,122 @@ func TestCertificateService_CertificateWithSANs(t *testing.T) { }) } +func TestCertificateService_IsCertificateInUse(t *testing.T) { + tmpDir := t.TempDir() + dsn := fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name()) + db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{})) + + cs := newTestCertificateService(tmpDir, db) + + t.Run("certificate not in use", func(t *testing.T) { + // Create certificate without any proxy hosts + domain := "unused.com" + expiry := time.Now().Add(24 * time.Hour) + certPEM := generateTestCert(t, domain, expiry) + cert, err := cs.UploadCertificate("Unused", string(certPEM), "FAKE KEY") + require.NoError(t, err) + + inUse, err := cs.IsCertificateInUse(cert.ID) + assert.NoError(t, err) + assert.False(t, inUse) + }) + + t.Run("certificate used by one proxy host", func(t *testing.T) { + // Create certificate + domain := "used.com" + expiry := time.Now().Add(24 * time.Hour) + certPEM := generateTestCert(t, domain, expiry) + cert, err := cs.UploadCertificate("Used", string(certPEM), "FAKE KEY") + require.NoError(t, err) + + // Create proxy host using this certificate + ph := models.ProxyHost{ + UUID: "ph-1", + Name: "Test Host 1", + DomainNames: "used.com", + ForwardHost: "localhost", + ForwardPort: 8080, + CertificateID: &cert.ID, + } + require.NoError(t, db.Create(&ph).Error) + + inUse, err := cs.IsCertificateInUse(cert.ID) + assert.NoError(t, err) + assert.True(t, inUse) + }) + + t.Run("certificate used by multiple proxy hosts", func(t *testing.T) { + // Create certificate + domain := "shared.com" + expiry := time.Now().Add(24 * time.Hour) + certPEM := generateTestCert(t, domain, expiry) + cert, err := cs.UploadCertificate("Shared", string(certPEM), "FAKE KEY") + require.NoError(t, err) + + // Create multiple proxy hosts using this certificate + for i := 1; i <= 3; i++ { + ph := models.ProxyHost{ + UUID: fmt.Sprintf("ph-shared-%d", i), + Name: fmt.Sprintf("Test Host %d", i), + DomainNames: fmt.Sprintf("host%d.shared.com", i), + ForwardHost: "localhost", + ForwardPort: 8080 + i, + CertificateID: &cert.ID, + } + require.NoError(t, db.Create(&ph).Error) + } + + inUse, err := cs.IsCertificateInUse(cert.ID) + assert.NoError(t, err) + assert.True(t, inUse) + }) + + t.Run("non-existent certificate", func(t *testing.T) { + inUse, err := cs.IsCertificateInUse(99999) + assert.NoError(t, err) // No error, just returns false + assert.False(t, inUse) + }) + + t.Run("certificate freed after proxy host deletion", func(t *testing.T) { + // Create certificate + domain := "freed.com" + expiry := time.Now().Add(24 * time.Hour) + certPEM := generateTestCert(t, domain, expiry) + cert, err := cs.UploadCertificate("Freed", string(certPEM), "FAKE KEY") + require.NoError(t, err) + + // Create proxy host using this certificate + ph := models.ProxyHost{ + UUID: "ph-freed", + Name: "Test Host Freed", + DomainNames: "freed.com", + ForwardHost: "localhost", + ForwardPort: 8080, + CertificateID: &cert.ID, + } + require.NoError(t, db.Create(&ph).Error) + + // Verify in use + inUse, err := cs.IsCertificateInUse(cert.ID) + assert.NoError(t, err) + assert.True(t, inUse) + + // Delete the proxy host + require.NoError(t, db.Delete(&ph).Error) + + // Verify no longer in use + inUse, err = cs.IsCertificateInUse(cert.ID) + assert.NoError(t, err) + assert.False(t, inUse) + + // Now deletion should succeed + err = cs.DeleteCertificate(cert.ID) + assert.NoError(t, err) + }) +} + func TestCertificateService_CacheBehavior(t *testing.T) { t.Run("cache returns consistent results", func(t *testing.T) { tmpDir := t.TempDir() diff --git a/docs/api.md b/docs/api.md index 9aff26d5..f4731de0 100644 --- a/docs/api.md +++ b/docs/api.md @@ -187,6 +187,95 @@ Response 200: `{ "deleted": true }` --- +### SSL Certificates + +#### List All Certificates + +```http +GET /certificates +``` + +**Response 200:** +```json +[ + { + "id": 1, + "uuid": "cert-uuid-123", + "name": "My Custom Cert", + "provider": "custom", + "domains": "example.com, www.example.com", + "expires_at": "2026-01-01T00:00:00Z", + "created_at": "2025-01-01T10:00:00Z" + } +] +``` + +#### Upload Certificate + +```http +POST /certificates/upload +Content-Type: multipart/form-data +``` + +**Request Body:** +- `name` (required) - Certificate name +- `certificate_file` (required) - Certificate file (.crt or .pem) +- `key_file` (required) - Private key file (.key or .pem) + +**Response 201:** +```json +{ + "id": 1, + "uuid": "cert-uuid-123", + "name": "My Custom Cert", + "provider": "custom", + "domains": "example.com" +} +``` + +#### Delete Certificate + +Delete a certificate. Requires that the certificate is not currently in use by any proxy hosts. + +```http +DELETE /certificates/:id +``` + +**Parameters:** +- `id` (path) - Certificate ID (numeric) + +**Response 200:** +```json +{ + "message": "certificate deleted" +} +``` + +**Response 400:** +```json +{ + "error": "invalid id" +} +``` + +**Response 409:** +```json +{ + "error": "certificate is in use by one or more proxy hosts" +} +``` + +**Response 500:** +```json +{ + "error": "failed to delete certificate" +} +``` + +**Note:** A backup is automatically created before deletion. The certificate files are removed from disk along with the database record. + +--- + ### Proxy Hosts #### List All Proxy Hosts diff --git a/docs/features.md b/docs/features.md index 2cf9ec57..2a176964 100644 --- a/docs/features.md +++ b/docs/features.md @@ -12,6 +12,28 @@ Here's everything Charon can do for you, explained simply. **What you do:** Nothing. Charon gets free certificates from Let's Encrypt and renews them automatically. +### Smart Certificate Cleanup + +**What it does:** When you delete websites, Charon asks if you want to delete unused certificates too. + +**Why you care:** Custom and staging certificates can pile up over time. This helps you keep things tidy. + +**How it works:** +- Delete a website → Charon checks if its certificate is used elsewhere +- If the certificate is custom or staging (not Let's Encrypt) and orphaned → you get a prompt +- Choose to keep or delete the certificate +- Default is "keep" (safe choice) + +**When it prompts:** +- ✅ Custom certificates you uploaded +- ✅ Staging certificates (for testing) +- ❌ Let's Encrypt certificates (managed automatically) + +**What you do:** +- See the prompt after clicking Delete on a proxy host +- Check the box if you want to delete the orphaned certificate +- Leave unchecked to keep the certificate (in case you need it later) + --- ## \ud83d\udee1\ufe0f Security (Optional) diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md new file mode 100644 index 00000000..3c3dbc04 --- /dev/null +++ b/docs/plans/current_spec.md @@ -0,0 +1,699 @@ +# Certificate Management Enhancement - Execution Plan + +**Issue**: The Certificates page has no actions for deleting certificates, and proxy host deletion doesn't prompt about certificate cleanup. + +**Date**: December 5, 2025 +**Status**: Planning Complete - Ready for Implementation + +--- + +## Overview + +This plan implements two related features: +1. **Certificate Deletion Actions**: Add delete buttons to the Certificates page actions column for expired/unused certificates +2. **Proxy Host Deletion Certificate Prompt**: When deleting a proxy host, prompt user to confirm deletion of the associated certificate (default: No) + +Both features prioritize user safety with confirmation dialogs, automatic backups, and sensible defaults. + +--- + +## Architecture Analysis + +### Current State + +**Backend**: +- Certificate model: `backend/internal/models/ssl_certificate.go` - SSLCertificate with ID, UUID, Name, Provider, Domains, etc. +- ProxyHost model: `backend/internal/models/proxy_host.go` - Has `CertificateID *uint` (nullable foreign key) and `Certificate *SSLCertificate` relationship +- Certificate service: `backend/internal/services/certificate_service.go` + - Already has `DeleteCertificate(id uint) error` method + - Already has `IsCertificateInUse(id uint) (bool, error)` - checks if cert is linked to any ProxyHost + - Returns `ErrCertInUse` error if certificate is in use +- Certificate handler: `backend/internal/api/handlers/certificate_handler.go` + - Already has `Delete(c *gin.Context)` endpoint at `DELETE /api/v1/certificates/:id` + - Creates backup before deletion (if backupService available) + - Checks if certificate is in use and returns 409 Conflict if so + - Returns appropriate error messages + +**Frontend**: +- CertificateList component: `frontend/src/components/CertificateList.tsx` + - Already checks if certificate is in use: `hosts.some(h => h.certificate_id === cert.id)` + - Already has delete button for custom and staging certificates + - Already shows appropriate confirmation messages + - Already creates backup before deletion +- ProxyHostForm: `frontend/src/components/ProxyHostForm.tsx` + - Certificate selector with dropdown showing available certificates + - No certificate deletion logic on proxy host deletion +- ProxyHosts page: `frontend/src/pages/ProxyHosts.tsx` + - Delete handler calls `deleteHost(uuid, deleteUptime?)` + - Currently prompts about uptime monitors but not certificates + +**Key Relationships**: +- One certificate can be used by multiple proxy hosts (one-to-many) +- Proxy hosts can have no certificate (certificate_id is nullable) +- Backend prevents deletion of certificates in use (409 Conflict) +- Frontend already checks usage and blocks deletion + +--- + +## Backend Requirements + +### Current Implementation is COMPLETE ✅ + +The backend already has all required functionality: + +1. ✅ **DELETE /api/v1/certificates/:id** endpoint exists +2. ✅ Certificate usage validation (`IsCertificateInUse`) +3. ✅ Backup creation before deletion +4. ✅ Proper error responses (400, 404, 409, 500) +5. ✅ Notification service integration +6. ✅ GORM relationship handling + +**No backend changes required** - the API fully supports certificate deletion with proper validation. + +### Proxy Host Deletion - No Changes Needed + +The proxy host deletion endpoint (`DELETE /api/v1/proxy-hosts/:uuid`) already: +- Deletes the proxy host +- GORM cascade rules handle the relationship cleanup +- Does NOT delete the certificate (certificate is shared resource) + +This is **correct behavior** - certificates should not be auto-deleted when a proxy host is removed, as they may be: +- Used by other proxy hosts +- Reusable for future proxy hosts +- Auto-managed by Let's Encrypt (shouldn't be manually deleted) + +**Frontend will handle certificate cleanup prompting** - no backend API changes needed. + +--- + +## Frontend Requirements + +### 1. Certificate Actions Column (Already Working) + +**Status**: ✅ **IMPLEMENTED** in `frontend/src/components/CertificateList.tsx` + +The actions column already shows delete buttons for: +- Custom certificates (`cert.provider === 'custom'`) +- Staging certificates (`cert.issuer?.toLowerCase().includes('staging')`) + +The delete logic already: +- Checks if certificate is in use by proxy hosts +- Shows appropriate confirmation messages +- Creates backup before deletion +- Handles errors properly + +**Current implementation is correct and complete.** + +### 2. Proxy Host Deletion Certificate Prompt (NEW FEATURE) + +**File**: `frontend/src/pages/ProxyHosts.tsx` +**Location**: `handleDelete` function (lines ~119-162) + +**Required Changes**: + +1. **Update `handleDelete` function** to check for associated certificates: + ```tsx + const handleDelete = async (uuid: string) => { + const host = hosts.find(h => h.uuid === uuid) + if (!host) return + + if (!confirm('Are you sure you want to delete this proxy host?')) return + + try { + // Check for uptime monitors (existing code) + let associatedMonitors: UptimeMonitor[] = [] + // ... existing uptime monitor logic ... + + // NEW: Check for associated certificate + let shouldDeleteCert = false + if (host.certificate_id && host.certificate) { + const cert = host.certificate + + // Check if this is the ONLY proxy host using this certificate + const otherHostsUsingCert = hosts.filter(h => + h.uuid !== uuid && h.certificate_id === host.certificate_id + ).length + + if (otherHostsUsingCert === 0) { + // This is the only host using the certificate + // Only prompt for custom/staging certs (not production Let's Encrypt) + if (cert.provider === 'custom' || cert.issuer?.toLowerCase().includes('staging')) { + shouldDeleteCert = confirm( + `This proxy host uses certificate "${cert.name || cert.domain}". ` + + `Do you want to delete the certificate as well?\n\n` + + `Click "Cancel" to keep the certificate (default).` + ) + } + } + } + + // Delete uptime monitors if confirmed (existing) + if (associatedMonitors.length > 0) { + const deleteUptime = confirm('...') + await deleteHost(uuid, deleteUptime) + } else { + await deleteHost(uuid) + } + + // NEW: Delete certificate if user confirmed + if (shouldDeleteCert && host.certificate_id) { + try { + await deleteCertificate(host.certificate_id) + toast.success('Proxy host and certificate deleted') + } catch (err) { + // Host is already deleted, just log cert deletion failure + toast.error(`Proxy host deleted but failed to delete certificate: ${err instanceof Error ? err.message : 'Unknown error'}`) + } + } + } catch (err) { + alert(err instanceof Error ? err.message : 'Failed to delete') + } + } + ``` + +2. **Import required API function**: + ```tsx + import { deleteCertificate } from '../api/certificates' + ``` + +3. **UI/UX Considerations**: + - Show certificate prompt AFTER proxy host deletion confirmation + - Default is "No" (Cancel button) - safer option + - Only prompt for custom/staging certificates (not production Let's Encrypt) + - Only prompt if this is the ONLY host using the certificate + - Certificate deletion happens AFTER host deletion (host must be removed first to pass backend validation) + - Show appropriate toast messages for both actions + +### 3. Bulk Proxy Host Deletion (Enhancement) + +**File**: `frontend/src/pages/ProxyHosts.tsx` +**Location**: `handleBulkDelete` function (lines ~204-242) + +**Required Changes** (similar pattern): + +```tsx +const handleBulkDelete = async () => { + const hostUUIDs = Array.from(selectedHosts) + setIsCreatingBackup(true) + + try { + // Create automatic backup (existing) + toast.loading('Creating backup before deletion...') + const backup = await createBackup() + toast.dismiss() + toast.success(`Backup created: ${backup.filename}`) + + // NEW: Collect certificates to potentially delete + const certsToConsider: Set = new Set() + hostUUIDs.forEach(uuid => { + const host = hosts.find(h => h.uuid === uuid) + if (host?.certificate_id && host.certificate) { + const cert = host.certificate + // Only consider custom/staging certs + if (cert.provider === 'custom' || cert.issuer?.toLowerCase().includes('staging')) { + // Check if this cert is ONLY used by hosts being deleted + const otherHosts = hosts.filter(h => + h.certificate_id === host.certificate_id && + !hostUUIDs.includes(h.uuid) + ) + if (otherHosts.length === 0) { + certsToConsider.add(host.certificate_id) + } + } + } + }) + + // NEW: Prompt for certificate deletion if any are orphaned + let shouldDeleteCerts = false + if (certsToConsider.size > 0) { + shouldDeleteCerts = confirm( + `${certsToConsider.size} certificate(s) will no longer be used after deleting these hosts. ` + + `Do you want to delete the unused certificates as well?\n\n` + + `Click "Cancel" to keep the certificates (default).` + ) + } + + // Delete each host (existing) + let deleted = 0 + let failed = 0 + for (const uuid of hostUUIDs) { + try { + await deleteHost(uuid) + deleted++ + } catch { + failed++ + } + } + + // NEW: Delete certificates if user confirmed + if (shouldDeleteCerts && certsToConsider.size > 0) { + let certsDeleted = 0 + let certsFailed = 0 + for (const certId of certsToConsider) { + try { + await deleteCertificate(certId) + certsDeleted++ + } catch { + certsFailed++ + } + } + + if (certsFailed > 0) { + toast.error(`Deleted ${deleted} host(s) and ${certsDeleted} certificate(s), ${certsFailed} certificate(s) failed`) + } else if (certsDeleted > 0) { + toast.success(`Deleted ${deleted} host(s) and ${certsDeleted} certificate(s)`) + } + } else { + // No certs deleted (existing logic) + if (failed > 0) { + toast.error(`Deleted ${deleted} host(s), ${failed} failed`) + } else { + toast.success(`Successfully deleted ${deleted} host(s). Backup available for restore.`) + } + } + + setSelectedHosts(new Set()) + setShowBulkDeleteModal(false) + } catch (err) { + toast.dismiss() + toast.error('Failed to create backup. Deletion cancelled.') + } finally { + setIsCreatingBackup(false) + } +} +``` + +--- + +## Testing Strategy + +### Backend Tests (Already Exist) ✅ + +Location: `backend/internal/api/handlers/certificate_handler_test.go` + +Existing tests cover: +- ✅ Delete certificate in use (409 Conflict) +- ✅ Delete certificate not in use (success with backup) +- ✅ Delete invalid ID (400 Bad Request) +- ✅ Delete non-existent certificate (404 Not Found) +- ✅ Delete without backup service (still succeeds) + +**No new backend tests required** - coverage is complete. + +### Frontend Tests (Need Updates) + +#### 1. CertificateList Component Tests ✅ +Location: `frontend/src/components/__tests__/CertificateList.test.tsx` + +Already has tests for: +- ✅ Delete custom certificate with confirmation +- ✅ Delete staging certificate +- ✅ Block deletion when certificate is in use +- ✅ Block deletion when certificate is active (valid/expiring) + +**Current tests are sufficient.** + +#### 2. ProxyHosts Component Tests (Need New Tests) +Location: `frontend/src/pages/__tests__/ProxyHosts-coverage.test.tsx` + +**New tests required**: + +```typescript +describe('ProxyHosts - Certificate Deletion Prompts', () => { + it('prompts to delete certificate when deleting proxy host with unique custom cert', async () => { + const cert = { id: 1, provider: 'custom', name: 'CustomCert', domain: 'test.com' } + const host = baseHost({ + uuid: 'h1', + name: 'Host1', + certificate_id: 1, + certificate: cert + }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([cert]) + + const confirmSpy = vi.spyOn(window, 'confirm') + .mockReturnValueOnce(true) // Confirm proxy host deletion + .mockReturnValueOnce(true) // Confirm certificate deletion + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByText('Delete') + await userEvent.click(deleteBtn) + + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(certificatesApi.deleteCertificate).toHaveBeenCalledWith(1) + }) + + confirmSpy.mockRestore() + }) + + it('does NOT prompt for certificate deletion when cert is shared by multiple hosts', async () => { + const cert = { id: 1, provider: 'custom', name: 'SharedCert' } + const host1 = baseHost({ uuid: 'h1', certificate_id: 1, certificate: cert }) + const host2 = baseHost({ uuid: 'h2', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host1, host2]) + + const confirmSpy = vi.spyOn(window, 'confirm') + .mockReturnValueOnce(true) // Only asked once (proxy host deletion) + + renderWithProviders() + await waitFor(() => expect(screen.getByText(host1.name)).toBeTruthy()) + + const deleteBtn = screen.getAllByText('Delete')[0] + await userEvent.click(deleteBtn) + + await waitFor(() => expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1')) + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + expect(confirmSpy).toHaveBeenCalledTimes(1) // Only proxy host confirmation + + confirmSpy.mockRestore() + }) + + it('does NOT prompt for production Let\'s Encrypt certificates', async () => { + const cert = { id: 1, provider: 'letsencrypt', issuer: 'Let\'s Encrypt', name: 'LE Prod' } + const host = baseHost({ uuid: 'h1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + + const confirmSpy = vi.spyOn(window, 'confirm') + .mockReturnValueOnce(true) // Only proxy host deletion + + renderWithProviders() + await waitFor(() => expect(screen.getByText(host.name)).toBeTruthy()) + + const deleteBtn = screen.getByText('Delete') + await userEvent.click(deleteBtn) + + expect(confirmSpy).toHaveBeenCalledTimes(1) // No cert prompt + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + + confirmSpy.mockRestore() + }) + + it('prompts for staging certificates', async () => { + const cert = { + id: 1, + provider: 'letsencrypt-staging', + issuer: 'Let\'s Encrypt Staging', + name: 'Staging Cert' + } + const host = baseHost({ uuid: 'h1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + + const confirmSpy = vi.spyOn(window, 'confirm') + .mockReturnValueOnce(true) // Proxy host deletion + .mockReturnValueOnce(false) // Decline certificate deletion (default) + + renderWithProviders() + await waitFor(() => expect(screen.getByText(host.name)).toBeTruthy()) + + const deleteBtn = screen.getByText('Delete') + await userEvent.click(deleteBtn) + + await waitFor(() => expect(confirmSpy).toHaveBeenCalledTimes(2)) + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + + confirmSpy.mockRestore() + }) + + it('handles certificate deletion failure gracefully', async () => { + const cert = { id: 1, provider: 'custom', name: 'CustomCert' } + const host = baseHost({ uuid: 'h1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + vi.mocked(certificatesApi.deleteCertificate).mockRejectedValue( + new Error('Certificate is still in use') + ) + + const confirmSpy = vi.spyOn(window, 'confirm') + .mockReturnValueOnce(true) // Proxy host + .mockReturnValueOnce(true) // Certificate + + renderWithProviders() + await waitFor(() => expect(screen.getByText(host.name)).toBeTruthy()) + + const deleteBtn = screen.getByText('Delete') + await userEvent.click(deleteBtn) + + await waitFor(() => { + expect(toast.error).toHaveBeenCalledWith( + expect.stringContaining('failed to delete certificate') + ) + }) + + confirmSpy.mockRestore() + }) + + it('bulk delete prompts for orphaned certificates', async () => { + const cert = { id: 1, provider: 'custom', name: 'BulkCert' } + const host1 = baseHost({ uuid: 'h1', certificate_id: 1, certificate: cert }) + const host2 = baseHost({ uuid: 'h2', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host1, host2]) + vi.mocked(backupsApi.createBackup).mockResolvedValue({ filename: 'backup.db' }) + + const confirmSpy = vi.spyOn(window, 'confirm') + .mockReturnValueOnce(true) // Confirm bulk delete modal + .mockReturnValueOnce(true) // Confirm certificate deletion + + renderWithProviders() + await waitFor(() => expect(screen.getByText(host1.name)).toBeTruthy()) + + // Select both hosts + const checkboxes = screen.getAllByRole('checkbox') + await userEvent.click(checkboxes[0]) // Select all + + // Click bulk delete + const bulkDeleteBtn = screen.getByText('Delete') + await userEvent.click(bulkDeleteBtn) + + // Confirm in modal + await userEvent.click(screen.getByText('Delete Permanently')) + + await waitFor(() => { + expect(confirmSpy).toHaveBeenCalledTimes(2) + expect(certificatesApi.deleteCertificate).toHaveBeenCalledWith(1) + }) + + confirmSpy.mockRestore() + }) +}) +``` + +#### 3. Integration Tests + +**Manual Testing Checklist**: +- [ ] Delete custom certificate from Certificates page +- [ ] Attempt to delete certificate in use (should show error) +- [ ] Delete proxy host with unique custom certificate (should prompt) +- [ ] Delete proxy host with shared certificate (should NOT prompt) +- [ ] Delete proxy host with production Let's Encrypt cert (should NOT prompt) +- [ ] Delete proxy host with staging certificate (should prompt) +- [ ] Decline certificate deletion (default) - only host deleted +- [ ] Accept certificate deletion - both deleted +- [ ] Bulk delete hosts with orphaned certificates +- [ ] Verify backups are created before deletions +- [ ] Check certificate deletion failure doesn't block host deletion + +--- + +## Security Considerations + +### Authorization +- ✅ All certificate endpoints protected by authentication middleware +- ✅ All proxy host endpoints protected by authentication middleware +- ✅ Only authenticated users can delete resources + +### Validation +- ✅ Backend validates certificate not in use before deletion (409 Conflict) +- ✅ Backend validates certificate ID is numeric and exists (400/404) +- ✅ Frontend checks certificate usage before allowing deletion +- ✅ Frontend validates proxy host UUID before deletion + +### Data Protection +- ✅ Automatic backup created before all deletions +- ✅ Soft deletes NOT used (certificates are fully removed) +- ✅ File system cleanup for Let's Encrypt certificates +- ✅ Database cascade rules properly configured + +### User Safety +- ✅ Confirmation dialogs required for all deletions +- ✅ Certificate deletion default is "No" (safer) +- ✅ Clear messaging about what will be deleted +- ✅ Descriptive toast messages for success/failure +- ✅ Only prompt for custom/staging certs (not production) +- ✅ Only prompt when certificate is orphaned (no other hosts) + +### Error Handling +- ✅ Graceful handling of certificate deletion failures +- ✅ Host deletion succeeds even if cert deletion fails +- ✅ Appropriate error messages shown to user +- ✅ Failed deletions don't block other operations + +--- + +## Implementation Order + +### Phase 1: Certificate Actions Column ✅ +**Status**: COMPLETE - Already implemented correctly + +No changes needed. + +### Phase 2: Single Proxy Host Deletion Certificate Prompt +**Priority**: HIGH +**Estimated Time**: 2 hours + +1. Update `frontend/src/pages/ProxyHosts.tsx`: + - Modify `handleDelete` function to check for certificates + - Add certificate deletion prompt logic + - Handle certificate deletion after host deletion + - Import `deleteCertificate` API function + +2. Write unit tests in `frontend/src/pages/__tests__/ProxyHosts-coverage.test.tsx`: + - Test certificate prompt for unique custom cert + - Test no prompt for shared certificate + - Test no prompt for production Let's Encrypt + - Test prompt for staging certificates + - Test default "No" behavior + - Test certificate deletion failure handling + +3. Manual testing: + - Test all scenarios in Testing Strategy checklist + - Verify toast messages are clear + - Verify backups are created + - Test error cases + +### Phase 3: Bulk Proxy Host Deletion Certificate Prompt +**Priority**: MEDIUM +**Estimated Time**: 2 hours + +1. Update `frontend/src/pages/ProxyHosts.tsx`: + - Modify `handleBulkDelete` function + - Add logic to identify orphaned certificates + - Add certificate deletion prompt + - Handle bulk certificate deletion + +2. Write unit tests: + - Test bulk deletion with orphaned certificates + - Test bulk deletion with shared certificates + - Test mixed scenarios + +3. Manual testing: + - Bulk delete scenarios + - Multiple certificate handling + - Error recovery + +### Phase 4: Documentation & Polish +**Priority**: LOW +**Estimated Time**: 1 hour + +1. Update `docs/features.md`: + - Document certificate deletion feature + - Document proxy host certificate cleanup + +2. Update `docs/api.md` (if needed): + - Verify certificate deletion endpoint documented + +3. Code review: + - Review all changes + - Ensure consistent error messages + - Verify test coverage + +--- + +## Risk Assessment + +### Low Risk ✅ +- Backend API already exists and is well-tested +- Certificate deletion already works correctly +- Backup system already in place +- Frontend certificate list already handles deletion + +### Medium Risk ⚠️ +- User confusion about certificate deletion prompts + - **Mitigation**: Clear messaging, sensible defaults (No), only prompt for custom/staging +- Race conditions with shared certificates + - **Mitigation**: Check certificate usage at deletion time (backend validation) +- Certificate deletion failure after host deleted + - **Mitigation**: Graceful error handling, informative toast messages + +### No Risk ❌ +- Data loss: Backups created before all deletions +- Accidental deletion: Multiple confirmations required +- Production certs: Never prompted for deletion + +--- + +## Success Criteria + +### Must Have ✅ +1. Certificate delete buttons visible in Certificates page actions column +2. Delete buttons only shown for custom and staging certificates +3. Certificate deletion blocked if in use by any proxy host +4. Automatic backup created before certificate deletion +5. Proxy host deletion prompts for certificate cleanup (default: No) +6. Certificate prompt only shown for custom/staging certs +7. Certificate prompt only shown when orphaned (no other hosts) +8. All operations have clear confirmation dialogs +9. All operations show appropriate toast messages +10. Backend validation prevents invalid deletions + +### Nice to Have ✨ +1. Show certificate usage count in Certificates table +2. Highlight orphaned certificates in the list +3. Batch certificate cleanup tool +4. Certificate expiry warnings before deletion + +--- + +## Open Questions + +1. ✅ Should production Let's Encrypt certificates ever be manually deletable? + - **Answer**: No, they are auto-managed by Caddy + +2. ✅ Should certificate deletion be allowed if status is "valid" or "expiring"? + - **Answer**: Yes, if not in use (user may want to replace) + +3. ✅ What happens if certificate deletion fails after host is deleted? + - **Answer**: Show error toast, certificate remains, user can delete later + +4. ✅ Should bulk host deletion prompt for each certificate individually? + - **Answer**: No, single prompt for all orphaned certificates + +--- + +## Notes + +- Certificate deletion is a **shared resource operation** - multiple hosts can use the same certificate +- The backend correctly prevents deletion of in-use certificates (409 Conflict) +- The frontend already has all the UI components and logic needed +- Focus is on **adding prompts** to the proxy host deletion flow +- Default behavior is **conservative** (don't delete certificates) for safety +- Only custom and staging certificates are considered for cleanup +- Production Let's Encrypt certificates should never be manually deleted + +--- + +## Definition of Done + +- [ ] Certificate delete buttons visible and functional +- [ ] Proxy host deletion prompts for certificate cleanup +- [ ] All confirmation dialogs use appropriate defaults +- [ ] Unit tests written and passing +- [ ] Manual testing completed +- [ ] Documentation updated +- [ ] Code reviewed +- [ ] No console errors or warnings +- [ ] Pre-commit checks pass +- [ ] Feature tested in local Docker environment + +--- + +**Plan Created**: December 5, 2025 +**Plan Author**: Planning Agent (Architect) +**Ready for Implementation**: ✅ YES diff --git a/docs/security.md b/docs/security.md index e7ce484d..4a396747 100644 --- a/docs/security.md +++ b/docs/security.md @@ -129,6 +129,27 @@ Now only devices on `192.168.x.x` or `10.x.x.x` can access it. The public intern --- +## Certificate Management Security + +**What it protects:** Certificate deletion is a destructive operation that requires proper authorization. + +**How it works:** +- Certificates cannot be deleted while in use by proxy hosts (conflict error) +- Automatic backup is created before any certificate deletion +- Authentication required (when auth is implemented) + +**Backup & Recovery:** +- Every certificate deletion triggers an automatic backup +- Find backups in the "Backups" page +- Restore from backup if you accidentally delete the wrong certificate + +**Best Practice:** +- Review which proxy hosts use a certificate before deleting it +- When deleting proxy hosts, use the cleanup prompt to delete orphaned certificates +- Keep custom certificates you might reuse later + +--- + ## Don't Lock Yourself Out! **Problem:** If you turn on security and misconfigure it, you might block yourself. diff --git a/frontend/src/components/dialogs/CertificateCleanupDialog.tsx b/frontend/src/components/dialogs/CertificateCleanupDialog.tsx new file mode 100644 index 00000000..0c087bdc --- /dev/null +++ b/frontend/src/components/dialogs/CertificateCleanupDialog.tsx @@ -0,0 +1,117 @@ +import { AlertTriangle } from 'lucide-react' + +interface CertificateCleanupDialogProps { + onConfirm: (deleteCerts: boolean) => void + onCancel: () => void + certificates: Array<{ id: number; name: string; domain: string }> + hostNames: string[] + isBulk?: boolean +} + +export default function CertificateCleanupDialog({ + onConfirm, + onCancel, + certificates, + hostNames, + isBulk = false +}: CertificateCleanupDialogProps) { + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault() + const formData = new FormData(e.currentTarget) + const deleteCerts = formData.get('delete_certs') === 'on' + onConfirm(deleteCerts) + } + + return ( +
+
e.stopPropagation()} + > +
+
+
+ +
+
+

+ Delete {isBulk ? `${hostNames.length} Proxy Hosts` : 'Proxy Host'}? +

+

+ {isBulk ? 'These hosts will be permanently deleted.' : 'This host will be permanently deleted.'} +

+
+
+ + {/* Host names */} +
+

+ {isBulk ? 'Hosts to be deleted:' : 'Host to be deleted:'} +

+
    + {hostNames.map((name, idx) => ( +
  • + • + {name} +
    • + ))} +
+
+ + {/* Certificate cleanup option */} + {certificates.length > 0 && ( +
+
+ +
+ +

+ {certificates.length === 1 + ? 'This custom/staging certificate will no longer be used by any hosts.' + : 'These custom/staging certificates will no longer be used by any hosts.'} +

+
    + {certificates.map((cert) => ( +
  • + → + {cert.name || cert.domain} + ({cert.domain}) +
    • + ))} +
+
+
+
+ )} + + {/* Confirmation buttons */} +
+ + +
+
+
+
+ ) +} diff --git a/frontend/src/pages/ProxyHosts.tsx b/frontend/src/pages/ProxyHosts.tsx index 79995e14..dc53b8f4 100644 --- a/frontend/src/pages/ProxyHosts.tsx +++ b/frontend/src/pages/ProxyHosts.tsx @@ -7,6 +7,7 @@ import { useCertificates } from '../hooks/useCertificates' import { useAccessLists } from '../hooks/useAccessLists' import { getSettings } from '../api/settings' import { createBackup } from '../api/backups' +import { deleteCertificate } from '../api/certificates' import type { ProxyHost } from '../api/proxyHosts' import compareHosts from '../utils/compareHosts' import type { AccessList } from '../api/accessLists' @@ -15,6 +16,7 @@ import { Switch } from '../components/ui/Switch' import { toast } from 'react-hot-toast' import { formatSettingLabel, settingHelpText, applyBulkSettingsToHosts } from '../utils/proxyHostsHelpers' import { ConfigReloadOverlay } from '../components/LoadingStates' +import CertificateCleanupDialog from '../components/dialogs/CertificateCleanupDialog' // Helper functions extracted for unit testing and reuse // Helpers moved to ../utils/proxyHostsHelpers to keep component files component-only for fast refresh @@ -35,6 +37,13 @@ export default function ProxyHosts() { const [showBulkApplyModal, setShowBulkApplyModal] = useState(false) const [showBulkDeleteModal, setShowBulkDeleteModal] = useState(false) const [isCreatingBackup, setIsCreatingBackup] = useState(false) + const [showCertCleanupDialog, setShowCertCleanupDialog] = useState(false) + const [certCleanupData, setCertCleanupData] = useState<{ + hostUUIDs: string[] + hostNames: string[] + certificates: Array<{ id: number; name: string; domain: string }> + isBulk: boolean + } | null>(null) const [selectedACLs, setSelectedACLs] = useState>(new Set()) const [bulkACLAction, setBulkACLAction] = useState<'apply' | 'remove'>('apply') const [applyProgress, setApplyProgress] = useState<{ current: number; total: number } | null>(null) @@ -139,6 +148,44 @@ export default function ProxyHosts() { const host = hosts.find(h => h.uuid === uuid) if (!host) return + // Check for orphaned certificates that would need cleanup + const orphanedCerts: Array<{ id: number; name: string; domain: string }> = [] + + if (host.certificate_id && host.certificate) { + const cert = host.certificate + + // Check if this is the ONLY proxy host using this certificate + const otherHostsUsingCert = hosts.filter(h => + h.uuid !== uuid && h.certificate_id === host.certificate_id + ).length + + if (otherHostsUsingCert === 0) { + // This is the only host using the certificate + // Only consider custom/staging certs (not production Let's Encrypt) + const isCustomOrStaging = cert.provider === 'custom' || cert.provider?.toLowerCase().includes('staging') + if (isCustomOrStaging) { + orphanedCerts.push({ + id: cert.id!, + name: cert.name || '', + domain: cert.domains + }) + } + } + } + + // If there are orphaned certificates, show cleanup dialog + if (orphanedCerts.length > 0) { + setCertCleanupData({ + hostUUIDs: [uuid], + hostNames: [host.name || host.domain_names], + certificates: orphanedCerts, + isBulk: false + }) + setShowCertCleanupDialog(true) + return + } + + // No orphaned certificates, proceed with standard deletion if (!confirm('Are you sure you want to delete this proxy host?')) return try { @@ -162,6 +209,95 @@ export default function ProxyHosts() { } } + const handleCertCleanupConfirm = async (deleteCerts: boolean) => { + if (!certCleanupData) return + + setShowCertCleanupDialog(false) + + try { + // Delete hosts first + if (certCleanupData.isBulk) { + // Bulk deletion + let deleted = 0 + let failed = 0 + + for (const uuid of certCleanupData.hostUUIDs) { + try { + await deleteHost(uuid) + deleted++ + } catch { + failed++ + } + } + + // Delete certificates if user confirmed + if (deleteCerts && certCleanupData.certificates.length > 0) { + let certsDeleted = 0 + let certsFailed = 0 + + for (const cert of certCleanupData.certificates) { + try { + await deleteCertificate(cert.id) + certsDeleted++ + } catch { + certsFailed++ + } + } + + if (certsFailed > 0) { + toast.error(`Deleted ${deleted} host(s) and ${certsDeleted} certificate(s), ${certsFailed} certificate(s) failed`) + } else { + toast.success(`Deleted ${deleted} host(s) and ${certsDeleted} certificate(s)`) + } + } else { + if (failed > 0) { + toast.error(`Deleted ${deleted} host(s), ${failed} failed`) + } else { + toast.success(`Successfully deleted ${deleted} host(s)`) + } + } + } else { + // Single deletion + const uuid = certCleanupData.hostUUIDs[0] + const host = hosts.find(h => h.uuid === uuid) + + // Check for uptime monitors + let associatedMonitors: UptimeMonitor[] = [] + try { + const monitors = await getMonitors() + associatedMonitors = monitors.filter(m => + host && (m.upstream_host === host.forward_host || (m.proxy_host_id && m.proxy_host_id === (host as unknown as { id?: number }).id)) + ) + } catch { + // ignore errors + } + + if (associatedMonitors.length > 0) { + const deleteUptime = confirm('This proxy host has uptime monitors associated with it. Delete the monitors as well?') + await deleteHost(uuid, deleteUptime) + } else { + await deleteHost(uuid) + } + + // Delete certificate if user confirmed + if (deleteCerts && certCleanupData.certificates.length > 0) { + try { + await deleteCertificate(certCleanupData.certificates[0].id) + toast.success('Proxy host and certificate deleted') + } catch (err) { + toast.error(`Proxy host deleted but failed to delete certificate: ${err instanceof Error ? err.message : 'Unknown error'}`) + } + } + } + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Failed to delete') + } finally { + setCertCleanupData(null) + setSelectedHosts(new Set()) + setShowBulkDeleteModal(false) + } + } + const toggleHostSelection = (uuid: string) => { setSelectedHosts(prev => { const next = new Set(prev) @@ -212,7 +348,51 @@ export default function ProxyHosts() { toast.dismiss() toast.success(`Backup created: ${backup.filename}`) - // Delete each host + // Collect certificates to potentially delete + const certsToConsider: Map = new Map() + + hostUUIDs.forEach(uuid => { + const host = hosts.find(h => h.uuid === uuid) + if (host?.certificate_id && host.certificate) { + const cert = host.certificate + // Only consider custom/staging certs + const isCustomOrStaging = cert.provider === 'custom' || cert.provider?.toLowerCase().includes('staging') + if (isCustomOrStaging) { + // Check if this cert is ONLY used by hosts being deleted + const otherHosts = hosts.filter(h => + h.certificate_id === host.certificate_id && + !hostUUIDs.includes(h.uuid) + ) + if (otherHosts.length === 0 && cert.id) { + certsToConsider.set(cert.id, { + id: cert.id, + name: cert.name || '', + domain: cert.domains + }) + } + } + } + }) + + // If there are orphaned certificates, show cleanup dialog + if (certsToConsider.size > 0) { + const hostNames = hostUUIDs.map(uuid => { + const host = hosts.find(h => h.uuid === uuid) + return host?.name || host?.domain_names || 'Unnamed' + }) + + setCertCleanupData({ + hostUUIDs, + hostNames, + certificates: Array.from(certsToConsider.values()), + isBulk: true + }) + setShowCertCleanupDialog(true) + setIsCreatingBackup(false) + return + } + + // No orphaned certificates, proceed with deletion let deleted = 0 let failed = 0 @@ -908,6 +1088,20 @@ export default function ProxyHosts() { )} + + {/* Certificate Cleanup Dialog */} + {showCertCleanupDialog && certCleanupData && ( + { + setShowCertCleanupDialog(false) + setCertCleanupData(null) + }} + certificates={certCleanupData.certificates} + hostNames={certCleanupData.hostNames} + isBulk={certCleanupData.isBulk} + /> + )} ) diff --git a/frontend/src/pages/__tests__/ProxyHosts-cert-cleanup.test.tsx b/frontend/src/pages/__tests__/ProxyHosts-cert-cleanup.test.tsx new file mode 100644 index 00000000..1d8042d2 --- /dev/null +++ b/frontend/src/pages/__tests__/ProxyHosts-cert-cleanup.test.tsx @@ -0,0 +1,442 @@ +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { MemoryRouter } from 'react-router-dom' +import { vi, describe, it, expect, beforeEach } from 'vitest' +import type { ProxyHost, Certificate } from '../../api/proxyHosts' +import ProxyHosts from '../ProxyHosts' +import * as proxyHostsApi from '../../api/proxyHosts' +import * as certificatesApi from '../../api/certificates' +import * as accessListsApi from '../../api/accessLists' +import * as settingsApi from '../../api/settings' +import * as uptimeApi from '../../api/uptime' +import * as backupsApi from '../../api/backups' +import { createMockProxyHost } from '../../testUtils/createMockProxyHost' + +vi.mock('react-hot-toast', () => ({ + toast: { + success: vi.fn(), + error: vi.fn(), + loading: vi.fn(), + dismiss: vi.fn() + } +})) + +vi.mock('../../api/proxyHosts', () => ({ + getProxyHosts: vi.fn(), + createProxyHost: vi.fn(), + updateProxyHost: vi.fn(), + deleteProxyHost: vi.fn(), + bulkUpdateACL: vi.fn(), + testProxyHostConnection: vi.fn(), +})) + +vi.mock('../../api/certificates', () => ({ + getCertificates: vi.fn(), + deleteCertificate: vi.fn(), +})) +vi.mock('../../api/accessLists', () => ({ accessListsApi: { list: vi.fn() } })) +vi.mock('../../api/settings', () => ({ getSettings: vi.fn() })) +vi.mock('../../api/backups', () => ({ createBackup: vi.fn() })) +vi.mock('../../api/uptime', () => ({ getMonitors: vi.fn() })) + +const createQueryClient = () => new QueryClient({ + defaultOptions: { + queries: { retry: false, gcTime: 0 }, + mutations: { retry: false } + } +}) + +const renderWithProviders = (ui: React.ReactNode) => { + const queryClient = createQueryClient() + return render( + + {ui} + + ) +} + +const baseHost = (overrides: Partial = {}) => createMockProxyHost(overrides) + +describe('ProxyHosts - Certificate Cleanup Prompts', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(accessListsApi.accessListsApi.list).mockResolvedValue([]) + vi.mocked(settingsApi.getSettings).mockResolvedValue({}) + vi.mocked(uptimeApi.getMonitors).mockResolvedValue([]) + vi.mocked(backupsApi.createBackup).mockResolvedValue({ filename: 'backup.db' } as any) + }) + + it('prompts to delete certificate when deleting proxy host with unique custom cert', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'CustomCert', + domains: 'test.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host = baseHost({ + uuid: 'h1', + name: 'Host1', + certificate_id: 1, + certificate: cert + }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + vi.mocked(certificatesApi.deleteCertificate).mockResolvedValue() + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByRole('button', { name: /delete/i }) + await userEvent.click(deleteBtn) + + // Certificate cleanup dialog should appear + await waitFor(() => { + expect(screen.getByText('Delete Proxy Host?')).toBeTruthy() + expect(screen.getByText(/Also delete.*orphaned certificate/i)).toBeTruthy() + expect(screen.getByText('CustomCert')).toBeTruthy() + }) + + // Checkbox for certificate deletion (should be unchecked by default) + const checkbox = screen.getByRole('checkbox', { name: /Also delete/i }) as HTMLInputElement + expect(checkbox.checked).toBe(false) + + // Check the checkbox to delete certificate + await userEvent.click(checkbox) + + // Confirm deletion - get all Delete buttons and use the one in the dialog (last one) + const deleteButtons = screen.getAllByRole('button', { name: 'Delete' }) + await userEvent.click(deleteButtons[deleteButtons.length - 1]) + + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(certificatesApi.deleteCertificate).toHaveBeenCalledWith(1) + }) + }) + + it('does NOT prompt for certificate deletion when cert is shared by multiple hosts', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'SharedCert', + domains: 'shared.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host1 = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + const host2 = baseHost({ uuid: 'h2', name: 'Host2', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host1, host2]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + + const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true) + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteButtons = screen.getAllByRole('button', { name: /delete/i }) + await userEvent.click(deleteButtons[0]) + + // Should show standard confirmation, not certificate cleanup dialog + await waitFor(() => expect(confirmSpy).toHaveBeenCalledWith('Are you sure you want to delete this proxy host?')) + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + + confirmSpy.mockRestore() + }) + + it('does NOT prompt for production Let\'s Encrypt certificates', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'letsencrypt', + name: 'LE Prod', + domains: 'prod.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + + const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true) + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByRole('button', { name: /delete/i }) + await userEvent.click(deleteBtn) + + // Should show standard confirmation only + await waitFor(() => expect(confirmSpy).toHaveBeenCalledTimes(1)) + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + + confirmSpy.mockRestore() + }) + + it('prompts for staging certificates', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'letsencrypt-staging', + name: 'Staging Cert', + domains: 'staging.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByRole('button', { name: /delete/i }) + await userEvent.click(deleteBtn) + + // Certificate cleanup dialog should appear + await waitFor(() => { + expect(screen.getByText('Delete Proxy Host?')).toBeTruthy() + expect(screen.getByText(/Also delete.*orphaned certificate/i)).toBeTruthy() + }) + + // Decline certificate deletion (click Delete without checking the box) + const deleteButtons = screen.getAllByRole('button', { name: 'Delete' }) + await userEvent.click(deleteButtons[deleteButtons.length - 1]) + + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + }) + }) + + it('handles certificate deletion failure gracefully', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'CustomCert', + domains: 'custom.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + vi.mocked(certificatesApi.deleteCertificate).mockRejectedValue( + new Error('Certificate is still in use') + ) + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByRole('button', { name: /delete/i }) + await userEvent.click(deleteBtn) + + await waitFor(() => expect(screen.getByText('Delete Proxy Host?')).toBeTruthy()) + + // Check the certificate deletion checkbox + const checkbox = screen.getByRole('checkbox', { name: /Also delete/i }) + await userEvent.click(checkbox) + + // Confirm deletion + const deleteButtons = screen.getAllByRole('button', { name: 'Delete' }) + await userEvent.click(deleteButtons[deleteButtons.length - 1]) + + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(certificatesApi.deleteCertificate).toHaveBeenCalledWith(1) + }) + + // Toast should show error about certificate but host was deleted + const toast = await import('react-hot-toast') + await waitFor(() => { + expect(toast.toast.error).toHaveBeenCalledWith( + expect.stringContaining('failed to delete certificate') + ) + }) + }) + + it('bulk delete prompts for orphaned certificates', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'BulkCert', + domains: 'bulk.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host1 = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + const host2 = baseHost({ uuid: 'h2', name: 'Host2', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host1, host2]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + vi.mocked(certificatesApi.deleteCertificate).mockResolvedValue() + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + // Select all hosts + const selectAllCheckbox = screen.getAllByRole('checkbox')[0] + await userEvent.click(selectAllCheckbox) + + // Click bulk delete button (the one with Trash icon in toolbar) + const bulkDeleteButtons = screen.getAllByRole('button', { name: /delete/i }) + await userEvent.click(bulkDeleteButtons[0]) // First is the bulk delete button in the toolbar + + // Confirm in bulk delete modal + await waitFor(() => expect(screen.getByText(/Delete 2 Proxy Hosts/)).toBeTruthy()) + const deletePermBtn = screen.getByRole('button', { name: /Delete Permanently/i }) + await userEvent.click(deletePermBtn) + + // Should show certificate cleanup dialog + await waitFor(() => { + expect(screen.getByText(/Also delete.*orphaned certificate/i)).toBeTruthy() + expect(screen.getByText('BulkCert')).toBeTruthy() + }) + + // Check the certificate deletion checkbox + const certCheckbox = screen.getByRole('checkbox', { name: /Also delete/i }) + await userEvent.click(certCheckbox) + + // Confirm + const deleteButtons = screen.getAllByRole('button', { name: 'Delete' }) + await userEvent.click(deleteButtons[deleteButtons.length - 1]) + + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h2') + expect(certificatesApi.deleteCertificate).toHaveBeenCalledWith(1) + }) + }) + + it('bulk delete does NOT prompt when certificate is still used by other hosts', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'SharedCert', + domains: 'shared.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host1 = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + const host2 = baseHost({ uuid: 'h2', name: 'Host2', certificate_id: 1, certificate: cert }) + const host3 = baseHost({ uuid: 'h3', name: 'Host3', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host1, host2, host3]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + // Select only host1 and host2 (host3 still uses the cert) + const host1Row = screen.getByText('Host1').closest('tr') as HTMLTableRowElement + const host2Row = screen.getByText('Host2').closest('tr') as HTMLTableRowElement + const host1Checkbox = within(host1Row).getByRole('checkbox', { name: /Select Host1/ }) + const host2Checkbox = within(host2Row).getByRole('checkbox', { name: /Select Host2/ }) + + await userEvent.click(host1Checkbox) + await userEvent.click(host2Checkbox) + + // Wait for bulk operations to be available + await waitFor(() => expect(screen.getByText('Bulk Apply')).toBeTruthy()) + + // Click bulk delete + const bulkDeleteButtons = screen.getAllByRole('button', { name: /delete/i }) + await userEvent.click(bulkDeleteButtons[0]) // First is the bulk delete button in the toolbar + + // Confirm in modal + await waitFor(() => expect(screen.getByText(/Delete 2 Proxy Hosts/)).toBeTruthy()) + const deletePermBtn = screen.getByRole('button', { name: /Delete Permanently/i }) + await userEvent.click(deletePermBtn) + + // Should NOT show certificate cleanup dialog (host3 still uses it) + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h2') + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + }) + }) + + it('allows cancelling certificate cleanup dialog', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'CustomCert', + domains: 'test.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByRole('button', { name: /delete/i }) + await userEvent.click(deleteBtn) + + // Certificate cleanup dialog appears + await waitFor(() => expect(screen.getByText('Delete Proxy Host?')).toBeTruthy()) + + // Click Cancel + const cancelBtn = screen.getByRole('button', { name: 'Cancel' }) + await userEvent.click(cancelBtn) + + // Dialog should close, nothing deleted + await waitFor(() => { + expect(screen.queryByText('Delete Proxy Host?')).toBeFalsy() + expect(proxyHostsApi.deleteProxyHost).not.toHaveBeenCalled() + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + }) + }) + + it('default state is unchecked for certificate deletion (conservative)', async () => { + const cert: Certificate = { + id: 1, + uuid: 'cert-1', + provider: 'custom', + name: 'CustomCert', + domains: 'test.com', + expires_at: '2026-01-01T00:00:00Z' + } + const host = baseHost({ uuid: 'h1', name: 'Host1', certificate_id: 1, certificate: cert }) + + vi.mocked(proxyHostsApi.getProxyHosts).mockResolvedValue([host]) + vi.mocked(certificatesApi.getCertificates).mockResolvedValue([]) + vi.mocked(proxyHostsApi.deleteProxyHost).mockResolvedValue() + + renderWithProviders() + await waitFor(() => expect(screen.getByText('Host1')).toBeTruthy()) + + const deleteBtn = screen.getByRole('button', { name: /delete/i }) + await userEvent.click(deleteBtn) + + await waitFor(() => expect(screen.getByText('Delete Proxy Host?')).toBeTruthy()) + + // Checkbox should be unchecked by default + const checkbox = screen.getByRole('checkbox', { name: /Also delete/i }) as HTMLInputElement + expect(checkbox.checked).toBe(false) + + // Confirm deletion without checking the box + const deleteButtons = screen.getAllByRole('button', { name: 'Delete' }) + await userEvent.click(deleteButtons[deleteButtons.length - 1]) + + await waitFor(() => { + expect(proxyHostsApi.deleteProxyHost).toHaveBeenCalledWith('h1') + expect(certificatesApi.deleteCertificate).not.toHaveBeenCalled() + }) + }) +})