Or copy and paste this link into your browser:
%s
-This link expires in 24 hours.
`, name, verifyURL, verifyURL)) +This link expires in 24 hours.
`, html.EscapeString(name), verifyURL, verifyURL)) } func renderPasswordResetEmail(name, resetURL string) string { @@ -37,7 +40,7 @@ func renderPasswordResetEmail(name, resetURL string) string {Or copy and paste this link into your browser:
%s
-This link expires in 1 hour. If you didn't request this, please ignore this email.
`, name, resetURL, resetURL)) +This link expires in 1 hour. If you didn't request this, please ignore this email.
`, html.EscapeString(name), resetURL, resetURL)) } func renderTicketClosedEmail(name, ticketTitle, ticketURL string) string { @@ -48,7 +51,7 @@ func renderTicketClosedEmail(name, ticketTitle, ticketURL string) string { -If you believe the issue is not fully resolved, you can add a comment on the ticket page.
`, name, ticketTitle, ticketURL)) +If you believe the issue is not fully resolved, you can add a comment on the ticket page.
`, html.EscapeString(name), html.EscapeString(ticketTitle), ticketURL)) } func renderTicketReplyEmail(name, ticketTitle, ticketURL string) string { @@ -58,7 +61,7 @@ func renderTicketReplyEmail(name, ticketTitle, ticketURL string) string {There is a new reply on your ticket "%s".
`, name, ticketTitle, ticketURL)) +`, html.EscapeString(name), html.EscapeString(ticketTitle), ticketURL)) } func renderAccountApprovedEmail(name, loginURL string) string { @@ -68,7 +71,7 @@ func renderAccountApprovedEmail(name, loginURL string) string {Your account request has been approved. You can now log in and start creating tickets.
Log In -
`, name, loginURL)) +`, html.EscapeString(name), loginURL)) } func renderWelcomeEmail(name, email, tempPassword, loginURL string) string { @@ -83,5 +86,5 @@ func renderWelcomeEmail(name, email, tempPassword, loginURL string) string { -Please change your password after logging in.
`, name, email, tempPassword, loginURL)) +Please change your password after logging in.
`, html.EscapeString(name), html.EscapeString(email), html.EscapeString(tempPassword), loginURL)) } diff --git a/internal/forgejo/client.go b/internal/forgejo/client.go index 14b5a6a..69dd7fe 100644 --- a/internal/forgejo/client.go +++ b/internal/forgejo/client.go @@ -868,7 +868,20 @@ func (c *Client) GetAttachmentURL(apiURL string) (string, error) { } // ProxyDownload fetches a file from the given Forgejo URL with authentication and streams it back. +// The URL host must match the configured Forgejo base URL to prevent SSRF. func (c *Client) ProxyDownload(downloadURL string) (*http.Response, error) { + parsed, err := url.Parse(downloadURL) + if err != nil { + return nil, fmt.Errorf("invalid download URL: %w", err) + } + base, err := url.Parse(c.baseURL) + if err != nil { + return nil, fmt.Errorf("invalid base URL: %w", err) + } + if parsed.Host != base.Host { + return nil, fmt.Errorf("download URL host %q does not match Forgejo host %q", parsed.Host, base.Host) + } + httpReq, err := http.NewRequest("GET", downloadURL, nil) if err != nil { return nil, err diff --git a/internal/forgejo/webhook.go b/internal/forgejo/webhook.go index 75fd0a1..3f3f307 100644 --- a/internal/forgejo/webhook.go +++ b/internal/forgejo/webhook.go @@ -39,7 +39,7 @@ func VerifyWebhookSignature(r *http.Request, secret string) ([]byte, error) { return nil, fmt.Errorf("missing X-Forgejo-Signature header") } - body, err := io.ReadAll(r.Body) + body, err := io.ReadAll(io.LimitReader(r.Body, 1<<20)) if err != nil { return nil, fmt.Errorf("read body: %w", err) } diff --git a/internal/handlers/admin/users.go b/internal/handlers/admin/users.go index f8c8a09..c072c53 100644 --- a/internal/handlers/admin/users.go +++ b/internal/handlers/admin/users.go @@ -4,12 +4,12 @@ import ( "crypto/rand" "encoding/hex" "net/http" - "net/url" "strings" "github.com/gin-gonic/gin" "github.com/google/uuid" "github.com/mattnite/forgejo-tickets/internal/forgejo" + "github.com/mattnite/forgejo-tickets/internal/middleware" "github.com/mattnite/forgejo-tickets/internal/models" "github.com/rs/zerolog/log" ) @@ -202,11 +202,8 @@ func (h *UserHandler) Approve(c *gin.Context) { log.Error().Err(err).Msg("send approval email error") } - redirectURL := "/admin/users/pending?" + url.Values{ - "flash": {"User " + user.Email + " has been approved"}, - "flash_type": {"success"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "success", "User "+user.Email+" has been approved") + c.Redirect(http.StatusSeeOther, "/admin/users/pending") } func (h *UserHandler) Reject(c *gin.Context) { @@ -222,11 +219,8 @@ func (h *UserHandler) Reject(c *gin.Context) { return } - redirectURL := "/admin/users/pending?" + url.Values{ - "flash": {"User request has been rejected"}, - "flash_type": {"success"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "success", "User request has been rejected") + c.Redirect(http.StatusSeeOther, "/admin/users/pending") } func (h *UserHandler) UpdateRepos(c *gin.Context) { @@ -257,9 +251,6 @@ func (h *UserHandler) UpdateRepos(c *gin.Context) { h.deps.DB.Create(&models.UserRepo{UserID: userID, RepoID: repoID}) } - redirectURL := "/admin/users/" + userID.String() + "?" + url.Values{ - "flash": {"Project assignments updated"}, - "flash_type": {"success"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "success", "Project assignments updated") + c.Redirect(http.StatusSeeOther, "/admin/users/"+userID.String()) } diff --git a/internal/handlers/public/auth.go b/internal/handlers/public/auth.go index d61a06b..2b32abc 100644 --- a/internal/handlers/public/auth.go +++ b/internal/handlers/public/auth.go @@ -2,15 +2,38 @@ package public import ( "net/http" - "net/url" "strings" + "unicode" "github.com/gin-gonic/gin" "github.com/mattnite/forgejo-tickets/internal/auth" + "github.com/mattnite/forgejo-tickets/internal/middleware" "github.com/mattnite/forgejo-tickets/internal/models" "github.com/rs/zerolog/log" ) +// validatePassword checks password complexity requirements. +func validatePassword(password string) string { + if len(password) < 8 { + return "Password must be at least 8 characters" + } + var hasUpper, hasLower, hasDigit bool + for _, r := range password { + switch { + case unicode.IsUpper(r): + hasUpper = true + case unicode.IsLower(r): + hasLower = true + case unicode.IsDigit(r): + hasDigit = true + } + } + if !hasUpper || !hasLower || !hasDigit { + return "Password must contain at least one uppercase letter, one lowercase letter, and one digit" + } + return "" +} + type AuthHandler struct { deps Dependencies } @@ -85,8 +108,8 @@ func (h *AuthHandler) Register(c *gin.Context) { return } - if len(password) < 8 { - data["Error"] = "Password must be at least 8 characters" + if errMsg := validatePassword(password); errMsg != "" { + data["Error"] = errMsg h.deps.Renderer.Render(c.Writer, c.Request, "register", data) return } @@ -111,11 +134,8 @@ func (h *AuthHandler) Register(c *gin.Context) { } } - redirectURL := "/login?" + url.Values{ - "flash": {"Account requested! Please check your email to verify your address. After verification, an admin will review your request."}, - "flash_type": {"success"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "success", "Account requested! Please check your email to verify your address. After verification, an admin will review your request.") + c.Redirect(http.StatusSeeOther, "/login") } func (h *AuthHandler) Logout(c *gin.Context) { @@ -138,11 +158,8 @@ func (h *AuthHandler) VerifyEmail(c *gin.Context) { return } - redirectURL := "/login?" + url.Values{ - "flash": {"Email verified successfully. Your account is pending admin approval."}, - "flash_type": {"success"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "success", "Email verified successfully. Your account is pending admin approval.") + c.Redirect(http.StatusSeeOther, "/login") } func (h *AuthHandler) ForgotPasswordForm(c *gin.Context) { @@ -189,10 +206,10 @@ func (h *AuthHandler) ResetPassword(c *gin.Context) { return } - if len(password) < 8 { + if errMsg := validatePassword(password); errMsg != "" { h.deps.Renderer.Render(c.Writer, c.Request, "reset-password", map[string]interface{}{ "Token": token, - "Error": "Password must be at least 8 characters", + "Error": errMsg, }) return } @@ -206,9 +223,6 @@ func (h *AuthHandler) ResetPassword(c *gin.Context) { return } - redirectURL := "/login?" + url.Values{ - "flash": {"Password reset successfully. You can now log in."}, - "flash_type": {"success"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "success", "Password reset successfully. You can now log in.") + c.Redirect(http.StatusSeeOther, "/login") } diff --git a/internal/handlers/public/oauth.go b/internal/handlers/public/oauth.go index 839f4b0..60b9a10 100644 --- a/internal/handlers/public/oauth.go +++ b/internal/handlers/public/oauth.go @@ -4,11 +4,11 @@ import ( "crypto/rand" "encoding/hex" "net/http" - "net/url" "strings" "github.com/gin-gonic/gin" "github.com/mattnite/forgejo-tickets/internal/auth" + "github.com/mattnite/forgejo-tickets/internal/middleware" "github.com/rs/zerolog/log" "golang.org/x/oauth2" ) @@ -53,12 +53,16 @@ func (h *OAuthHandler) Login(c *gin.Context) { } state := generateState() - session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") - session.Values["state"] = state - session.Values["user_id"] = "00000000-0000-0000-0000-000000000000" // placeholder for save - if err := session.Save(c.Request, c.Writer); err != nil { - log.Error().Err(err).Msg("save oauth state error") - } + isSecure := strings.HasPrefix(h.deps.Config.BaseURL, "https") + http.SetCookie(c.Writer, &http.Cookie{ + Name: "oauth_state", + Value: state, + Path: "/", + MaxAge: 600, // 10 minutes + HttpOnly: true, + Secure: isSecure, + SameSite: http.SameSiteLaxMode, + }) url := provider.Config.AuthCodeURL(state, oauth2.AccessTypeOffline) c.Redirect(http.StatusTemporaryRedirect, url) @@ -73,12 +77,17 @@ func (h *OAuthHandler) Callback(c *gin.Context) { } // Verify state - session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") - expectedState, _ := session.Values["state"].(string) - if c.Query("state") != expectedState { + stateCookie, err := c.Request.Cookie("oauth_state") + if err != nil || c.Query("state") != stateCookie.Value { c.String(http.StatusBadRequest, "Invalid state parameter") return } + // Clear the state cookie + http.SetCookie(c.Writer, &http.Cookie{ + Name: "oauth_state", + Path: "/", + MaxAge: -1, + }) code := c.Query("code") token, err := provider.Config.Exchange(c.Request.Context(), code) @@ -98,11 +107,8 @@ func (h *OAuthHandler) Callback(c *gin.Context) { user, err := h.deps.Auth.FindOrCreateOAuthUser(c.Request.Context(), provider.Name, info) if err != nil { if strings.Contains(err.Error(), "pending admin approval") { - redirectURL := "/login?" + url.Values{ - "flash": {err.Error()}, - "flash_type": {"info"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "info", err.Error()) + c.Redirect(http.StatusSeeOther, "/login") return } log.Error().Err(err).Msg("find or create oauth user error") @@ -137,12 +143,16 @@ func (h *OAuthHandler) appleLogin(c *gin.Context) { } state := generateState() - session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") - session.Values["state"] = state - session.Values["user_id"] = "00000000-0000-0000-0000-000000000000" - if err := session.Save(c.Request, c.Writer); err != nil { - log.Error().Err(err).Msg("save oauth state error") - } + isSecure := strings.HasPrefix(h.deps.Config.BaseURL, "https") + http.SetCookie(c.Writer, &http.Cookie{ + Name: "oauth_state", + Value: state, + Path: "/", + MaxAge: 600, // 10 minutes + HttpOnly: true, + Secure: isSecure, + SameSite: http.SameSiteNoneMode, // Apple uses form_post cross-origin + }) url := appleProvider.Config.AuthCodeURL(state, oauth2.AccessTypeOffline, auth.AppleAuthCodeOption()) c.Redirect(http.StatusTemporaryRedirect, url) @@ -164,12 +174,17 @@ func (h *OAuthHandler) AppleCallback(c *gin.Context) { code := c.PostForm("code") state := c.PostForm("state") - session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") - expectedState, _ := session.Values["state"].(string) - if state != expectedState { + stateCookie, err := c.Request.Cookie("oauth_state") + if err != nil || state != stateCookie.Value { c.String(http.StatusBadRequest, "Invalid state parameter") return } + // Clear the state cookie + http.SetCookie(c.Writer, &http.Cookie{ + Name: "oauth_state", + Path: "/", + MaxAge: -1, + }) token, err := appleProvider.ExchangeCode(c.Request.Context(), code) if err != nil { @@ -196,11 +211,8 @@ func (h *OAuthHandler) AppleCallback(c *gin.Context) { user, err := h.deps.Auth.FindOrCreateOAuthUser(c.Request.Context(), "apple", info) if err != nil { if strings.Contains(err.Error(), "pending admin approval") { - redirectURL := "/login?" + url.Values{ - "flash": {err.Error()}, - "flash_type": {"info"}, - }.Encode() - c.Redirect(http.StatusSeeOther, redirectURL) + middleware.SetFlash(c, "info", err.Error()) + c.Redirect(http.StatusSeeOther, "/login") return } log.Error().Err(err).Msg("find or create apple user error") diff --git a/internal/handlers/public/routes.go b/internal/handlers/public/routes.go index beccad6..3185e3e 100644 --- a/internal/handlers/public/routes.go +++ b/internal/handlers/public/routes.go @@ -3,6 +3,7 @@ package public import ( "net/http" "strings" + "time" "github.com/gin-gonic/gin" "github.com/mattnite/forgejo-tickets/internal/auth" @@ -31,6 +32,7 @@ func NewRouter(deps Dependencies) *gin.Engine { r.Use(middleware.RequestID) r.Use(middleware.Logging) r.Use(middleware.Recovery) + r.Use(middleware.SecurityHeaders(strings.HasPrefix(deps.Config.BaseURL, "https"))) r.Use(deps.Auth.SessionMiddleware) csrfSecret := []byte(deps.Config.SessionSecret) @@ -41,7 +43,7 @@ func NewRouter(deps Dependencies) *gin.Engine { c.String(http.StatusOK, "ok") }) - r.Static("/static", "web/static") + r.StaticFS("/static", gin.Dir("web/static", false)) webhookHandler := &WebhookHandler{deps: deps} r.POST("/webhooks/forgejo/:repoSlug", webhookHandler.HandleForgejoWebhook) @@ -49,6 +51,8 @@ func NewRouter(deps Dependencies) *gin.Engine { ssoHandler := &SSOHandler{deps: deps} r.GET("/sso/:slug", ssoHandler.HandleSSO) + authRateLimiter := middleware.NewRateLimiter(10, 1*time.Minute) + csrf := r.Group("/") csrf.Use(csrfMiddleware) { @@ -57,13 +61,13 @@ func NewRouter(deps Dependencies) *gin.Engine { authHandler := &AuthHandler{deps: deps} csrf.GET("/login", authHandler.LoginForm) - csrf.POST("/login", authHandler.Login) + csrf.POST("/login", authRateLimiter.Middleware(), authHandler.Login) csrf.GET("/register", authHandler.RegisterForm) - csrf.POST("/register", authHandler.Register) + csrf.POST("/register", authRateLimiter.Middleware(), authHandler.Register) csrf.POST("/logout", authHandler.Logout) csrf.GET("/verify-email", authHandler.VerifyEmail) csrf.GET("/forgot-password", authHandler.ForgotPasswordForm) - csrf.POST("/forgot-password", authHandler.ForgotPassword) + csrf.POST("/forgot-password", authRateLimiter.Middleware(), authHandler.ForgotPassword) csrf.GET("/reset-password", authHandler.ResetPasswordForm) csrf.POST("/reset-password", authHandler.ResetPassword) diff --git a/internal/handlers/public/sso.go b/internal/handlers/public/sso.go index 940a315..5adbb47 100644 --- a/internal/handlers/public/sso.go +++ b/internal/handlers/public/sso.go @@ -98,7 +98,11 @@ func (h *SSOHandler) HandleSSO(c *gin.Context) { c.String(http.StatusInternalServerError, "failed to create user") return } + } else { + log.Info().Str("email", email).Str("name", name).Str("repo", slug).Msg("SSO: created new user") } + } else { + log.Info().Str("email", email).Str("repo", slug).Msg("SSO: existing user logged in") } // Update existing user if needed diff --git a/internal/handlers/public/tickets.go b/internal/handlers/public/tickets.go index de704bb..a8ebfcc 100644 --- a/internal/handlers/public/tickets.go +++ b/internal/handlers/public/tickets.go @@ -2,6 +2,7 @@ package public import ( "io" + "mime" "net/http" "sort" "strconv" @@ -531,7 +532,7 @@ func (h *TicketHandler) proxyAssetDownload(c *gin.Context, assetURL, filename st contentType = "application/octet-stream" } c.Header("Content-Type", contentType) - c.Header("Content-Disposition", "attachment; filename=\""+filename+"\"") + c.Header("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{"filename": filename})) if cl := resp.Header.Get("Content-Length"); cl != "" { c.Header("Content-Length", cl) } diff --git a/internal/middleware/flash.go b/internal/middleware/flash.go new file mode 100644 index 0000000..fd3167c --- /dev/null +++ b/internal/middleware/flash.go @@ -0,0 +1,46 @@ +package middleware + +import ( + "encoding/base64" + "net/http" + "strings" + + "github.com/gin-gonic/gin" +) + +// SetFlash sets a flash message cookie that will be consumed on the next render. +func SetFlash(c *gin.Context, flashType, message string) { + // Encode as "type:message" in base64 to avoid cookie value issues + value := base64.StdEncoding.EncodeToString([]byte(flashType + ":" + message)) + http.SetCookie(c.Writer, &http.Cookie{ + Name: "flash", + Value: value, + Path: "/", + MaxAge: 60, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }) +} + +// GetFlash reads and clears the flash message cookie. +func GetFlash(r *http.Request, w http.ResponseWriter) (flashType, message string) { + cookie, err := r.Cookie("flash") + if err != nil || cookie.Value == "" { + return "", "" + } + // Clear the cookie + http.SetCookie(w, &http.Cookie{ + Name: "flash", + Path: "/", + MaxAge: -1, + }) + data, err := base64.StdEncoding.DecodeString(cookie.Value) + if err != nil { + return "", "" + } + parts := strings.SplitN(string(data), ":", 2) + if len(parts) != 2 { + return "", "" + } + return parts[0], parts[1] +} diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 7fc3d03..48378a1 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -44,3 +44,15 @@ func Recovery(c *gin.Context) { }() c.Next() } + +func SecurityHeaders(secure bool) gin.HandlerFunc { + return func(c *gin.Context) { + c.Header("X-Content-Type-Options", "nosniff") + c.Header("X-Frame-Options", "DENY") + c.Header("Referrer-Policy", "strict-origin-when-cross-origin") + if secure { + c.Header("Strict-Transport-Security", "max-age=63072000; includeSubDomains") + } + c.Next() + } +} diff --git a/internal/middleware/ratelimit.go b/internal/middleware/ratelimit.go new file mode 100644 index 0000000..da1c670 --- /dev/null +++ b/internal/middleware/ratelimit.go @@ -0,0 +1,86 @@ +package middleware + +import ( + "net/http" + "sync" + "time" + + "github.com/gin-gonic/gin" +) + +type ipRecord struct { + mu sync.Mutex + timestamps []time.Time +} + +// RateLimiter holds the per-IP rate limiting state. +type RateLimiter struct { + ips sync.Map // map[string]*ipRecord + limit int + window time.Duration +} + +// NewRateLimiter creates a rate limiter that allows `limit` requests per `window` per IP. +func NewRateLimiter(limit int, window time.Duration) *RateLimiter { + rl := &RateLimiter{ + limit: limit, + window: window, + } + // Periodically clean up stale entries + go rl.cleanup() + return rl +} + +// cleanup removes entries that have no recent timestamps every 5 minutes. +func (rl *RateLimiter) cleanup() { + ticker := time.NewTicker(5 * time.Minute) + for range ticker.C { + now := time.Now() + rl.ips.Range(func(key, value any) bool { + rec := value.(*ipRecord) + rec.mu.Lock() + if len(rec.timestamps) == 0 || now.Sub(rec.timestamps[len(rec.timestamps)-1]) > rl.window { + rec.mu.Unlock() + rl.ips.Delete(key) + } else { + rec.mu.Unlock() + } + return true + }) + } +} + +// Middleware returns a Gin middleware that enforces the rate limit. +func (rl *RateLimiter) Middleware() gin.HandlerFunc { + return func(c *gin.Context) { + ip := c.ClientIP() + + val, _ := rl.ips.LoadOrStore(ip, &ipRecord{}) + rec := val.(*ipRecord) + + rec.mu.Lock() + now := time.Now() + cutoff := now.Add(-rl.window) + + // Remove timestamps outside the sliding window + valid := 0 + for _, t := range rec.timestamps { + if t.After(cutoff) { + rec.timestamps[valid] = t + valid++ + } + } + rec.timestamps = rec.timestamps[:valid] + + if len(rec.timestamps) >= rl.limit { + rec.mu.Unlock() + c.AbortWithStatus(http.StatusTooManyRequests) + return + } + + rec.timestamps = append(rec.timestamps, now) + rec.mu.Unlock() + + c.Next() + } +} diff --git a/internal/templates/render.go b/internal/templates/render.go index f9872c7..3e1d1e7 100644 --- a/internal/templates/render.go +++ b/internal/templates/render.go @@ -12,6 +12,7 @@ import ( "github.com/gorilla/csrf" "github.com/mattnite/forgejo-tickets/internal/auth" + "github.com/mattnite/forgejo-tickets/internal/middleware" "github.com/mattnite/forgejo-tickets/internal/models" ) @@ -103,12 +104,8 @@ func (r *Renderer) Render(w http.ResponseWriter, req *http.Request, name string, Data: data, } - if msg := req.URL.Query().Get("flash"); msg != "" { - flashType := req.URL.Query().Get("flash_type") - if flashType == "" { - flashType = "info" - } - pd.Flash = &Flash{Type: flashType, Message: msg} + if flashType, flashMsg := middleware.GetFlash(req, w); flashMsg != "" { + pd.Flash = &Flash{Type: flashType, Message: flashMsg} } var buf bytes.Buffer diff --git a/web/templates/layouts/base.html b/web/templates/layouts/base.html index d8dc45f..3e1d79d 100644 --- a/web/templates/layouts/base.html +++ b/web/templates/layouts/base.html @@ -15,11 +15,17 @@ -