Skip to content
Merged

fix #21

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand 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 |
Expand All @@ -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
Expand Down
6 changes: 0 additions & 6 deletions cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 11 additions & 3 deletions docs/horizontal-scaling.md
Original file line number Diff line number Diff line change
Expand Up @@ -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=<paste-generated-secret-here>
```

Never rotate this secret without a coordinated rollout strategy — all existing tokens become invalid immediately.
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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`) |
Expand Down
17 changes: 10 additions & 7 deletions docs/security-report.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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).
Expand All @@ -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.
52 changes: 52 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package config

import (
"errors"
"fmt"
"net"
"strings"
"time"
"unicode"

"github.com/spf13/viper"
)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
113 changes: 112 additions & 1 deletion internal/config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package config

import "testing"
import (
"strings"
"testing"
)

const validJWTSecret = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"

func TestParseTrustedProxyCIDRs(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -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)
}
})
}
}
Loading