refactor middleware

internal/infrastructure/http/middlewares.go

@@ -0,0 +1,311 @@
+// Package http provides reusable net/http middleware shared by every service
+// that exposes an HTTP surface. It holds no domain types — handlers belong in
+// the owning service, not here.
+package http
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"log/slog"
+	"net/http"
+	"os"
+	"strconv"
+	"strings"
+	"time"
+)
+
+// contextKey is unexported so values stashed by this package cannot collide
+// with values stashed by any other package using the same string.
+type contextKey string
+
+const (
+	userIDContextKey   contextKey = "user_id"
+	usernameContextKey contextKey = "username"
+)
+
+var (
+	defaultAllowedMethods = []string{"GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"}
+	defaultAllowedHeaders = []string{"Content-Type", "Authorization", "X-Requested-With", "X-API-Key"}
+)
+
+// CORSConfig describes which cross-origin requests are permitted.
+//
+// AllowedOrigins is an explicit allowlist matched exactly against the request
+// Origin. The single entry "*" permits any origin, but only for unauthenticated
+// requests: "*" together with AllowCredentials is rejected by CORS, because it
+// would let any site on the internet issue credentialed requests and read the
+// responses. An empty AllowedOrigins denies every cross-origin request, which
+// leaves same-origin traffic working normally.
+type CORSConfig struct {
+	AllowedOrigins   []string
+	AllowedMethods   []string
+	AllowedHeaders   []string
+	ExposedHeaders   []string
+	AllowCredentials bool
+	MaxAge           time.Duration
+}
+
+// LoadCORSConfigFromEnv reads CORS settings from the environment.
+//
+// CORS_ALLOWED_ORIGINS is a comma-separated allowlist and defaults to empty,
+// so an unconfigured service denies cross-origin requests rather than allowing
+// them. CORS_ALLOW_CREDENTIALS defaults to false.
+func LoadCORSConfigFromEnv() CORSConfig {
+	return CORSConfig{
+		AllowedOrigins:   splitAndTrim(getEnv("CORS_ALLOWED_ORIGINS", "")),
+		AllowedMethods:   splitAndTrim(getEnv("CORS_ALLOWED_METHODS", strings.Join(defaultAllowedMethods, ","))),
+		AllowedHeaders:   splitAndTrim(getEnv("CORS_ALLOWED_HEADERS", strings.Join(defaultAllowedHeaders, ","))),
+		ExposedHeaders:   splitAndTrim(getEnv("CORS_EXPOSED_HEADERS", "Content-Length,Content-Range")),
+		AllowCredentials: getEnv("CORS_ALLOW_CREDENTIALS", "false") == "true",
+		MaxAge:           24 * time.Hour,
+	}
+}
+
+// CORS returns a middleware enforcing cfg.
+//
+// It returns an error for configurations that cannot be served safely, so a
+// service fails at startup instead of running with permissive CORS.
+func CORS(cfg CORSConfig) (func(http.Handler) http.Handler, error) {
+	allowAny := len(cfg.AllowedOrigins) == 1 && cfg.AllowedOrigins[0] == "*"
+
+	if allowAny && cfg.AllowCredentials {
+		return nil, fmt.Errorf(`CORS: AllowedOrigins "*" cannot be combined with AllowCredentials; list the permitted origins explicitly`)
+	}
+
+	for _, origin := range cfg.AllowedOrigins {
+		if origin != "*" && !strings.Contains(origin, "://") {
+			return nil, fmt.Errorf("CORS: AllowedOrigins entry %q must include a scheme, e.g. https://%s", origin, origin)
+		}
+	}
+
+	// Origin comparison is case-insensitive on scheme and host, so normalise the
+	// allowlist once here rather than on every request.
+	allowed := make(map[string]struct{}, len(cfg.AllowedOrigins))
+	for _, origin := range cfg.AllowedOrigins {
+		allowed[strings.ToLower(origin)] = struct{}{}
+	}
+
+	// Defaults are applied here, not only in LoadCORSConfigFromEnv, so a
+	// directly constructed CORSConfig still answers preflights usefully instead
+	// of sending empty header values.
+	if len(cfg.AllowedMethods) == 0 {
+		cfg.AllowedMethods = defaultAllowedMethods
+	}
+	if len(cfg.AllowedHeaders) == 0 {
+		cfg.AllowedHeaders = defaultAllowedHeaders
+	}
+
+	methods := strings.Join(cfg.AllowedMethods, ", ")
+	headers := strings.Join(cfg.AllowedHeaders, ", ")
+	exposed := strings.Join(cfg.ExposedHeaders, ", ")
+
+	maxAge := ""
+	if cfg.MaxAge > 0 {
+		maxAge = strconv.Itoa(int(cfg.MaxAge.Seconds()))
+	}
+
+	if len(cfg.AllowedOrigins) == 0 {
+		slog.Warn("CORS: no allowed origins configured, cross-origin requests will be denied (set CORS_ALLOWED_ORIGINS)")
+	}
+
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			origin := r.Header.Get("Origin")
+			isPreflight := r.Method == http.MethodOptions && r.Header.Get("Access-Control-Request-Method") != ""
+
+			// Responses vary by Origin whenever the allowlist is consulted, so
+			// caches must not serve one origin's response to another.
+			if !allowAny {
+				w.Header().Add("Vary", "Origin")
+			}
+
+			if origin != "" && originAllowed(allowed, allowAny, origin) {
+				if allowAny {
+					w.Header().Set("Access-Control-Allow-Origin", "*")
+				} else {
+					w.Header().Set("Access-Control-Allow-Origin", origin)
+				}
+
+				if cfg.AllowCredentials {
+					w.Header().Set("Access-Control-Allow-Credentials", "true")
+				}
+				if exposed != "" {
+					w.Header().Set("Access-Control-Expose-Headers", exposed)
+				}
+
+				if isPreflight {
+					w.Header().Set("Access-Control-Allow-Methods", methods)
+					w.Header().Set("Access-Control-Allow-Headers", headers)
+					if maxAge != "" {
+						w.Header().Set("Access-Control-Max-Age", maxAge)
+					}
+				}
+			}
+
+			// Preflights are answered here whether or not the origin passed: with
+			// no Allow-Origin header the browser blocks the real request anyway,
+			// and handlers never have to deal with OPTIONS.
+			if isPreflight {
+				w.WriteHeader(http.StatusNoContent)
+				return
+			}
+
+			next.ServeHTTP(w, r)
+		})
+	}, nil
+}
+
+func originAllowed(allowed map[string]struct{}, allowAny bool, origin string) bool {
+	if allowAny {
+		return true
+	}
+	_, ok := allowed[strings.ToLower(origin)]
+	return ok
+}
+
+// Logs logs each request with its method, path, status and duration.
+func Logs(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		start := time.Now()
+		recorder := &statusRecorder{ResponseWriter: w, status: http.StatusOK}
+
+		next.ServeHTTP(recorder, r)
+
+		slog.Info("http request",
+			"method", r.Method,
+			"path", r.URL.Path,
+			"status", recorder.status,
+			"duration", time.Since(start),
+		)
+	})
+}
+
+// statusRecorder captures the response status for logging.
+type statusRecorder struct {
+	http.ResponseWriter
+	status      int
+	wroteHeader bool
+}
+
+func (s *statusRecorder) WriteHeader(status int) {
+	if s.wroteHeader {
+		return
+	}
+	s.status = status
+	s.wroteHeader = true
+	s.ResponseWriter.WriteHeader(status)
+}
+
+// AuthConfig supplies the credential checks used by Auth. Every validator is
+// injected by the calling service — this package deliberately ships no
+// credentials of its own. A nil validator disables that scheme.
+type AuthConfig struct {
+	// ValidateBearerToken resolves a Bearer token to a user ID.
+	ValidateBearerToken func(token string) (userID string, err error)
+	// ValidateAPIKey resolves an X-API-Key value to a user ID.
+	ValidateAPIKey func(apiKey string) (userID string, ok bool)
+	// ValidateBasicAuth reports whether HTTP basic credentials are valid.
+	ValidateBasicAuth func(username, password string) bool
+	// SkipPrefixes are path prefixes served without authentication, e.g.
+	// "/swagger/" or "/health".
+	SkipPrefixes []string
+}
+
+// Auth authenticates requests via Bearer token, API key or basic auth, in that
+// order, and rejects anything else with 401.
+func Auth(cfg AuthConfig) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			for _, prefix := range cfg.SkipPrefixes {
+				if strings.HasPrefix(r.URL.Path, prefix) {
+					next.ServeHTTP(w, r)
+					return
+				}
+			}
+
+			if cfg.ValidateBearerToken != nil {
+				if token, ok := bearerToken(r); ok {
+					if userID, err := cfg.ValidateBearerToken(token); err == nil {
+						next.ServeHTTP(w, r.WithContext(
+							context.WithValue(r.Context(), userIDContextKey, userID)))
+						return
+					}
+				}
+			}
+
+			if cfg.ValidateAPIKey != nil {
+				if apiKey := r.Header.Get("X-API-Key"); apiKey != "" {
+					if userID, ok := cfg.ValidateAPIKey(apiKey); ok {
+						next.ServeHTTP(w, r.WithContext(
+							context.WithValue(r.Context(), userIDContextKey, userID)))
+						return
+					}
+				}
+			}
+
+			if cfg.ValidateBasicAuth != nil {
+				if username, password, ok := r.BasicAuth(); ok {
+					if cfg.ValidateBasicAuth(username, password) {
+						next.ServeHTTP(w, r.WithContext(
+							context.WithValue(r.Context(), usernameContextKey, username)))
+						return
+					}
+				}
+			}
+
+			slog.Warn("unauthorized request", "method", r.Method, "path", r.URL.Path)
+
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusUnauthorized)
+			json.NewEncoder(w).Encode(map[string]any{
+				"error":   "Unauthorized",
+				"message": "Valid authentication required. Use Bearer token, API key, or basic auth.",
+				"code":    http.StatusUnauthorized,
+			})
+		})
+	}
+}
+
+func bearerToken(r *http.Request) (string, bool) {
+	header := r.Header.Get("Authorization")
+	if !strings.HasPrefix(header, "Bearer ") {
+		return "", false
+	}
+	token := strings.TrimPrefix(header, "Bearer ")
+	return token, token != ""
+}
+
+// UserIDFromContext returns the user ID stored by Auth, if any.
+func UserIDFromContext(ctx context.Context) (string, bool) {
+	userID, ok := ctx.Value(userIDContextKey).(string)
+	return userID, ok
+}
+
+// UsernameFromContext returns the username stored by Auth, if any.
+func UsernameFromContext(ctx context.Context) (string, bool) {
+	username, ok := ctx.Value(usernameContextKey).(string)
+	return username, ok
+}
+
+func getEnv(key, defaultValue string) string {
+	if value := os.Getenv(key); value != "" {
+		return value
+	}
+	return defaultValue
+}
+
+func splitAndTrim(value string) []string {
+	if value == "" {
+		return nil
+	}
+
+	parts := strings.Split(value, ",")
+	result := make([]string, 0, len(parts))
+	for _, part := range parts {
+		if trimmed := strings.TrimSpace(part); trimmed != "" {
+			result = append(result, trimmed)
+		}
+	}
+	return result
+}

internal/infrastructure/http/middlewares_test.go

@@ -0,0 +1,137 @@
+package http
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func corsHandler(t *testing.T, cfg CORSConfig) http.Handler {
+	t.Helper()
+
+	middleware, err := CORS(cfg)
+	if err != nil {
+		t.Fatalf("CORS(%+v): %v", cfg, err)
+	}
+
+	return middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+}
+
+func request(h http.Handler, method, origin string, preflight bool) *httptest.ResponseRecorder {
+	r := httptest.NewRequest(method, "/jobs", nil)
+	if origin != "" {
+		r.Header.Set("Origin", origin)
+	}
+	if preflight {
+		r.Header.Set("Access-Control-Request-Method", "POST")
+	}
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, r)
+	return w
+}
+
+// A wildcard origin combined with credentials would let any site issue
+// credentialed requests and read the responses, so it must not be constructible.
+func TestCORSRejectsWildcardWithCredentials(t *testing.T) {
+	if _, err := CORS(CORSConfig{AllowedOrigins: []string{"*"}, AllowCredentials: true}); err == nil {
+		t.Fatal("expected an error for wildcard origins combined with credentials")
+	}
+}
+
+func TestCORSRejectsSchemelessOrigin(t *testing.T) {
+	if _, err := CORS(CORSConfig{AllowedOrigins: []string{"app.example.com"}}); err == nil {
+		t.Fatal("expected an error for an origin without a scheme")
+	}
+}
+
+func TestCORSDoesNotEchoDisallowedOrigin(t *testing.T) {
+	h := corsHandler(t, CORSConfig{
+		AllowedOrigins:   []string{"https://app.example.com"},
+		AllowCredentials: true,
+	})
+
+	w := request(h, http.MethodGet, "https://evil.example.com", false)
+
+	if got := w.Header().Get("Access-Control-Allow-Origin"); got != "" {
+		t.Errorf("Access-Control-Allow-Origin = %q, want empty for a disallowed origin", got)
+	}
+	if got := w.Header().Get("Access-Control-Allow-Credentials"); got != "" {
+		t.Errorf("Access-Control-Allow-Credentials = %q, want empty for a disallowed origin", got)
+	}
+	if w.Code != http.StatusOK {
+		t.Errorf("status = %d, want %d: the handler should still run", w.Code, http.StatusOK)
+	}
+}
+
+func TestCORSAllowsListedOrigin(t *testing.T) {
+	h := corsHandler(t, CORSConfig{
+		AllowedOrigins:   []string{"https://app.example.com"},
+		AllowCredentials: true,
+	})
+
+	w := request(h, http.MethodGet, "https://app.example.com", false)
+
+	if got, want := w.Header().Get("Access-Control-Allow-Origin"), "https://app.example.com"; got != want {
+		t.Errorf("Access-Control-Allow-Origin = %q, want %q", got, want)
+	}
+	if got, want := w.Header().Get("Access-Control-Allow-Credentials"), "true"; got != want {
+		t.Errorf("Access-Control-Allow-Credentials = %q, want %q", got, want)
+	}
+	// Without Vary, a shared cache could serve one origin's response to another.
+	if got, want := w.Header().Get("Vary"), "Origin"; got != want {
+		t.Errorf("Vary = %q, want %q", got, want)
+	}
+}
+
+func TestCORSOriginMatchIsCaseInsensitive(t *testing.T) {
+	h := corsHandler(t, CORSConfig{AllowedOrigins: []string{"https://App.Example.com"}})
+
+	w := request(h, http.MethodGet, "https://app.example.com", false)
+
+	if w.Header().Get("Access-Control-Allow-Origin") == "" {
+		t.Error("origin differing only in case should match the allowlist")
+	}
+}
+
+// An unconfigured service must fail closed rather than allowing every origin.
+func TestCORSEmptyAllowlistDeniesCrossOrigin(t *testing.T) {
+	h := corsHandler(t, CORSConfig{})
+
+	w := request(h, http.MethodGet, "https://anything.example.com", false)
+
+	if got := w.Header().Get("Access-Control-Allow-Origin"); got != "" {
+		t.Errorf("Access-Control-Allow-Origin = %q, want empty when no origins are configured", got)
+	}
+}
+
+func TestCORSPreflightShortCircuits(t *testing.T) {
+	h := corsHandler(t, CORSConfig{AllowedOrigins: []string{"https://app.example.com"}})
+
+	w := request(h, http.MethodOptions, "https://app.example.com", true)
+
+	if w.Code != http.StatusNoContent {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusNoContent)
+	}
+	// Defaults must apply to a directly constructed config, not only to one
+	// built by LoadCORSConfigFromEnv.
+	if w.Header().Get("Access-Control-Allow-Methods") == "" {
+		t.Error("preflight response is missing Access-Control-Allow-Methods")
+	}
+	if w.Header().Get("Access-Control-Allow-Headers") == "" {
+		t.Error("preflight response is missing Access-Control-Allow-Headers")
+	}
+}
+
+func TestCORSPlainOptionsReachesHandler(t *testing.T) {
+	h := corsHandler(t, CORSConfig{AllowedOrigins: []string{"https://app.example.com"}})
+
+	// OPTIONS without Access-Control-Request-Method is not a preflight.
+	w := request(h, http.MethodOptions, "https://app.example.com", false)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("status = %d, want %d", w.Code, http.StatusOK)
+	}
+}

services/api/cmd/server/main.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/jobs-scraper/internal/infrastructure"
+	httpmw "github.com/jobs-scraper/internal/infrastructure/http"
 	"github.com/jobs-scraper/internal/infrastructure/rabbitmq"
 	"github.com/jobs-scraper/internal/repo"
 	"github.com/jobs-scraper/services/api/internal/app"
@@ -64,8 +65,13 @@ func main() {
 
 	router := mux.NewRouter()
 
-	router.Use(httpHandler.CORSMiddleware)
+	cors, err := httpmw.CORS(httpmw.LoadCORSConfigFromEnv())
-	router.Use(httpHandler.LogsMiddleware)
+	if err != nil {
+		log.Fatalf("Invalid CORS configuration: %v", err)
+	}
+
+	router.Use(cors)
+	router.Use(httpmw.Logs)
 
 	// Swagger endpoint
 	router.PathPrefix("/swagger/").Handler(httpSwagger.WrapHandler)

services/api/pkg/http/job.go

@@ -36,7 +36,9 @@ var decoder = schema.NewDecoder()
 func (h *JobHandler) RegisterRoutes(router *mux.Router) {
 	jobs := router.PathPrefix("/jobs").Subrouter()
 
-	//jobs.Use(AuthMiddleware)
+	// Not authenticated. To require auth, apply httpmw.Auth from
+	// internal/infrastructure/http here with real validators supplied by this
+	// service — it ships none of its own.
 
 	jobs.HandleFunc("", utils.Make(h.CreateJob)).Methods("POST")
 	jobs.HandleFunc("/analysis", utils.Make(h.GetAllAnalysisResults)).Methods("GET")

services/api/pkg/http/middlewares.go

@@ -1,149 +0,0 @@
-package http
-
-import (
-	"context"
-	"encoding/json"
-	"log"
-	"net/http"
-	"strings"
-	"time"
-)
-
-// CORSMiddleware handles Cross-Origin Resource Sharing
-func CORSMiddleware(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Set CORS headers
-		origin := r.Header.Get("Origin")
-		if origin == "" {
-			origin = "*"
-		}
-
-		w.Header().Set("Access-Control-Allow-Origin", origin)
-		w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS, PATCH")
-		w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With, X-API-Key")
-		w.Header().Set("Access-Control-Allow-Credentials", "true")
-		w.Header().Set("Access-Control-Max-Age", "86400")
-		w.Header().Set("Access-Control-Expose-Headers", "Content-Length, Content-Range")
-
-		// Handle preflight OPTIONS requests
-		if r.Method == "OPTIONS" {
-			w.WriteHeader(http.StatusOK)
-			return
-		}
-
-		next.ServeHTTP(w, r)
-	})
-}
-
-// LogsMiddleware logs HTTP requests with timing information
-func LogsMiddleware(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		start := time.Now()
-		next.ServeHTTP(w, r)
-		log.Printf("%s %s %s", r.Method, r.RequestURI, time.Since(start))
-	})
-}
-
-// AuthMiddleware handles JWT token validation, API keys, and basic auth
-func AuthMiddleware(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Skip auth for swagger endpoints and health checks
-		if strings.HasPrefix(r.URL.Path, "/swagger/") || r.URL.Path == "/health" {
-			next.ServeHTTP(w, r)
-			return
-		}
-
-		// Try Bearer token authentication first
-		authHeader := r.Header.Get("Authorization")
-		if authHeader != "" {
-			if strings.HasPrefix(authHeader, "Bearer ") {
-				token := strings.TrimPrefix(authHeader, "Bearer ")
-				if token != "" {
-					userID, err := validateJWTToken(token)
-					if err == nil {
-						ctx := context.WithValue(r.Context(), "user_id", userID)
-						next.ServeHTTP(w, r.WithContext(ctx))
-						return
-					}
-				}
-			}
-		}
-
-		// Try API Key authentication
-		apiKey := r.Header.Get("X-API-Key")
-		if apiKey != "" {
-			if isValidAPIKey(apiKey) {
-				ctx := context.WithValue(r.Context(), "user_id", getUserIDFromAPIKey(apiKey))
-				next.ServeHTTP(w, r.WithContext(ctx))
-				return
-			}
-		}
-
-		// Try Basic authentication
-		username, password, ok := r.BasicAuth()
-		if ok {
-			if isValidCredentials(username, password) {
-				ctx := context.WithValue(r.Context(), "username", username)
-				next.ServeHTTP(w, r.WithContext(ctx))
-				return
-			}
-		}
-
-		// Fallback to cookie-based auth (existing behavior)
-		token, err := r.Cookie("token")
-		if err == nil && token.Value == "123" {
-			next.ServeHTTP(w, r)
-			return
-		}
-
-		// No valid authentication found
-		response := map[string]interface{}{
-			"error":   "Unauthorized",
-			"message": "Valid authentication required. Use Bearer token, API key, or basic auth.",
-			"code":    http.StatusUnauthorized,
-		}
-		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusUnauthorized)
-		json.NewEncoder(w).Encode(response)
-	})
-}
-
-// Helper functions for authentication validation
-func validateJWTToken(token string) (string, error) {
-	// TODO: Implement JWT validation logic with your preferred JWT library
-	// For demonstration, accept a demo token
-	if token == "valid-jwt-token" {
-		return "user-123", nil
-	}
-	return "", http.ErrAbortHandler
-}
-
-func isValidAPIKey(apiKey string) bool {
-	// TODO: Implement API key validation logic
-	// For demonstration, accept a demo API key
-	return apiKey == "demo-api-key-123"
-}
-
-func getUserIDFromAPIKey(apiKey string) string {
-	// TODO: Implement logic to get user ID from API key
-	// For demonstration, return a mock user ID
-	return "api-user-123"
-}
-
-func isValidCredentials(username, password string) bool {
-	// TODO: Implement credential validation logic
-	// For demonstration, accept demo credentials
-	return username == "admin" && password == "password"
-}
-
-// GetUserIDFromContext extracts user ID from request context
-func GetUserIDFromContext(ctx context.Context) (string, bool) {
-	userID, ok := ctx.Value("user_id").(string)
-	return userID, ok
-}
-
-// GetUsernameFromContext extracts username from request context
-func GetUsernameFromContext(ctx context.Context) (string, bool) {
-	username, ok := ctx.Value("username").(string)
-	return username, ok
-}