Harden API-key model: per-key scoping + revocable keys, and configurable/whitelist-aware rate limiting (#16)

* API scope and rate limiting

* improvements from review

internal/config/config.go

@@ -2,12 +2,17 @@
 package config
 
 import (
+	"bytes"
+	"encoding/json"
 	"fmt"
 	"net"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 	"time"
+
+	"github.com/Turbootzz/vaultwarden-api/internal/auth"
 )
 
 // Config holds all application configuration
@@ -19,7 +24,7 @@ type Config struct {
 	WriteTimeout time.Duration
 
 	// Security
-	APIKey               string
+	APIKeys              []auth.APIKey
 	AllowedIPs           []string
 	EnableGitHubIPRanges bool
 
@@ -30,6 +35,10 @@ type Config struct {
 	// Performance
 	CacheTTL           time.Duration
 	CORSAllowedOrigins string
+
+	// Rate limiting
+	RateLimitMax    int
+	RateLimitWindow time.Duration
 }
 
 // Load reads configuration from environment variables
@@ -37,19 +46,28 @@ func Load() (*Config, error) {
 	cfg := &Config{
 		Port:        getEnv("API_PORT", "8080"),
 		Environment: getEnv("ENVIRONMENT", "development"),
-		APIKey:      os.Getenv("API_KEY"),
 
 		VaultwardenURL:   os.Getenv("VAULTWARDEN_URL"),
 		VaultwardenToken: os.Getenv("VAULTWARDEN_ACCESS_TOKEN"),
 
-		ReadTimeout:        parseDuration(getEnv("READ_TIMEOUT", "10s")),
-		WriteTimeout:       parseDuration(getEnv("WRITE_TIMEOUT", "10s")),
-		CacheTTL:           parseDuration(getEnv("CACHE_TTL", "5m")),
+		ReadTimeout:        parseDuration(os.Getenv("READ_TIMEOUT"), "10s"),
+		WriteTimeout:       parseDuration(os.Getenv("WRITE_TIMEOUT"), "10s"),
+		CacheTTL:           parseDuration(os.Getenv("CACHE_TTL"), "5m"),
 		CORSAllowedOrigins: getEnv("CORS_ALLOWED_ORIGINS", "http://localhost:3000"),
 
 		EnableGitHubIPRanges: getEnv("ENABLE_GITHUB_IP_RANGES", "false") == "true",
+
+		RateLimitMax:    parseInt(getEnv("RATE_LIMIT_MAX", "30"), 30),
+		RateLimitWindow: parseDuration(os.Getenv("RATE_LIMIT_WINDOW"), "1m"),
 	}
 
+	// Load API keys from API_KEYS_FILE / API_KEYS / legacy API_KEY.
+	apiKeys, err := loadAPIKeys()
+	if err != nil {
+		return nil, err
+	}
+	cfg.APIKeys = apiKeys
+
 	// Parse allowed IPs
 	if allowedIPsStr := os.Getenv("ALLOWED_IPS"); allowedIPsStr != "" {
 		ips := strings.Split(allowedIPsStr, ",")
@@ -65,12 +83,6 @@ func Load() (*Config, error) {
 	}
 
 	// Validate required fields
-	if cfg.APIKey == "" {
-		return nil, fmt.Errorf("API_KEY is required")
-	}
-	if len(cfg.APIKey) < 32 {
-		return nil, fmt.Errorf("API_KEY must be at least 32 characters for security (run: openssl rand -base64 32)")
-	}
 	if cfg.VaultwardenURL == "" {
 		return nil, fmt.Errorf("VAULTWARDEN_URL is required")
 	}
@@ -102,15 +114,109 @@ func getEnv(key, defaultValue string) string {
 	return defaultValue
 }
 
-// parseDuration parses a duration string with a fallback
-func parseDuration(s string) time.Duration {
-	d, err := time.ParseDuration(s)
-	if err != nil {
-		return 10 * time.Second
+// parseDuration parses a duration string, falling back to the given default
+// string for empty or malformed input (the fallback is a known-good constant).
+func parseDuration(s, fallback string) time.Duration {
+	if d, err := time.ParseDuration(s); err == nil {
+		return d
 	}
+	d, _ := time.ParseDuration(fallback)
 	return d
 }
 
+// parseInt parses a positive integer with a fallback for empty/invalid/non-positive input.
+func parseInt(s string, fallback int) int {
+	n, err := strconv.Atoi(s)
+	if err != nil || n <= 0 {
+		return fallback
+	}
+	return n
+}
+
+// apiKeyJSON is the on-disk/env JSON schema for a scoped API key.
+type apiKeyJSON struct {
+	Name          string   `json:"name"`
+	Key           string   `json:"key"`
+	Organizations []string `json:"organizations"`
+	Collections   []string `json:"collections"`
+}
+
+// loadAPIKeys assembles the configured keys from API_KEYS_FILE (preferred) or
+// API_KEYS (inline JSON), plus a legacy unscoped API_KEY if set. At least one
+// key is required and each must be at least 32 characters.
+func loadAPIKeys() ([]auth.APIKey, error) {
+	var keys []auth.APIKey
+
+	if path := os.Getenv("API_KEYS_FILE"); path != "" {
+		data, err := os.ReadFile(path)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read API_KEYS_FILE: %w", err)
+		}
+		parsed, err := parseAPIKeysJSON(data, "API_KEYS_FILE")
+		if err != nil {
+			return nil, err
+		}
+		keys = append(keys, parsed...)
+	} else if raw := os.Getenv("API_KEYS"); raw != "" {
+		parsed, err := parseAPIKeysJSON([]byte(raw), "API_KEYS")
+		if err != nil {
+			return nil, err
+		}
+		keys = append(keys, parsed...)
+	}
+
+	// Legacy single key remains a full-access (unscoped) key.
+	if legacy := os.Getenv("API_KEY"); legacy != "" {
+		keys = append(keys, auth.APIKey{Name: "legacy", Key: legacy})
+	}
+
+	if len(keys) == 0 {
+		return nil, fmt.Errorf("no API keys configured: set API_KEY, API_KEYS, or API_KEYS_FILE")
+	}
+
+	seen := make(map[string]struct{}, len(keys))
+	for i, k := range keys {
+		if len(k.Key) < 32 {
+			return nil, fmt.Errorf("API key #%d (%q) must be at least 32 characters for security (run: openssl rand -base64 32)", i+1, k.Name)
+		}
+		if _, dup := seen[k.Key]; dup {
+			return nil, fmt.Errorf("duplicate API key material for key #%d (%q): each key must be unique so it cannot silently override another key's scope", i+1, k.Name)
+		}
+		seen[k.Key] = struct{}{}
+	}
+
+	return keys, nil
+}
+
+// parseAPIKeysJSON parses a JSON array of scoped API keys. Unknown fields are
+// rejected so a misspelled scope field (e.g. "collection") fails loudly at
+// startup instead of silently leaving the key unscoped (full access).
+func parseAPIKeysJSON(data []byte, source string) ([]auth.APIKey, error) {
+	dec := json.NewDecoder(bytes.NewReader(data))
+	dec.DisallowUnknownFields()
+
+	var entries []apiKeyJSON
+	if err := dec.Decode(&entries); err != nil {
+		return nil, fmt.Errorf("invalid JSON in %s: %w", source, err)
+	}
+
+	keys := make([]auth.APIKey, 0, len(entries))
+	for i, e := range entries {
+		if e.Key == "" {
+			return nil, fmt.Errorf("%s entry #%d is missing \"key\"", source, i+1)
+		}
+		keys = append(keys, auth.APIKey{
+			Name: e.Name,
+			Key:  e.Key,
+			Scope: auth.Scope{
+				Organizations: e.Organizations,
+				Collections:   e.Collections,
+			},
+		})
+	}
+	return keys, nil
+}
+
 // validateIPOrCIDR validates if a string is a valid IP address or CIDR range
 func validateIPOrCIDR(s string) error {
 	// Try parsing as CIDR first

internal/config/config_test.go

@@ -0,0 +1,184 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+const (
+	key32a = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" // 35 chars
+	key32b = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+)
+
+// clearKeyEnv removes all key-related env vars so each case starts clean.
+func clearKeyEnv(t *testing.T) {
+	t.Helper()
+	for _, k := range []string{"API_KEY", "API_KEYS", "API_KEYS_FILE"} {
+		t.Setenv(k, "")
+	}
+}
+
+func TestLoadAPIKeys(t *testing.T) {
+	t.Run("legacy single key is full access", func(t *testing.T) {
+		clearKeyEnv(t)
+		t.Setenv("API_KEY", key32a)
+
+		keys, err := loadAPIKeys()
+		if err != nil {
+			t.Fatalf("loadAPIKeys: %v", err)
+		}
+		if len(keys) != 1 || keys[0].Key != key32a || !keys[0].Scope.IsEmpty() {
+			t.Fatalf("unexpected keys: %+v", keys)
+		}
+	})
+
+	t.Run("inline API_KEYS with scope", func(t *testing.T) {
+		clearKeyEnv(t)
+		t.Setenv("API_KEYS", `[{"name":"dev","key":"`+key32a+`","collections":["Secrets - DEV"]}]`)
+
+		keys, err := loadAPIKeys()
+		if err != nil {
+			t.Fatalf("loadAPIKeys: %v", err)
+		}
+		if len(keys) != 1 {
+			t.Fatalf("want 1 key, got %d", len(keys))
+		}
+		if keys[0].Name != "dev" || len(keys[0].Scope.Collections) != 1 {
+			t.Errorf("unexpected key: %+v", keys[0])
+		}
+	})
+
+	t.Run("API_KEYS_FILE preferred over inline and merged with legacy", func(t *testing.T) {
+		clearKeyEnv(t)
+		dir := t.TempDir()
+		path := filepath.Join(dir, "keys.json")
+		if err := os.WriteFile(path, []byte(`[{"name":"file","key":"`+key32b+`"}]`), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		t.Setenv("API_KEYS_FILE", path)
+		t.Setenv("API_KEYS", `[{"name":"ignored","key":"`+key32a+`"}]`)
+		t.Setenv("API_KEY", key32a)
+
+		keys, err := loadAPIKeys()
+		if err != nil {
+			t.Fatalf("loadAPIKeys: %v", err)
+		}
+		// File entry + legacy entry; inline API_KEYS is ignored when file is set.
+		if len(keys) != 2 {
+			t.Fatalf("want 2 keys (file + legacy), got %d: %+v", len(keys), keys)
+		}
+		if keys[0].Name != "file" || keys[1].Name != "legacy" {
+			t.Errorf("unexpected key order/names: %+v", keys)
+		}
+	})
+
+	t.Run("no keys configured", func(t *testing.T) {
+		clearKeyEnv(t)
+		if _, err := loadAPIKeys(); err == nil {
+			t.Error("expected error when no keys configured")
+		}
+	})
+
+	t.Run("short key rejected", func(t *testing.T) {
+		clearKeyEnv(t)
+		t.Setenv("API_KEY", "too-short")
+		if _, err := loadAPIKeys(); err == nil {
+			t.Error("expected error for short key")
+		}
+	})
+
+	t.Run("malformed JSON rejected", func(t *testing.T) {
+		clearKeyEnv(t)
+		t.Setenv("API_KEYS", `not json`)
+		if _, err := loadAPIKeys(); err == nil {
+			t.Error("expected error for malformed JSON")
+		}
+	})
+
+	t.Run("unknown field rejected", func(t *testing.T) {
+		clearKeyEnv(t)
+		// "collection" (singular) is a typo for "collections"; must fail loudly
+		// rather than silently leaving the key unscoped (full access).
+		t.Setenv("API_KEYS", `[{"name":"dev","key":"`+key32a+`","collection":["DEV"]}]`)
+		if _, err := loadAPIKeys(); err == nil {
+			t.Error("expected error for unknown JSON field")
+		}
+	})
+
+	t.Run("entry missing key rejected", func(t *testing.T) {
+		clearKeyEnv(t)
+		t.Setenv("API_KEYS", `[{"name":"x"}]`)
+		if _, err := loadAPIKeys(); err == nil {
+			t.Error("expected error for entry without key")
+		}
+	})
+
+	t.Run("duplicate key material rejected", func(t *testing.T) {
+		clearKeyEnv(t)
+		// Same key string used twice would let one entry silently override the
+		// other's scope in the store.
+		t.Setenv("API_KEYS", `[{"name":"a","key":"`+key32a+`"},{"name":"b","key":"`+key32a+`"}]`)
+		if _, err := loadAPIKeys(); err == nil {
+			t.Error("expected error for duplicate key material")
+		}
+	})
+}
+
+func TestParseInt(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		in       string
+		fallback int
+		want     int
+	}{
+		{"50", 30, 50},
+		{"", 30, 30},
+		{"abc", 30, 30},
+		{"0", 30, 30},
+		{"-5", 30, 30},
+	}
+	for _, tt := range tests {
+		if got := parseInt(tt.in, tt.fallback); got != tt.want {
+			t.Errorf("parseInt(%q, %d) = %d, want %d", tt.in, tt.fallback, got, tt.want)
+		}
+	}
+}
+
+func TestLoadRateLimitDefaultsAndOverrides(t *testing.T) {
+	clearKeyEnv(t)
+	t.Setenv("API_KEY", key32a)
+	t.Setenv("VAULTWARDEN_URL", "https://vault.example.com")
+
+	t.Run("defaults", func(t *testing.T) {
+		// Clear any inherited overrides so defaults are evaluated deterministically.
+		t.Setenv("RATE_LIMIT_MAX", "")
+		t.Setenv("RATE_LIMIT_WINDOW", "")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("Load: %v", err)
+		}
+		if cfg.RateLimitMax != 30 {
+			t.Errorf("RateLimitMax = %d, want 30", cfg.RateLimitMax)
+		}
+		if cfg.RateLimitWindow != time.Minute {
+			t.Errorf("RateLimitWindow = %v, want 1m", cfg.RateLimitWindow)
+		}
+	})
+
+	t.Run("overrides", func(t *testing.T) {
+		t.Setenv("RATE_LIMIT_MAX", "100")
+		t.Setenv("RATE_LIMIT_WINDOW", "30s")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("Load: %v", err)
+		}
+		if cfg.RateLimitMax != 100 {
+			t.Errorf("RateLimitMax = %d, want 100", cfg.RateLimitMax)
+		}
+		if cfg.RateLimitWindow != 30*time.Second {
+			t.Errorf("RateLimitWindow = %v, want 30s", cfg.RateLimitWindow)
+		}
+	})
+}