From e491e063cf5be276253677f890a892599c6494ec Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 27 Sep 2026 14:04:34 +0800 Subject: [PATCH] =?UTF-8?q?fix(api):=20=E6=B6=88=E8=B4=B9=20op-login=20?= =?UTF-8?q?=E8=AF=B7=E6=B1=82=E6=97=B6=E8=A6=81=E6=B1=82=E5=B7=B2=E5=AE=A1?= =?UTF-8?q?=E6=89=B9=EF=BC=8Cfake=20=E5=90=8C=E6=AD=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/api/api_test.go | 10 +++++----- internal/api/pgrepo.go | 2 +- internal/pgint/pgint_test.go | 9 +++++++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 9fa5c65..50bbcb3 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -1734,13 +1734,13 @@ func (f *fakeRepo) OpLoginRequestByID(_ context.Context, id string) (*OpLoginReq }, nil } -// ConsumeOpLoginRequest stamps consumed on an unconsumed, unexpired request (the -// finish path's single-use guard), mirroring the PG zero-rows-else UPDATE. The -// approval gate is read by the handler BEFORE this call, so consume only checks -// consumed_at and expiry (exactly as PG does). +// ConsumeOpLoginRequest stamps consumed on an approved, unconsumed, unexpired +// request (the finish path's single-use guard), mirroring the PG zero-rows-else +// UPDATE. The handler reads the approval first too; the store refuses a pending +// request on its own so the single-use guard never depends on that read. func (f *fakeRepo) ConsumeOpLoginRequest(_ context.Context, id string, now time.Time) error { r, ok := f.opLogins[id] - if !ok || r.consumed || !r.expiresAt.After(now) { + if !ok || r.status != "approved" || r.consumed || !r.expiresAt.After(now) { return ErrNotFound } r.consumed = true diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index b86cd64..05fbe0b 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -2948,7 +2948,7 @@ func (p *PGRepo) ApproveOpLogin(ctx context.Context, id, approverUserID string, func (p *PGRepo) ConsumeOpLoginRequest(ctx context.Context, id string, now time.Time) error { res, err := p.db.ExecContext(ctx, `UPDATE op_login_requests SET consumed_at = $2 - WHERE id = $1 AND consumed_at IS NULL AND expires_at > $2`, + WHERE id = $1 AND approved_at IS NOT NULL AND consumed_at IS NULL AND expires_at > $2`, id, now) if err != nil { return err diff --git a/internal/pgint/pgint_test.go b/internal/pgint/pgint_test.go index 71371be..75bdbe7 100644 --- a/internal/pgint/pgint_test.go +++ b/internal/pgint/pgint_test.go @@ -1260,6 +1260,15 @@ func TestOpLoginStateMachine(t *testing.T) { t.Fatal("fresh pending request missing from the list") } + // A pending request is not a ticket: the store refuses to consume it on its own, + // whatever the caller checked, and the refusal leaves it pending and unconsumed. + if err := repo.ConsumeOpLoginRequest(ctx, id, now); !errors.Is(err, api.ErrNotFound) { + t.Fatalf("consume pending = %v, want ErrNotFound", err) + } + if req, err := repo.OpLoginRequestByID(ctx, id); err != nil || req.Status != "pending" || req.Consumed { + t.Fatalf("after refused consume = %+v, %v; want pending+unconsumed", req, err) + } + // Approve -> consume -> single use; second approve/consume are ErrNotFound. if err := repo.ApproveOpLogin(ctx, id, approver.ID, now); err != nil { t.Fatalf("ApproveOpLogin: %v", err)