diff --git a/.env.example b/.env.example index 6e14c8a..82e1340 100644 --- a/.env.example +++ b/.env.example @@ -12,7 +12,8 @@ DATABASE_URL=postgres://uniauth:uniauth@localhost:5432/uniauth?sslmode=disable REDIS_URL=redis://localhost:6379/0 # ─── Auth ───────────────────────────────────────────────────────────────────── -# REQUIRED: Generate with: openssl rand -base64 32 +# REQUIRED: Generate once with: openssl rand -hex 32 +# Reuse the same generated value on every UniAuth instance. JWT_SECRET= ACCESS_TOKEN_DURATION=15m diff --git a/README.md b/README.md index 1ef2286..83e9e08 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ git clone https://github.com/osama1998h/uniauth.git && cd uniauth # 2. Configure cp .env.example .env -# Edit .env — at minimum set JWT_SECRET +# Edit .env — generate JWT_SECRET with: openssl rand -hex 32 # 3. Start docker compose up @@ -136,7 +136,7 @@ All configuration is via environment variables (see `.env.example`): | `TRUSTED_PROXY_CIDRS` | — | Comma-separated proxy CIDRs or IPs allowed to supply forwarded client IP headers; empty means trust no proxies | | `DATABASE_URL` | — | PostgreSQL connection string (required) | | `REDIS_URL` | `redis://localhost:6379/0` | Redis connection string | -| `JWT_SECRET` | — | HMAC secret for JWTs (required, min 32 chars) | +| `JWT_SECRET` | — | HMAC secret for JWTs (required, random 32+ chars, not a placeholder/default) | | `ACCESS_TOKEN_DURATION` | `15m` | Access token lifetime | | `REFRESH_TOKEN_DURATION` | `168h` | Refresh token lifetime (7 days) | | `RESET_TOKEN_DURATION` | `1h` | Password reset token lifetime | @@ -154,10 +154,12 @@ If UniAuth is deployed behind a reverse proxy or load balancer, set `TRUSTED_PRO ### Docker ```bash +export JWT_SECRET="$(openssl rand -hex 32)" + docker run -d \ -e DATABASE_URL="postgres://..." \ -e REDIS_URL="redis://..." \ - -e JWT_SECRET="your-secret" \ + -e JWT_SECRET="$JWT_SECRET" \ -e TRUSTED_PROXY_CIDRS="10.0.0.0/8" \ -p 8080:8080 \ ghcr.io/osama1998h/uniauth:latest diff --git a/cmd/server/main.go b/cmd/server/main.go index d67d5f8..6aaea9c 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -112,12 +112,6 @@ func main() { defer func() { _ = redisCache.Close() }() logger.Info("redis connected") - // JWT secret validation - if cfg.Auth.JWTSecret == "" { - logger.Error("JWT_SECRET must be set") - os.Exit(1) - } - // HTTP server handler := api.NewRouter(cfg, store, redisCache, logger) srv := &http.Server{ diff --git a/docker-compose.yml b/docker-compose.yml index 7c696d2..0810875 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,7 +12,7 @@ services: environment: DATABASE_URL: postgres://uniauth:uniauth@postgres:5432/uniauth?sslmode=disable REDIS_URL: redis://redis:6379/0 - JWT_SECRET: change-me-in-production-use-a-long-random-string + JWT_SECRET: ${JWT_SECRET:?set JWT_SECRET in .env or environment} ENVIRONMENT: development PORT: 8080 APP_BASE_URL: http://localhost:8080 diff --git a/docs/horizontal-scaling.md b/docs/horizontal-scaling.md index a2adf0a..e3cea60 100644 --- a/docs/horizontal-scaling.md +++ b/docs/horizontal-scaling.md @@ -55,7 +55,9 @@ This means Redis is part of UniAuth's auth control plane. It should be highly av Every instance must use the **same** `JWT_SECRET`. Tokens are signed with HMAC-SHA256; a token signed by one instance must be verifiable by any other. ``` -JWT_SECRET=your-secret-minimum-32-characters-long +# Generate once and reuse across every instance: +# openssl rand -hex 32 +JWT_SECRET= ``` Never rotate this secret without a coordinated rollout strategy — all existing tokens become invalid immediately. @@ -111,6 +113,12 @@ The default Kubernetes rolling update strategy works correctly because: For local testing or simple deployments, you can run multiple app instances behind an Nginx or Traefik reverse proxy: +Generate and export a shared secret once before starting the stack: + +```bash +export JWT_SECRET="$(openssl rand -hex 32)" +``` + ```yaml # docker-compose.scale.yml version: "3.8" @@ -122,7 +130,7 @@ services: environment: DATABASE_URL: postgres://postgres:postgres@postgres:5432/uniauth?sslmode=disable REDIS_URL: redis://redis:6379/0 - JWT_SECRET: your-secret-minimum-32-characters + JWT_SECRET: ${JWT_SECRET} depends_on: - postgres - redis @@ -197,7 +205,7 @@ All instances must share these values: | Variable | Requirement | |----------|-------------| -| `JWT_SECRET` | Identical on all instances; min 32 chars | +| `JWT_SECRET` | Identical on all instances; random 32+ chars; do not use placeholders/defaults | | `DATABASE_URL` | Points to the same PostgreSQL instance/cluster | | `REDIS_URL` | Points to the same Redis instance/cluster | | `ACCESS_TOKEN_DURATION` | Should be consistent (default `15m`) | diff --git a/docs/security-report.md b/docs/security-report.md index f37268d..17fd7f5 100644 --- a/docs/security-report.md +++ b/docs/security-report.md @@ -1,9 +1,9 @@ **1. Executive Summary** -This repo’s remaining main security problems are now concentrated in configuration and deployment hardening. The highest-risk remaining issue is weak JWT secret acceptance, followed by the remaining lower-severity readiness and supply-chain hardening gaps. +This repo’s remaining main security problems are now concentrated in lower-severity readiness and deployment hardening gaps. -Update (2026-03-28): findings 1, 2, 3, 4, 5, 6, 7, and 8 have been fixed in the repo. The remaining findings below are still outstanding unless explicitly marked otherwise. +Update (2026-03-28): findings 1, 2, 3, 4, 5, 6, 7, 8, and 9 have been fixed in the repo. The remaining findings below are still outstanding unless explicitly marked otherwise. -This report began as a read-only review of the codebase across auth/session logic, authorization/tenant boundaries, and config/deployment. Findings 1, 2, 3, 4, 5, 6, 7, and 8 have since been remediated in the repo. `go test ./...` passed locally; `govulncheck` was not installed, so dependency-CVE verification remains open. +This report began as a read-only review of the codebase across auth/session logic, authorization/tenant boundaries, and config/deployment. Findings 1, 2, 3, 4, 5, 6, 7, 8, and 9 have since been remediated in the repo. `go test ./...` passed locally; `govulncheck` was not installed, so dependency-CVE verification remains open. **2. Architecture and Attack Surface** - Entry points: public auth routes, `/health`, `/ready`, `/swagger/*`, and JWT-protected `/api/v1/*` in [router.go](/Users/osamamuhammed/uniauth/internal/api/router.go#L76). @@ -51,7 +51,9 @@ This report began as a read-only review of the codebase across auth/session logi Status: Fixed. Remediation implemented: Redis-backed auth controls now use a strict-auth fail-closed policy: `JWTAuth` returns `503` when blacklist lookups fail, `/api/v1/auth/*` requests return `503` when rate limiting cannot be enforced, and logout/password-change flows abort with `ErrServiceUnavailable` before mutating DB-backed auth state if the access-token blacklist write fails. Supporting docs were updated in [horizontal-scaling.md](/Users/osamamuhammed/uniauth/docs/horizontal-scaling.md#L1). Regression coverage was added in [auth_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/auth_test.go#L1), [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1), [auth_redis_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_redis_test.go#L1), and [response_test.go](/Users/osamamuhammed/uniauth/internal/api/handlers/response_test.go#L1). -9. **JWT secret strength is documented but not enforced** — Medium, Medium confidence. Affected: [main.go](/Users/osamamuhammed/uniauth/cmd/server/main.go#L115), [config.go](/Users/osamamuhammed/uniauth/internal/config/config.go#L53), [docker-compose](/Users/osamamuhammed/uniauth/docker-compose.yml#L12). Evidence: startup only checks `JWT_SECRET != ""`; docs say “minimum 32 characters,” but code does not enforce it. Risk: weak or placeholder HMAC secrets make token forgery practical in misconfigured self-hosted deployments. Preconditions: operator sets a weak/default secret. Fix: reject short or known-placeholder secrets at startup. Verify with config validation tests. OWASP: A02 Cryptographic Failures. +9. **JWT secret strength is documented but not enforced** — Medium, Medium confidence. Affected: [config load](/Users/osamamuhammed/uniauth/internal/config/config.go#L75), [JWT secret validator](/Users/osamamuhammed/uniauth/internal/config/config.go#L148), [main.go](/Users/osamamuhammed/uniauth/cmd/server/main.go#L78), [docker-compose](/Users/osamamuhammed/uniauth/docker-compose.yml#L15). Evidence before remediation: startup only checked `JWT_SECRET != ""`; docs said “minimum 32 characters,” but code did not enforce it. Risk: weak or placeholder HMAC secrets make token forgery practical in misconfigured self-hosted deployments. Preconditions: operator sets a weak/default secret. Fix: reject short or known-placeholder secrets at startup. Verify with config validation tests. OWASP: A02 Cryptographic Failures. + Status: Fixed. + Remediation implemented: `config.Load()` now rejects blank, short, and normalized placeholder/default `JWT_SECRET` values before any dependency startup, startup relies on config-load failure instead of a late empty-only check, and local Docker Compose now requires the secret from the operator environment instead of shipping a repo placeholder. Regression coverage was added in [config_test.go](/Users/osamamuhammed/uniauth/internal/config/config_test.go#L1). **Lower-Severity Findings** - **`/ready` leaks backend error details** — Low, High confidence. Affected: [health handler](/Users/osamamuhammed/uniauth/internal/api/handlers/health.go#L40). It returns raw DB/Redis error text to unauthenticated callers, which can expose internal topology or outage details. Fix: return generic health states and log details server-side. @@ -69,7 +71,8 @@ This report began as a read-only review of the codebase across auth/session logi - Completed hotfix 4: stop logging reset tokens and fail closed when reset-email delivery is unavailable. Complexity: Small. Regression risk: Low. Order: 4. - Completed hardening 1: add webhook URL validation and outbound SSRF guards. Complexity: Medium. Regression risk: Medium. Order: 5. - Completed hardening 2: replace naive forwarded-header trust with trusted-proxy-aware IP extraction and explicit proxy CIDR configuration. Complexity: Small-Medium. Regression risk: Medium. Order: 6. -- Short-term hardening 3: enforce JWT secret quality and sanitize `/ready`. Complexity: Small. Regression risk: Low-Medium. Order: 7. +- Completed hardening 3: enforce JWT secret quality at config load and require operator-supplied secrets in Docker Compose. Complexity: Small. Regression risk: Low-Medium. Order: 7. +- Short-term hardening 4: sanitize `/ready`. Complexity: Small. Regression risk: Low. Order: 8. - Completed structural improvement 1: introduce reusable authorization middleware/policy mapping instead of ad hoc handler checks. Complexity: High. Regression risk: Medium. - Completed structural improvement 2: add DB-level same-org enforcement for role assignments and auto-remove legacy cross-tenant links during migration. Complexity: Medium. Regression risk: Medium. - Structural improvement 3: pin CI actions/artifacts/images and add `govulncheck` to CI. Complexity: Small. Regression risk: Low. @@ -83,7 +86,7 @@ This report began as a read-only review of the codebase across auth/session logi - Implemented: RBAC service tests now reject foreign-org role assignment and permission assignment, and verify `Authorize` returns `ErrForbidden` for missing grants while allowing superuser bypass in [rbac_scope_test.go](/Users/osamamuhammed/uniauth/internal/service/rbac_scope_test.go#L1). - Implemented: trusted-proxy-aware IP resolution tests now confirm untrusted peers cannot spoof forwarded headers, trusted proxy chains resolve the correct client IP, and rate limiting buckets remain stable under header spoofing unless the peer is explicitly trusted in [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1). - Implemented: webhook validation and delivery tests now reject private/link-local/metadata webhook targets and ensure blocked destinations are not dialed in [webhook_test.go](/Users/osamamuhammed/uniauth/internal/service/webhook_test.go#L1) and [webhooks_test.go](/Users/osamamuhammed/uniauth/internal/api/handlers/webhooks_test.go#L1). -- Add startup/config tests that weak or placeholder `JWT_SECRET` values fail fast. +- Implemented: startup/config tests now prove blank, short, and placeholder `JWT_SECRET` values fail fast without echoing the configured secret, while valid secrets continue to load successfully in [config_test.go](/Users/osamamuhammed/uniauth/internal/config/config_test.go#L1). - Implemented: password reset email tests now confirm reset tokens and links are never written to logs in [email_test.go](/Users/osamamuhammed/uniauth/internal/service/email_test.go#L1). - Implemented: auth reset-flow tests now confirm failed delivery deletes reset-token rows while successful delivery preserves a usable token in [auth_reset_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_reset_test.go#L1). - Implemented: Redis outage regression tests now confirm blacklist lookup failures return `503`, auth-route rate limiting fails closed with `503`, non-auth public routes remain available, and logout/password-change flows do not mutate DB-backed auth state when blacklist writes fail in [auth_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/auth_test.go#L1), [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1), and [auth_redis_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_redis_test.go#L1). @@ -94,4 +97,4 @@ This report began as a read-only review of the codebase across auth/session logi - I did not verify pod/network egress policy. Existing egress controls remain useful defense-in-depth on top of the new webhook validation and delivery guards. - I could not verify dependency-level vulns because `govulncheck` is not installed in this workspace. -Quick note: 3 security issues remain open in the report: finding 9 plus the 2 lower-severity hardening items. +Quick note: 2 security issues remain open in the report: the 2 lower-severity hardening items. diff --git a/internal/config/config.go b/internal/config/config.go index 6d1c3d7..4cf6b28 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,10 +1,12 @@ package config import ( + "errors" "fmt" "net" "strings" "time" + "unicode" "github.com/spf13/viper" ) @@ -54,6 +56,22 @@ type EmailConfig struct { BaseURL string `mapstructure:"APP_BASE_URL"` // used in reset email links } +const minJWTSecretLength = 32 + +var ( + errInvalidJWTSecret = errors.New("invalid JWT_SECRET: must be non-empty, at least 32 characters, and not use a known placeholder; generate one with: openssl rand -hex 32") + + blockedJWTSecrets = map[string]struct{}{ + "secret": {}, + "jwtsecret": {}, + "changeme": {}, + "yoursecret": {}, + "yoursecretminimum32characters": {}, + "yoursecretminimum32characterslong": {}, + "changemeinproductionusealongrandomstring": {}, + } +) + // Load reads configuration from environment variables (and optionally a .env file). func Load() (*Config, error) { v := viper.New() @@ -120,9 +138,43 @@ func Load() (*Config, error) { BaseURL: v.GetString("APP_BASE_URL"), } + if err := validateJWTSecret(cfg.Auth.JWTSecret); err != nil { + return nil, err + } + return cfg, nil } +func validateJWTSecret(secret string) error { + trimmed := strings.TrimSpace(secret) + if trimmed == "" { + return errInvalidJWTSecret + } + + if len(trimmed) < minJWTSecretLength { + return errInvalidJWTSecret + } + + if _, blocked := blockedJWTSecrets[normalizeJWTSecretCandidate(trimmed)]; blocked { + return errInvalidJWTSecret + } + + return nil +} + +func normalizeJWTSecretCandidate(secret string) string { + var normalized strings.Builder + normalized.Grow(len(secret)) + + for _, r := range strings.ToLower(secret) { + if unicode.IsLetter(r) || unicode.IsDigit(r) { + normalized.WriteRune(r) + } + } + + return normalized.String() +} + func parseTrustedProxyCIDRs(raw string) ([]*net.IPNet, error) { if strings.TrimSpace(raw) == "" { return nil, nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index fd55c84..97283f7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,11 @@ package config -import "testing" +import ( + "strings" + "testing" +) + +const validJWTSecret = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" func TestParseTrustedProxyCIDRs(t *testing.T) { t.Parallel() @@ -43,3 +48,109 @@ func TestParseTrustedProxyCIDRs(t *testing.T) { } }) } + +func TestValidateJWTSecret(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + secret string + wantErr bool + }{ + { + name: "accepts random 32 plus character secret", + secret: validJWTSecret, + wantErr: false, + }, + { + name: "rejects blank secret", + secret: " ", + wantErr: true, + }, + { + name: "rejects short secret", + secret: "short-secret", + wantErr: true, + }, + { + name: "rejects placeholder variant after normalization", + secret: "Change-Me in production, use a long random string!!!", + wantErr: true, + }, + { + name: "rejects placeholder variant with punctuation and case changes", + secret: "Your-Secret minimum_32 characters long!!!", + wantErr: true, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := validateJWTSecret(tc.secret) + if tc.wantErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tc.wantErr && err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + } +} + +func TestLoadValidatesJWTSecret(t *testing.T) { + tests := []struct { + name string + secret string + wantErr bool + checkMessage bool + }{ + { + name: "rejects short secret", + secret: "too-short-secret", + wantErr: true, + }, + { + name: "rejects placeholder secret without echoing it", + secret: "change-me-in-production-use-a-long-random-string", + wantErr: true, + checkMessage: true, + }, + { + name: "accepts valid secret", + secret: validJWTSecret, + wantErr: false, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Setenv("JWT_SECRET", tc.secret) + t.Setenv("TRUSTED_PROXY_CIDRS", "") + + cfg, err := Load() + if tc.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "openssl rand -hex 32") { + t.Fatalf("error %q does not include generation guidance", err.Error()) + } + if tc.checkMessage && strings.Contains(err.Error(), tc.secret) { + t.Fatalf("error %q leaked configured secret", err.Error()) + } + return + } + + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Auth.JWTSecret != tc.secret { + t.Fatalf("cfg.Auth.JWTSecret = %q, want %q", cfg.Auth.JWTSecret, tc.secret) + } + }) + } +}