From 495edffde33f48880a9528d4ab1b09501bb25281 Mon Sep 17 00:00:00 2001 From: Thijs Herman <64640608+Turbootzz@users.noreply.github.com> Date: Thu, 28 May 2026 11:45:16 +0200 Subject: [PATCH] Harden API-key model: per-key scoping + revocable keys, and configurable/whitelist-aware rate limiting (#16) * API scope and rate limiting * improvements from review --- .env.example | 23 +++- README.md | 49 +++++++- cmd/api/main.go | 9 +- internal/auth/middleware.go | 69 ++++++++++- internal/auth/store_test.go | 145 ++++++++++++++++++++++ internal/config/config.go | 138 ++++++++++++++++++--- internal/config/config_test.go | 184 ++++++++++++++++++++++++++++ internal/handlers/handlers.go | 69 +++++++++++ internal/handlers/handlers_test.go | 107 +++++++++++++++- internal/vaultwarden/client.go | 47 +++++-- internal/vaultwarden/client_test.go | 45 ++++++- 11 files changed, 843 insertions(+), 42 deletions(-) create mode 100644 internal/auth/store_test.go create mode 100644 internal/config/config_test.go diff --git a/.env.example b/.env.example index 13888f4..6cda14a 100644 --- a/.env.example +++ b/.env.example @@ -15,12 +15,28 @@ VAULTWARDEN_EMAIL=you@example.com # Your Vaultwarden master password VAULTWARDEN_PASSWORD=your-master-password -# API key for authenticating to this service (min 32 chars) +# API key for authenticating to this service (min 32 chars). +# This single key has FULL access to every secret the account can decrypt. # Generate one: openssl rand -base64 32 +# At least one of API_KEY / API_KEYS / API_KEYS_FILE is required. API_KEY= # --- Optional --- +# Multiple, individually-revocable keys, each scoped to specific organizations +# and/or collections. Scope is enforced SERVER-SIDE regardless of query filters, +# so a scoped key can only read secrets within its scope. Each key needs >= 32 chars. +# Refs may be friendly names or UUIDs (UUIDs recommended — names are case-insensitive, +# break on rename, and a duplicate name binds to the lowest matching UUID). A key with +# no organizations/collections is unscoped (full access). Note: an org-scoped key cannot +# read personal/no-org items. +# +# Inline JSON (string env var): +# API_KEYS=[{"name":"dev-team","key":"<32+ chars>","organizations":["MyOrg"],"collections":["Secrets - DEV"]}] +# +# Or a mounted JSON file (takes precedence over API_KEYS; ideal as a Docker secret): +# API_KEYS_FILE=/run/secrets/api-keys.json + # Restrict access to specific IPs/CIDRs # ALLOWED_IPS=192.168.1.0/24,10.0.0.1 @@ -33,6 +49,11 @@ API_KEY= # How often to re-sync the vault (default: 5m) # SYNC_INTERVAL=5m +# Rate limiting (per client IP). Whitelisted IPs (ALLOWED_IPS / TRUSTED_PROXY_IP) +# bypass the limiter entirely. Defaults: 30 requests per 1m window. +# RATE_LIMIT_MAX=30 +# RATE_LIMIT_WINDOW=1m + # Environment (development shows detailed errors, production hides them) # ENVIRONMENT=production diff --git a/README.md b/README.md index 1384127..76df3ae 100644 --- a/README.md +++ b/README.md @@ -110,17 +110,57 @@ You can also use **custom fields** or **notes** — the API returns the most rel | `VAULTWARDEN_URL` | **Yes** | — | Your Vaultwarden instance URL | | `VAULTWARDEN_EMAIL` | **Yes** | — | Your Vaultwarden email | | `VAULTWARDEN_PASSWORD` | **Yes** | — | Your master password | -| `API_KEY` | **Yes** | — | API key for this service (min 32 chars) | +| `API_KEY` | Yes\* | — | Single full-access key for this service (min 32 chars) | +| `API_KEYS` | Yes\* | — | Inline JSON array of scoped keys (see [Scoped API keys](#scoped-api-keys)) | +| `API_KEYS_FILE` | Yes\* | — | Path to a JSON file of scoped keys; takes precedence over `API_KEYS` | | `VAULTWARDEN_CLIENT_ID` | No | — | API key client ID (bypasses 2FA — see below) | | `VAULTWARDEN_CLIENT_SECRET` | No | — | API key client secret (bypasses 2FA — see below) | | `ALLOWED_IPS` | No | (all) | Comma-separated IPs/CIDRs to whitelist | | `ENABLE_GITHUB_IP_RANGES` | No | `false` | Auto-whitelist GitHub Actions IPs | | `SYNC_INTERVAL` | No | `5m` | How often to re-sync the vault | | `CACHE_TTL` | No | `5m` | Secret cache duration | +| `RATE_LIMIT_MAX` | No | `30` | Max requests per window, per IP | +| `RATE_LIMIT_WINDOW` | No | `1m` | Rate-limit window duration | | `TRUSTED_PROXY_IP` | No | `localhost` | Trusted reverse proxy IPs | | `ENVIRONMENT` | No | `development` | Set to `production` to hide errors | | `DEBUG` | No | `false` | Enable debug logging | +\* At least one of `API_KEY`, `API_KEYS`, or `API_KEYS_FILE` is required. + +### Scoped API keys + +`API_KEY` is a single, full-access key — any holder can read every secret the account +can decrypt. For least-privilege, per-consumer access, define multiple keys with +`API_KEYS` (inline JSON) or `API_KEYS_FILE` (a JSON file, ideal as a mounted Docker +secret). Each key is scoped to allowed organizations and/or collections, and is +individually revocable (remove it from the config and re-deploy). + +```json +[ + { + "name": "dev-team", + "key": "<32+ character secret>", + "organizations": ["MyOrg"], + "collections": ["Secrets - DEV"] + } +] +``` + +- **Scope is enforced server-side**, regardless of the `organization_*` / `collection_*` + query filters. A scoped key can only ever read secrets within its scope — the query + filters can narrow within scope but never widen beyond it. +- **A key with no `organizations` and no `collections` is unscoped** (full access), + same as the legacy `API_KEY`. `API_KEY` keeps working alongside scoped keys. +- **Matching:** a secret is in scope when its organization is in the key's allowed + organizations (if any) **and** it belongs to at least one allowed collection (if any). + Consequently, an org-scoped key **cannot read personal / no-org items**. +- **Refs may be names or UUIDs.** UUIDs are recommended: name matching is + case-insensitive, silently stops matching if the org/collection is renamed, and a + duplicate name binds to the lowest matching UUID. Unresolvable refs **fail closed** + (the key matches nothing), including the brief window before the first vault sync. + +`API_KEYS_FILE` takes precedence over `API_KEYS` when both are set. + ### 2FA / Two-Step Login If your Vaultwarden account has 2FA enabled, password login will be blocked. You need to use API key login instead: @@ -251,8 +291,9 @@ resp, _ := http.DefaultClient.Do(req) ## Security - **API key authentication** with constant-time comparison (timing-attack resistant) +- **Per-key scoping** — multiple revocable keys, each restricted server-side to specific organizations/collections ([Scoped API keys](#scoped-api-keys)) - **IP whitelisting** with CIDR support + optional GitHub Actions IP auto-import -- **Rate limiting** (30 requests/minute per IP) +- **Rate limiting** (configurable via `RATE_LIMIT_MAX` / `RATE_LIMIT_WINDOW`, default 30/min per IP; whitelisted IPs are exempt) - **Read-only filesystem** in Docker (only `/tmp` writable) - **Non-root user** in container - **No capabilities** (`cap_drop: ALL`) @@ -327,8 +368,8 @@ However, for each dimension (organization | collection | folder) you can only fi | `Two factor required` | Account has 2FA enabled | Set `VAULTWARDEN_CLIENT_ID` and `VAULTWARDEN_CLIENT_SECRET` (see [2FA section](#2fa--two-step-login)) | | `MAC verification failed` | Wrong password or org-owned items | Normal for items shared via organizations — they use a different key | | `missing authorization header` | No Bearer token in request | Add `-H "Authorization: Bearer YOUR_API_KEY"` to your request | -| `secret not found` | Item name doesn't match | Check the exact name in your Vaultwarden vault (matching is case-insensitive) | -| Container exits immediately | Missing required env vars | Ensure `VAULTWARDEN_URL`, `VAULTWARDEN_EMAIL`, `VAULTWARDEN_PASSWORD`, and `API_KEY` are all set | +| `secret not found` | Item name doesn't match, or out of the key's scope | Check the exact name in your Vaultwarden vault (matching is case-insensitive); for a scoped key, confirm the secret is within its allowed orgs/collections | +| Container exits immediately | Missing required env vars | Ensure `VAULTWARDEN_URL`, `VAULTWARDEN_EMAIL`, `VAULTWARDEN_PASSWORD`, and one of `API_KEY` / `API_KEYS` / `API_KEYS_FILE` are set | **Debug mode:** Set `DEBUG=true` to see detailed logs including secret names being synced (don't use in production). diff --git a/cmd/api/main.go b/cmd/api/main.go index aabf6be..fb6c7f0 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -107,14 +107,19 @@ func main() { api := app.Group("/") api.Use(ipWhitelist.Middleware()) api.Use(limiter.New(limiter.Config{ - Max: 30, + Max: cfg.RateLimitMax, + Expiration: cfg.RateLimitWindow, + // Whitelisted/trusted IPs bypass rate limiting entirely. + Next: func(c *fiber.Ctx) bool { + return ipWhitelist.IsAllowed(c.IP()) + }, LimitReached: func(c *fiber.Ctx) error { return c.Status(fiber.StatusTooManyRequests).JSON(fiber.Map{ "error": "too many requests, please slow down", }) }, })) - api.Use(auth.Middleware(cfg.APIKey)) + api.Use(auth.Middleware(auth.NewStore(cfg.APIKeys))) api.Get("/secret/:name", h.GetSecret) api.Post("/refresh", h.RefreshCache) diff --git a/internal/auth/middleware.go b/internal/auth/middleware.go index 57b9033..382ffd4 100644 --- a/internal/auth/middleware.go +++ b/internal/auth/middleware.go @@ -5,12 +5,69 @@ import ( "crypto/subtle" "strings" - "github.com/gofiber/fiber/v2" "github.com/Turbootzz/vaultwarden-api/pkg/logger" + "github.com/gofiber/fiber/v2" ) -// Middleware creates an authentication middleware for API key validation -func Middleware(apiKey string) fiber.Handler { +// Scope limits which secrets a key may read, enforced server-side. +// Entries may be organization/collection names or UUIDs (resolved per-request). +// An empty scope (no organizations and no collections) grants full access. +type Scope struct { + Organizations []string + Collections []string +} + +// IsEmpty reports whether the scope imposes no constraint (full access). +func (s Scope) IsEmpty() bool { + return len(s.Organizations) == 0 && len(s.Collections) == 0 +} + +// APIKey is a single configured key with its server-side scope. +type APIKey struct { + Name string + Key string + Scope Scope +} + +// Store holds the configured API keys and resolves a presented key to its scope. +type Store struct { + keys []APIKey +} + +// NewStore builds a key store from the configured keys. +func NewStore(keys []APIKey) *Store { + return &Store{keys: keys} +} + +// Match returns the configured key matching the presented secret, if any. +// It compares against every key without short-circuiting so that timing does +// not reveal a key's position in the list. +func (s *Store) Match(provided string) (APIKey, bool) { + var matched APIKey + found := false + for _, k := range s.keys { + if secureCompare(provided, k.Key) { + matched = k + found = true + } + } + return matched, found +} + +// ctxKey is the unexported type for the scope stored in the request context. +type ctxKey struct{} + +var scopeKey ctxKey + +// ScopeFromCtx returns the authenticated key's scope from the request context. +func ScopeFromCtx(c *fiber.Ctx) (Scope, bool) { + scope, ok := c.Locals(scopeKey).(Scope) + return scope, ok +} + +// Middleware creates an authentication middleware that validates the bearer +// API key against the store and attaches the matched key's scope to the context. +func Middleware(store *Store) fiber.Handler { return func(c *fiber.Ctx) error { // Get the Authorization header authHeader := c.Get("Authorization") @@ -33,14 +90,16 @@ func Middleware(apiKey string) fiber.Handler { providedKey := parts[1] - // Use constant-time comparison to prevent timing attacks - if !secureCompare(providedKey, apiKey) { + key, ok := store.Match(providedKey) + if !ok { logger.Warn.Printf("Invalid API key from IP: %s", c.IP()) return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{ "error": "invalid api key", }) } + c.Locals(scopeKey, key.Scope) + // Authentication successful return c.Next() } diff --git a/internal/auth/store_test.go b/internal/auth/store_test.go new file mode 100644 index 0000000..d1d23b3 --- /dev/null +++ b/internal/auth/store_test.go @@ -0,0 +1,145 @@ +package auth + +import ( + "io" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + + "github.com/gofiber/fiber/v2" + "github.com/valyala/fasthttp" +) + +const ( + keyFull = "full-access-key-0000000000000000000000" + keyScoped = "scoped-key-1111111111111111111111111111" +) + +func testStore() *Store { + return NewStore([]APIKey{ + {Name: "full", Key: keyFull}, + {Name: "dev", Key: keyScoped, Scope: Scope{Collections: []string{"Secrets - DEV"}}}, + }) +} + +func TestStoreMatch(t *testing.T) { + t.Parallel() + + store := testStore() + + tests := []struct { + name string + provided string + wantOK bool + wantName string + wantScope Scope + }{ + {"full access key", keyFull, true, "full", Scope{}}, + {"scoped key", keyScoped, true, "dev", Scope{Collections: []string{"Secrets - DEV"}}}, + {"unknown key", "nope-nope-nope-nope-nope-nope-nope-nope", false, "", Scope{}}, + {"empty provided", "", false, "", Scope{}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, ok := store.Match(tt.provided) + if ok != tt.wantOK { + t.Fatalf("Match ok = %v, want %v", ok, tt.wantOK) + } + if !tt.wantOK { + return + } + if got.Name != tt.wantName { + t.Errorf("Match name = %q, want %q", got.Name, tt.wantName) + } + if !reflect.DeepEqual(got.Scope, tt.wantScope) { + t.Errorf("Match scope = %+v, want %+v", got.Scope, tt.wantScope) + } + }) + } +} + +func TestStoreMatchEmptyStore(t *testing.T) { + t.Parallel() + if _, ok := NewStore(nil).Match(keyFull); ok { + t.Error("empty store should not match any key") + } +} + +func TestScopeIsEmpty(t *testing.T) { + t.Parallel() + if !(Scope{}).IsEmpty() { + t.Error("zero scope should be empty") + } + if (Scope{Organizations: []string{"x"}}).IsEmpty() { + t.Error("scope with organizations should not be empty") + } + if (Scope{Collections: []string{"x"}}).IsEmpty() { + t.Error("scope with collections should not be empty") + } +} + +func TestMiddleware(t *testing.T) { + t.Parallel() + + app := fiber.New() + app.Use(Middleware(testStore())) + app.Get("/", func(c *fiber.Ctx) error { + scope, ok := ScopeFromCtx(c) + if !ok { + return c.Status(fiber.StatusInternalServerError).SendString("no scope") + } + if scope.IsEmpty() { + return c.SendString("full") + } + return c.SendString("scoped") + }) + + tests := []struct { + name string + header string + wantStatus int + wantBody string + }{ + {"missing header", "", http.StatusUnauthorized, "missing authorization header"}, + {"malformed header", "Token abc", http.StatusUnauthorized, "invalid authorization header format"}, + {"unknown key", "Bearer wrong-key", http.StatusUnauthorized, "invalid api key"}, + {"full access key", "Bearer " + keyFull, http.StatusOK, "full"}, + {"scoped key", "Bearer " + keyScoped, http.StatusOK, "scoped"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + if tt.header != "" { + req.Header.Set("Authorization", tt.header) + } + resp, err := app.Test(req, -1) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != tt.wantStatus { + t.Errorf("status = %d, want %d", resp.StatusCode, tt.wantStatus) + } + body, _ := io.ReadAll(resp.Body) + if !strings.Contains(string(body), tt.wantBody) { + t.Errorf("body = %q, want substring %q", body, tt.wantBody) + } + }) + } +} + +func TestScopeFromCtxAbsent(t *testing.T) { + t.Parallel() + app := fiber.New() + ctx := app.AcquireCtx(&fasthttp.RequestCtx{}) + defer app.ReleaseCtx(ctx) + if _, ok := ScopeFromCtx(ctx); ok { + t.Error("ScopeFromCtx should report false when no scope set") + } +} diff --git a/internal/config/config.go b/internal/config/config.go index d822236..5d80a15 100644 --- a/internal/config/config.go +++ b/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 diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..48fda72 --- /dev/null +++ b/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) + } + }) +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 244a0e4..045a5fa 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -7,6 +7,7 @@ import ( "net/url" "strings" + "github.com/Turbootzz/vaultwarden-api/internal/auth" "github.com/Turbootzz/vaultwarden-api/internal/validators" "github.com/Turbootzz/vaultwarden-api/internal/vaultwarden" "github.com/Turbootzz/vaultwarden-api/pkg/logger" @@ -91,6 +92,14 @@ func (h *Handler) GetSecret(c *fiber.Ctx) error { }) } + // Enforce the authenticated key's scope server-side, regardless of query filters. + if !h.applyKeyScope(c, &filter) { + logger.Warn.Printf("Request denied by key scope from IP: %s", c.IP()) + return c.Status(fiber.StatusNotFound).JSON(fiber.Map{ + "error": "secret not found", + }) + } + value, err := h.vaultClient.GetSecret(secretName, filter) if err != nil { logger.Error.Printf("Failed to fetch secret (requested by IP: %s)", c.IP()) @@ -135,6 +144,66 @@ func resolveDim(dim, name, id string, nameMap map[string]string, out *string) er return nil } +// resolveRef resolves a scope reference that may be either a UUID or a friendly name. +// UUIDs pass through (existence not verified); names are looked up via NameMaps. +func resolveRef(nameMap map[string]string, ref string) (string, bool) { + ref = strings.TrimSpace(ref) + if ref == "" { + return "", false + } + if parsed, err := uuid.Parse(ref); err == nil { + return parsed.String(), true + } + return vaultwarden.LookupIDByName(nameMap, ref) +} + +// resolveScopeRefs maps scope refs (UUIDs or names) to UUIDs, dropping any that +// don't resolve. The caller treats an all-unresolved dimension as deny (fail closed). +func resolveScopeRefs(refs []string, nameMap map[string]string) []string { + out := make([]string, 0, len(refs)) + for _, ref := range refs { + if id, ok := resolveRef(nameMap, ref); ok { + out = append(out, id) + } + } + return out +} + +// applyKeyScope sets the server-side scope fields on the filter from the +// authenticated key's scope. It returns false to deny the request (404) when a +// constrained dimension resolves to nothing — including unknown names and the +// pre-first-sync window — so a scoped key can never read outside its scope. +func (h *Handler) applyKeyScope(c *fiber.Ctx, filter *vaultwarden.SecretFilter) bool { + scope, ok := auth.ScopeFromCtx(c) + if !ok { + // No scope in context means the auth middleware did not run for this + // request. Fail closed rather than silently granting full access. + return false + } + if scope.IsEmpty() { + return true // unscoped key: full access + } + + nm := h.vaultClient.NameMaps() + + if len(scope.Organizations) > 0 { + ids := resolveScopeRefs(scope.Organizations, nm.Organizations) + if len(ids) == 0 { + return false + } + filter.OrganizationIDs = ids + } + if len(scope.Collections) > 0 { + ids := resolveScopeRefs(scope.Collections, nm.Collections) + if len(ids) == 0 { + return false + } + filter.CollectionIDs = ids + } + + return true +} + // parseSecretFilters reads placement query params: at most one of id or name per dimension. // Name-based filters are resolved against h.vaultClient.NameMaps(); unknown names fail. // Id-based filters are accepted as-is after UUID parsing (existence is not checked here). diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 8e2acbc..1947445 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -5,9 +5,11 @@ import ( "io" "net/http" "net/http/httptest" + "reflect" "strings" "testing" + "github.com/Turbootzz/vaultwarden-api/internal/auth" "github.com/Turbootzz/vaultwarden-api/internal/vaultwarden" "github.com/gofiber/fiber/v2" "github.com/valyala/fasthttp" @@ -221,7 +223,7 @@ func TestParseSecretFilters(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != tt.want { + if !reflect.DeepEqual(got, tt.want) { t.Errorf("filter = %+v, want %+v", got, tt.want) } }) @@ -229,8 +231,11 @@ func TestParseSecretFilters(t *testing.T) { } func TestGetSecret(t *testing.T) { + const fullKey = "full-access-key-for-getsecret-test-000000" h := NewHandler(vaultwarden.NewClient(nil, 0, 0, vaultwarden.WithState(testVaultItems(), testNameMaps()))) app := fiber.New() + // Mirror production: auth runs first and attaches a (here unscoped) scope. + app.Use(auth.Middleware(auth.NewStore([]auth.APIKey{{Name: "full", Key: fullKey}}))) app.Get("/secret/:name", h.GetSecret) // Test the GetSecret handler with various input scenarios @@ -315,6 +320,106 @@ func TestGetSecret(t *testing.T) { url += "?" + tt.query } req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, url, nil) + req.Header.Set("Authorization", "Bearer "+fullKey) + resp, err := app.Test(req, -1) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != tt.wantStatus { + t.Errorf("status = %d, want %d", resp.StatusCode, tt.wantStatus) + } + + body, _ := io.ReadAll(resp.Body) + if tt.wantStatus == http.StatusOK { + var payload map[string]string + if err := json.Unmarshal(body, &payload); err != nil { + t.Fatalf("json: %v", err) + } + if payload["value"] != tt.wantBody { + t.Errorf("value = %q, want %q", payload["value"], tt.wantBody) + } + return + } + if !strings.Contains(string(body), tt.wantBody) { + t.Errorf("body = %s, want substring %q", body, tt.wantBody) + } + }) + } +} + +// TestGetSecretFailsClosedWithoutAuth verifies that if the handler is reached +// without the auth middleware (no scope in context), it denies rather than +// granting full access. +func TestGetSecretFailsClosedWithoutAuth(t *testing.T) { + h := NewHandler(vaultwarden.NewClient(nil, 0, 0, vaultwarden.WithState(testVaultItems(), testNameMaps()))) + app := fiber.New() + app.Get("/secret/:name", h.GetSecret) // intentionally no auth.Middleware + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/secret/db-password", nil) + resp, err := app.Test(req, -1) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNotFound { + t.Errorf("status = %d, want %d (fail closed)", resp.StatusCode, http.StatusNotFound) + } +} + +func TestGetSecretScoped(t *testing.T) { + h := NewHandler(vaultwarden.NewClient(nil, 0, 0, vaultwarden.WithState(testVaultItems(), testNameMaps()))) + + // Keys wired through the real auth middleware so scope flows via c.Locals. + const ( + fullKey = "full-access-0000000000000000000000000000" + colKey = "collection-scoped-11111111111111111111111" + orgKey = "org-scoped-2222222222222222222222222222222" + badScopeKey = "bad-scope-33333333333333333333333333333333" + ) + store := auth.NewStore([]auth.APIKey{ + {Name: "full", Key: fullKey}, + {Name: "dev", Key: colKey, Scope: auth.Scope{Collections: []string{"Shared"}}}, + {Name: "acme", Key: orgKey, Scope: auth.Scope{Organizations: []string{"Acme"}}}, + {Name: "broken", Key: badScopeKey, Scope: auth.Scope{Collections: []string{"Nonexistent"}}}, + }) + + app := fiber.New() + app.Use(auth.Middleware(store)) + app.Get("/secret/:name", h.GetSecret) + + tests := []struct { + name string + key string + path string + query string + wantStatus int + wantBody string + }{ + // db-password (cipher-1) lives in org "Acme" / collection "Shared". + {"collection scope can read in-scope secret", colKey, "/secret/db-password", "", http.StatusOK, "s3cret"}, + // other-password (cipher-2) has no collection -> out of a collection scope. + {"collection scope blocks out-of-scope secret", colKey, "/secret/other-password", "", http.StatusNotFound, "secret not found"}, + {"org scope can read in-scope secret", orgKey, "/secret/db-password", "", http.StatusOK, "s3cret"}, + // other-password is in a different org -> blocked server-side regardless of query. + {"org scope blocks other org secret", orgKey, "/secret/other-password", "", http.StatusNotFound, "secret not found"}, + {"client filter cannot widen beyond org scope", orgKey, "/secret/other-password", "organization_id=" + testOtherOrgID, http.StatusNotFound, "secret not found"}, + // Unscoped (full-access) key sees everything. + {"full access reads other org secret", fullKey, "/secret/other-password", "", http.StatusOK, "other-org"}, + // Scope referencing an unknown collection name fails closed. + {"unresolvable scope fails closed", badScopeKey, "/secret/db-password", "", http.StatusNotFound, "secret not found"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + url := tt.path + if tt.query != "" { + url += "?" + tt.query + } + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, url, nil) + req.Header.Set("Authorization", "Bearer "+tt.key) resp, err := app.Test(req, -1) if err != nil { t.Fatalf("app.Test: %v", err) diff --git a/internal/vaultwarden/client.go b/internal/vaultwarden/client.go index 8c331c6..308fcfe 100644 --- a/internal/vaultwarden/client.go +++ b/internal/vaultwarden/client.go @@ -73,32 +73,55 @@ func (c *Client) Initialize() error { } // SecretFilter limits lookup by vault placement. Empty fields are ignored (no constraint). -// Use at most one of organization id vs name, etc., enforced at the HTTP layer. +// +// The singular fields are client-supplied query filters (use at most one of id vs +// name per dimension, enforced at the HTTP layer). The plural fields are server-set +// from the authenticated key's scope and act as a hard boundary: a request must match +// both the client filter and the scope, so a client filter can only narrow within the +// scope, never widen beyond it. Empty plural slices impose no constraint. type SecretFilter struct { OrganizationID string CollectionID string FolderID string + + OrganizationIDs []string + CollectionIDs []string +} + +func containsFold(ids []string, target string) bool { + for _, id := range ids { + if strings.EqualFold(id, target) { + return true + } + } + return false +} + +func intersectsFold(a, b []string) bool { + for _, id := range a { + if containsFold(b, id) { + return true + } + } + return false } func matchesSecretFilter(item DecryptedItem, f SecretFilter) bool { if f.OrganizationID != "" && !strings.EqualFold(item.OrganizationID, f.OrganizationID) { return false } - if f.CollectionID != "" { - found := false - for _, id := range item.CollectionIDs { - if strings.EqualFold(id, f.CollectionID) { - found = true - break - } - } - if !found { - return false - } + if f.CollectionID != "" && !containsFold(item.CollectionIDs, f.CollectionID) { + return false } if f.FolderID != "" && !strings.EqualFold(item.FolderID, f.FolderID) { return false } + if len(f.OrganizationIDs) > 0 && !containsFold(f.OrganizationIDs, item.OrganizationID) { + return false + } + if len(f.CollectionIDs) > 0 && !intersectsFold(item.CollectionIDs, f.CollectionIDs) { + return false + } return true } diff --git a/internal/vaultwarden/client_test.go b/internal/vaultwarden/client_test.go index c9de93c..719ce07 100644 --- a/internal/vaultwarden/client_test.go +++ b/internal/vaultwarden/client_test.go @@ -55,7 +55,7 @@ func TestMatchesSecretFilter(t *testing.T) { SecretFilter{ OrganizationID: testOrgID, CollectionID: testCollectionID, - FolderID: testFolderID, + FolderID: testFolderID, }, true, }, @@ -64,6 +64,35 @@ func TestMatchesSecretFilter(t *testing.T) { SecretFilter{OrganizationID: testOrgID, CollectionID: "77777777-7777-4777-8777-777777777777"}, false, }, + // Server-side scope (plural fields). + {"scope org in set", SecretFilter{OrganizationIDs: []string{testOrgID2, testOrgID}}, true}, + {"scope org not in set", SecretFilter{OrganizationIDs: []string{testOrgID2}}, false}, + {"scope collection intersects", SecretFilter{CollectionIDs: []string{testCollectionID}}, true}, + { + "scope collection disjoint", + SecretFilter{CollectionIDs: []string{"77777777-7777-4777-8777-777777777777"}}, + false, + }, + { + "scope org and collection both match", + SecretFilter{OrganizationIDs: []string{testOrgID}, CollectionIDs: []string{testCollectionID}}, + true, + }, + { + "scope org ok but collection disjoint", + SecretFilter{OrganizationIDs: []string{testOrgID}, CollectionIDs: []string{"77777777-7777-4777-8777-777777777777"}}, + false, + }, + { + "scope narrowed further by client filter", + SecretFilter{OrganizationID: testOrgID, OrganizationIDs: []string{testOrgID}}, + true, + }, + { + "client filter cannot widen beyond scope", + SecretFilter{OrganizationID: testOrgID, OrganizationIDs: []string{testOrgID2}}, + false, + }, } for _, tt := range tests { @@ -75,3 +104,17 @@ func TestMatchesSecretFilter(t *testing.T) { }) } } + +func TestMatchesSecretFilter_PersonalItemExcludedByOrgScope(t *testing.T) { + t.Parallel() + + // A personal (no-org) item must be excluded once a key scopes to organizations. + personal := DecryptedItem{ID: "personal-1", Name: "personal-secret"} + + if matchesSecretFilter(personal, SecretFilter{OrganizationIDs: []string{testOrgID}}) { + t.Error("personal item should be excluded by an org scope") + } + if !matchesSecretFilter(personal, SecretFilter{}) { + t.Error("personal item should match an empty (full-access) scope") + } +}