diff --git a/contrib/receiptnext.service b/contrib/receiptnext.service index 63312c6..e21161e 100644 --- a/contrib/receiptnext.service +++ b/contrib/receiptnext.service @@ -11,7 +11,7 @@ WorkingDirectory=/opt/receiptnext ExecStart=/opt/receiptnext/app Restart=always RestartSec=5 -EnvironmentFile=/opt/receiptnext/.env +EnvironmentFile=-/opt/receiptnext/.env StandardOutput=append:/var/log/receiptnext.log StandardError=append:/var/log/receiptnext.log diff --git a/install.sh b/install.sh index a9bca3f..683580b 100755 --- a/install.sh +++ b/install.sh @@ -295,7 +295,7 @@ WorkingDirectory=${INSTALL_DIR} ExecStart=${INSTALL_DIR}/app Restart=always RestartSec=5 -EnvironmentFile=${INSTALL_DIR}/.env +EnvironmentFile=-${INSTALL_DIR}/.env StandardOutput=append:/var/log/receiptnext.log StandardError=append:/var/log/receiptnext.log diff --git a/internal/auth/otp.go b/internal/auth/otp.go index 5713e7c..e53dd43 100644 --- a/internal/auth/otp.go +++ b/internal/auth/otp.go @@ -4,6 +4,7 @@ package auth import ( "crypto/rand" + "crypto/subtle" "fmt" "sync" "time" @@ -33,7 +34,8 @@ func ValidateOTP(provided, stored string, expiresAt time.Time) bool { if time.Now().After(expiresAt) { return false } - return provided == stored + // Use constant-time comparison to prevent timing side-channel attacks. + return subtle.ConstantTimeCompare([]byte(provided), []byte(stored)) == 1 } // attemptData stores the failure count and timestamp for a single email. diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 2037a24..7745c59 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -10,6 +10,7 @@ import ( "log" "net/http" "strings" + "sync" "time" "github.com/cclohmar/ReceiptNext/internal/auth" @@ -35,6 +36,8 @@ type AuthHandler struct { Sessions *auth.SessionStore FailureTracker *auth.FailureTracker EmailSender *email.Sender + + otpMu sync.Mutex // prevents OTP reuse via race conditions } // --------------------------------------------------------------------------- @@ -164,19 +167,21 @@ func (h *AuthHandler) VerifyOTP(w http.ResponseWriter, r *http.Request) { return } - // Validate the OTP code and expiry. + // Validate + delete OTP atomically to prevent race-condition reuse. + h.otpMu.Lock() if !auth.ValidateOTP(otpCode, stored.OTPCode, expiresAt) { + h.otpMu.Unlock() h.FailureTracker.RecordFailure(emailAddr) renderOTPForm(w, emailAddr, "Invalid or expired OTP code. Please try again.") return } - // Successful verification: clean up and create session. + // Successful verification: delete OTP immediately (still under lock). h.FailureTracker.Reset(emailAddr) if err := database.DeleteOTP(h.DB, emailAddr); err != nil { log.Printf("ERROR [%s] handlers: VerifyOTP DeleteOTP(%s): %v", time.Now().Format(time.RFC3339), emailAddr, err) - // Non-fatal — the OTP is already validated. } + h.otpMu.Unlock() // Retrieve the user record to obtain the user ID. user, err := database.GetUserByEmail(h.DB, emailAddr) @@ -292,6 +297,25 @@ func renderOTPForm(w http.ResponseWriter, email string, errMsg string) { // collectOTP reads the 6 individual digit form values and concatenates them // into a single 6-character OTP code string. Returns an empty string if any // digit is missing. +// Logout clears the session cookie and invalidates the server-side session. +func (h *AuthHandler) Logout(w http.ResponseWriter, r *http.Request) { + // Invalidate the server-side session. + if cookie, err := r.Cookie("session_token"); err == nil && cookie.Value != "" { + h.Sessions.Delete(cookie.Value) + } + // Clear the cookie on the client side. + http.SetCookie(w, &http.Cookie{ + Name: "session_token", + Value: "", + Path: "/", + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + MaxAge: -1, + }) + w.Header().Set("HX-Redirect", "/") + w.WriteHeader(http.StatusOK) +} + func collectOTP(r *http.Request) string { var b strings.Builder for i := 0; i < 6; i++ { diff --git a/internal/handlers/expenses.go b/internal/handlers/expenses.go index ad62f32..74ccc07 100644 --- a/internal/handlers/expenses.go +++ b/internal/handlers/expenses.go @@ -359,6 +359,13 @@ func (h *ExpenseHandler) EditExpense(w http.ResponseWriter, r *http.Request) { return } + // Verify ownership: the expense's event must belong to the current user. + event, err := database.GetEventByID(h.DB, expense.EventID) + if err != nil || event == nil || event.UserID != getUserID(r) { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + tmpl, err := template.ParseFiles("templates/receipt_form.html") if err != nil { log.Printf("ERROR [%s] handlers: EditExpense: parse template: %v", @@ -426,6 +433,13 @@ func (h *ExpenseHandler) UpdateExpense(w http.ResponseWriter, r *http.Request) { return } + // Verify ownership: the expense's event must belong to the current user. + event, err := database.GetEventByID(h.DB, existing.EventID) + if err != nil || event == nil || event.UserID != getUserID(r) { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + expense := database.Expense{ ID: expenseID, EventID: existing.EventID, diff --git a/internal/handlers/file.go b/internal/handlers/file.go index 9a507ce..45af5ec 100644 --- a/internal/handlers/file.go +++ b/internal/handlers/file.go @@ -364,11 +364,19 @@ func createReceiptZip(eventName string, expenses []database.Expense) (*email.Att continue } + // Prevent path traversal — only allow files within the storage directory. + cleanPath := filepath.Clean(exp.ImagePath) + if !strings.HasPrefix(cleanPath, "storage") && !strings.HasPrefix(cleanPath, "./storage") { + log.Printf("WARN [%s] handlers: createReceiptZip: blocked path traversal attempt: %q", + time.Now().Format(time.RFC3339), exp.ImagePath) + continue + } + // Read the image file from disk. - data, err := os.ReadFile(exp.ImagePath) + data, err := os.ReadFile(cleanPath) if err != nil { log.Printf("WARN [%s] handlers: createReceiptZip: reading %q: %v", - time.Now().Format(time.RFC3339), exp.ImagePath, err) + time.Now().Format(time.RFC3339), cleanPath, err) continue } diff --git a/main.go b/main.go index 51ad43d..a0fd924 100644 --- a/main.go +++ b/main.go @@ -156,20 +156,9 @@ func main() { r.Post("/request-otp", authHandler.RequestOTP) r.Post("/verify-otp", authHandler.VerifyOTP) - // ---- Logout ---- + // ---- Logout (invalidates server-side session + clears cookie) ---- - r.Post("/logout", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Delete the session cookie. - http.SetCookie(w, &http.Cookie{ - Name: "session_token", - Value: "", - Path: "/", - HttpOnly: true, - MaxAge: -1, - }) - w.Header().Set("HX-Redirect", "/") - w.WriteHeader(http.StatusOK) - })) + r.Post("/logout", authHandler.Logout) // ---- Protected routes (auth required) ----