fix(migrate): 兑换迁移码在双方认领锁下检查目标配额,超出返回 403 migrate_quota_exceeded 且码不消耗
This commit is contained in:
12 files changed
+362
-11
No files matched your search
@@ -47,6 +47,9 @@ type fakeRepo struct {
|
||||
// claimQuotaRefuse simulates ClaimServer's atomic quota gate (audit #4)
|
||||
// refusing a name whose advisory pre-check already passed.
|
||||
claimQuotaRefuse map[string]bool
|
||||
// migrateQuotaRefuse names redeeming targets whose quota RedeemMigration's gate
|
||||
// finds too small for the servers moving in; it refuses only a source that owns one.
|
||||
migrateQuotaRefuse map[string]bool
|
||||
// serverResources / resourceUpdates mirror the cached resource columns:
|
||||
// ServerResources is what the resize path reads (to preserve storage), and
|
||||
// UpdateServerResources records the write for assertions.
|
||||
@@ -1471,6 +1474,13 @@ func (f *fakeRepo) RedeemMigration(_ context.Context, targetUserID, codeHash str
|
||||
if mig == nil {
|
||||
return "", nil, ErrLinkCodeInvalid
|
||||
}
|
||||
if f.migrateQuotaRefuse[targetUserID] {
|
||||
for _, rec := range f.byName {
|
||||
if rec.OwnerID == mig.sourceUserID {
|
||||
return "", nil, ErrQuotaExceeded // before anything moves, as the real gate
|
||||
}
|
||||
}
|
||||
}
|
||||
// Re-point every server the source owns to the target (byName holds pointers).
|
||||
var moved []string
|
||||
for name, rec := range f.byName {
|
||||
|
||||
@@ -410,6 +410,11 @@ func (a *API) handleMigrateIssueCode(w http.ResponseWriter, r *http.Request) {
|
||||
var errMigrateNotConfirmed = newError(http.StatusConflict, "not_confirmed",
|
||||
"confirm the migration on this browser first; a confirmation lasts 10 minutes")
|
||||
|
||||
// errMigrateQuotaExceeded refuses a redeem whose servers would push the target over
|
||||
// its quota. The code stays unspent, so raising the quota and redeeming again works.
|
||||
var errMigrateQuotaExceeded = newError(http.StatusForbidden, "migrate_quota_exceeded",
|
||||
"the servers this migration moves do not fit this account's quota; ask an admin to raise it, then redeem the same code again before it expires")
|
||||
|
||||
// migrateRedeemRequest is the redeem body: the one-time code the target received.
|
||||
type migrateRedeemRequest struct {
|
||||
Code string `json:"code"`
|
||||
@@ -441,6 +446,10 @@ func (a *API) handleMigrateRedeem(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "invalid_code", "migrate code is invalid or expired"))
|
||||
return
|
||||
}
|
||||
if errors.Is(err, ErrQuotaExceeded) {
|
||||
writeError(w, r, errMigrateQuotaExceeded)
|
||||
return
|
||||
}
|
||||
writeError(w, r, err)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -315,6 +315,67 @@ func TestMigrateRedeemBinding(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestMigrateRedeemOverQuota: a target whose quota cannot take the source's servers is
|
||||
// refused with 403 migrate_quota_exceeded, and the refusal leaves everything as it was —
|
||||
// no server moved, the source live, the migration still code_issued, nothing audited or
|
||||
// mailed — so the same code redeems once an admin raises the quota.
|
||||
func TestMigrateRedeemOverQuota(t *testing.T) {
|
||||
const uuid = "88888888-8888-8888-8888-888888888888"
|
||||
src := &Principal{UserID: "u1", Email: "[email protected]", Role: "user"}
|
||||
tgt := &Principal{UserID: "u2", Email: "[email protected]", Role: "user"}
|
||||
|
||||
repo := newFakeRepo()
|
||||
repo.seedUser(UserView{ID: "u1", Username: "old", Email: "[email protected]", Role: "user"})
|
||||
repo.seedUser(UserView{ID: "u2", Username: "new", Email: "[email protected]", Role: "user"})
|
||||
repo.links[uuid] = "u1"
|
||||
repo.byName["alpha"] = &ServerRecord{Name: "alpha", OwnerID: "u1"}
|
||||
repo.migrateQuotaRefuse = map[string]bool{"u2": true}
|
||||
|
||||
mk, mailer, _ := migrateEnv(repo)
|
||||
ehSrc := mk(src).ExternalHandler()
|
||||
if w := startMigrate(t, mk(src).InternalHandler(), uuid); w.Code != http.StatusCreated {
|
||||
t.Fatalf("start: %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if w := do(ehSrc, "POST", "/api/v1/account/migrate/confirm/otp/start", "", jsonHeader); w.Code != http.StatusAccepted {
|
||||
t.Fatalf("otp start: %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if w := do(ehSrc, "POST", "/api/v1/account/migrate/confirm/otp/verify", `{"code":"`+mailer.code+`"}`, jsonHeader); w.Code != http.StatusOK {
|
||||
t.Fatalf("otp verify: %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
w := do(ehSrc, "POST", "/api/v1/account/migrate/issue-code", `{"target_user_id":"u2"}`, jsonHeader)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("issue-code: %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
mcode, _ := acctBody(t, w)["code"].(string)
|
||||
audits, notices := len(repo.audits), len(mailer.notices)
|
||||
|
||||
ehTgt := mk(tgt).ExternalHandler()
|
||||
w = do(ehTgt, "POST", "/api/v1/account/migrate/redeem", `{"code":"`+mcode+`"}`, jsonHeader)
|
||||
if w.Code != http.StatusForbidden || decodeErr(t, w) != "migrate_quota_exceeded" {
|
||||
t.Fatalf("redeem over quota: code = %d body %s, want 403 migrate_quota_exceeded", w.Code, w.Body.String())
|
||||
}
|
||||
if repo.byName["alpha"].OwnerID != "u1" {
|
||||
t.Fatalf("server moved on a refused redeem: owner=%s", repo.byName["alpha"].OwnerID)
|
||||
}
|
||||
if d, _ := repo.UserDetail(context.Background(), "u1"); d.DeletedAt != nil || d.Disabled {
|
||||
t.Fatalf("source retired on a refused redeem: %+v", d)
|
||||
}
|
||||
if m, _ := repo.MigrationForSource(context.Background(), "u1"); m == nil || m.State != "code_issued" {
|
||||
t.Fatalf("migration after a refused redeem = %+v, want still code_issued", m)
|
||||
}
|
||||
if len(repo.audits) != audits || len(mailer.notices) != notices {
|
||||
t.Fatalf("a refused redeem audited %v / mailed %q", repo.audits[audits:], mailer.notices[notices:])
|
||||
}
|
||||
|
||||
delete(repo.migrateQuotaRefuse, "u2")
|
||||
if w := do(ehTgt, "POST", "/api/v1/account/migrate/redeem", `{"code":"`+mcode+`"}`, jsonHeader); w.Code != http.StatusOK {
|
||||
t.Fatalf("redeem once the quota fits: code = %d body %s, want 200", w.Code, w.Body.String())
|
||||
}
|
||||
if repo.byName["alpha"].OwnerID != "u2" {
|
||||
t.Fatalf("server not moved once the quota fits: owner=%s", repo.byName["alpha"].OwnerID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestMigrateGuards covers the input/state refusals: an unlinked UUID has no account to
|
||||
// migrate; a code cannot be issued before confirmation; the target may be neither the
|
||||
// source itself nor an unknown account.
|
||||
|
||||
+55
-6
@@ -396,7 +396,7 @@ func (p *PGRepo) QuotaCheck(ctx context.Context, userID string, excludeName stri
|
||||
return false, err
|
||||
}
|
||||
|
||||
return quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum, incoming), nil
|
||||
return quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum, 1, incoming), nil
|
||||
}
|
||||
|
||||
// UpdateServerResources updates the resource cache for a server after a spec
|
||||
@@ -498,7 +498,7 @@ func (p *PGRepo) ClaimServer(ctx context.Context, name, userID string) (bool, er
|
||||
return false, err
|
||||
}
|
||||
if !quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum,
|
||||
ResourceSpec{CPUMilli: cpu, MemoryMB: mem, StorageMB: stor}) {
|
||||
1, ResourceSpec{CPUMilli: cpu, MemoryMB: mem, StorageMB: stor}) {
|
||||
return false, ErrQuotaExceeded
|
||||
}
|
||||
|
||||
@@ -526,13 +526,14 @@ func (p *PGRepo) ClaimServer(ctx context.Context, name, userID string) (bool, er
|
||||
}
|
||||
|
||||
// quotaAllows applies the four spec §9.3 caps to one per-owner aggregate plus
|
||||
// the incoming spec. Shared by QuotaCheck (the advisory pre-check) and
|
||||
// ClaimServer (the atomic gate) so the two can never drift. An invalid (NULL or
|
||||
// incomingCount servers whose specs sum to incoming. Shared by QuotaCheck (the
|
||||
// advisory pre-check), ClaimServer (the atomic gate) and RedeemMigration (a
|
||||
// migration's servers arriving at once) so they can never drift. An invalid (NULL or
|
||||
// missing) cap means unlimited for that dimension; storage is compared in MB
|
||||
// against max_storage_gb × 1024.
|
||||
func quotaAllows(maxServers, maxCPU, maxMem, maxStor sql.NullInt64,
|
||||
count, cpuSum, memSum, storSum int64, incoming ResourceSpec) bool {
|
||||
if maxServers.Valid && count >= maxServers.Int64 {
|
||||
count, cpuSum, memSum, storSum, incomingCount int64, incoming ResourceSpec) bool {
|
||||
if maxServers.Valid && count+incomingCount > maxServers.Int64 {
|
||||
return false
|
||||
}
|
||||
if maxCPU.Valid && cpuSum+int64(incoming.CPUMilli) > maxCPU.Int64 {
|
||||
@@ -2492,6 +2493,54 @@ func (p *PGRepo) RedeemMigration(ctx context.Context, targetUserID, codeHash str
|
||||
return "", nil, err
|
||||
}
|
||||
|
||||
// Take both accounts' claim lanes, ClaimServer's lock, in id order so two
|
||||
// migrations crossing between the same pair cannot deadlock. The target's makes
|
||||
// the quota read below and the move one consistent decision against its own
|
||||
// claims; the source's keeps a claim of its own from landing between the count
|
||||
// of its servers and their move.
|
||||
lanes := []string{sourceUserID, targetUserID}
|
||||
if lanes[1] < lanes[0] {
|
||||
lanes[0], lanes[1] = lanes[1], lanes[0]
|
||||
}
|
||||
for _, id := range lanes {
|
||||
if _, err := tx.ExecContext(ctx, `SELECT pg_advisory_xact_lock(hashtext($1))`, id); err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
}
|
||||
|
||||
// The target's four caps (spec §9.3) must hold with every server the source owns
|
||||
// added in, as a claim of each would. Over any cap, nothing moves and the code
|
||||
// stays unspent, so the target can have an admin raise its quota and redeem again
|
||||
// before the code expires. A source that owns nothing moves nothing, and retires
|
||||
// even into a target already over a cap.
|
||||
var n, cpu, mem, stor int64
|
||||
if err := tx.QueryRowContext(ctx,
|
||||
`SELECT COUNT(*), COALESCE(SUM(cached_cpu_milli), 0), COALESCE(SUM(cached_memory_mb), 0), COALESCE(SUM(cached_storage_mb), 0)
|
||||
FROM servers WHERE owner_id = $1 AND deleted_at IS NULL`,
|
||||
sourceUserID).Scan(&n, &cpu, &mem, &stor); err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
if n > 0 {
|
||||
var maxServers, maxCPU, maxMem, maxStor sql.NullInt64
|
||||
if err := tx.QueryRowContext(ctx,
|
||||
`SELECT max_servers, max_cpu_milli, max_memory_mb, max_storage_gb
|
||||
FROM quotas WHERE user_id = $1`, targetUserID).Scan(
|
||||
&maxServers, &maxCPU, &maxMem, &maxStor); err != nil && !errors.Is(err, sql.ErrNoRows) {
|
||||
return "", nil, err
|
||||
}
|
||||
var count, cpuSum, memSum, storSum int64
|
||||
if err := tx.QueryRowContext(ctx,
|
||||
`SELECT COUNT(*), COALESCE(SUM(cached_cpu_milli), 0), COALESCE(SUM(cached_memory_mb), 0), COALESCE(SUM(cached_storage_mb), 0)
|
||||
FROM servers WHERE owner_id = $1 AND deleted_at IS NULL`,
|
||||
targetUserID).Scan(&count, &cpuSum, &memSum, &storSum); err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
if !quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum,
|
||||
n, ResourceSpec{CPUMilli: int(cpu), MemoryMB: int(mem), StorageMB: int(stor)}) {
|
||||
return "", nil, ErrQuotaExceeded
|
||||
}
|
||||
}
|
||||
|
||||
// Re-point every server the source owns to the target, collecting the names for
|
||||
// the audit trail. Server ownership is the only thing that moves.
|
||||
rows, err := tx.QueryContext(ctx,
|
||||
|
||||
@@ -825,9 +825,12 @@ type Repo interface {
|
||||
// the migration 'redeemed' — all in one transaction. It returns the source user id
|
||||
// and the moved server names for the audit trail. No matching or expired code, or a
|
||||
// code whose named target is a different user → ErrLinkCodeInvalid (an intercepted
|
||||
// code is useless to anyone but the named target). Server ownership is the only thing
|
||||
// moved — the mc_uuid link and web credentials stay with their accounts. now drives
|
||||
// expiry and the terminal timestamps.
|
||||
// code is useless to anyone but the named target). When the source owns servers, the
|
||||
// target's quota must hold with all of them added in, checked under ClaimServer's
|
||||
// lock for both accounts; over any cap → ErrQuotaExceeded, with nothing moved and the
|
||||
// code left unspent. Server ownership is the only thing moved — the mc_uuid link and
|
||||
// web credentials stay with their accounts. now drives expiry and the terminal
|
||||
// timestamps.
|
||||
RedeemMigration(ctx context.Context, targetUserID, codeHash string, now time.Time) (sourceUserID string, movedServers []string, err error)
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user