fix: round-2 audit fixes — security, code quality, and UI improvements
Security (HIGH): - Normalize all Redis cache keys to lowercase (verifyCode, passwordReset) - Fix verify code TTL renewal on failed attempts: use remaining TTL via ExpiresAt field instead of resetting to full 15-minute window - Add 3 missing fields to diffSettings audit log (promo_code, invitation_code, custom_endpoints) Code quality (MEDIUM): - Extract filterVerifiedEmails shared helper (balance_notify_service.go) - Add Pricing array non-empty validation for channel pricing rules - Add platform token semantics comment in gateway_service.go - Complete validatePlanPatch test coverage (+10 test cases) - Replace string types with QuotaThresholdType/QuotaResetMode across frontend - Remove duplicate getPlatformTextColor/getRateBadgeClass in ChannelsView - Return EMAIL_NOT_FOUND error on RemoveNotifyEmail miss UI improvements: - Reorder cost tooltip: user billing above separator, account billing below - Add NaN guard to accountBilled function - Move timezone selector inline into reset-mode row (no longer standalone)
This commit is contained in:
@@ -357,6 +357,11 @@ func (h *ChannelHandler) Create(c *gin.Context) {
|
||||
fmt.Sprintf("pricing rule #%d must have at least one group or account", i+1)))
|
||||
return
|
||||
}
|
||||
if len(r.Pricing) == 0 {
|
||||
response.ErrorFrom(c, infraerrors.BadRequest("PRICING_RULE_EMPTY_PRICING",
|
||||
fmt.Sprintf("pricing rule #%d must have at least one pricing entry", i+1)))
|
||||
return
|
||||
}
|
||||
rule := accountStatsPricingRuleRequestToService(r)
|
||||
rule.SortOrder = i
|
||||
statsRules = append(statsRules, rule)
|
||||
@@ -420,6 +425,11 @@ func (h *ChannelHandler) Update(c *gin.Context) {
|
||||
fmt.Sprintf("pricing rule #%d must have at least one group or account", i+1)))
|
||||
return
|
||||
}
|
||||
if len(r.Pricing) == 0 {
|
||||
response.ErrorFrom(c, infraerrors.BadRequest("PRICING_RULE_EMPTY_PRICING",
|
||||
fmt.Sprintf("pricing rule #%d must have at least one pricing entry", i+1)))
|
||||
return
|
||||
}
|
||||
rule := accountStatsPricingRuleRequestToService(r)
|
||||
rule.SortOrder = i
|
||||
statsRules = append(statsRules, rule)
|
||||
|
||||
@@ -1138,6 +1138,12 @@ func diffSettings(before *service.SystemSettings, after *service.SystemSettings,
|
||||
if !equalStringSlice(before.RegistrationEmailSuffixWhitelist, after.RegistrationEmailSuffixWhitelist) {
|
||||
changed = append(changed, "registration_email_suffix_whitelist")
|
||||
}
|
||||
if before.PromoCodeEnabled != after.PromoCodeEnabled {
|
||||
changed = append(changed, "promo_code_enabled")
|
||||
}
|
||||
if before.InvitationCodeEnabled != after.InvitationCodeEnabled {
|
||||
changed = append(changed, "invitation_code_enabled")
|
||||
}
|
||||
if before.PasswordResetEnabled != after.PasswordResetEnabled {
|
||||
changed = append(changed, "password_reset_enabled")
|
||||
}
|
||||
@@ -1348,6 +1354,9 @@ func diffSettings(before *service.SystemSettings, after *service.SystemSettings,
|
||||
if before.CustomMenuItems != after.CustomMenuItems {
|
||||
changed = append(changed, "custom_menu_items")
|
||||
}
|
||||
if before.CustomEndpoints != after.CustomEndpoints {
|
||||
changed = append(changed, "custom_endpoints")
|
||||
}
|
||||
if before.EnableFingerprintUnification != after.EnableFingerprintUnification {
|
||||
changed = append(changed, "enable_fingerprint_unification")
|
||||
}
|
||||
|
||||
@@ -20,8 +20,9 @@ const (
|
||||
)
|
||||
|
||||
// verifyCodeKey generates the Redis key for email verification code.
|
||||
// Email is lowercased for case-insensitive consistency.
|
||||
func verifyCodeKey(email string) string {
|
||||
return verifyCodeKeyPrefix + email
|
||||
return verifyCodeKeyPrefix + strings.ToLower(email)
|
||||
}
|
||||
|
||||
// notifyVerifyKey generates the Redis key for notify email verification code.
|
||||
@@ -33,12 +34,12 @@ func notifyVerifyKey(email string) string {
|
||||
|
||||
// passwordResetKey generates the Redis key for password reset token.
|
||||
func passwordResetKey(email string) string {
|
||||
return passwordResetKeyPrefix + email
|
||||
return passwordResetKeyPrefix + strings.ToLower(email)
|
||||
}
|
||||
|
||||
// passwordResetSentAtKey generates the Redis key for password reset email sent timestamp.
|
||||
func passwordResetSentAtKey(email string) string {
|
||||
return passwordResetSentAtKeyPrefix + email
|
||||
return passwordResetSentAtKeyPrefix + strings.ToLower(email)
|
||||
}
|
||||
|
||||
type emailCache struct {
|
||||
|
||||
@@ -283,6 +283,20 @@ func (s *BalanceNotifyService) getAccountQuotaNotifyEmails(ctx context.Context)
|
||||
return nil
|
||||
}
|
||||
|
||||
return filterVerifiedEmails(entries)
|
||||
}
|
||||
|
||||
// getSiteName reads site name from settings with fallback.
|
||||
func (s *BalanceNotifyService) getSiteName(ctx context.Context) string {
|
||||
name, err := s.settingRepo.GetValue(ctx, SettingKeySiteName)
|
||||
if err != nil || name == "" {
|
||||
return defaultSiteName
|
||||
}
|
||||
return name
|
||||
}
|
||||
|
||||
// filterVerifiedEmails returns deduplicated, non-disabled, verified emails.
|
||||
func filterVerifiedEmails(entries []NotifyEmailEntry) []string {
|
||||
var recipients []string
|
||||
seen := make(map[string]bool)
|
||||
for _, entry := range entries {
|
||||
@@ -303,38 +317,10 @@ func (s *BalanceNotifyService) getAccountQuotaNotifyEmails(ctx context.Context)
|
||||
return recipients
|
||||
}
|
||||
|
||||
// getSiteName reads site name from settings with fallback.
|
||||
func (s *BalanceNotifyService) getSiteName(ctx context.Context) string {
|
||||
name, err := s.settingRepo.GetValue(ctx, SettingKeySiteName)
|
||||
if err != nil || name == "" {
|
||||
return defaultSiteName
|
||||
}
|
||||
return name
|
||||
}
|
||||
|
||||
// collectBalanceNotifyRecipients returns verified, non-disabled email recipients.
|
||||
// Only emails with verified=true and disabled=false are included.
|
||||
func (s *BalanceNotifyService) collectBalanceNotifyRecipients(user *User) []string {
|
||||
var recipients []string
|
||||
seen := make(map[string]bool)
|
||||
|
||||
for _, entry := range user.BalanceNotifyExtraEmails {
|
||||
if entry.Disabled || !entry.Verified {
|
||||
continue
|
||||
}
|
||||
email := strings.TrimSpace(entry.Email)
|
||||
if email == "" {
|
||||
continue
|
||||
}
|
||||
lower := strings.ToLower(email)
|
||||
if seen[lower] {
|
||||
continue
|
||||
}
|
||||
seen[lower] = true
|
||||
recipients = append(recipients, email)
|
||||
}
|
||||
|
||||
return recipients
|
||||
return filterVerifiedEmails(user.BalanceNotifyExtraEmails)
|
||||
}
|
||||
|
||||
// sendEmails sends an email to all recipients with shared timeout and error logging.
|
||||
|
||||
@@ -55,6 +55,7 @@ type VerificationCodeData struct {
|
||||
Code string
|
||||
Attempts int
|
||||
CreatedAt time.Time
|
||||
ExpiresAt time.Time // absolute expiry; used to preserve remaining TTL when updating attempts
|
||||
}
|
||||
|
||||
// PasswordResetTokenData represents password reset token data
|
||||
@@ -263,6 +264,7 @@ func (s *EmailService) SendVerifyCode(ctx context.Context, email, siteName strin
|
||||
Code: code,
|
||||
Attempts: 0,
|
||||
CreatedAt: time.Now(),
|
||||
ExpiresAt: time.Now().Add(verifyCodeTTL),
|
||||
}
|
||||
if err := s.cache.SetVerificationCode(ctx, email, data, verifyCodeTTL); err != nil {
|
||||
return fmt.Errorf("save verify code: %w", err)
|
||||
@@ -295,7 +297,11 @@ func (s *EmailService) VerifyCode(ctx context.Context, email, code string) error
|
||||
// 验证码不匹配 (constant-time comparison to prevent timing attacks)
|
||||
if subtle.ConstantTimeCompare([]byte(data.Code), []byte(code)) != 1 {
|
||||
data.Attempts++
|
||||
if err := s.cache.SetVerificationCode(ctx, email, data, verifyCodeTTL); err != nil {
|
||||
remaining := time.Until(data.ExpiresAt)
|
||||
if remaining <= 0 {
|
||||
return ErrInvalidVerifyCode
|
||||
}
|
||||
if err := s.cache.SetVerificationCode(ctx, email, data, remaining); err != nil {
|
||||
slog.Error("failed to update verification attempt count", "email", email, "error", err)
|
||||
}
|
||||
if data.Attempts >= maxVerifyCodeAttempts {
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -128,3 +128,66 @@ func TestValidatePlanPatch_NilOriginalPrice(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{OriginalPrice: nil})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
// --- validatePlanPatch: other fields ---
|
||||
|
||||
func ptrStr(s string) *string { return &s }
|
||||
func ptrInt(i int) *int { return &i }
|
||||
func ptrInt64(i int64) *int64 { return &i }
|
||||
func ptrFloat(f float64) *float64 { return &f }
|
||||
|
||||
func TestValidatePlanPatch_EmptyName(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{Name: ptrStr("")})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "plan name")
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_ValidName(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{Name: ptrStr("Basic")})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_ZeroGroupID(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{GroupID: ptrInt64(0)})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "group")
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_NegativePrice(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{Price: ptrFloat(-1)})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "price")
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_ZeroPrice(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{Price: ptrFloat(0)})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "price")
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_ValidPrice(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{Price: ptrFloat(9.99)})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_ZeroValidityDays(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{ValidityDays: ptrInt(0)})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "validity days")
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_EmptyValidityUnit(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{ValidityUnit: ptrStr("")})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "validity unit")
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_ValidValidityUnit(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{ValidityUnit: ptrStr("days")})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestValidatePlanPatch_AllNil(t *testing.T) {
|
||||
err := validatePlanPatch(UpdatePlanRequest{})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
@@ -330,6 +330,7 @@ func saveNotifyVerifyCode(ctx context.Context, cache EmailCache, email, code str
|
||||
Code: code,
|
||||
Attempts: 0,
|
||||
CreatedAt: time.Now(),
|
||||
ExpiresAt: time.Now().Add(verifyCodeTTL),
|
||||
}
|
||||
if err := cache.SetNotifyVerifyCode(ctx, email, data, verifyCodeTTL); err != nil {
|
||||
return fmt.Errorf("save verify code: %w", err)
|
||||
@@ -370,7 +371,11 @@ func verifyNotifyCode(ctx context.Context, cache EmailCache, email, code string)
|
||||
}
|
||||
if subtle.ConstantTimeCompare([]byte(data.Code), []byte(code)) != 1 {
|
||||
data.Attempts++
|
||||
if err := cache.SetNotifyVerifyCode(ctx, email, data, verifyCodeTTL); err != nil {
|
||||
remaining := time.Until(data.ExpiresAt)
|
||||
if remaining <= 0 {
|
||||
return ErrInvalidVerifyCode
|
||||
}
|
||||
if err := cache.SetNotifyVerifyCode(ctx, email, data, remaining); err != nil {
|
||||
slog.Error("failed to update notify verify code attempts", "email", email, "error", err)
|
||||
}
|
||||
if data.Attempts >= maxVerifyCodeAttempts {
|
||||
@@ -418,11 +423,17 @@ func (s *UserService) RemoveNotifyEmail(ctx context.Context, userID int64, email
|
||||
}
|
||||
|
||||
filtered := make([]NotifyEmailEntry, 0, len(user.BalanceNotifyExtraEmails))
|
||||
found := false
|
||||
for _, e := range user.BalanceNotifyExtraEmails {
|
||||
if !strings.EqualFold(e.Email, email) {
|
||||
if strings.EqualFold(e.Email, email) {
|
||||
found = true
|
||||
} else {
|
||||
filtered = append(filtered, e)
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
return infraerrors.BadRequest("EMAIL_NOT_FOUND", "notification email not found")
|
||||
}
|
||||
user.BalanceNotifyExtraEmails = filtered
|
||||
return s.userRepo.Update(ctx, user)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user