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

fix(api): op-login finish 拒绝审批后被停用或删除的账号

parent 11244138
Loading
Loading
Loading
Loading
+6 −4
Changes for docs/openapi.yaml: 6 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -3300,8 +3300,9 @@ paths:
        Public, pre-session final leg: mints a host-only staff session only when BOTH
        factors have landed — the request is approved-and-live AND the mailed code
        verifies. Every failure (unknown handle, not-yet-approved, wrong or locked code,
        an account past its daily wrong-code budget, lost race) collapses into one uniform 400 op_login_invalid, so a code-less
        caller learns nothing. Admin is re-asserted before the session is issued.
        an account past its daily wrong-code budget, an account disabled or deleted since
        the start, lost race) collapses into one uniform 400 op_login_invalid, so a
        code-less caller learns nothing. Admin is re-asserted before the session is issued.
      x-felis-face: [external]
      x-felis-tier: public
      security: []
@@ -3329,8 +3330,9 @@ paths:
        '400':
          description: >-
            request_id and code are required (bad_request); or the login could not be
            completed — unknown handle, not approved, wrong or locked code, or lost
            race, all uniform (op_login_invalid).
            completed — unknown handle, not approved, wrong or locked code, an account
            disabled or deleted since the start, or lost race, all uniform
            (op_login_invalid).
          content:
            application/json:
              schema: { $ref: '#/components/schemas/Error' }
+4 −0
Changes for internal/api/api_test.go: 4 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -95,6 +95,7 @@ type fakeRepo struct {
	failMarkReauth   error
	failGetSetting   error
	failRedeemSetup  error
	failUserDetail   error
	// player email OTPs (spec §B2). Keyed by row id; the verify path scans for the
	// newest live (user, purpose) just as the PG query does.
	otps map[string]*fakeEmailOTP
@@ -1271,6 +1272,9 @@ func (f *fakeRepo) ListUsers(_ context.Context, opts ListUsersOpts) ([]UserView,
}

func (f *fakeRepo) UserDetail(_ context.Context, userID string) (*UserDetail, error) {
	if f.failUserDetail != nil {
		return nil, f.failUserDetail
	}
	deletedAt := time.Unix(1_700_000_000, 0)
	for _, su := range f.seededUsers {
		if su.view.ID == userID {
+14 −0
Changes for internal/api/handlers_op_login.go: 14 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -313,6 +313,20 @@ func (a *API) handleOpLoginFinish(w http.ResponseWriter, r *http.Request) {
		writeError(w, r, invalid)
		return
	}
	// A disabled or soft-deleted account finishes nothing, whatever it proved: start
	// found it through the live-only UserByEmail, but an admin may have retired it
	// since (audit #33; a soft delete disables the account too). Checked before the
	// code is touched, like the approval, and answered with the same uniform failure.
	d, err := a.Repo.UserDetail(r.Context(), loginReq.UserID)
	if err != nil {
		writeError(w, r, err)
		return
	}
	if d.Disabled {
		a.authFailure(r, "op_login", "account_retired", a.opLoginAccount(r, loginReq.UserID))
		writeError(w, r, invalid)
		return
	}
	// Consume the mailed code (op_login purpose). A wrong/expired/locked code charges an
	// attempt without minting anything and returns the uniform failure — the code, not
	// the request, is the problem, and the request stays approved for a retry.
+60 −0
Changes for internal/api/handlers_op_login_test.go: 60 added lines, 0 removed lines.
Original line number Diff line number Diff line
package api

import (
	"errors"
	"net/http"
	"net/http/httptest"
	"strings"
@@ -438,6 +439,65 @@ func TestOpLoginFinishUniform(t *testing.T) {
	})
}

// An account an admin retires between the approval and the finish signs in
// nowhere: finish answers the uniform failure, sets no cookie, stores no session,
// and leaves the code as it was.
func TestOpLoginFinishRefusesRetiredAccount(t *testing.T) {
	for _, tc := range []struct {
		name   string
		retire func(repo *fakeRepo)
	}{
		{"disabled", func(repo *fakeRepo) {
			u := UserView{ID: "a1", Username: "op", Email: "[email protected]", Role: "admin", Disabled: true}
			repo.seededUsers = append(repo.seededUsers, seededUser{view: u, detail: UserDetail{UserView: u}})
		}},
		{"deleted", func(repo *fakeRepo) { repo.deletedIDs["a1"] = true }},
	} {
		t.Run(tc.name, func(t *testing.T) {
			api, repo, mailer := seedOpLoginAPI(t)
			eh, ih := api.ExternalHandler(), api.InternalHandler()
			reqID := acctBody(t, startOp(eh, "[email protected]"))["request_id"].(string)
			if w := approveOp(ih, reqID, opUUID, "op"); w.Code != http.StatusOK {
				t.Fatalf("approve: %d (%s)", w.Code, w.Body.String())
			}
			tc.retire(repo)

			w := finishOp(eh, reqID, mailer.code)
			if w.Code != http.StatusBadRequest || decodeErr(t, w) != "op_login_invalid" || len(w.Result().Cookies()) != 0 {
				t.Fatalf("finish: code = %d body %s cookies %v, want 400 op_login_invalid and none",
					w.Code, w.Body.String(), w.Result().Cookies())
			}
			if len(repo.sessions) != 0 {
				t.Fatalf("sessions = %d, want none", len(repo.sessions))
			}
			for _, o := range repo.otps {
				if o.consumed || o.attempts != 0 {
					t.Errorf("the code must stay untouched: consumed=%v attempts=%d", o.consumed, o.attempts)
				}
			}
		})
	}

	// A store that cannot say whether the account is live is an outage: the finish
	// can be retried with the same approval, so it is not told to start over.
	t.Run("store outage", func(t *testing.T) {
		api, repo, mailer := seedOpLoginAPI(t)
		eh, ih := api.ExternalHandler(), api.InternalHandler()
		reqID := acctBody(t, startOp(eh, "[email protected]"))["request_id"].(string)
		if w := approveOp(ih, reqID, opUUID, "op"); w.Code != http.StatusOK {
			t.Fatalf("approve: %d (%s)", w.Code, w.Body.String())
		}
		repo.failUserDetail = errors.New("connection refused")
		if w := finishOp(eh, reqID, mailer.code); w.Code != http.StatusInternalServerError {
			t.Fatalf("finish: code = %d body %s, want 500", w.Code, w.Body.String())
		}
		repo.failUserDetail = nil
		if w := finishOp(eh, reqID, mailer.code); w.Code != http.StatusOK {
			t.Fatalf("retry: code = %d body %s, want 200", w.Code, w.Body.String())
		}
	})
}

// TestOpLoginApproveGate pins the in-game approval gate: only a linked staff UUID
// (role admin or owner) may vouch (all refusals share one 403 not_admin), a
// missing/no-longer-pending request is 404, and a bare request without an
+2 −2
Changes for panel/src/lib/openapi.gen.ts: 2 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -944,7 +944,7 @@ export interface paths {
        put?: never;
        /**
         * Redeem an approved op.console request plus its mailed code into a staff session (spec §B).
         * @description Public, pre-session final leg: mints a host-only staff session only when BOTH factors have landed — the request is approved-and-live AND the mailed code verifies. Every failure (unknown handle, not-yet-approved, wrong or locked code, an account past its daily wrong-code budget, lost race) collapses into one uniform 400 op_login_invalid, so a code-less caller learns nothing. Admin is re-asserted before the session is issued.
         * @description Public, pre-session final leg: mints a host-only staff session only when BOTH factors have landed — the request is approved-and-live AND the mailed code verifies. Every failure (unknown handle, not-yet-approved, wrong or locked code, an account past its daily wrong-code budget, an account disabled or deleted since the start, lost race) collapses into one uniform 400 op_login_invalid, so a code-less caller learns nothing. Admin is re-asserted before the session is issued.
         */
        post: operations["opLoginFinish"];
        delete?: never;
@@ -5254,7 +5254,7 @@ export interface operations {
                    };
                };
            };
            /** @description request_id and code are required (bad_request); or the login could not be completed — unknown handle, not approved, wrong or locked code, or lost race, all uniform (op_login_invalid). */
            /** @description request_id and code are required (bad_request); or the login could not be completed — unknown handle, not approved, wrong or locked code, an account disabled or deleted since the start, or lost race, all uniform (op_login_invalid). */
            400: {
                headers: {
                    [name: string]: unknown;