fix: cleanup, adding more security fixes

assets/custom.css

@@ -22,16 +22,6 @@
     }
 }
 
-.htmx-indicator {
-  display: none;
-}
-.htmx-request .htmx-indicator {
-  display: inline-block;
-}
-.htmx-request.htmx-indicator {
-  display: inline-block;
-}
-
 input.invalid {
   border-color: #dc2626;
 }
@@ -82,11 +72,6 @@ input.invalid {
   font-size: 0.6rem;
 }
 
-.h {
-  /* bottom: 0;*/
-  height: 10rem !important;
-}
-
 /* Audio player skeleton prior to loading */
 .audio-player-container {
   background: linear-gradient(to bottom, rgba(128, 128, 128, 0.4), rgba(128, 128, 128, 0.1));

internal/conf/config.go

@@ -409,7 +409,7 @@ func createDefaultConfig() error {
 	defaultConfig := getDefaultConfig()
 
 	// If the basicauth secret is not set, generate a random one
-	if viper.GetString("security.basicauth.secret") == "" {
+	if viper.GetString("security.basicauth.clientsecret") == "" {
 		viper.Set("security.basicauth.clientsecret", GenerateRandomSecret())
 	}
 

internal/conf/defaults.go

@@ -185,7 +185,7 @@ func setDefaultConfig() {
 	viper.SetDefault("security.google.redirecturi", "/settings")
 	viper.SetDefault("security.google.userid", "")
 
-	// GittHub OAuth2 configuration
+	// GitHub OAuth2 configuration
 	viper.SetDefault("security.github.enabled", false)
 	viper.SetDefault("security.github.clientid", "")
 	viper.SetDefault("security.github.clientsecret", "")

internal/conf/validate.go

@@ -166,7 +166,7 @@ func validateWebServerSettings(settings *struct {
 func validateSecuritySettings(settings *Security) error {
 	// Check if any OAuth provider is enabled
 	if (settings.BasicAuth.Enabled || settings.GoogleAuth.Enabled || settings.GithubAuth.Enabled) && settings.Host == "" {
-		return fmt.Errorf("security host must be set when using authentication providers")
+		return fmt.Errorf("security.host must be set when using authentication providers")
 	}
 
 	// Validate the subnet bypass setting against the allowed pattern

internal/httpcontroller/auth_routes.go

@@ -7,24 +7,29 @@ import (
 	"strings"
 
 	"github.com/labstack/echo/v4"
+	"github.com/labstack/echo/v4/middleware"
 	"github.com/markbates/goth/gothic"
 )
 
 // initAuthRoutes initializes all authentication related routes
 func (s *Server) initAuthRoutes() {
+	// Add rate limiter for auth and login routes
+	g := s.Echo.Group("")
+	g.Use(middleware.RateLimiter(middleware.NewRateLimiterMemoryStore(10)))
+
 	// OAuth2 routes
-	s.Echo.GET("/oauth2/authorize", s.Handlers.WithErrorHandling(s.OAuth2Server.HandleBasicAuthorize))
-	s.Echo.POST("/oauth2/token", s.Handlers.WithErrorHandling(s.OAuth2Server.HandleBasicAuthToken))
-	s.Echo.GET("/callback", s.Handlers.WithErrorHandling(s.OAuth2Server.HandleBasicAuthCallback))
+	g.GET("/oauth2/authorize", s.Handlers.WithErrorHandling(s.OAuth2Server.HandleBasicAuthorize))
+	g.POST("/oauth2/token", s.Handlers.WithErrorHandling(s.OAuth2Server.HandleBasicAuthToken))
+	g.GET("/callback", s.Handlers.WithErrorHandling(s.OAuth2Server.HandleBasicAuthCallback))
 
 	// Social authentication routes
-	s.Echo.GET("/auth/:provider", s.Handlers.WithErrorHandling(handleGothProvider))
-	s.Echo.GET("/auth/:provider/callback", s.Handlers.WithErrorHandling(handleGothCallback))
+	g.GET("/auth/:provider", s.Handlers.WithErrorHandling(handleGothProvider))
+	g.GET("/auth/:provider/callback", s.Handlers.WithErrorHandling(handleGothCallback))
 
 	// Basic authentication routes
-	s.Echo.GET("/login", s.Handlers.WithErrorHandling(s.handleLoginPage))
-	s.Echo.POST("/login", s.handleBasicAuthLogin)
-	s.Echo.GET("/logout", s.Handlers.WithErrorHandling(s.handleLogout))
+	g.GET("/login", s.Handlers.WithErrorHandling(s.handleLoginPage))
+	g.POST("/login", s.handleBasicAuthLogin)
+	g.GET("/logout", s.Handlers.WithErrorHandling(s.handleLogout))
 }
 
 func handleGothProvider(c echo.Context) error {
@@ -100,9 +105,14 @@ func (s *Server) handleLoginPage(c echo.Context) error {
 }
 
 // isValidRedirect ensures the redirect path is safe and internal
-func isValidRedirect(redirect string) bool {
-	// Allow only relative paths starting with '/'
-	return strings.HasPrefix(redirect, "/") && !strings.Contains(redirect, "//")
+func isValidRedirect(redirectPath string) bool {
+	// Allow only relative paths
+	return strings.HasPrefix(redirectPath, "/") &&
+		!strings.Contains(redirectPath, "//") &&
+		!strings.Contains(redirectPath, "\\") &&
+		!strings.Contains(redirectPath, "://") &&
+		!strings.Contains(redirectPath, "..") &&
+		len(redirectPath) < 512
 }
 
 // handleBasicAuthLogin handles password login POST request
@@ -118,7 +128,11 @@ func (s *Server) handleBasicAuthLogin(c echo.Context) error {
 	if err != nil {
 		return c.HTML(http.StatusUnauthorized, "<div class='text-red-500'>Unable to login at this time</div>")
 	}
-	redirectURL := fmt.Sprintf("/callback?code=%s&redirect=%s", authCode, c.FormValue("redirect"))
+	redirect := c.FormValue("redirect")
+	if !isValidRedirect(redirect) {
+		redirect = "/"
+	}
+	redirectURL := fmt.Sprintf("/callback?code=%s&redirect=%s", authCode, redirect)
 	c.Response().Header().Set("HX-Redirect", redirectURL)
 	return c.String(http.StatusOK, "")
 }

internal/httpcontroller/middleware.go

@@ -11,8 +11,8 @@ import (
 
 // configureMiddleware sets up middleware for the server.
 func (s *Server) configureMiddleware() {
-	s.Echo.Use(s.AuthMiddleware)
 	s.Echo.Use(middleware.Recover())
+	s.Echo.Use(s.AuthMiddleware)
 	s.Echo.Use(middleware.GzipWithConfig(middleware.GzipConfig{
 		Level:     6,
 		MinLength: 2048,
@@ -63,6 +63,10 @@ func (s *Server) AuthMiddleware(next echo.HandlerFunc) echo.HandlerFunc {
 			if s.OAuth2Server.IsAuthenticationEnabled(s.RealIP(c)) {
 				if !s.IsAccessAllowed(c) {
 					redirectPath := url.QueryEscape(c.Request().URL.Path)
+					// Validate redirect path against whitelist
+					if !isValidRedirect(redirectPath) {
+						redirectPath = "/"
+					}
 					if c.Request().Header.Get("HX-Request") == "true" {
 						c.Response().Header().Set("HX-Redirect", "/login?redirect="+redirectPath)
 						return c.String(http.StatusUnauthorized, "")

internal/httpcontroller/server.go

@@ -4,6 +4,7 @@ package httpcontroller
 import (
 	"fmt"
 	"log"
+	"net"
 	"strings"
 
 	"github.com/labstack/echo/v4"
@@ -115,14 +116,25 @@ func (s *Server) IsAccessAllowed(c echo.Context) bool {
 }
 
 func (s *Server) RealIP(c echo.Context) string {
-	var ip string
+	// If Cloudflare Access is enabled, prioritize CF-Connecting-IP
+	if s.CloudflareAccess.IsEnabled(c) {
+		if cfIP := c.Request().Header.Get("CF-Connecting-IP"); cfIP != "" {
+			return strings.TrimSpace(cfIP)
+		}
+	}
 
-	if forwardedFor := c.Request().Header.Get("X-Forwarded-For"); forwardedFor != "" {
-		ip = strings.Split(forwardedFor, ",")[0]
-		ip = strings.TrimSpace(ip)
-	} else {
-		ip = strings.Split(c.Request().RemoteAddr, ":")[0]
+	// Get the X-Forwarded-For header
+	if xff := c.Request().Header.Get("X-Forwarded-For"); xff != "" {
+		// Split and get the first IP in the chain
+		ips := strings.Split(xff, ",")
+		if len(ips) > 0 {
+			// Take the last IP which should be the original client IP
+			return strings.TrimSpace(ips[len(ips)-1])
+		}
 	}
+
+	// Fallback to direct RemoteAddr
+	ip, _, _ := net.SplitHostPort(c.Request().RemoteAddr)
 	return ip
 }
 

internal/security/cloudflare.go

@@ -119,6 +119,7 @@ func (ca *CloudflareAccess) fetchCerts(issuer string) error {
 func (ca *CloudflareAccess) IsEnabled(c echo.Context) bool {
 
 	if !ca.settings.Enabled {
+		ca.Debug("Cloudflare Access is disabled")
 		return false
 	}
 

internal/security/cloudflare_test.go

@@ -299,6 +299,15 @@ func TestFetchCertsLogging(t *testing.T) {
 
 	var logs logWriter
 	log.SetOutput(io.Discard)
+
+	// Setup logging
+	originalOutput := log.Writer()
+	originalFlags := log.Flags()
+	defer func() {
+		// Restore original log settings after test
+		log.SetOutput(originalOutput)
+		log.SetFlags(originalFlags)
+	}()
 	log.SetFlags(0)
 	log.SetOutput(&logs)
 

internal/security/oauth.go

@@ -205,7 +205,6 @@ func (s *OAuth2Server) IsRequestFromAllowedSubnet(ip string) bool {
 	}
 
 	clientIP := net.ParseIP(ip)
-	log.Printf("*** %s", clientIP)
 	if clientIP == nil {
 		s.Debug("Invalid IP address: %s", ip)
 		return false

internal/security/oauth_test.go

@@ -28,9 +28,6 @@ func TestIsUserAuthenticatedValidAccessToken(t *testing.T) {
 
 	// Initialize gothic exactly as in production
 	gothic.Store = sessions.NewCookieStore([]byte(settings.Security.SessionSecret))
-	gothic.SetState = func(req *http.Request) string {
-		return ""
-	}
 
 	e := echo.New()
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
@@ -87,9 +84,6 @@ func TestIsUserAuthenticated(t *testing.T) {
 
 			// Initialize gothic exactly as in production
 			gothic.Store = sessions.NewCookieStore([]byte(settings.Security.SessionSecret))
-			gothic.SetState = func(req *http.Request) string {
-				return ""
-			}
 
 			e := echo.New()
 			req := httptest.NewRequest(http.MethodGet, "/", nil)

reset_auth.sh

@@ -42,13 +42,19 @@ for CONFIG_PATH in "${CONFIG_PATHS[@]}"; do
         cp "$CONFIG_PATH" "$BACKUP"        
         # Reset auth settings
         sed -i '
-            /^security:/,/^[^ ]/ {
+            /^security:/,/^[^[:space:]]/ {
                 s/\(host:\).*/\1 ""/
                 s/\(autotls:\).*/\1 false/
                 s/\(redirecttohttps:\).*/\1 false/
-                s/\(googleauth.enabled:\).*/\1 false/
-                s/\(githubauth.enabled:\).*/\1 false/
-                s/\(basicauth.enabled:\).*/\1 false/
+            }
+            /^[[:space:]]*basicauth:/,/^[[:space:]]*[^[:space:]]/ {
+                s/\(enabled:\).*/\1 false/
+            }
+            /^[[:space:]]*googleauth:/,/^[[:space:]]*[^[:space:]]/ {
+                s/\(enabled:\).*/\1 false/
+            }
+            /^[[:space:]]*githubauth:/,/^[[:space:]]*[^[:space:]]/ {
+                s/\(enabled:\).*/\1 false/
             }
         ' "$CONFIG_PATH"