fix account email validation and inactive reset

Author: ppolariss

apis/account.go

@@ -37,6 +37,9 @@ func Register(c *fiber.Ctx) (err error) {
 	if err != nil {
 		return err
 	}
+	if err = validateAccountEmail(body.Email); err != nil {
+		return err
+	}
 
 	// check verification code
 	ok := auth.CheckVerificationCode(body.Email, scope, string(body.Verification))
@@ -116,6 +119,11 @@ func RegisterDebugInBatch(c *fiber.Ctx) (err error) {
 	if err != nil {
 		return err
 	}
+	for _, data := range body.Data {
+		if err := validateAccountEmail(data.Email); err != nil {
+			return err
+		}
+	}
 
 	// check registered
 	var emailHashes []string
@@ -276,6 +284,10 @@ func RegisterDebugInBatch(c *fiber.Ctx) (err error) {
 }
 
 func register(c *fiber.Ctx, email, password string, batch bool) error {
+	if err := validateAccountEmail(email); err != nil {
+		return err
+	}
+
 	registered, err := HasRegisteredEmail(DB, email)
 	if err != nil {
 		return err
@@ -365,6 +377,9 @@ func ChangePassword(c *fiber.Ctx) error {
 	if err != nil {
 		return err
 	}
+	if err = validateAccountEmail(body.Email); err != nil {
+		return err
+	}
 
 	registered, err := HasRegisteredEmail(DB, body.Email)
 	if err != nil {
@@ -397,7 +412,10 @@ func ChangePassword(c *fiber.Ctx) error {
 			return err
 		}
 
-		user.IsActive = true
+		if !user.IsActive {
+			return common.Forbidden("账号已停用")
+		}
+
 		user.Password, err = auth.MakePassword(body.Password)
 		if err != nil {
 			return err
@@ -473,10 +491,7 @@ func VerifyWithEmail(c *fiber.Ctx) error {
 }
 
 func verifyWithEmail(c *fiber.Ctx, email string, check bool) error {
-	if !utils.ValidateEmail(email) {
-		return common.BadRequest("email invalid")
-	}
-	err := utils.ValidateEmailFudan(email)
+	err := validateAccountEmail(email)
 	if err != nil {
 		return err
 	}
@@ -542,6 +557,13 @@ func verifyWithEmail(c *fiber.Ctx, email string, check bool) error {
 	})
 }
 
+func validateAccountEmail(email string) error {
+	if !utils.ValidateEmail(email) {
+		return common.BadRequest("email invalid")
+	}
+	return utils.ValidateEmailFudan(email)
+}
+
 // VerifyWithApikey godoc
 //
 // @Summary verify with email in query and apikey

apis/account_test.go

@@ -0,0 +1,113 @@
+package apis
+
+import (
+	"bytes"
+	"database/sql"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gofiber/fiber/v2"
+	"github.com/opentreehole/go-common"
+	"gorm.io/driver/sqlite"
+	"gorm.io/gorm"
+	"gorm.io/gorm/schema"
+
+	"auth_next/config"
+	"auth_next/models"
+	"auth_next/utils/auth"
+)
+
+func TestChangePasswordRejectsInactiveUser(t *testing.T) {
+	oldDB := models.DB
+	oldStandalone := config.Config.Standalone
+	oldShamirFeature := config.Config.ShamirFeature
+	oldEmailWhitelist := config.Config.EmailWhitelist
+	oldVerificationCodeExpires := config.Config.VerificationCodeExpires
+	t.Cleanup(func() {
+		models.DB = oldDB
+		config.Config.Standalone = oldStandalone
+		config.Config.ShamirFeature = oldShamirFeature
+		config.Config.EmailWhitelist = oldEmailWhitelist
+		config.Config.VerificationCodeExpires = oldVerificationCodeExpires
+	})
+
+	db, err := gorm.Open(sqlite.Open("file::memory:?cache=shared"), &gorm.Config{
+		NamingStrategy: schema.NamingStrategy{SingularTable: true},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	sqlDB, err := db.DB()
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		_ = sqlDB.Close()
+	})
+	if err := db.AutoMigrate(models.User{}, models.DeleteIdentifier{}, models.UserJwtSecret{}); err != nil {
+		t.Fatal(err)
+	}
+
+	models.DB = db
+	config.Config.Standalone = true
+	config.Config.ShamirFeature = false
+	config.Config.EmailWhitelist = nil
+	config.Config.VerificationCodeExpires = 5
+	auth.InitVerificationCodeCache()
+
+	const email = "inactive@example.com"
+	oldPassword, err := auth.MakePassword("old-password")
+	if err != nil {
+		t.Fatal(err)
+	}
+	user := models.User{
+		Identifier: sql.NullString{String: auth.MakeIdentifier(email), Valid: true},
+		Password:   oldPassword,
+	}
+	if err := db.Create(&user).Error; err != nil {
+		t.Fatal(err)
+	}
+	if err := db.Model(&user).Update("is_active", false).Error; err != nil {
+		t.Fatal(err)
+	}
+
+	code, err := auth.SetVerificationCode(email, "reset")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app := fiber.New(fiber.Config{ErrorHandler: common.ErrorHandler})
+	app.Put("/register", ChangePassword)
+
+	body := fmt.Sprintf(
+		`{"email":%q,"password":"new-password","verification":%q}`,
+		email,
+		code,
+	)
+	req := httptest.NewRequest(http.MethodPut, "/register", bytes.NewBufferString(body))
+	req.Header.Set("Content-Type", "application/json")
+	resp, err := app.Test(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusForbidden {
+		t.Fatalf("expected status %d, got %d", http.StatusForbidden, resp.StatusCode)
+	}
+
+	var updated models.User
+	if err := db.Take(&updated, user.ID).Error; err != nil {
+		t.Fatal(err)
+	}
+	if updated.IsActive {
+		t.Fatal("expected inactive user to remain inactive after password reset attempt")
+	}
+	ok, err := auth.CheckPassword("new-password", updated.Password)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if ok {
+		t.Fatal("expected inactive user's password not to be changed")
+	}
+}

utils/validate.go

@@ -4,6 +4,7 @@ import (
 	"golang.org/x/exp/slices"
 	"strconv"
 	"strings"
+	"unicode"
 
 	"github.com/go-playground/validator/v10"
 	"github.com/opentreehole/go-common"
@@ -20,6 +21,10 @@ func init() {
 }
 
 func ValidateEmail(email string) bool {
+	if strings.ContainsFunc(email, unicode.IsSpace) {
+		return false
+	}
+
 	if err := common.Validate.Var(email, "required,email"); err != nil {
 		return false
 	}

utils/validate_test.go

@@ -18,6 +18,7 @@ func TestValidateEmail(t *testing.T) {
 	assert.Equal(t, ValidateEmail(" abcd@fudan.edu.cn"), false)
 	assert.Equal(t, ValidateEmail("abcd @fudan.edu.cn"), false)
 	assert.Equal(t, ValidateEmail("\tabcd@fudan.edu.cn"), false)
+	assert.Equal(t, ValidateEmail("\"abc def\"@fudan.edu.cn"), false)
 }
 
 func TestValidateEmailFudan(t *testing.T) {