feat: harden PUT /users/me — validate display name, re-verify on email change

internal/handlers/user.go

@@ -59,6 +59,12 @@ func (h *UserHandler) CreateUser(c *gin.Context) {
 		return
 	}
 
+	// Validate the optional display name
+	if err := h.Service.ValidateFirstName(newUser.FirstName); err != nil {
+		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+		return
+	}
+
 	// Validate email
 	if err := h.Service.ValidateEmail(newUser.Email); err != nil {
 		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
@@ -354,13 +360,50 @@ func (h *UserHandler) UpdateUser(c *gin.Context) {
 		return
 	}
 
+	req.FirstName = strings.TrimSpace(req.FirstName)
+	req.Email = strings.TrimSpace(req.Email)
+
+	if err := h.Service.ValidateFirstName(req.FirstName); err != nil {
+		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+		return
+	}
+
+	// Note the address change before the update rewrites the in-memory user.
+	emailChanged := req.Email != "" && !strings.EqualFold(req.Email, user.Email)
+	if emailChanged {
+		if err := h.Service.ValidateEmail(req.Email); err != nil {
+			c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+			return
+		}
+	}
+
 	if err := h.Service.UpdateUser(user, req.FirstName, req.Email); err != nil {
+		if errors.Is(err, repository.ErrEmailTaken) {
+			c.JSON(http.StatusConflict, gin.H{"error": "email already in use"})
+			return
+		}
 		logger.Get().Error("failed to update user", zap.Uint("user_id", user.ID), zap.Error(err))
-		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update user"})
 		return
 	}
 
-	c.JSON(http.StatusOK, gin.H{"message": "User updated successfully"})
+	// The address changed, so UpdateUser un-verified the account. Send a code to
+	// the new address — best-effort, exactly like signup: a mail hiccup must not
+	// fail the update, and the verify screen offers a resend.
+	verificationRequired := false
+	if emailChanged && h.EmailVerification != nil && h.EmailVerification.Enabled() {
+		verificationRequired = true
+		if err := h.EmailVerification.StartVerification(c.Request.Context(), user); err != nil {
+			logger.Get().Warn("failed to send verification email after address change",
+				zap.Uint("user_id", user.ID), zap.Error(err))
+		}
+	}
+
+	c.JSON(http.StatusOK, gin.H{
+		"message": "User updated successfully",
+		// Tells a client it has to collect a code before AI features work again.
+		"email_verification_required": verificationRequired,
+	})
 }
 
 // UpdateSettings updates a user's settings.

internal/repository/interfaces.go

@@ -137,7 +137,7 @@ type UserRepo interface {
 	GetUserAuthByUsername(username string) (*models.User, error)
 	GetUserAuthByEmail(email string) (*models.User, error)
 	UpdateUserFirstName(userID uint, firstName string) error
-	UpdateUserEmail(userID uint, email string) error
+	UpdateUserEmail(userID uint, email string, verifiedAt *time.Time) error
 	UpdateUserSettingsKeepScreenAwake(userID uint, keepScreenAwake bool) error
 	UpdatePersonalization(userID uint, update *models.PersonalizationUpdate) error
 	UsernameExists(username string) (bool, error)

internal/repository/user.go

@@ -124,11 +124,18 @@ func (r *UserRepository) UpdateUserFirstName(userID uint, firstName string) erro
 	return err
 }
 
-// UpdateUserEmail updates a user's email address.
-func (r *UserRepository) UpdateUserEmail(userID uint, email string) error {
+// UpdateUserEmail updates a user's email address and, in the same statement,
+// its verification state: a nil verifiedAt marks the new address unverified.
+// Both columns move together so an account can never be left looking verified
+// on an address nobody proved. A map is used rather than a struct because GORM
+// skips zero values on struct updates, and nil is the value that matters here.
+func (r *UserRepository) UpdateUserEmail(userID uint, email string, verifiedAt *time.Time) error {
 	err := r.DB.Model(&models.User{}).
 		Where("id = ?", userID).
-		Update("Email", email).Error
+		Updates(map[string]interface{}{
+			"email":             email,
+			"email_verified_at": verifiedAt,
+		}).Error
 	if err != nil {
 		logger.Get().Error("failed to update user email", zap.Uint("user_id", userID), zap.Error(err))
 		return mapUserUniqueViolation(err)

internal/router/router.go

@@ -133,6 +133,8 @@ func SetupRouter(cfg *config.Config, database *gorm.DB) *gin.Engine {
 	emailVerificationService := service.NewEmailVerificationService(cfg, userRepo, emailVerificationRepo, emailSender)
 	emailVerificationHandler := handlers.NewEmailVerificationHandler(emailVerificationService)
 	userHandler.EmailVerification = emailVerificationService
+	// Lets UpdateUser decide whether a changed address starts out unverified.
+	userService.Verification = emailVerificationService
 
 	// Gate for AI-cost endpoints: throwaway signups must verify their email
 	// before they can spend AI quota. No-op while verification is disabled.

internal/service/profile_update_test.go

@@ -0,0 +1,197 @@
+package service
+
+import (
+	"errors"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/windoze95/saltybytes-api/internal/models"
+	"github.com/windoze95/saltybytes-api/internal/repository"
+	"github.com/windoze95/saltybytes-api/internal/testutil"
+)
+
+// stubVerification stands in for EmailVerificationService.
+type stubVerification struct{ enabled bool }
+
+func (s stubVerification) Enabled() bool { return s.enabled }
+
+// newProfileHarness builds a user service with a verified account already in it.
+// The returned user is the object the repo holds — tests should pass a
+// sessionCopy of it to UpdateUser, not this pointer.
+func newProfileHarness(t *testing.T, verificationLive bool) (*UserService, *testutil.MockUserRepo, *models.User) {
+	t.Helper()
+
+	repo := testutil.NewMockUserRepo()
+	svc := &UserService{
+		Cfg:          verificationEnabledConfig(),
+		Repo:         repo,
+		Verification: stubVerification{enabled: verificationLive},
+	}
+
+	verifiedAt := time.Now()
+	user := &models.User{
+		Username:        "cook",
+		Email:           "cook@example.com",
+		EmailVerifiedAt: &verifiedAt,
+		Auth:            &models.UserAuth{AuthType: models.Standard},
+	}
+	repo.CreateUser(user)
+	return svc, repo, user
+}
+
+// sessionCopy mimics what the handler actually passes in: a user loaded from the
+// database, which is a *different* object from the row the repo will write.
+//
+// This matters. MockUserRepo hands back the very pointer it stores, so calling
+// UpdateUser with the harness's user would make every "did it persist?"
+// assertion vacuous — they'd pass on the service's in-memory mutation alone,
+// even if it never touched the repo.
+func sessionCopy(stored *models.User) *models.User {
+	c := *stored
+	return &c
+}
+
+func TestUpdateUser_EmailChangeUnverifiesAccount(t *testing.T) {
+	svc, repo, stored := newProfileHarness(t, true)
+	user := sessionCopy(stored)
+
+	if err := svc.UpdateUser(user, "", "new@example.com"); err != nil {
+		t.Fatalf("UpdateUser error: %v", err)
+	}
+
+	// The in-memory user has to move too: the handler mails user.Email next, and
+	// StartVerification refuses to send to an account that still looks verified.
+	if user.Email != "new@example.com" {
+		t.Errorf("in-memory email = %q, want new@example.com", user.Email)
+	}
+	if user.EmailVerified() {
+		t.Error("in-memory user still looks verified after an address change")
+	}
+
+	// And it has to be persisted, not just mutated in memory.
+	written, _ := repo.GetUserByID(stored.ID)
+	if written.Email != "new@example.com" {
+		t.Errorf("stored email = %q, want new@example.com", written.Email)
+	}
+	if written.EmailVerified() {
+		t.Error("stored account still verified on an address nobody proved")
+	}
+}
+
+func TestUpdateUser_EmailChangeAutoVerifiesWhileVerificationOff(t *testing.T) {
+	svc, repo, stored := newProfileHarness(t, false)
+
+	if err := svc.UpdateUser(sessionCopy(stored), "", "new@example.com"); err != nil {
+		t.Fatalf("UpdateUser error: %v", err)
+	}
+
+	written, _ := repo.GetUserByID(stored.ID)
+	if !written.EmailVerified() {
+		t.Error("with verification off, a changed address must be auto-verified — the gate is a no-op and nothing would ever send a code")
+	}
+}
+
+func TestUpdateUser_SameAddressDifferentCaseKeepsVerification(t *testing.T) {
+	svc, repo, stored := newProfileHarness(t, true)
+
+	if err := svc.UpdateUser(sessionCopy(stored), "", "Cook@Example.com"); err != nil {
+		t.Fatalf("UpdateUser error: %v", err)
+	}
+
+	written, _ := repo.GetUserByID(stored.ID)
+	if !written.EmailVerified() {
+		t.Error("retyping the same address in different case must not cost the user their verified status")
+	}
+}
+
+func TestUpdateUser_TakenEmailIsRejected(t *testing.T) {
+	svc, repo, stored := newProfileHarness(t, true)
+
+	otherVerified := time.Now()
+	repo.CreateUser(&models.User{
+		Username:        "taken",
+		Email:           "taken@example.com",
+		EmailVerifiedAt: &otherVerified,
+		Auth:            &models.UserAuth{AuthType: models.Standard},
+	})
+
+	err := svc.UpdateUser(sessionCopy(stored), "", "taken@example.com")
+	if !errors.Is(err, repository.ErrEmailTaken) {
+		t.Errorf("UpdateUser with a taken email = %v, want ErrEmailTaken", err)
+	}
+
+	written, _ := repo.GetUserByID(stored.ID)
+	if written.Email != "cook@example.com" || !written.EmailVerified() {
+		t.Error("a rejected email change must leave the account untouched")
+	}
+}
+
+func TestUpdateUser_FirstNameOnlyKeepsVerification(t *testing.T) {
+	svc, repo, stored := newProfileHarness(t, true)
+
+	if err := svc.UpdateUser(sessionCopy(stored), "Julia", ""); err != nil {
+		t.Fatalf("UpdateUser error: %v", err)
+	}
+
+	written, _ := repo.GetUserByID(stored.ID)
+	if written.FirstName != "Julia" {
+		t.Errorf("stored first name = %q, want Julia", written.FirstName)
+	}
+	if !written.EmailVerified() {
+		t.Error("changing only the display name must not touch verification")
+	}
+}
+
+func TestValidateFirstName(t *testing.T) {
+	svc := newTestUserService(testutil.NewMockUserRepo())
+
+	tests := []struct {
+		name      string
+		firstName string
+		wantErr   bool
+	}{
+		{"empty is allowed (optional)", "", false},
+		{"plain", "Julian", false},
+		{"accented", "José", false},
+		{"umlaut", "Zoë", false},
+		{"non-latin script", "李", false},
+		{"hyphenated", "Mary-Jane", false},
+		{"apostrophe", "O'Brien", false},
+		{"curly apostrophe", "O’Brien", false},
+		{"initials", "J. R.", false},
+		{"two words", "Ann Marie", false},
+		{"at the length limit", strings.Repeat("a", maxFirstNameLength), false},
+
+		{"over the length limit", strings.Repeat("a", maxFirstNameLength+1), true},
+		{"absurdly long", strings.Repeat("a", 5000), true},
+		{"digits", "Chef2000", true},
+		{"emoji", "Julian🍳", true},
+		{"newline", "Julian\nAdmin", true},
+		{"tab", "Julian\tAdmin", true},
+		{"control character", "Julian\x00", true},
+		{"punctuation only", "---", true},
+		{"symbols", "<script>", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := svc.ValidateFirstName(tt.firstName)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ValidateFirstName(%q) error = %v, wantErr %v", tt.firstName, err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestValidateEmail_LengthCap(t *testing.T) {
+	svc := newTestUserService(testutil.NewMockUserRepo())
+
+	long := strings.Repeat("a", 250) + "@example.com" // valid shape, 262 chars
+	if err := svc.ValidateEmail(long); err == nil {
+		t.Error("ValidateEmail: an over-long address should fail")
+	}
+	if err := svc.ValidateEmail("fine@example.com"); err != nil {
+		t.Errorf("ValidateEmail: a normal address should pass, got %v", err)
+	}
+}