fix(api): in-game identity resolution and link takeover ignore dead accounts
UserByMCUUID now resolves only live accounts: claim, menu, wake authorization, op-login vouch and the QR link-status poll treat a disabled or soft-deleted link holder exactly like an unlinked UUID instead of a retired identity. VerifyLinkCode lets a soft-deleted link be taken over by a fresh in-game code (the deleted account is gone, e.g. a migrated source), while a disabled holder still 409s so the lockout is not bypassable; failed attempts still do not consume the code. Fake repo and pgint coverage pin both branches.
This commit is contained in:
5 files changed
+182
-7
No files matched your search
@@ -287,7 +287,13 @@ func (f *fakeRepo) VerifyLinkCode(_ context.Context, userID, code string, now ti
|
||||
return "", "", ErrLinkCodeInvalid
|
||||
}
|
||||
if existing, ok := f.links[rec.mcUUID]; ok && existing != userID {
|
||||
return "", "", ErrConflict // do not consume another user's pending code
|
||||
// A soft-deleted link's identity is unclaimed: the fresh in-game code lets a
|
||||
// live caller take it over (mirrors PGRepo). Disabled-but-not-deleted stays a
|
||||
// conflict — takeover there would bypass the lockout. Neither arm consumes
|
||||
// the code.
|
||||
if !f.seededDeleted(existing) {
|
||||
return "", "", ErrConflict
|
||||
}
|
||||
}
|
||||
f.links[rec.mcUUID] = userID
|
||||
f.linkAuthSource[rec.mcUUID] = rec.authSource // copy/refresh, mirrors DO UPDATE
|
||||
@@ -626,7 +632,7 @@ func (f *fakeRepo) UUIDInAllowlist(_ context.Context, n, uuid string) (bool, err
|
||||
return f.allowUUID[n][uuid], nil
|
||||
}
|
||||
func (f *fakeRepo) UserByMCUUID(_ context.Context, uuid string) (string, error) {
|
||||
if u, ok := f.links[uuid]; ok {
|
||||
if u, ok := f.links[uuid]; ok && !f.seededDead(u) {
|
||||
return u, nil
|
||||
}
|
||||
return "", ErrNotFound
|
||||
@@ -1177,6 +1183,20 @@ func (f *fakeRepo) seededDead(id string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// seededDeleted is the narrower liveness query: soft-deleted only (a disabled
|
||||
// account still holds its identity, mirroring VerifyLinkCode's takeover rule).
|
||||
func (f *fakeRepo) seededDeleted(id string) bool {
|
||||
if f.deletedIDs[id] {
|
||||
return true
|
||||
}
|
||||
for _, su := range f.seededUsers {
|
||||
if su.view.ID == id {
|
||||
return su.detail.DeletedAt != nil
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (f *fakeRepo) GetQuotas(_ context.Context, userID string) (*QuotaView, error) {
|
||||
if !f.liveUserExists(userID) {
|
||||
return nil, ErrNotFound
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
@@ -252,6 +254,58 @@ func TestLinkVerifyIdempotent(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A link whose account was SOFT-DELETED is unclaimed: a fresh in-game code lets a
|
||||
// live account take it over (the migrated-source path — retire keeps the link but
|
||||
// kills the account), while a merely disabled holder keeps its identity so the
|
||||
// lockout cannot be re-linked away, and neither dead link has in-game standing
|
||||
// (UserByMCUUID reads it exactly like an unlinked UUID). Audit #33's in-game half.
|
||||
func TestLinkVerifyTakesOverDeletedLinkOnly(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
const mcGone = "55555555-5555-5555-5555-555555555555"
|
||||
const mcLocked = "66666666-6666-6666-6666-666666666666"
|
||||
user := &Principal{UserID: "u-take", Email: "[email protected]", Role: "user"}
|
||||
repo := newFakeRepo()
|
||||
repo.seedUser(UserView{ID: "u-gone", Username: "gone", Email: "[email protected]", Role: "user"})
|
||||
repo.seedUser(UserView{ID: "u-locked", Username: "locked", Email: "[email protected]", Role: "user"})
|
||||
if err := repo.DeleteUser(ctx, "u-gone", "test"); err != nil {
|
||||
t.Fatalf("DeleteUser: %v", err)
|
||||
}
|
||||
repo.links[mcGone] = "u-gone" // a retired source's link outlives the account
|
||||
|
||||
if _, err := repo.UserByMCUUID(ctx, mcGone); !errors.Is(err, ErrNotFound) {
|
||||
t.Fatalf("UserByMCUUID(deleted link) = %v, want ErrNotFound (no in-game standing)", err)
|
||||
}
|
||||
|
||||
repo.linkCodes["TAKEOVER"] = fakeLinkCode{mcUUID: mcGone, expiresAt: time.Unix(1_700_000_600, 0)}
|
||||
api := newTestAPI(repo, newFakeCluster())
|
||||
api.External = staticExternal{p: user}
|
||||
if w := do(api.ExternalHandler(), "POST", "/api/v1/account/link/verify", `{"code":"TAKEOVER"}`, nil); w.Code != http.StatusOK {
|
||||
t.Fatalf("takeover verify: code = %d, want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if repo.links[mcGone] != "u-take" {
|
||||
t.Errorf("links[%s] = %q after takeover, want u-take", mcGone, repo.links[mcGone])
|
||||
}
|
||||
|
||||
// A disabled (not deleted) holder keeps the identity: 409, code survives, link unmoved.
|
||||
if err := repo.SetUserDisabled(ctx, "u-locked", true); err != nil {
|
||||
t.Fatalf("disable: %v", err)
|
||||
}
|
||||
repo.links[mcLocked] = "u-locked"
|
||||
repo.linkCodes["LOCKED12"] = fakeLinkCode{mcUUID: mcLocked, expiresAt: time.Unix(1_700_000_600, 0)}
|
||||
if _, err := repo.UserByMCUUID(ctx, mcLocked); !errors.Is(err, ErrNotFound) {
|
||||
t.Fatalf("UserByMCUUID(disabled link) = %v, want ErrNotFound", err)
|
||||
}
|
||||
if w := do(api.ExternalHandler(), "POST", "/api/v1/account/link/verify", `{"code":"LOCKED12"}`, nil); w.Code != http.StatusConflict {
|
||||
t.Fatalf("takeover of a disabled holder: code = %d, want 409 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if repo.links[mcLocked] != "u-locked" {
|
||||
t.Errorf("disabled holder's link moved to %q", repo.links[mcLocked])
|
||||
}
|
||||
if _, ok := repo.linkCodes["LOCKED12"]; !ok {
|
||||
t.Error("refused verify consumed the code")
|
||||
}
|
||||
}
|
||||
|
||||
// TestLinkAuthSourcePropagates proves auth_source survives the whole §10 flow: a
|
||||
// thirdparty source captured in-game at mint reaches the durable link and the
|
||||
// verify response — the value the web side can never originate itself.
|
||||
|
||||
+29
-5
@@ -94,8 +94,13 @@ func (p *PGRepo) VerifyLinkCode(ctx context.Context, userID, code string, now ti
|
||||
return "", "", err
|
||||
}
|
||||
|
||||
// If this UUID is already linked, only the same user may re-verify (idempotent);
|
||||
// a different user is a conflict and must not consume the code.
|
||||
// If this UUID is already linked, only the same user may re-verify (idempotent).
|
||||
// A different LIVE user is a conflict and must not consume the code. A link whose
|
||||
// account was soft-deleted is the exception: the identity is unclaimed (the
|
||||
// account is gone; e.g. a migrated source, whose retire keeps the link but is
|
||||
// otherwise dead), and the fresh in-game code proves the caller still holds this
|
||||
// UUID, so the live caller takes the link over. Disabled-but-not-deleted stays a
|
||||
// conflict — taking over a locked account's identity would bypass the lockout.
|
||||
var existingUser string
|
||||
switch err := tx.QueryRowContext(ctx,
|
||||
`SELECT user_id FROM account_links WHERE mc_uuid = $1`, mcUUID).Scan(&existingUser); {
|
||||
@@ -105,7 +110,21 @@ func (p *PGRepo) VerifyLinkCode(ctx context.Context, userID, code string, now ti
|
||||
return "", "", err
|
||||
default:
|
||||
if existingUser != userID {
|
||||
return "", "", ErrConflict
|
||||
var linkedDeleted bool
|
||||
if err := tx.QueryRowContext(ctx,
|
||||
`SELECT deleted_at IS NOT NULL FROM users WHERE id = $1`,
|
||||
existingUser).Scan(&linkedDeleted); err != nil {
|
||||
return "", "", err
|
||||
}
|
||||
if !linkedDeleted {
|
||||
return "", "", ErrConflict
|
||||
}
|
||||
if _, err := tx.ExecContext(ctx,
|
||||
`UPDATE account_links SET user_id = $1, auth_source = $2, verified_at = now()
|
||||
WHERE mc_uuid = $3`,
|
||||
userID, authSource, mcUUID); err != nil {
|
||||
return "", "", fmt.Errorf("take over retired link: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -513,11 +532,16 @@ func (p *PGRepo) UUIDInAllowlist(ctx context.Context, name, mcUUID string) (bool
|
||||
}
|
||||
|
||||
// UserByMCUUID resolves a verified in-game UUID to its linked user_id (spec §10
|
||||
// account_links), or ErrNotFound when the UUID is not linked to any account.
|
||||
// account_links), or ErrNotFound when the UUID is not linked to any account. A
|
||||
// link whose account is dead reads the same as no link at all (audit #33), so the
|
||||
// in-game doors never act as a retired identity.
|
||||
func (p *PGRepo) UserByMCUUID(ctx context.Context, mcUUID string) (string, error) {
|
||||
var userID string
|
||||
switch err := p.db.QueryRowContext(ctx,
|
||||
`SELECT user_id FROM account_links WHERE mc_uuid = $1`, mcUUID).Scan(&userID); {
|
||||
`SELECT al.user_id FROM account_links al
|
||||
JOIN users u ON u.id = al.user_id
|
||||
WHERE al.mc_uuid = $1 AND u.disabled = false AND u.deleted_at IS NULL`,
|
||||
mcUUID).Scan(&userID); {
|
||||
case errors.Is(err, sql.ErrNoRows):
|
||||
return "", ErrNotFound
|
||||
case err != nil:
|
||||
|
||||
@@ -251,6 +251,10 @@ type Repo interface {
|
||||
// (spec §10 account_links), or ErrNotFound when the UUID is not linked. The
|
||||
// internal-face wake uses it to apply the owner bypass for a player known only
|
||||
// by UUID; an unlinked UUID simply falls through to the autostartPolicy gate.
|
||||
// Only a LIVE account resolves (audit #33): a link whose account is disabled or
|
||||
// soft-deleted carries no standing on the in-game doors — claim, menu, wake
|
||||
// authorization, op-login vouch and the QR link-status poll all read a dead
|
||||
// account exactly like an unlinked UUID, never as a retired identity.
|
||||
UserByMCUUID(ctx context.Context, mcUUID string) (userID string, err error)
|
||||
// RecordJoin updates last_active_at, clears reaper warnings, and auto-appends
|
||||
// the UUID to the allowlist (spec §7 join-event, §9.4).
|
||||
|
||||
Reference in new issue
Block a user