fix(profile): return sentinel errors for wrong password and duplicate email (#931)

On the profile-update path, wrong current password and duplicate email
both fell through to the generic 500 "Internal server error" because
UpdateUserProfile returned plain fmt.Errorf strings that the handler
did not recognise as client errors.

- Add ErrCurrentPasswordIncorrect sentinel to internal/auth/errors.go.
- Return the sentinel (not a plain string) from UpdateUserProfile when
  the current password does not match.
- Return the existing ErrEmailInUse sentinel (not a plain string) from
  updateUserEmail when the address is taken.
- Add mapProfileUpdateError in handler_auth.go: wrong password -> 401
  with the sentinel message; duplicate email -> 409 with the neutral
  privacy-safe message "Unable to update email" (does not confirm
  another account's existence, preventing enumeration).
- Add handler tests for both new error paths asserting the correct
  status code and message.
- Add service-layer tests asserting errors.Is on both sentinels so the
  contract is positively enforced.

Closes #929

Author: cristim

internal/api/handler_auth.go

@@ -394,12 +394,32 @@ func (h *Handler) updateProfile(ctx context.Context, req *events.LambdaFunctionU
 
 	// Update profile through auth service
 	if err := h.auth.UpdateUserProfile(ctx, session.UserID, profileReq.Email, currentPassword, newPassword); err != nil {
-		return nil, err
+		return nil, mapProfileUpdateError(err)
 	}
 
 	return map[string]string{"status": "profile updated"}, nil
 }
 
+// mapProfileUpdateError converts profile-update service errors to the correct
+// HTTP status code and a user-facing message.
+//
+//   - ErrCurrentPasswordIncorrect -> 401 with a precise message (the acting
+//     user is verifying their own credential, so specificity is safe).
+//   - ErrEmailInUse -> 409 with a neutral message that does NOT confirm whether
+//     another account holds the address; prevents account enumeration via the
+//     profile-update path (issue #929).
+//   - All other errors pass through unchanged for handleRequestError to render
+//     as 500.
+func mapProfileUpdateError(err error) error {
+	switch {
+	case errors.Is(err, auth.ErrCurrentPasswordIncorrect):
+		return NewClientError(401, err.Error())
+	case errors.Is(err, auth.ErrEmailInUse):
+		return NewClientError(409, "Unable to update email")
+	}
+	return err
+}
+
 // decodeProfilePasswords decodes the optional base64-encoded current and new
 // passwords from a ProfileUpdateRequest. Pulled out of updateProfile to keep
 // that function under the cyclomatic limit.

internal/api/handler_auth_test.go

@@ -947,6 +947,68 @@ func TestHandler_updateProfile_AcceptsValidEmail(t *testing.T) {
 	mockAuth.AssertCalled(t, "UpdateUserProfile", ctx, "12345678-1234-1234-1234-123456789abc", "new@example.com", "", "")
 }
 
+// TestHandler_updateProfile_WrongCurrentPassword verifies that a wrong current
+// password returned by the auth service is surfaced as 401, not a generic 500
+// (issue #929). The acting user is checking their own credential so a precise
+// message is safe and helpful.
+func TestHandler_updateProfile_WrongCurrentPassword(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	session := &Session{UserID: "12345678-1234-1234-1234-123456789abc"}
+	mockAuth.On("ValidateSession", ctx, "test-token").Return(session, nil)
+	mockAuth.On("UpdateUserProfile", mock.MatchedBy(func(c context.Context) bool { return c != nil }), "12345678-1234-1234-1234-123456789abc", "", "wrongpass", "").
+		Return(auth.ErrCurrentPasswordIncorrect)
+
+	handler := &Handler{auth: mockAuth}
+
+	encoded := base64.StdEncoding.EncodeToString([]byte("wrongpass"))
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer test-token"},
+		Body:    `{"email": "", "current_password": "` + encoded + `"}`,
+	}
+
+	_, err := handler.updateProfile(ctx, req)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "wrong-password error must be a ClientError, not a 500")
+	assert.Equal(t, 401, ce.code)
+	assert.Equal(t, auth.ErrCurrentPasswordIncorrect.Error(), ce.message)
+}
+
+// TestHandler_updateProfile_DuplicateEmail verifies that a duplicate-email
+// conflict is surfaced as 409 with a privacy-preserving message that does NOT
+// confirm whether another account holds the address (issue #929).
+func TestHandler_updateProfile_DuplicateEmail(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	session := &Session{UserID: "12345678-1234-1234-1234-123456789abc"}
+	mockAuth.On("ValidateSession", ctx, "test-token").Return(session, nil)
+	mockAuth.On("UpdateUserProfile", mock.MatchedBy(func(c context.Context) bool { return c != nil }), "12345678-1234-1234-1234-123456789abc", "taken@example.com", "mypass", "").
+		Return(auth.ErrEmailInUse)
+
+	handler := &Handler{auth: mockAuth}
+
+	encoded := base64.StdEncoding.EncodeToString([]byte("mypass"))
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer test-token"},
+		Body:    `{"email": "taken@example.com", "current_password": "` + encoded + `"}`,
+	}
+
+	_, err := handler.updateProfile(ctx, req)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "duplicate-email error must be a ClientError, not a 500")
+	assert.Equal(t, 409, ce.code)
+	// Must NOT expose the raw ErrEmailInUse message which would confirm another
+	// account exists. The privacy-preserving message is "Unable to update email".
+	assert.Equal(t, "Unable to update email", ce.message)
+	assert.NotContains(t, ce.message, "already in use", "response must not confirm another account's existence")
+}
+
 // TestHandler_resetPassword_DecodesBase64 verifies issue #356: the
 // resetPassword handler must base64-decode new_password before forwarding to
 // the service, matching the pattern used by login / change-password /

internal/auth/errors.go

@@ -34,6 +34,12 @@ var (
 	// permission. Mapped to 403 (issue #907).
 	ErrSelfEscalation = errors.New("cannot escalate your own group membership")
 
+	// ErrCurrentPasswordIncorrect is returned by UpdateUserProfile when the
+	// caller-supplied current password does not match the stored hash. Mapped
+	// to 401 at the API layer (the acting user is verifying their own
+	// credential, so a precise message is safe -- issue #929).
+	ErrCurrentPasswordIncorrect = errors.New("Current password is incorrect")
+
 	// MFA login-gate sentinels — used by the login API handler to map
 	// to machine-readable response codes (mfa_required /
 	// invalid_mfa_code) so the frontend can branch on the error class

internal/auth/service_user.go

@@ -452,7 +452,7 @@ func (s *Service) UpdateUserProfile(ctx context.Context, userID string, email st
 	}
 
 	if !s.verifyPassword(currentPassword, user.PasswordHash) {
-		return fmt.Errorf("current password is incorrect")
+		return ErrCurrentPasswordIncorrect
 	}
 
 	if err := s.updateUserEmail(ctx, user, email); err != nil {
@@ -492,7 +492,7 @@ func (s *Service) updateUserEmail(ctx context.Context, user *User, email string)
 			return err
 		}
 		if existing != nil {
-			return fmt.Errorf("email already in use")
+			return ErrEmailInUse
 		}
 		user.Email = email
 	}

internal/auth/service_user_test.go

@@ -2,6 +2,7 @@ package auth
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -794,7 +795,41 @@ func TestService_UpdateUserProfile(t *testing.T) {
 
 		err := service.UpdateUserProfile(ctx, "user-123", "new@example.com", "WrongPassword", "SecureTest@456")
 		assert.Error(t, err)
-		assert.Contains(t, err.Error(), "current password is incorrect")
+		// The handler must receive the sentinel so it can produce a 401 instead
+		// of a 500. Assert errors.Is in addition to the string check (issue #929).
+		assert.True(t, errors.Is(err, ErrCurrentPasswordIncorrect),
+			"UpdateUserProfile wrong-password must return ErrCurrentPasswordIncorrect sentinel")
+
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("duplicate email returns ErrEmailInUse sentinel", func(t *testing.T) {
+		// Verifies issue #929: updateUserEmail must return the ErrEmailInUse
+		// sentinel (not a plain fmt.Errorf string) so the API handler can map
+		// it to a 409 with a privacy-preserving message.
+		mockStore := new(MockStore)
+		mockEmail := new(MockEmailSender)
+		service := createTestService(mockStore, mockEmail)
+
+		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
+		testUser := &User{
+			ID:           "user-123",
+			Email:        "old@example.com",
+			PasswordHash: string(hash),
+			Active:       true,
+		}
+		otherUser := &User{
+			ID:    "other-456",
+			Email: "taken@example.com",
+		}
+
+		mockStore.On("GetUserByID", ctx, "user-123").Return(testUser, nil).Once()
+		mockStore.On("GetUserByEmail", ctx, "taken@example.com").Return(otherUser, nil).Once()
+
+		err := service.UpdateUserProfile(ctx, "user-123", "taken@example.com", "OldPassword123", "")
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, ErrEmailInUse),
+			"UpdateUserProfile duplicate-email must return ErrEmailInUse sentinel")
 
 		mockStore.AssertExpectations(t)
 	})