diff --git a/internal/infrastructure/http/middlewares.go b/internal/infrastructure/http/middlewares.go new file mode 100644 index 0000000..5222add --- /dev/null +++ b/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 +} diff --git a/internal/infrastructure/http/middlewares_test.go b/internal/infrastructure/http/middlewares_test.go new file mode 100644 index 0000000..e868bf6 --- /dev/null +++ b/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) + } +} diff --git a/services/api/cmd/server/main.go b/services/api/cmd/server/main.go index 502b008..588f3a8 100644 --- a/services/api/cmd/server/main.go +++ b/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) - router.Use(httpHandler.LogsMiddleware) + cors, err := httpmw.CORS(httpmw.LoadCORSConfigFromEnv()) + 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) diff --git a/services/api/pkg/http/job.go b/services/api/pkg/http/job.go index d051dbb..2014f49 100644 --- a/services/api/pkg/http/job.go +++ b/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") diff --git a/services/api/pkg/http/middlewares.go b/services/api/pkg/http/middlewares.go deleted file mode 100644 index bcf4c7d..0000000 --- a/services/api/pkg/http/middlewares.go +++ /dev/null @@ -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 -}