From 9ed60e57199dac919d650688f7987ecd31b21b7d Mon Sep 17 00:00:00 2001 From: senke Date: Thu, 16 Apr 2026 14:52:46 +0200 Subject: [PATCH] fix(backend,infra): send real verification emails + fail-loud in prod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registration was setting `IsVerified: true` at user-create time and the "send email" block was a `logger.Info("Sending verification email")` — no SMTP call. On production this meant any attacker-typo or typosquat email got a fully-verified account because the user never had to prove ownership. In development the hack let people "log in" without checking MailHog, masking SMTP misconfiguration. Changes: * `core/auth/service.go`: new users start with `IsVerified: false`. The existing `POST /auth/verify-email` flow (unchanged) flips the bit when the user clicks the link. * Registration now calls `emailService.SendVerificationEmail(...)` for real. On SMTP failure the handler returns `500` in production (no stuck account with no recovery path) and logs a warning in development (local sign-ups keep flowing). * Same treatment for `password_reset_handler.RequestPasswordReset` — production fails loud instead of returning the generic success message after a silent SMTP drop. * New helper `isProductionEnv()` centralises the `APP_ENV=="production"` check in both `core/auth` and `handlers`. * `docker-compose.yml` + `docker-compose.dev.yml` now ship MailHog (`mailhog/mailhog:v1.0.1`, SMTP 1025, UI 8025). Backend dev env vars `SMTP_HOST=mailhog SMTP_PORT=1025` pre-wired so dev sign-ups actually deliver. Tests: auth test mocks updated (`expectRegister` adds a `SendVerificationEmail` mock). `TestAuthService_Login_Success` + `TestAuthHandler_Login_Success` flip `is_verified` directly after `Register` to simulate the verification click. `TestLogin_EmailNotVerified` now asserts `403` (previously asserted `200` — the test was codifying the bug this commit fixes). Co-Authored-By: Claude Opus 4.6 (1M context) --- docker-compose.dev.yml | 18 +++++ docker-compose.yml | 23 ++++++ .../internal/core/auth/handler_test.go | 28 +++++--- .../internal/core/auth/service.go | 71 +++++++++++++++---- .../internal/core/auth/service_test.go | 7 +- .../internal/handlers/auth_handler_test.go | 6 +- .../handlers/password_reset_handler.go | 17 ++++- 7 files changed, 138 insertions(+), 32 deletions(-) diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 5b4bc7bc0..c546b2161 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -111,6 +111,24 @@ services: cpus: '0.5' memory: 1G + # MailHog - Local SMTP capture (dev only) + # Backend points SMTP_HOST=localhost:1025 (when running on host) or mailhog:1025 + # (container) and outbound mail shows up in the web UI on port 8025. + mailhog: + image: mailhog/mailhog:v1.0.1 + container_name: veza_mailhog + restart: unless-stopped + ports: + - "${PORT_MAILHOG_SMTP:-1025}:1025" + - "${PORT_MAILHOG_UI:-8025}:8025" + networks: + - veza-net + deploy: + resources: + limits: + cpus: '0.10' + memory: 64M + minio: image: minio/minio:latest container_name: veza_minio diff --git a/docker-compose.yml b/docker-compose.yml index bdfe1b8eb..d3d1f1a64 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -57,6 +57,25 @@ services: reservations: memory: 32M + # MailHog - Local SMTP capture for development + # Receives every outbound mail from backend-api (SMTP 1025) and exposes a + # web UI (8025) where devs can inspect verification and password-reset + # emails without wiring a real SMTP provider. + mailhog: + image: mailhog/mailhog:v1.0.1 + container_name: veza_mailhog + restart: unless-stopped + ports: + - "${PORT_MAILHOG_SMTP:-1025}:1025" + - "${PORT_MAILHOG_UI:-8025}:8025" + networks: + - veza-net + deploy: + resources: + limits: + cpus: '0.10' + memory: 64M + # ClamAV - Virus scanning for uploads # SECURITY(MEDIUM-003): Pin ClamAV image to specific version instead of :latest clamav: @@ -190,6 +209,10 @@ services: - AWS_REGION=us-east-1 - HLS_STREAMING=true - HLS_STORAGE_DIR=/data/hls + - SMTP_HOST=mailhog + - SMTP_PORT=1025 + - FROM_EMAIL=${FROM_EMAIL:-no-reply@veza.local} + - FROM_NAME=${FROM_NAME:-Veza (dev)} volumes: - hls-data:/data/hls ports: diff --git a/veza-backend-api/internal/core/auth/handler_test.go b/veza-backend-api/internal/core/auth/handler_test.go index 4c2d26972..94e1d84f7 100644 --- a/veza-backend-api/internal/core/auth/handler_test.go +++ b/veza-backend-api/internal/core/auth/handler_test.go @@ -16,10 +16,11 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" + "gorm.io/gorm" ) -func setupTestAuthHandler(t *testing.T) (*AuthHandler, *gin.Engine, *TestMocks, func()) { - service, _, mocks, cleanupService := setupTestAuthService(t) +func setupTestAuthHandler(t *testing.T) (*AuthHandler, *gin.Engine, *TestMocks, *gorm.DB, func()) { + service, db, mocks, cleanupService := setupTestAuthService(t) gin.SetMode(gin.TestMode) router := gin.New() @@ -30,7 +31,7 @@ func setupTestAuthHandler(t *testing.T) (*AuthHandler, *gin.Engine, *TestMocks, zaptest.NewLogger(t), ) - return handler, router, mocks, func() { + return handler, router, mocks, db, func() { cleanupService() } } @@ -38,13 +39,14 @@ func setupTestAuthHandler(t *testing.T) (*AuthHandler, *gin.Engine, *TestMocks, func expectRegister(mocks *TestMocks) { mocks.EmailVerification.On("GenerateToken").Return("verification-token", nil).Maybe() mocks.EmailVerification.On("StoreToken", mock.Anything, mock.Anything, "verification-token").Return(nil).Maybe() + mocks.Email.On("SendVerificationEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(nil).Maybe() mocks.JWT.On("GenerateAccessToken", mock.AnythingOfType("*models.User")).Return("access-token", nil).Once() mocks.JWT.On("GenerateRefreshToken", mock.AnythingOfType("*models.User")).Return("refresh-token", nil).Once() mocks.RefreshToken.On("Store", mock.Anything, "refresh-token", mock.Anything).Return(nil).Once() } func TestAuthHandler_Register_Success(t *testing.T) { - handler, router, mocks, cleanup := setupTestAuthHandler(t) + handler, router, mocks, _, cleanup := setupTestAuthHandler(t) defer cleanup() router.POST("/register", handler.Register) @@ -76,7 +78,7 @@ func TestAuthHandler_Register_Success(t *testing.T) { } func TestAuthHandler_Login_Success(t *testing.T) { - handler, router, mocks, cleanup := setupTestAuthHandler(t) + handler, router, mocks, db, cleanup := setupTestAuthHandler(t) defer cleanup() router.POST("/login", handler.Login) @@ -86,9 +88,13 @@ func TestAuthHandler_Login_Success(t *testing.T) { expectRegister(mocks) - _, _, err := handler.authService.Register(ctx, "login_h@example.com", "login_h", "StrongPassword123!") + registeredUser, _, err := handler.authService.Register(ctx, "login_h@example.com", "login_h", "StrongPassword123!") require.NoError(t, err) + // Simulate the user clicking the verification link — Register now leaves + // is_verified=false and Login refuses unverified users. + require.NoError(t, db.Model(&models.User{}).Where("id = ?", registeredUser.ID).Update("is_verified", true).Error) + reqBody := dto.LoginRequest{ Email: "login_h@example.com", Password: "StrongPassword123!", @@ -116,7 +122,7 @@ func TestAuthHandler_Login_Success(t *testing.T) { } func TestAuthHandler_Login_InvalidCredentials(t *testing.T) { - handler, router, _, cleanup := setupTestAuthHandler(t) + handler, router, _, _, cleanup := setupTestAuthHandler(t) defer cleanup() router.POST("/login", handler.Login) @@ -137,7 +143,7 @@ func TestAuthHandler_Login_InvalidCredentials(t *testing.T) { } func TestAuthHandler_Refresh_Success(t *testing.T) { - handler, _, mocks, cleanup := setupTestAuthHandler(t) + handler, _, mocks, _, cleanup := setupTestAuthHandler(t) defer cleanup() expectRegister(mocks) @@ -176,7 +182,7 @@ func TestAuthHandler_Refresh_Success(t *testing.T) { } func TestAuthHandler_CheckUsername_Available(t *testing.T) { - handler, _, _, cleanup := setupTestAuthHandler(t) + handler, _, _, _, cleanup := setupTestAuthHandler(t) defer cleanup() w := httptest.NewRecorder() @@ -196,7 +202,7 @@ func TestAuthHandler_CheckUsername_Available(t *testing.T) { } func TestAuthHandler_GetMe_Success(t *testing.T) { - handler, _, mocks, cleanup := setupTestAuthHandler(t) + handler, _, mocks, _, cleanup := setupTestAuthHandler(t) defer cleanup() ctx := context.Background() @@ -223,7 +229,7 @@ func TestAuthHandler_GetMe_Success(t *testing.T) { } func TestAuthHandler_Logout_Success(t *testing.T) { - handler, _, mocks, cleanup := setupTestAuthHandler(t) + handler, _, mocks, _, cleanup := setupTestAuthHandler(t) defer cleanup() ctx := context.Background() diff --git a/veza-backend-api/internal/core/auth/service.go b/veza-backend-api/internal/core/auth/service.go index 9d82fdb00..abbbbc1a2 100644 --- a/veza-backend-api/internal/core/auth/service.go +++ b/veza-backend-api/internal/core/auth/service.go @@ -197,7 +197,7 @@ func (s *AuthService) Register(ctx context.Context, email, username, password st PasswordHash: string(hashedPassword), Role: "user", // Valeur par défaut (doit correspondre à l'ENUM PostgreSQL) IsActive: true, // Valeur par défaut - IsVerified: true, // MVP: Auto-verify email pour permettre login immédiat + IsVerified: false, // Verified via POST /auth/verify-email after the user clicks the link IsBanned: false, // Valeur par défaut (required NOT NULL field) TokenVersion: 0, // Valeur par défaut (required NOT NULL field) LoginCount: 0, // Valeur par défaut (required NOT NULL field) @@ -354,27 +354,62 @@ func (s *AuthService) Register(ctx context.Context, email, username, password st ) } - // Générer le token de vérification d'email (non-bloquant) - // Si la génération échoue, on continue quand même avec l'inscription - // L'utilisateur pourra demander un nouveau token plus tard + // Generate the verification token and dispatch the email. + // In production an SMTP failure is a hard error — we refuse to create the + // account silently, otherwise the user ends up stuck with no way to verify. + // In development we log a warning so local sign-ups continue to work even + // when MailHog/SMTP is not running (dev can verify by reading the log). if s.emailVerificationService != nil { - token, err := s.emailVerificationService.GenerateToken() - if err != nil { - s.logger.Warn("Failed to generate email verification token (non-blocking)", zap.Error(err)) - } else { - // Stocker le token - if err := s.emailVerificationService.StoreToken(user.ID, user.Email, token); err != nil { - s.logger.Warn("Failed to store email verification token (non-blocking)", zap.Error(err)) - } else { - // Envoyer l'email de vérification (simulation pour l'instant) - s.logger.Info("Sending verification email", + token, tokenErr := s.emailVerificationService.GenerateToken() + if tokenErr != nil { + s.logger.Error("Failed to generate email verification token", + zap.String("user_id", user.ID.String()), + zap.Error(tokenErr), + ) + if isProductionEnv() { + return nil, nil, fmt.Errorf("failed to generate verification token: %w", tokenErr) + } + } else if storeErr := s.emailVerificationService.StoreToken(user.ID, user.Email, token); storeErr != nil { + s.logger.Error("Failed to store email verification token", + zap.String("user_id", user.ID.String()), + zap.Error(storeErr), + ) + if isProductionEnv() { + return nil, nil, fmt.Errorf("failed to store verification token: %w", storeErr) + } + } else if s.emailService != nil { + if sendErr := s.emailService.SendVerificationEmail(user.Email, token); sendErr != nil { + s.logger.Error("Failed to send verification email", + zap.String("user_id", user.ID.String()), zap.String("email", user.Email), + zap.Error(sendErr), + ) + if isProductionEnv() { + return nil, nil, fmt.Errorf("failed to send verification email: %w", sendErr) + } + s.logger.Warn("Continuing registration in non-production mode despite SMTP failure — verify via token in logs or MailHog UI (http://localhost:8025)", zap.String("token", token), - zap.String("user_id", user.ID.String())) + ) + } else { + s.logger.Info("Verification email sent", + zap.String("user_id", user.ID.String()), + zap.String("email", user.Email), + ) + } + } else { + s.logger.Warn("Email service not wired — verification email not sent", + zap.String("user_id", user.ID.String()), + zap.String("token", token), + ) + if isProductionEnv() { + return nil, nil, fmt.Errorf("email service unavailable in production") } } } else { s.logger.Warn("Email verification service not available - skipping token generation") + if isProductionEnv() { + return nil, nil, fmt.Errorf("email verification service unavailable in production") + } } s.logger.Info("User registered successfully", zap.String("user_id", user.ID.String())) @@ -1020,3 +1055,9 @@ func min(a, b int) int { } return b } + +// isProductionEnv reports whether APP_ENV is set to "production". Used to +// decide whether SMTP / email-delivery failures should be hard errors. +func isProductionEnv() bool { + return strings.EqualFold(os.Getenv("APP_ENV"), "production") +} diff --git a/veza-backend-api/internal/core/auth/service_test.go b/veza-backend-api/internal/core/auth/service_test.go index 82a900652..eabe6b699 100644 --- a/veza-backend-api/internal/core/auth/service_test.go +++ b/veza-backend-api/internal/core/auth/service_test.go @@ -313,7 +313,7 @@ func TestAuthService_Logout(t *testing.T) { } func TestAuthService_Login_Success(t *testing.T) { - service, _, mocks, cleanup := setupTestAuthService(t) + service, db, mocks, cleanup := setupTestAuthService(t) defer cleanup() ctx := context.Background() @@ -340,10 +340,15 @@ func TestAuthService_Login_Success(t *testing.T) { mocks.RefreshToken.On("Store", mock.AnythingOfType("uuid.UUID"), "refresh-token", mock.Anything).Return(nil).Once() mocks.EmailVerification.On("GenerateToken").Return("verify-token", nil).Once() mocks.EmailVerification.On("StoreToken", mock.AnythingOfType("uuid.UUID"), email, "verify-token").Return(nil).Once() + mocks.Email.On("SendVerificationEmail", email, "verify-token").Return(nil).Once() user, _, err := service.Register(ctx, email, "loginuser", password) require.NoError(t, err) + // Simulate the user clicking the verification link — Register now leaves + // is_verified=false (v1.0.4 hardening) and Login refuses unverified users. + require.NoError(t, db.Model(&models.User{}).Where("id = ?", user.ID).Update("is_verified", true).Error) + // Now Login // Login also needs JWT generation expectations mocks.JWT.On("GenerateAccessToken", mock.AnythingOfType("*models.User")).Return("new-access-token", nil).Once() diff --git a/veza-backend-api/internal/handlers/auth_handler_test.go b/veza-backend-api/internal/handlers/auth_handler_test.go index 68fbe524a..0e6352679 100644 --- a/veza-backend-api/internal/handlers/auth_handler_test.go +++ b/veza-backend-api/internal/handlers/auth_handler_test.go @@ -211,7 +211,7 @@ func TestLogin_EmailNotVerified(t *testing.T) { user, _, err := authService.Register(ctx, "test@example.com", "testuser", "SecurePassword123!") require.NoError(t, err) require.NotNil(t, user) - // User is not verified by default + // User is not verified by default (v1.0.4: Register leaves is_verified=false). reqBody := dto.LoginRequest{ Email: "test@example.com", @@ -226,8 +226,8 @@ func TestLogin_EmailNotVerified(t *testing.T) { router.ServeHTTP(w, req) - // Current behavior: unverified users can login (StatusOK). Product may require StatusForbidden in future. - assert.Equal(t, http.StatusOK, w.Code) + // Login refuses unverified users; the user must POST /auth/verify-email first. + assert.Equal(t, http.StatusForbidden, w.Code) } func TestLogin_Requires2FA(t *testing.T) { diff --git a/veza-backend-api/internal/handlers/password_reset_handler.go b/veza-backend-api/internal/handlers/password_reset_handler.go index ffe4fe044..c706db822 100644 --- a/veza-backend-api/internal/handlers/password_reset_handler.go +++ b/veza-backend-api/internal/handlers/password_reset_handler.go @@ -3,6 +3,8 @@ package handlers import ( "context" "net/http" + "os" + "strings" "veza-backend-api/internal/core/auth" // Added import for authcore "veza-backend-api/internal/services" @@ -12,6 +14,12 @@ import ( "go.uber.org/zap" ) +// isProductionEnv reports whether APP_ENV is set to "production". Used to +// decide whether SMTP delivery failures should surface as HTTP 500. +func isProductionEnv() bool { + return strings.EqualFold(os.Getenv("APP_ENV"), "production") +} + // RequestPasswordResetRequest represents a request to reset password // T0193: Request structure for password reset endpoint // MOD-P1-001: Ajout tags validate pour validation systématique @@ -129,14 +137,19 @@ func RequestPasswordResetWithInterfaces( return } - // Send email + // Send email. In production SMTP must work — a silent log-only failure + // would leave the user stuck with no way to reset. In development we + // keep the generic success response so local sign-ups keep flowing. if err := emailService.SendPasswordResetEmail(user.ID, user.Email, token); err != nil { - // Log but don't fail - user should still get success message logger.Error("Failed to send password reset email", zap.String("user_id", user.ID.String()), zap.String("email", user.Email), zap.Error(err), ) + if isProductionEnv() { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to send password reset email"}) + return + } } // BE-SEC-013: Log password reset request