Crussell/backend/handlers/user/account_round9_fixes_test.go

//go:build test

package user

import (
	"bytes"
	"context"
	"database/sql"
	"encoding/json"
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"
	"sync"
	"testing"
	"time"

	"crussell/clock"
	"crussell/db"
	"crussell/internal/s3"
	"crussell/mw"
	"crussell/testutils"
	"crussell/testutils/fixtures"

	"github.com/stretchr/testify/require"
	"golang.org/x/crypto/bcrypt"
)

// ============================================================================
// FIX 2 — DeleteAccountHandler current-password lockout
// ============================================================================

// TestDeleteAccount_CurrentPasswordLockout verifies the FIX 2 budget: after 5
// consecutive wrong current passwords the account is locked (even the CORRECT
// password is rejected with 429), and a cleared lockout lets the correct
// password through. Sequential (no t.Parallel): the handler reads the
// process-global s3.Client / payments.SquareClient.
func TestDeleteAccount_CurrentPasswordLockout(t *testing.T) {
	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)

	// Five wrong current passwords: each rejected with 401 and counted toward
	// the shared failed_attempts budget.
	for i := 0; i < 5; i++ {
		req := deleteAccountRequest(t, ctx, userID, "not-the-password", "")
		rr := httptest.NewRecorder()
		DeleteAccountHandler(rr, req)
		require.Equal(t, http.StatusUnauthorized, rr.Code, "wrong current password must be rejected (attempt %d)", i+1)
	}

	// The budget is now locked: even the correct password is rejected. FIX 4:
	// a locked account returns the SAME uniform 401 as a wrong password (never
	// distinguishable), with a distinct body the UI can surface.
	req := deleteAccountRequest(t, ctx, userID, "testpassword123", "")
	rr := httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusUnauthorized, rr.Code, "delete-account must be rejected with a lockout after 5 wrong current passwords")
	require.Contains(t, rr.Body.String(), "too many failed attempts", "the locked body must stay distinct for the UI")

	// The account survives the lockout.
	var firstName string
	require.NoError(t, tx.QueryRow(ctx, `SELECT n_first_name FROM users WHERE id = $1`, userID).Scan(&firstName))
	require.Equal(t, "Test", firstName)

	// Clearing the lockout (the documented operator / password-reset recovery)
	// lets the correct password through.
	_, err = tx.Exec(ctx, `UPDATE users SET failed_attempts = 0, locked_until = NULL WHERE id = $1`, userID)
	require.NoError(t, err)
	req = deleteAccountRequest(t, ctx, userID, "testpassword123", "")
	rr = httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusNoContent, rr.Code, "correct password must succeed after the lockout is reset")
}

// ============================================================================
// FIX 3 — ChangePasswordHandler current-password lockout
// ============================================================================

// changePasswordRequest builds the PUT /api/user/change-password request with
// the given current/new password and an authenticated context.
func changePasswordRequest(t *testing.T, ctx context.Context, userID, currentPassword, newPassword string) *http.Request {
	t.Helper()
	body, err := json.Marshal(ChangePasswordRequest{
		CurrentPassword: currentPassword,
		NewPassword:     newPassword,
	})
	require.NoError(t, err)
	req := httptest.NewRequest(http.MethodPut, "/api/user/change-password", bytes.NewReader(body))
	req = req.WithContext(context.WithValue(ctx, mw.UserIDKey, userID))
	req.Header.Set("Content-Type", "application/json")
	return req
}

// TestPasswordChange_CurrentPasswordLockout verifies the FIX 3 budget shares
// the same failed-attempt/lockout columns as delete-account: 5 wrong current
// passwords lock the change-password flow (correct password → 429), and a
// cleared lockout lets it through.
func TestPasswordChange_CurrentPasswordLockout(t *testing.T) {
	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)

	for i := 0; i < 5; i++ {
		req := changePasswordRequest(t, ctx, userID, "not-the-password", "newpassword456")
		rr := httptest.NewRecorder()
		ChangePasswordHandler(rr, req)
		require.Equal(t, http.StatusUnauthorized, rr.Code, "wrong current password must be rejected (attempt %d)", i+1)
	}

	// Locked out: the correct current password is rejected too. FIX 4: uniform
	// 401 (never distinguishable from a wrong password), distinct body text.
	req := changePasswordRequest(t, ctx, userID, "testpassword123", "newpassword456")
	rr := httptest.NewRecorder()
	ChangePasswordHandler(rr, req)
	require.Equal(t, http.StatusUnauthorized, rr.Code, "change-password must be rejected with a lockout after 5 wrong current passwords")
	require.Contains(t, rr.Body.String(), "too many failed attempts", "the locked body must stay distinct for the UI")

	// The correct password works again once the lockout is cleared.
	_, err = tx.Exec(ctx, `UPDATE users SET failed_attempts = 0, locked_until = NULL WHERE id = $1`, userID)
	require.NoError(t, err)
	req = changePasswordRequest(t, ctx, userID, "testpassword123", "newpassword456")
	rr = httptest.NewRecorder()
	ChangePasswordHandler(rr, req)
	require.Equal(t, http.StatusOK, rr.Code, "correct current password must succeed after the lockout is reset")
}

// ============================================================================
// FIX 1 — Durable S3/R2 profile-picture deletion outbox
// ============================================================================

// recordingUploader records S3/R2 Delete calls so the account-deletion outbox
// tests can assert what was (and wasn't) deleted. Every Uploader method is
// implemented explicitly (no embedded nil) so parallel profile-picture tests
// that read the swapped global s3.Client never panic.
type recordingUploader struct {
	mu      sync.Mutex
	deleted []string // "bucket/key"
}

func (r *recordingUploader) Upload(context.Context, string, string, io.Reader, string) error {
	return nil
}

func (r *recordingUploader) Download(context.Context, string, string, io.Writer) error {
	return nil
}

func (r *recordingUploader) GetURL(context.Context, string, string) (string, error) {
	return "https://cdn.example.com/x/y", nil
}

func (r *recordingUploader) HealthCheck(context.Context) error {
	return nil
}

func (r *recordingUploader) Delete(_ context.Context, bucket, key string) error {
	r.mu.Lock()
	defer r.mu.Unlock()
	r.deleted = append(r.deleted, bucket+"/"+key)
	return nil
}

func (r *recordingUploader) Deleted() []string {
	r.mu.Lock()
	defer r.mu.Unlock()
	return append([]string(nil), r.deleted...)
}

// waitFor polls cond until it holds or the timeout elapses, failing the test.
func waitFor(t *testing.T, timeout time.Duration, cond func() bool) {
	t.Helper()
	deadline := clock.Now().Add(timeout)
	for clock.Now().Before(deadline) {
		if cond() {
			return
		}
		time.Sleep(5 * time.Millisecond)
	}
	t.Fatal("timed out waiting for condition")
}

// TestDeleteAccount_S3OutboxPersisted_AndDrained verifies the FIX 1 durable
// pattern: with a configured bucket the erasure transaction persists a
// pending_s3_deletions outbox row, and the async goroutine drains it after
// deleting the object. Sequential: swaps the process-global s3.Client and env.
func TestDeleteAccount_S3OutboxPersisted_AndDrained(t *testing.T) {
	savedClient := s3.Client
	rec := &recordingUploader{}
	s3.Client = rec
	t.Cleanup(func() { s3.Client = savedClient })
	t.Setenv("S3_PROFILE_PICS_BUCKET", "test-profile-pics-bucket")

	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)

	_, err = tx.Exec(ctx, `UPDATE users SET profile_pic_url = 'https://cdn.example.com/pics/old.jpg' WHERE id = $1`, userID)
	require.NoError(t, err)

	req := deleteAccountRequest(t, ctx, userID, "testpassword123", "")
	rr := httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusNoContent, rr.Code, rr.Body.String())

	// The durable outbox row was written inside the erasure transaction.
	var outboxID, bucket, objectKey string
	err = tx.QueryRow(ctx, `SELECT id, bucket, object_key FROM pending_s3_deletions WHERE user_id = $1`, userID).
		Scan(&outboxID, &bucket, &objectKey)
	require.NoError(t, err, "pending_s3_deletions outbox row must be persisted in the erasure transaction")
	require.NotEmpty(t, outboxID)
	require.Equal(t, "test-profile-pics-bucket", bucket)
	require.Equal(t, "profiles/"+userID+".jpg", objectKey)

	// The async goroutine (primary drain) deletes the object from the store.
	waitFor(t, 2*time.Second, func() bool { return len(rec.Deleted()) > 0 })
	require.Equal(t, []string{"test-profile-pics-bucket/profiles/" + userID + ".jpg"}, rec.Deleted())
}

// TestDeleteAccount_S3Outbox_FailClosedOnEmptyBucket verifies the FIX 4
// fail-closed behavior: with S3_PROFILE_PICS_BUCKET unset the handler must
// NOT guess the dev bucket — no outbox row is written and no deletion is
// attempted, even though a profile picture exists.
func TestDeleteAccount_S3Outbox_FailClosedOnEmptyBucket(t *testing.T) {
	savedClient := s3.Client
	rec := &recordingUploader{}
	s3.Client = rec
	t.Cleanup(func() { s3.Client = savedClient })
	t.Setenv("S3_PROFILE_PICS_BUCKET", "")

	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)

	_, err = tx.Exec(ctx, `UPDATE users SET profile_pic_url = 'https://cdn.example.com/pics/old.jpg' WHERE id = $1`, userID)
	require.NoError(t, err)

	req := deleteAccountRequest(t, ctx, userID, "testpassword123", "")
	rr := httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusNoContent, rr.Code, rr.Body.String())

	// Fail-closed: no outbox row, no deletion attempt against a guessed bucket.
	var count int
	require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM pending_s3_deletions WHERE user_id = $1`, userID).Scan(&count))
	require.Zero(t, count, "no outbox row may be written when the bucket is unset")
	require.Empty(t, rec.Deleted(), "no S3 deletion may be attempted against a guessed bucket")
}

// ============================================================================
// FIX 3 — atomic current-password failure record (no lost updates)
// ============================================================================

// TestDeleteAccount_ConcurrentWrongPassword_NoLostUpdates verifies the FIX 3
// atomic failure record: 20 concurrent wrong-current-password requests each
// increment the shared failed-attempt budget exactly once (no lost updates —
// the final count is 20, not a smaller racy subset) and the escalating lockout
// is armed. Runs against the real pool because a per-test pgx.Tx cannot serve
// concurrent queries; the requests carry no test-tx context.
func TestDeleteAccount_ConcurrentWrongPassword_NoLostUpdates(t *testing.T) {
	savedClient := s3.Client
	s3.Client = nil
	t.Cleanup(func() { s3.Client = savedClient })

	hash, err := bcrypt.GenerateFromPassword([]byte("testpassword123"), bcrypt.DefaultCost)
	require.NoError(t, err)

	ctx := context.Background()
	var userID string
	require.NoError(t, db.Conn.QueryRow(ctx, `
		INSERT INTO users (n_first_name, n_last_name, email, phone, date_of_birth, password_hash, account_role, account_type)
		VALUES ('Concurrent', 'User', $1, '+447123456789', '1990-01-01', $2, 'verified_email', 'email')
		RETURNING id
	`, fmt.Sprintf("concurrent.%d@test.com", time.Now().UnixNano()), string(hash)).Scan(&userID))
	t.Cleanup(func() {
		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, userID)
	})

	const n = 20
	var wg sync.WaitGroup
	for i := 0; i < n; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			req := httptest.NewRequest(http.MethodDelete, "/api/user/account", bytes.NewBufferString(`{"current_password":"not-the-password"}`))
			req = req.WithContext(context.WithValue(context.Background(), mw.UserIDKey, userID))
			rr := httptest.NewRecorder()
			DeleteAccountHandler(rr, req)
		}()
	}
	wg.Wait()

	var failedAttempts int
	var lockedUntil *time.Time
	require.NoError(t, db.Conn.QueryRow(ctx, `SELECT failed_attempts, locked_until FROM users WHERE id = $1`, userID).Scan(&failedAttempts, &lockedUntil))
	require.Equal(t, n, failedAttempts, "every concurrent wrong password must be counted — no lost updates")
	require.NotNil(t, lockedUntil, "the lockout must be armed after the burst")
	require.True(t, lockedUntil.After(clock.Now()), "locked_until must be in the future")
}

// ============================================================================
// FIX 5 — passwordless (NULL password_hash) accounts
// ============================================================================

// TestDeleteAccount_Passwordless_Requires2FAUnconditionally verifies FIX 5a: a
// NULL-password-hash (social-only) account has no current password to
// re-verify, so deleting it requires the 2FA code gate UNCONDITIONALLY — even
// when 2FA is not otherwise enforced — so a session holder cannot erase a
// passwordless account with zero credential proof.
func TestDeleteAccount_Passwordless_Requires2FAUnconditionally(t *testing.T) {
	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)
	_, err = tx.Exec(ctx, `UPDATE users SET password_hash = NULL WHERE id = $1`, userID)
	require.NoError(t, err)

	// No code → rejected with the exact message the frontend uses to reveal the
	// 2FA step (deleteRevealTwoFactor).
	req := deleteAccountRequest(t, ctx, userID, "", "")
	rr := httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String())
	require.Contains(t, rr.Body.String(), "a two-factor verification code is required to delete the account")

	// A wrong code is rejected too (the account survives).
	seedPendingTwoFA(t, ctx, tx, userID, "424242")
	req = deleteAccountRequest(t, ctx, userID, "", "000000")
	rr = httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String())

	var firstName string
	require.NoError(t, tx.QueryRow(ctx, `SELECT n_first_name FROM users WHERE id = $1`, userID).Scan(&firstName))
	require.Equal(t, "Test", firstName, "the account must survive a rejected code")

	// A correct fresh code is the sole credential — it deletes the account.
	req = deleteAccountRequest(t, ctx, userID, "", "424242")
	rr = httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusNoContent, rr.Code, rr.Body.String())
}

// TestPasswordChange_NullHash_NoPasswordToChange verifies FIX 5b: changing the
// password on a passwordless (NULL hash) account is a clear 400 with an
// actionable message — not the old 500 from scanning NULL into a plain string.
func TestPasswordChange_NullHash_NoPasswordToChange(t *testing.T) {
	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)
	_, err = tx.Exec(ctx, `UPDATE users SET password_hash = NULL WHERE id = $1`, userID)
	require.NoError(t, err)

	req := changePasswordRequest(t, ctx, userID, "whatever", "newpassword456")
	rr := httptest.NewRecorder()
	ChangePasswordHandler(rr, req)
	require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String())
	require.Contains(t, rr.Body.String(), "this account has no password to change")

	// The password must be untouched.
	var stored sql.NullString
	require.NoError(t, tx.QueryRow(ctx, `SELECT password_hash FROM users WHERE id = $1`, userID).Scan(&stored))
	require.False(t, stored.Valid, "a passwordless account must remain passwordless after a rejected change")
}

// ============================================================================
// FIX 1a — CardDAV vCard deleted inside the erasure transaction
// ============================================================================

// TestDeleteAccount_DavCardDeletedInErasureTx verifies FIX 1a: the dav_cards
// row (full name/email/phone/DOB/photo URL PII) is deleted INSIDE the erasure
// transaction — the old fire-and-forget dav.Service.DeleteContact goroutine is
// gone, so the contact PII can never be stranded by a crash or a log-only
// failure.
func TestDeleteAccount_DavCardDeletedInErasureTx(t *testing.T) {
	ctx, tx := testutils.SetupTestTx(t)
	userID, err := fixtures.CreateTestUser(tx)
	require.NoError(t, err)

	uri := userID + ".vcf"
	_, err = tx.Exec(ctx, `
		INSERT INTO dav_cards (addressbookid, uri, carddata, lastmodified, etag, size)
		VALUES (1, $1, 'BEGIN:VCARD', 0, '0', 0)
	`, uri)
	require.NoError(t, err)

	var countBefore int
	require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM dav_cards WHERE uri = $1`, uri).Scan(&countBefore))
	require.Equal(t, 1, countBefore)

	req := deleteAccountRequest(t, ctx, userID, "testpassword123", "")
	rr := httptest.NewRecorder()
	DeleteAccountHandler(rr, req)
	require.Equal(t, http.StatusNoContent, rr.Code, rr.Body.String())

	var countAfter int
	require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM dav_cards WHERE uri = $1`, uri).Scan(&countAfter))
	require.Zero(t, countAfter, "the dav_cards row must be deleted inside the erasure transaction")
}