fix: close auth, permission, contract and e2e review blockers

internal/service/auth.go

@@ -122,7 +122,7 @@ type LoginResponse struct {
 	ExpiresIn    int64     `json:"expires_in,omitempty"`
 	User         *UserInfo `json:"user,omitempty"`
 	// RequiresTOTP 指示登录需要额外的TOTP验证（当设备未信任时）
-	RequiresTOTP bool   `json:"requires_totp,omitempty"`
+	RequiresTOTP bool `json:"requires_totp,omitempty"`
 	// TempToken 临时令牌，用于TOTP验证阶段（短生命周期，不可用于常规API）
 	TempToken string `json:"temp_token,omitempty"`
 	// UserID 当RequiresTOTP为true时返回，用于后续TOTP验证
@@ -759,11 +759,16 @@ func (s *AuthService) Login(ctx context.Context, req *LoginRequest, ip string) (
 
 	// P0-07 安全修复：检查是否需要TOTP验证（用户启用了TOTP且设备未信任）
 	if s.isTOTPRequiredForLogin(ctx, user, req.DeviceID) {
+		tempToken, err := s.jwtManager.GenerateTOTPChallengeToken(user.ID, user.Username, req.DeviceID, user.PasswordChangedAt.Unix())
+		if err != nil {
+			return nil, err
+		}
 		// 返回RequiresTOTP指示前端需要完成TOTP验证
 		// 前端应调用 /auth/login/totp-verify 接口完成验证
 		return &LoginResponse{
 			RequiresTOTP: true,
-			UserID:      user.ID,
+			TempToken:    tempToken,
+			UserID:       user.ID,
 		}, nil
 	}
 
@@ -808,10 +813,27 @@ func (s *AuthService) isTOTPRequiredForLogin(ctx context.Context, user *domain.U
 // VerifyTOTPAfterPasswordLogin 完成密码登录后的TOTP验证
 // 当用户启用了TOTP但设备未信任时，密码登录会返回RequiresTOTP=true
 // 前端需要调用此接口完成TOTP验证以获取令牌
-func (s *AuthService) VerifyTOTPAfterPasswordLogin(ctx context.Context, userID int64, totpCode, deviceID string) (*LoginResponse, error) {
+func (s *AuthService) VerifyTOTPAfterPasswordLogin(ctx context.Context, userID int64, totpCode, deviceID, tempToken string) (*LoginResponse, error) {
 	if s == nil {
 		return nil, errors.New("auth service is not initialized")
 	}
+	if s.jwtManager == nil {
+		return nil, errors.New("jwt manager is not configured")
+	}
+
+	claims, err := s.jwtManager.ValidateTOTPChallengeToken(strings.TrimSpace(tempToken))
+	if err != nil {
+		return nil, errors.New("TOTP challenge is invalid or expired")
+	}
+	if claims == nil || claims.UserID != userID {
+		return nil, errors.New("TOTP challenge does not match user")
+	}
+	if strings.TrimSpace(claims.DeviceID) != strings.TrimSpace(deviceID) {
+		return nil, errors.New("TOTP challenge does not match device")
+	}
+	if s.IsTokenBlacklisted(ctx, claims.JTI) {
+		return nil, errors.New("TOTP challenge has already been used")
+	}
 
 	user, err := s.userRepo.GetByID(ctx, userID)
 	if err != nil {
@@ -826,6 +848,9 @@ func (s *AuthService) VerifyTOTPAfterPasswordLogin(ctx context.Context, userID i
 	if err := s.VerifyTOTP(ctx, userID, totpCode, deviceID); err != nil {
 		return nil, err
 	}
+	if err := s.blacklistTokenClaims(ctx, tempToken, s.jwtManager.ValidateTOTPChallengeToken); err != nil {
+		return nil, fmt.Errorf("totp challenge revocation failed: %w", err)
+	}
 
 	// TOTP验证成功，返回完整登录响应
 	return s.generateLoginResponseWithoutRemember(ctx, user)
@@ -902,18 +927,22 @@ func (s *AuthService) Logout(ctx context.Context, username string, req *LogoutRe
 		return nil
 	}
 
-	_ = s.blacklistTokenClaims(ctx, req.AccessToken, func(token string) (*auth.Claims, error) {
+	if err := s.blacklistTokenClaims(ctx, req.AccessToken, func(token string) (*auth.Claims, error) {
 		if s.jwtManager == nil {
 			return nil, nil
 		}
 		return s.jwtManager.ValidateAccessToken(token)
-	})
-	_ = s.blacklistTokenClaims(ctx, req.RefreshToken, func(token string) (*auth.Claims, error) {
+	}); err != nil {
+		return err
+	}
+	if err := s.blacklistTokenClaims(ctx, req.RefreshToken, func(token string) (*auth.Claims, error) {
 		if s.jwtManager == nil {
 			return nil, nil
 		}
 		return s.jwtManager.ValidateRefreshToken(token)
-	})
+	}); err != nil {
+		return err
+	}
 
 	if strings.TrimSpace(username) != "" {
 		s.publishEvent(ctx, domain.EventUserLogout, map[string]interface{}{

internal/service/auth_login_test.go

@@ -157,6 +157,41 @@ func TestAuthService_Login(t *testing.T) {
 			t.Error("nil service should return error")
 		}
 	})
+
+	t.Run("login with totp enabled returns temporary challenge token", func(t *testing.T) {
+		req := &service.RegisterRequest{
+			Username: "totploginuser",
+			Password: "Test123!",
+			Email:    "totplogin@test.com",
+		}
+		user, err := env.authSvc.Register(ctx, req)
+		if err != nil {
+			t.Fatalf("Register failed: %v", err)
+		}
+		if err := env.db.Model(&domain.User{}).Where("id = ?", user.ID).Updates(map[string]interface{}{
+			"totp_enabled": true,
+			"totp_secret":  "JBSWY3DPEHPK3PXP",
+		}).Error; err != nil {
+			t.Fatalf("enable totp failed: %v", err)
+		}
+
+		resp, err := env.authSvc.Login(ctx, &service.LoginRequest{
+			Username: "totploginuser",
+			Password: "Test123!",
+		}, "127.0.0.1")
+		if err != nil {
+			t.Fatalf("Login failed: %v", err)
+		}
+		if !resp.RequiresTOTP {
+			t.Fatal("expected requires_totp response")
+		}
+		if resp.TempToken == "" {
+			t.Fatal("expected temp_token for second-factor challenge")
+		}
+		if resp.AccessToken != "" || resp.RefreshToken != "" {
+			t.Fatal("totp challenge should not mint full session tokens before second factor verification")
+		}
+	})
 }
 
 func TestAuthService_Register(t *testing.T) {

internal/service/auth_logout_failclosed_test.go

@@ -0,0 +1,87 @@
+package service
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/user-management-system/internal/auth"
+	"github.com/user-management-system/internal/cache"
+	"github.com/user-management-system/internal/domain"
+	"github.com/user-management-system/internal/repository"
+	gormsqlite "gorm.io/driver/sqlite"
+	"gorm.io/gorm"
+	"gorm.io/gorm/logger"
+)
+
+type failingL2Cache struct {
+	setErr error
+}
+
+func (f *failingL2Cache) Set(ctx context.Context, key string, value interface{}, ttl time.Duration) error {
+	return f.setErr
+}
+func (f *failingL2Cache) Get(ctx context.Context, key string) (interface{}, error) { return nil, nil }
+func (f *failingL2Cache) Delete(ctx context.Context, key string) error             { return nil }
+func (f *failingL2Cache) Exists(ctx context.Context, key string) (bool, error)     { return false, nil }
+func (f *failingL2Cache) Clear(ctx context.Context) error                          { return nil }
+func (f *failingL2Cache) Increment(ctx context.Context, key string, delta int64, ttl time.Duration) (int64, error) {
+	return 0, nil
+}
+func (f *failingL2Cache) Close() error { return nil }
+
+func TestAuthService_Logout_FailsClosedWhenBlacklistWriteFails(t *testing.T) {
+	dsn := fmt.Sprintf("file:logoutfailclosed_%d?mode=memory&cache=shared", time.Now().UnixNano())
+	db, err := gorm.Open(gormsqlite.New(gormsqlite.Config{DriverName: "sqlite", DSN: dsn}), &gorm.Config{
+		Logger: logger.Default.LogMode(logger.Silent),
+	})
+	if err != nil {
+		t.Fatalf("open db failed: %v", err)
+	}
+	if err := db.AutoMigrate(&domain.User{}, &domain.Role{}, &domain.UserRole{}, &domain.LoginLog{}, &domain.PasswordHistory{}); err != nil {
+		t.Fatalf("migrate failed: %v", err)
+	}
+	for _, role := range domain.PredefinedRoles {
+		roleCopy := role
+		if err := db.Create(&roleCopy).Error; err != nil {
+			t.Fatalf("seed role %s failed: %v", role.Code, err)
+		}
+	}
+
+	jwtManager, err := auth.NewJWTWithOptions(auth.JWTOptions{
+		HS256Secret:        fmt.Sprintf("logout-failclosed-secret-%d", time.Now().UnixNano()),
+		AccessTokenExpire:  15 * time.Minute,
+		RefreshTokenExpire: 7 * 24 * time.Hour,
+	})
+	if err != nil {
+		t.Fatalf("create jwt manager failed: %v", err)
+	}
+
+	userRepo := repository.NewUserRepository(db)
+	userRoleRepo := repository.NewUserRoleRepository(db)
+	roleRepo := repository.NewRoleRepository(db)
+	cacheManager := cache.NewCacheManager(cache.NewL1Cache(), &failingL2Cache{setErr: errors.New("forced blacklist failure")})
+
+	authSvc := NewAuthService(userRepo, nil, jwtManager, cacheManager, 8, 5, 15*time.Minute)
+	authSvc.SetRoleRepositories(userRoleRepo, roleRepo)
+
+	ctx := context.Background()
+	if _, err := authSvc.Register(ctx, &RegisterRequest{Username: "logoutfail", Password: "Password123!"}); err != nil {
+		t.Fatalf("register failed: %v", err)
+	}
+	loginResp, err := authSvc.Login(ctx, &LoginRequest{Username: "logoutfail", Password: "Password123!"}, "127.0.0.1")
+	if err != nil {
+		t.Fatalf("login failed: %v", err)
+	}
+
+	err = authSvc.Logout(ctx, "logoutfail", &LogoutRequest{AccessToken: loginResp.AccessToken, RefreshToken: loginResp.RefreshToken})
+	if err == nil {
+		t.Fatal("expected logout to fail closed when blacklist write fails")
+	}
+	if !strings.Contains(err.Error(), "forced blacklist failure") {
+		t.Fatalf("expected propagated blacklist error, got: %v", err)
+	}
+}

internal/service/user_service.go

@@ -125,24 +125,49 @@ func (s *UserService) ChangePassword(ctx context.Context, userID int64, oldPassw
 		return errors.New("密码哈希失败")
 	}
 
-	// 保存新密码到历史记录（异步，不阻塞密码更新）
-	if s.passwordHistoryRepo != nil {
-		// #nosec G118 - 使用带超时的独立 context（不能使用请求 ctx，该 goroutine 在请求完成后仍可能运行）
-		go func(hashedPw string) { // #nosec G118
-			bgCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-			defer cancel()
-			_ = s.passwordHistoryRepo.Create(bgCtx, &domain.PasswordHistory{
-				UserID:       userID,
-				PasswordHash: hashedPw,
-			})
-			_ = s.passwordHistoryRepo.DeleteOldRecords(bgCtx, userID, passwordHistoryLimit)
-		}(newHashedPassword)
-	}
-
-	// 更新密码（使用同一哈希值）
+	oldPasswordHash := user.Password
+	oldPasswordChangedAt := user.PasswordChangedAt
 	user.Password = newHashedPassword
 	user.PasswordChangedAt = time.Now()
-	return s.userRepo.Update(ctx, user)
+
+	if s.passwordHistoryRepo == nil {
+		return s.userRepo.Update(ctx, user)
+	}
+
+	return s.userRepo.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+		if err := tx.Model(&domain.User{}).
+			Where("id = ?", user.ID).
+			Updates(map[string]interface{}{"password": user.Password, "password_changed_at": user.PasswordChangedAt}).Error; err != nil {
+			user.Password = oldPasswordHash
+			user.PasswordChangedAt = oldPasswordChangedAt
+			return err
+		}
+
+		if err := tx.Create(&domain.PasswordHistory{UserID: userID, PasswordHash: newHashedPassword}).Error; err != nil {
+			user.Password = oldPasswordHash
+			user.PasswordChangedAt = oldPasswordChangedAt
+			return err
+		}
+
+		var ids []int64
+		if err := tx.Model(&domain.PasswordHistory{}).
+			Where("user_id = ?", userID).
+			Order("created_at DESC").
+			Limit(passwordHistoryLimit).
+			Pluck("id", &ids).Error; err != nil {
+			user.Password = oldPasswordHash
+			user.PasswordChangedAt = oldPasswordChangedAt
+			return err
+		}
+		if len(ids) > 0 {
+			if err := tx.Where("user_id = ? AND id NOT IN ?", userID, ids).Delete(&domain.PasswordHistory{}).Error; err != nil {
+				user.Password = oldPasswordHash
+				user.PasswordChangedAt = oldPasswordChangedAt
+				return err
+			}
+		}
+		return nil
+	})
 }
 
 // GetByID 根据ID获取用户

internal/service/user_service_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/user-management-system/internal/auth"
 	"github.com/user-management-system/internal/domain"
+	"github.com/user-management-system/internal/repository"
 	"github.com/user-management-system/internal/service"
 )
 
@@ -339,6 +340,32 @@ func TestUserService_ChangePassword(t *testing.T) {
 			t.Error("Expected error for weak new password")
 		}
 	})
+
+	t.Run("Change password persists history synchronously", func(t *testing.T) {
+		hashedPassword, _ := auth.HashPassword("HistoryOld123!")
+		user := &domain.User{
+			Username: "historysync",
+			Password: hashedPassword,
+			Status:   domain.UserStatusActive,
+		}
+		env.userSvc.Create(ctx, user)
+
+		if err := env.userSvc.ChangePassword(ctx, user.ID, "HistoryOld123!", "HistoryNew456!"); err != nil {
+			t.Fatalf("ChangePassword failed: %v", err)
+		}
+
+		historyRepo := repository.NewPasswordHistoryRepository(env.db)
+		history, err := historyRepo.GetByUserID(ctx, user.ID, 10)
+		if err != nil {
+			t.Fatalf("GetByUserID failed: %v", err)
+		}
+		if len(history) == 0 {
+			t.Fatal("expected password history to be written synchronously")
+		}
+		if !auth.VerifyPassword(history[0].PasswordHash, "HistoryNew456!") {
+			t.Fatal("latest password history hash does not match new password")
+		}
+	})
 }
 
 func TestUserService_BatchUpdateStatus(t *testing.T) {