From 268df9886eab34d86c84e99aa8d577113e71ca53 Mon Sep 17 00:00:00 2001 From: Cipher Vance Date: Sat, 15 Nov 2025 13:18:36 -0600 Subject: [PATCH] refactor(pr): Fixed some PR issues --- internal/database/database.go | 12 +++++++++--- internal/email/email.go | 19 +++++++++++-------- internal/handlers/newsletter.go | 9 ++++++++- internal/handlers/subscribers.go | 3 ++- internal/middleware/auth.go | 13 +++++++++++-- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/internal/database/database.go b/internal/database/database.go index 6742644..b2aab5c 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -1,6 +1,8 @@ package database import ( + "context" + "time" "database/sql" "fmt" "log" @@ -27,7 +29,7 @@ type Admin struct { // cfg provides PostgreSQL connection parameters and the default admin credentials used to create the // default admin user when missing. func Init(cfg *config.Config) { - password := url.QueryEscape(cfg.PGPassword) + password := url.PathEscape(cfg.PGPassword) psqlInfo := fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=require", cfg.PGUser, password, cfg.PGHost, cfg.PGPort, cfg.PGDatabase, @@ -45,7 +47,9 @@ func Init(cfg *config.Config) { db.SetMaxOpenConns(25) db.SetMaxIdleConns(5) - if err = db.Ping(); err != nil { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel + if err = db.PingContext(ctx); err != nil { log.Fatalf("Failed to ping database: %v", err) } @@ -58,7 +62,9 @@ func Init(cfg *config.Config) { // It is safe to call multiple times; if no connection exists, the call is a no-op. func Close() { if db != nil { - db.Close() + if err := db.Close(); err != nil { + log.Printf("Error closing database connection: %v", err) + } } } diff --git a/internal/email/email.go b/internal/email/email.go index 9d9abd6..7798ba4 100644 --- a/internal/email/email.go +++ b/internal/email/email.go @@ -3,6 +3,7 @@ package email import ( "fmt" "log" + "net/url" "time" "github.com/rideaware/admin-panel/internal/config" @@ -23,22 +24,24 @@ func SendUpdate(subject, body string) (string, error) { if err != nil { return "Failed to retrieve subscribers", err } - if len(subscribers) == 0 { return "No subscribers found.", nil } - + var succeeded, failed int for _, email := range subscribers { - if !send(subject, body, email) { - return fmt.Sprintf("Failed to send to %s", email), nil + if send(subject, body, email) { + succeeded++ + } else { + failed++ } } - if err := database.LogNewsletter(subject, body); err != nil { log.Printf("Error logging newsletter: %v", err) } - - return "Email has been sent to all subscribers.", nil + if failed == 0 { + return fmt.Sprintf("Email sent to all %d subscribers.", succeeded), nil + } + return fmt.Sprintf("Sent to %d/%d subscribers; %d failed.", succeeded, succeeded+failed, failed), nil } // send constructs and sends an HTML newsletter update to the specified recipient using the current SMTP configuration. @@ -72,7 +75,7 @@ func send(subject, body, recipient string) bool { m.Subject(subject) unsubLink := fmt.Sprintf("https://%s/unsubscribe?email=%s", - cfg.BaseURL, recipient) + cfg.BaseURL, url.QueryEscape(recipient)) htmlBody := fmt.Sprintf( "%s

If you ever wish to unsubscribe, "+ diff --git a/internal/handlers/newsletter.go b/internal/handlers/newsletter.go index afb407d..0f80289 100644 --- a/internal/handlers/newsletter.go +++ b/internal/handlers/newsletter.go @@ -21,9 +21,16 @@ func SendUpdatePost(c *gin.Context) { subject := c.PostForm("subject") body := c.PostForm("body") + // validate inputs + if strings.TrimSpace(subject) == "" || strings.TrimSpace(body) == { + c.HTML(http,StatusBadRequest, "send_update.html", + gin.H{"error": "Subject and message cannot be empty"}) + return + } + message, err := email.SendUpdate(subject, body) if err != nil { - c.HTML(http.StatusOK, "send_update.html", + c.HTML(http.StatusInternalServerError, "send_update.html", gin.H{"error": message}) return } diff --git a/internal/handlers/subscribers.go b/internal/handlers/subscribers.go index 9ae5d2d..e99730c 100644 --- a/internal/handlers/subscribers.go +++ b/internal/handlers/subscribers.go @@ -14,7 +14,8 @@ import ( func IndexGet(c *gin.Context) { emails, err := database.GetAllEmails() if err != nil { - c.AbortWithError(http.StatusInternalServerError, err) + c.HTML(http.StatusInternalServerError, "admin_index.html", + gin.H{"error": "Failed to retrieve subscribers"}) return } c.HTML(http.StatusOK, "admin_index.html", diff --git a/internal/middleware/auth.go b/internal/middleware/auth.go index afcf803..e0ec67d 100644 --- a/internal/middleware/auth.go +++ b/internal/middleware/auth.go @@ -15,6 +15,10 @@ var store *sessions.CookieStore // It panics if config.Current.SecretKey is empty. // The created store is configured with Path "/", MaxAge one week, HttpOnly true, Secure false, and SameSite 0. func Init() { + if config.Current == nil { + panic("config was not loaded; call config.Load() before middleware.Init()") + } + if config.Current.SecretKey == "" { panic("SECRET_KEY not set") } @@ -23,8 +27,8 @@ func Init() { Path: "/", MaxAge: 86400 * 7, HttpOnly: true, - Secure: false, - SameSite: 0, + Secure: true, + SameSite: http.SameSiteStrictMode, } } @@ -40,6 +44,11 @@ func GetStore() *sessions.CookieStore { // Otherwise the middleware calls the next handler in the chain. func Auth() gin.HandlerFunc { return func(c *gin.Context) { + if store == nil { + c.String(http.StatusInternalServerError, "Session store not initialized.") + c.Abort() + return + } session, err := store.Get(c.Request, "session") if err != nil || session.Values["username"] == nil { c.Redirect(http.StatusFound, "/login")