fix(tests+security): custom script timeout tests, client-id metric, test fixes, ArangoDB hardening

CHANGELOG.md

@@ -5,6 +5,26 @@ All notable changes to Authorizer will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+
+- **`--rate-limit-fail-closed`**: when the rate-limit backend returns an error, respond with `503` instead of allowing the request (default remains fail-open).
+- **`--metrics-host`**: bind address for the dedicated `/metrics` listener (default `127.0.0.1`). Use `0.0.0.0` when a scraper on another host/pod must reach the metrics port over the network; keep the metrics port off public ingress.
+
+### Changed
+
+- **Prometheus `/metrics`**: always served on a **dedicated** HTTP listener (`--metrics-host`:`--metrics-port`, default `127.0.0.1:8081`). **`--http-port` and `--metrics-port` must differ**; `/metrics` is not registered on the main Gin server.
+- **HTTP metrics**: unmatched Gin routes use the fixed path label `unmatched` instead of the raw request URL (prevents cardinality attacks).
+- **GraphQL metrics**: the `operation` label is now `anonymous` or `op_<sha256-prefix>` so client-supplied operation names cannot explode time-series cardinality.
+- **Health/readiness JSON**: failure responses return a generic `error` string; details remain in server logs.
+- **OAuth callback JSON**: generic OAuth-style error body on provider processing failure; details remain in logs.
+- **`/playground`** is subject to the same per-IP rate limits as other routes (health and OIDC discovery paths stay exempt). **`/metrics`** is not on the main HTTP router.
+
+### Removed
+
+- **`authorizer_client_id_not_found_total`**: replaced by **`authorizer_client_id_header_missing_total`**, which matches the actual behavior (header omitted, request still allowed). Update dashboards and alerts accordingly.
+
 ## [2.0.0] - 2025-02-28
 
 ### Added

Dockerfile

@@ -69,7 +69,18 @@ RUN addgroup -g 1000 authorizer && \
 
 USER authorizer
 
+# Ports (see docs: deployment/docker, deployment/kubernetes)
+# - EXPOSE is documentation only: it does NOT publish ports on the Docker host.
+# - 8080: main HTTP API (OAuth, GraphQL, health on /healthz, etc.). This is what you
+#   typically map with -p 8080:8080 or put behind an Ingress / load balancer.
+# - 8081: dedicated Prometheus /metrics listener. By default the process binds it to
+#   127.0.0.1, so other containers cannot scrape until you pass --metrics-host=0.0.0.0.
+#   Even then: do not map 8081 to the public internet; keep scraping on internal networks
+#   only (Docker internal network, Kubernetes ClusterIP / pod network).
 EXPOSE 8080 8081
-HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD wget -qO- http://localhost:8080/ || exit 1
+
+# Liveness uses the main HTTP server only (metrics may be loopback-only).
+HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD wget -qO- http://127.0.0.1:8080/healthz || exit 1
+
 ENTRYPOINT [ "./authorizer" ]
 CMD []

MIGRATION.md

@@ -205,8 +205,13 @@ Use these v2 **CLI flags** instead of v1 env or dashboard config. Flag names use
 | `PORT`               | `--http-port` (default: 8080)    |
 | Host                 | `--host` (default: 0.0.0.0)      |
 | Metrics port         | `--metrics-port` (default: 8081) |
+| Metrics bind         | `--metrics-host` (default: `127.0.0.1`) for the dedicated metrics listener only |
 | `LOG_LEVEL`          | `--log-level`                    |
 
+**Metrics:** `GET /metrics` is **always** on a **separate** minimal HTTP server at **`--metrics-host`:`--metrics-port`** (default **`127.0.0.1:8081`**). **`--http-port` and `--metrics-port` must differ**; the main Gin server does not expose `/metrics`. Use `--metrics-host=0.0.0.0` when Prometheus scrapes from another container or pod (keep the metrics port off public load balancers).
+
+**Rate limiting:** `--rate-limit-fail-closed` rejects requests with `503` when the rate-limit backend errors; the default remains fail-open (allow) for availability.
+
 
 ### Database
 

ROADMAP_V2.md

@@ -97,21 +97,13 @@ These are table-stakes features that every competitor has. Without them, Authori
 
 **Why**: Keycloak has full Prometheus/Grafana support. Essential for production deployments.
 
-- [ ] **`/metrics` endpoint** (OpenMetrics/Prometheus format)
-  - `authorizer_login_total{method,status}` -- login attempts by method and success/failure
-  - `authorizer_signup_total{method,status}` -- signup attempts
-  - `authorizer_token_issued_total{type}` -- tokens issued by type
-  - `authorizer_active_sessions` -- current active sessions gauge
-  - `authorizer_request_duration_seconds{endpoint,method}` -- request latency histogram
-  - `authorizer_db_query_duration_seconds` -- database query latency
-  - `authorizer_failed_login_total` -- failed logins (for alerting)
-  - `authorizer_account_lockouts_total` -- lockout events
-  - Go runtime metrics (goroutines, memory, GC)
+- [x] **`/metrics` endpoint** (OpenMetrics/Prometheus format) — implemented (`authorizer_*` metrics; always on dedicated `--metrics-host`:`--metrics-port`). Further metric parity (below) remains roadmap.
+  - Planned / partial vs Keycloak-style names: `authorizer_login_total{method,status}`, `authorizer_signup_total{method,status}`, `authorizer_token_issued_total{type}`, `authorizer_db_query_duration_seconds`, `authorizer_failed_login_total`, `authorizer_account_lockouts_total`, Go runtime metrics (goroutines, memory, GC)
 - [ ] **Enhanced `/health` endpoint** returning JSON with component status
   ```json
   {"status": "healthy", "db": "ok", "redis": "ok", "uptime": "72h"}
   ```
-- [ ] **Readiness/liveness probes** (`/healthz`, `/readyz`) for Kubernetes
+- [x] **Readiness/liveness probes** (`/healthz`, `/readyz`, `/health`) for Kubernetes
 
 ### 1.5 Session Security Enhancements
 

cmd/root.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/signal"
 	"strings"
@@ -32,6 +33,7 @@ import (
 // Default values for flags (single source of truth for init and applyFlagDefaults).
 var (
 	defaultHost              = "0.0.0.0"
+	defaultMetricsHost       = "127.0.0.1"
 	defaultLogLevel          = "debug"
 	defaultHTTPPort          = 8080
 	defaultMetricsPort       = 8081
@@ -52,8 +54,9 @@ var (
 	defaultDiscordScopes     = []string{"identify", "email"}
 	defaultTwitterScopes     = []string{"tweet.read", "users.read"}
 	defaultRobloxScopes      = []string{"openid", "profile"}
-	defaultRateLimitRPS      = float64(10)
-	defaultRateLimitBurst    = 20
+	// Default RPS cap per IP; raised from 10 to reduce false positives on busy UIs.
+	defaultRateLimitRPS   = float64(30)
+	defaultRateLimitBurst = 20
 )
 
 var (
@@ -74,7 +77,8 @@ func init() {
 	// Server flags
 	f.StringVar(&rootArgs.server.Host, "host", defaultHost, "Host address to listen on")
 	f.IntVar(&rootArgs.server.HTTPPort, "http-port", defaultHTTPPort, "Port to serve HTTP requests on")
-	f.IntVar(&rootArgs.server.MetricsPort, "metrics-port", defaultMetricsPort, "Port to serve metrics requests on")
+	f.IntVar(&rootArgs.server.MetricsPort, "metrics-port", defaultMetricsPort, "Port for the dedicated /metrics listener (must differ from --http-port)")
+	f.StringVar(&rootArgs.server.MetricsHost, "metrics-host", defaultMetricsHost, "Bind address for the dedicated /metrics listener (default loopback; use 0.0.0.0 when Prometheus scrapes from another host/pod)")
 
 	// Logging flags
 	f.StringVar(&rootArgs.logLevel, "log-level", defaultLogLevel, "Log level to use")
@@ -159,6 +163,7 @@ func init() {
 	// Rate limiting flags
 	f.Float64Var(&rootArgs.config.RateLimitRPS, "rate-limit-rps", defaultRateLimitRPS, "Maximum requests per second per IP for rate limiting")
 	f.IntVar(&rootArgs.config.RateLimitBurst, "rate-limit-burst", defaultRateLimitBurst, "Maximum burst size per IP for rate limiting")
+	f.BoolVar(&rootArgs.config.RateLimitFailClosed, "rate-limit-fail-closed", false, "On rate-limit backend errors, reject with 503 instead of allowing the request")
 
 	// JWT flags
 	f.StringVar(&rootArgs.config.JWTType, "jwt-type", "", "Type of JWT to use")
@@ -230,6 +235,9 @@ func applyFlagDefaults() {
 	if s.MetricsPort == 0 {
 		s.MetricsPort = defaultMetricsPort
 	}
+	if strings.TrimSpace(s.MetricsHost) == "" {
+		s.MetricsHost = defaultMetricsHost
+	}
 	if strings.TrimSpace(rootArgs.logLevel) == "" {
 		rootArgs.logLevel = defaultLogLevel
 	}
@@ -298,6 +306,10 @@ func applyFlagDefaults() {
 // Run the service
 func runRoot(c *cobra.Command, args []string) {
 	applyFlagDefaults()
+	if rootArgs.server.HTTPPort == rootArgs.server.MetricsPort {
+		fmt.Fprintf(os.Stderr, "invalid server ports: --http-port and --metrics-port must differ (metrics are always served on a dedicated listener)\n")
+		os.Exit(1)
+	}
 
 	// Prepare logger
 	ctx := context.Background()
@@ -340,6 +352,11 @@ func runRoot(c *cobra.Command, args []string) {
 	if err != nil {
 		log.Fatal().Err(err).Msg("failed to create storage provider")
 	}
+	defer func() {
+		if err := storageProvider.Close(); err != nil {
+			log.Error().Err(err).Msg("failed to close storage provider")
+		}
+	}()
 
 	// Authenticator provider
 	authenticatorProvider, err := authenticators.New(&rootArgs.config, &authenticators.Dependencies{

internal/config/config.go

@@ -4,6 +4,10 @@ package config
 type Config struct {
 	// Env is the environment of the authorizer instance
 	Env string
+	// SkipTestEndpointSSRFValidation relaxes SSRF checks for the admin TestEndpoint GraphQL
+	// mutation (e.g. to hit localhost in tests). Must remain false in production; integration
+	// tests enable it together with Env=test.
+	SkipTestEndpointSSRFValidation bool
 	// OrganizationLogo is the logo of the organization
 	OrganizationLogo string
 	// OrganizationName is the name of the organization
@@ -253,4 +257,6 @@ type Config struct {
 	RateLimitRPS float64
 	// RateLimitBurst is the maximum burst size per IP
 	RateLimitBurst int
+	// RateLimitFailClosed rejects requests when the rate limit backend errors (default: fail-open).
+	RateLimitFailClosed bool
 }

internal/graphql/test_endpoint.go

@@ -72,10 +72,14 @@ func (g *graphqlProvider) TestEndpoint(ctx context.Context, params *model.TestEn
 		return nil, err
 	}
 
-	// SSRF protection: validate endpoint URL and resolved IPs
-	if err := validators.ValidateEndpointURL(params.Endpoint); err != nil {
-		log.Debug().Err(err).Str("endpoint", params.Endpoint).Msg("endpoint URL rejected by SSRF filter")
-		return nil, fmt.Errorf("invalid endpoint: %s", err.Error())
+	// SSRF protection: validate endpoint URL and resolved IPs. Skipped only when tests
+	// explicitly set SkipTestEndpointSSRFValidation (never enable that flag in production).
+	skipSSRF := g.Config.Env == constants.TestEnv && g.Config.SkipTestEndpointSSRFValidation
+	if !skipSSRF {
+		if err := validators.ValidateEndpointURL(params.Endpoint); err != nil {
+			log.Debug().Err(err).Str("endpoint", params.Endpoint).Msg("endpoint URL rejected by SSRF filter")
+			return nil, fmt.Errorf("invalid endpoint: %w", err)
+		}
 	}
 
 	req, err := http.NewRequest("POST", params.Endpoint, bytes.NewBuffer(requestBody))
@@ -107,4 +111,3 @@ func (g *graphqlProvider) TestEndpoint(ctx context.Context, params *model.TestEn
 		Response:   refs.NewStringRef(string(body)),
 	}, nil
 }
-

internal/http_handlers/app.go

@@ -68,6 +68,7 @@ func (h *httpProvider) AppHandler() gin.HandlerFunc {
 				"state":            state,
 				"organizationName": orgName,
 				"organizationLogo": orgLogo,
+				"clientId":         h.Config.ClientID,
 			},
 		})
 	}

internal/http_handlers/client_check.go

@@ -2,8 +2,11 @@ package http_handlers
 
 import (
 	"net/http"
+	"strings"
 
 	"github.com/gin-gonic/gin"
+
+	"github.com/authorizerdev/authorizer/internal/metrics"
 )
 
 // ClientCheckMiddleware is a middleware to verify the client ID
@@ -12,16 +15,25 @@ import (
 // (e.g., OAuth callbacks, JWKS, OpenID configuration, health checks).
 // The middleware only rejects requests with an explicitly wrong client ID.
 func (h *httpProvider) ClientCheckMiddleware() gin.HandlerFunc {
-	log := h.Log.With().Str("func", "ClientCheckMiddleware").Logger()
 	return func(c *gin.Context) {
+		log := h.Log.With().Str("func", "ClientCheckMiddleware").
+			Str("path", c.Request.URL.Path).
+			Logger()
 		clientID := c.Request.Header.Get("X-Authorizer-Client-ID")
 		if clientID == "" {
 			log.Debug().Msg("request received without client ID header")
+			metrics.RecordClientIDHeaderMissing()
 			c.Next()
 			return
 		}
 
 		if clientID != h.Config.ClientID {
+			// Record metric for client-id mismatch, but skip dashboard and app UI routes
+			// as those are internal requests that should not trigger security alerts.
+			path := c.Request.URL.Path
+			if !strings.HasPrefix(path, "/dashboard") && !strings.HasPrefix(path, "/app") {
+				metrics.RecordSecurityEvent("client_id_mismatch", "invalid_client_id")
+			}
 			log.Debug().Str("client_id", clientID).Msg("Client ID is invalid")
 			c.JSON(http.StatusBadRequest, gin.H{
 				"error":             "invalid_client_id",

internal/http_handlers/context.go

@@ -1,20 +1,15 @@
 package http_handlers
 
 import (
-	"context"
-
 	"github.com/gin-gonic/gin"
-)
 
-// Define a custom type for context key
-type contextKey string
-
-const ginContextKey contextKey = "GinContextKey"
+	"github.com/authorizerdev/authorizer/internal/utils"
+)
 
 // ContextMiddleware is a middleware to add gin context in context
 func (h *httpProvider) ContextMiddleware() gin.HandlerFunc {
 	return func(c *gin.Context) {
-		ctx := context.WithValue(c.Request.Context(), ginContextKey, c)
+		ctx := utils.ContextWithGin(c.Request.Context(), c)
 		c.Request = c.Request.WithContext(ctx)
 		c.Next()
 	}