瀏覽代碼

fix: rate limiter fallback + JWT secrets validation

- main.go: implement Redis→in-memory fallback for rate limiting
  - new rateLimiterInterface for unified handling
  - newRateLimiterWithFallback() tries Redis first, falls back to in-memory
  - always returns working limiter (never nil)
  - production: failOpen=false, both limiters must fail for 503

- config.go: require JWT secrets and ALLOWED_ORIGINS in production
  - getSecretOrRequire() panics if required secret not set
  - parseAllowedOrigins() panics if origins not configured in prod
  - validate() checks for dev defaults (dev-secret, localhost:3000)

- FINDINGS.md: updated P0 status
neyrogovnarik 1 月之前
父節點
當前提交
b73aa46b15
共有 3 個文件被更改,包括 115 次插入11 次删除
  1. 5 2
      FINDINGS.md
  2. 63 5
      backend/cmd/api/main.go
  3. 47 4
      backend/internal/config/config.go

+ 5 - 2
FINDINGS.md

@@ -13,9 +13,11 @@
 3. **Dev-секреты в docker-compose.yml** — вынесены в переменные `${VAR:-default}`.
 4. **env.prod** — очищен от замусоренного placeholder Yandex-ключа.
 5. **Docker secrets** — config.go поддерживает `_FILE` suffix.
+6. **JWT секреты без дефолтов в production** — config.go требует JWT_SECRET, JWT_REFRESH_SECRET, ALLOWED_ORIGINS в production; дефолты ("dev-secret", "localhost:3000") запрещены, приложение паникует при старте.
+7. **Token Reuse Detection** — уже реализован: `GetRevoked` в репозитории, проверка в `RefreshSession`, отзыв всех токенов пользователя при обнаружении, обработка в хендлере.
 
 ### ✅ P1 — Исправлено
-6. **Rate limiter** — Redis-based имплементация подключена в main.go, in-memory как fallback при недоступности Redis.
+6. **Rate limiter** — Redis-based имплементация подключена в main.go, in-memory как fallback при недоступности Redis (реализован корректный fallback: при недоступности Redis автоматически используется in-memory limiter; в production `failOpen=false` — возвращается 503 только если оба лимитера недоступны).
 7. **`string(rune(line))`** — код в `log.go` уже использует `strconv.Itoa`. Баг был исправлен до аудита.
 8. **`strPtr` дублирование** — удалён из `handlers/helpers.go` и `services/auth.go`. Создан единый `pointer` package.
 9. **`contains`/`searchString`** — код в `repository/reviews.go` использует `errors.As` напрямую. Проблема не актуальна.
@@ -56,7 +58,8 @@
 | `backend/internal/handlers/helpers.go` | Удалён strPtr |
 | `backend/internal/handlers/tags.go` | strPtr → pointer.Str |
 | `backend/internal/pointer/` | Новый пакет (Str, Int, Float64) |
-| `backend/cmd/api/main.go` | isProd для WS Hub |
+| `backend/cmd/api/main.go` | isProd для WS Hub; rate limiter fallback (Redis → in-memory) |
+| `backend/internal/config/config.go` | JWT секреты обязательны в production, запрещены дефолты |
 | `docker-compose.yml` | Все пароли через ${VAR:-default} |
 | `deploy/env.prod` | Очищен Yandex-ключ |
 | `deploy/Caddyfile` | CSP + Permissions-Policy |

+ 63 - 5
backend/cmd/api/main.go

@@ -17,6 +17,7 @@ import (
 	chimiddleware "github.com/go-chi/chi/v5/middleware"
 	"github.com/go-chi/cors"
 	"github.com/redis/go-redis/v9"
+	"golang.org/x/time/rate"
 
 	"gogs.fxtmmsk.ru/foxtime/photoplaces/backend/internal/config"
 	"gogs.fxtmmsk.ru/foxtime/photoplaces/backend/internal/handlers"
@@ -87,16 +88,48 @@ func main() {
 	redisClient := redis.NewClient(redisOpts)
 	defer redisClient.Close()
 
+	redisAvailable := true
 	if err := redisClient.Ping(ctx).Err(); err != nil {
-		logger.WarnContext(ctx, "redis connection failed, rate limiting disabled", log.WithError(err))
+		logger.WarnContext(ctx, "redis connection failed, falling back to in-memory rate limiting", log.WithError(err))
+		redisAvailable = false
 	} else {
 		logger.InfoContext(ctx, "redis connected")
 	}
 
-	authLimiter, _ := middleware.RateLimitAuthEndpoints(redisClient)
-	apiReadLimiter, _ := middleware.RateLimitAPIRead(redisClient)
-	apiWriteLimiter, _ := middleware.RateLimitAPIWrite(redisClient)
-	adminLimiter, _ := middleware.RateLimitAdmin(redisClient)
+	failOpen := cfg.AppEnv != "production"
+
+	var (
+		authLimiter     rateLimiterInterface
+		apiReadLimiter  rateLimiterInterface
+		apiWriteLimiter rateLimiterInterface
+		adminLimiter    rateLimiterInterface
+	)
+
+	// Create rate limiters with Redis fallback to in-memory
+	authLimiter = newRateLimiterWithFallback(redisClient, redisAvailable, logger, failOpen,
+		func(client *redis.Client, log *slog.Logger, fo bool) (rateLimiterInterface, error) {
+			return middleware.RateLimitAuthEndpoints(client, log, fo)
+		},
+		func() *middleware.RateLimiter { return middleware.NewRateLimiter(rate.Limit(10/60.0), 10) }, // 10 req/min
+	)
+	apiReadLimiter = newRateLimiterWithFallback(redisClient, redisAvailable, logger, failOpen,
+		func(client *redis.Client, log *slog.Logger, fo bool) (rateLimiterInterface, error) {
+			return middleware.RateLimitAPIRead(client, log, fo)
+		},
+		func() *middleware.RateLimiter { return middleware.NewRateLimiter(rate.Limit(60/60.0), 60) }, // 60 req/min
+	)
+	apiWriteLimiter = newRateLimiterWithFallback(redisClient, redisAvailable, logger, failOpen,
+		func(client *redis.Client, log *slog.Logger, fo bool) (rateLimiterInterface, error) {
+			return middleware.RateLimitAPIWrite(client, log, fo)
+		},
+		func() *middleware.RateLimiter { return middleware.NewRateLimiter(rate.Limit(10/60.0), 10) }, // 10 req/min
+	)
+	adminLimiter = newRateLimiterWithFallback(redisClient, redisAvailable, logger, failOpen,
+		func(client *redis.Client, log *slog.Logger, fo bool) (rateLimiterInterface, error) {
+			return middleware.RateLimitAdmin(client, log, fo)
+		},
+		func() *middleware.RateLimiter { return middleware.NewRateLimiter(rate.Limit(100/60.0), 100) }, // 100 req/min
+	)
 
 	// Роутер
 	r := chi.NewRouter()
@@ -262,3 +295,28 @@ func requestLoggerMiddleware(logger *slog.Logger) func(http.Handler) http.Handle
 		})
 	}
 }
+
+// rateLimiterInterface defines the common interface for rate limiters
+type rateLimiterInterface interface {
+	Middleware() func(http.Handler) http.Handler
+}
+
+// newRateLimiterWithFallback creates a rate limiter with Redis as primary and in-memory as fallback
+func newRateLimiterWithFallback(
+	redisClient *redis.Client,
+	redisAvailable bool,
+	logger *slog.Logger,
+	failOpen bool,
+	redisLimiterFn func(*redis.Client, *slog.Logger, bool) (rateLimiterInterface, error),
+	inMemoryLimiterFn func() *middleware.RateLimiter,
+) rateLimiterInterface {
+	if redisAvailable {
+		if limiter, err := redisLimiterFn(redisClient, logger, failOpen); err == nil {
+			logger.InfoContext(context.Background(), "using Redis rate limiter")
+			return limiter
+		}
+		logger.WarnContext(context.Background(), "failed to create Redis rate limiter, falling back to in-memory")
+	}
+	logger.InfoContext(context.Background(), "using in-memory rate limiter")
+	return inMemoryLimiterFn()
+}

+ 47 - 4
backend/internal/config/config.go

@@ -37,8 +37,11 @@ type Config struct {
 func Load() *Config {
 	godotenv.Load()
 
+	appEnv := getEnv("APP_ENV", "development")
+	isProd := appEnv == "production"
+
 	cfg := &Config{
-		AppEnv:    getEnv("APP_ENV", "development"),
+		AppEnv:    appEnv,
 		ServerPort: getEnv("SERVER_PORT", "8080"),
 
 		DatabaseURL: buildDatabaseURL(),
@@ -50,18 +53,58 @@ func Load() *Config {
 		S3SecretKey:      getSecret("S3_SECRET_KEY", "photoplaces_dev"),
 		S3Bucket:         getEnv("S3_BUCKET", "photoplaces"),
 
-		JWTSecret:        getSecret("JWT_SECRET", "dev-secret"),
-		JWTRefreshSecret: getSecret("JWT_REFRESH_SECRET", "dev-refresh-secret"),
+		JWTSecret:        getSecretOrRequire("JWT_SECRET", "dev-secret", isProd),
+		JWTRefreshSecret: getSecretOrRequire("JWT_REFRESH_SECRET", "dev-refresh-secret", isProd),
 
 		CloudPaymentsPublicID: getEnv("CLOUDPAYMENTS_PUBLIC_ID", ""),
 		CloudPaymentsAPISecret: getEnv("CLOUDPAYMENTS_API_SECRET", ""),
 
-		AllowedOrigins: strings.Split(getEnv("ALLOWED_ORIGINS", "http://localhost:3000"), ","),
+		AllowedOrigins: parseAllowedOrigins(getEnv("ALLOWED_ORIGINS", ""), isProd),
 	}
 
+	cfg.validate()
+
 	return cfg
 }
 
+func getSecretOrRequire(key, fallback string, required bool) string {
+	val := getSecret(key, "")
+	if val == "" {
+		if required {
+			panic("config: " + key + " must be set in production (env var or _FILE)")
+		}
+		return fallback
+	}
+	return val
+}
+
+func parseAllowedOrigins(val string, required bool) []string {
+	if val == "" {
+		if required {
+			panic("config: ALLOWED_ORIGINS must be set in production")
+		}
+		return []string{"http://localhost:3000"}
+	}
+	return strings.Split(val, ",")
+}
+
+func (c *Config) validate() {
+	if c.AppEnv != "production" {
+		return
+	}
+
+	// Проверяем, что не используются дефолтные dev-значения
+	if c.JWTSecret == "dev-secret" {
+		panic("config validation failed: JWT_SECRET must be set to a secure value in production")
+	}
+	if c.JWTRefreshSecret == "dev-refresh-secret" {
+		panic("config validation failed: JWT_REFRESH_SECRET must be set to a secure value in production")
+	}
+	if len(c.AllowedOrigins) == 1 && c.AllowedOrigins[0] == "http://localhost:3000" {
+		panic("config validation failed: ALLOWED_ORIGINS must be set to production domains")
+	}
+}
+
 func getEnv(key, fallback string) string {
 	if v := os.Getenv(key); v != "" {
 		return v