Unverified Commit d1ec40f7 authored by Lemon-miaow's avatar Lemon-miaow
Browse files

fix(users): an admin email edit must clear the stale verification 

UpdateUser wrote a new address but kept email_verified, so patching a verified
account asserted a proof of an address nobody had proven — and the
pre-session login mails and resolves on exactly that flag, so a typo'd edit
could hand the account's sign-in codes to the wrong mailbox.

Changing the address now clears the flag in the same write; a no-op patch that
passes the same value keeps it. The fake mirrors the semantics, and the pgint
suite pins both halves (same value keeps proof, new value drops it).
parent b6ef27cd

internal/api/api_test.go

+6 −0
Changes for internal/api/api_test.go: 6 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -921,6 +921,12 @@ func (f *fakeRepo) UpdateUser(_ context.Context, userID string, patch UpdateUser 
			f.seededUsers[i].detail.Username = *patch.Username

		}

		if patch.Email != nil {

			// Changing the address voids the proof of it, exactly like PGRepo:

			// only VerifyEmailOTP may assert a verified address.

			if *patch.Email != f.seededUsers[i].view.Email {

				f.seededUsers[i].view.EmailVerified = false

				f.seededUsers[i].detail.EmailVerified = false

			}

			f.seededUsers[i].view.Email = *patch.Email

			f.seededUsers[i].detail.Email = *patch.Email

		}

internal/api/pgrepo.go

+9 −1
Changes for internal/api/pgrepo.go: 9 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -1422,7 +1422,15 @@ func (p *PGRepo) UpdateUser(ctx context.Context, userID string, patch UpdateUser 
	}

	if patch.Email != nil {

		argn++

		sets = append(sets, fmt.Sprintf("email = NULLIF($%d, '')", argn))

		// Changing the address voids any proof of it: only VerifyEmailOTP may assert

		// a verified address (mirrors SetUserEmail's rationale — a fresh, unproven

		// value must not keep a stale verified flag that would let the pre-session

		// email login resolve the account). A no-op edit that passes the same value

		// keeps the flag; the second expression reads the OLD row, so comparing

		// there is exact.

		sets = append(sets,

			fmt.Sprintf("email = NULLIF($%d, '')", argn),

			fmt.Sprintf("email_verified = (email_verified AND email IS NOT DISTINCT FROM NULLIF($%d, ''))", argn))

		args = append(args, *patch.Email)

	}

	if patch.Role != nil {

internal/pgint/pgint_test.go

+34 −0
Changes for internal/pgint/pgint_test.go: 34 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -323,6 +323,40 @@ func TestConsumeLoginEmailOTPContract(t *testing.T) { 
	}

}



// ---- user admin (spec §7) -------------------------------------------------------



// An admin email edit must not carry a verification over to an address nobody

// proved: the verified flag is exactly what the pre-session login resolves on

// (UserByEmail), and only VerifyEmailOTP may assert it — the same rationale as

// SetUserEmail. A no-op edit that passes the same value keeps the proof.

func TestUserAdminEmailEditClearsVerification(t *testing.T) {

	ctx := context.Background()

	u := newUser(t, "user", "admin-edit")

	purpose := "onboard_email"

	addr := "edit-" + suffix(t) + "@example.net"

	now := mustNow()



	if err := repo.CreateEmailOTP(ctx, "ae-"+suffix(t), u.ID, addr, "h", purpose, now.Add(5*time.Minute)); err != nil {

		t.Fatalf("CreateEmailOTP: %v", err)

	}

	if _, err := repo.VerifyEmailOTP(ctx, u.ID, purpose, "h", now); err != nil {

		t.Fatalf("verify: %v", err)

	}

	assertEmailProven(t, u.ID, addr, true)



	same := addr

	if _, err := repo.UpdateUser(ctx, u.ID, api.UpdateUserInput{Email: &same}, "pgint"); err != nil {

		t.Fatalf("UpdateUser (same email): %v", err)

	}

	assertEmailProven(t, u.ID, addr, true)



	next := "edit2-" + suffix(t) + "@example.net"

	if _, err := repo.UpdateUser(ctx, u.ID, api.UpdateUserInput{Email: &next}, "pgint"); err != nil {

		t.Fatalf("UpdateUser (new email): %v", err)

	}

	assertEmailProven(t, u.ID, next, false)

}



// ---- op.console staff login state machine --------------------------------------



func TestOpLoginStateMachine(t *testing.T) {