mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
fix(multiplayer): bind signed workload tokens to allocation_id only
The just-landed signed workload token embedded (allocation_id, match_id, server_id) as claims. That doesn't actually work for its intended delivery channel: the token is meant to be requested as a GameServerAllocation annotation in the SAME request that asks Agones to pick a server, so at mint time the allocator knows allocation_id (it generates it) but not yet which server_id Agones will return -- server_id only exists in Agones's response, after the annotation request has already been sent. Embedding it was simply not possible for the real caller this was built for; only the (allocator -> signed_token) unit tests and hand-constructed integration tests happened to supply it directly, masking the gap. Fixes it by having the token bind only allocation_id (the one identifier actually known at mint time) plus expiry. match_id/server_id are resolved at verify time from the durable allocations table via the new store.AllocationBindingByAllocationID, keyed by allocation_id -- which the allocator already records immediately after Agones responds. This is strictly stronger, not just a workaround: a caller can no longer claim any match/server pairing at all, even one that happens to be internally consistent -- the binding returned is entirely durable-record-derived. Verified: server/workload's unit tests updated for the new two-field claim shape; server/api's Postgres integration suite gains TestWorkloadVerifierFromSignedTokenNeverTrustsCallerSuppliedBinding (two distinct real allocations each resolve to their own, and only their own, match/server pairing) replacing the now-inapplicable mismatched-triple test. Full `go build ./... && go vet ./... && gofmt -l . && go test ./... -race` and `go test -tags integration ./... -race` both clean; the api integration suite re-run 3x clean against a live postgres:17-alpine container.
This commit is contained in:
@@ -90,12 +90,16 @@ func WorkloadVerifierFromSignedToken(secret []byte, db *sql.DB) WorkloadVerifier
|
||||
return domain.WorkloadBinding{}, err
|
||||
}
|
||||
// WorkloadVerifier has no context parameter (see its type in
|
||||
// service.go) so the durable cross-check below cannot inherit the
|
||||
// service.go) so the durable lookup below cannot inherit the
|
||||
// caller's request context; bound it locally instead of running
|
||||
// unbounded against context.Background().
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
ok, err := store.AllocationBindingStillValid(ctx, db, claims.AllocationID, claims.MatchID, claims.ServerID)
|
||||
// The token only names allocation_id (see signed_token.go for why);
|
||||
// match_id/server_id come from the durable allocator record, never
|
||||
// from the caller, so a token can never claim a pairing that wasn't
|
||||
// actually, durably allocated.
|
||||
matchID, serverID, ok, err := store.AllocationBindingByAllocationID(ctx, db, claims.AllocationID)
|
||||
if err != nil {
|
||||
return domain.WorkloadBinding{}, err
|
||||
}
|
||||
@@ -104,8 +108,8 @@ func WorkloadVerifierFromSignedToken(secret []byte, db *sql.DB) WorkloadVerifier
|
||||
}
|
||||
return domain.WorkloadBinding{
|
||||
AllocationID: claims.AllocationID,
|
||||
MatchID: claims.MatchID,
|
||||
ServerID: claims.ServerID,
|
||||
MatchID: matchID,
|
||||
ServerID: serverID,
|
||||
}, nil
|
||||
}
|
||||
}
|
||||
|
||||
@@ -48,8 +48,8 @@ func openIntegrationPostgres(t *testing.T) *sql.DB {
|
||||
}
|
||||
|
||||
// seedRealAllocation claims a real ready server and allocation row, exactly
|
||||
// the durable state a signed workload token must later be cross-checked
|
||||
// against (see store.AllocationBindingStillValid).
|
||||
// the durable state a signed workload token's allocation_id must resolve
|
||||
// against (see store.AllocationBindingByAllocationID).
|
||||
func seedRealAllocation(t *testing.T, db *sql.DB, allocationID, matchID string, now time.Time) domain.Allocation {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
@@ -65,11 +65,13 @@ func seedRealAllocation(t *testing.T, db *sql.DB, allocationID, matchID string,
|
||||
}
|
||||
|
||||
// TestWorkloadVerifierFromSignedTokenAcceptsARealAllocation proves the full
|
||||
// wired path: a token issued by workload.IssueSignedWorkloadToken for a real
|
||||
// allocation row verifies successfully through
|
||||
// WorkloadVerifierFromSignedToken and returns a binding matching what
|
||||
// serverMutation actually checks (ServerID, MatchID). This is the "wired,
|
||||
// working" counterpart to cmd/control-plane's
|
||||
// wired path: a token issued by workload.IssueSignedWorkloadToken naming only
|
||||
// a real allocation_id verifies successfully through
|
||||
// WorkloadVerifierFromSignedToken and returns a binding whose match_id/
|
||||
// server_id came from the durable allocation record (the token itself never
|
||||
// carries them -- see signed_token.go), matching what serverMutation
|
||||
// actually checks (ServerID, MatchID). This is the "wired, working"
|
||||
// counterpart to cmd/control-plane's
|
||||
// TestServerRoutesRequireWorkloadVerifyToBeWired, which pins the
|
||||
// unconfigured-503 case.
|
||||
func TestWorkloadVerifierFromSignedTokenAcceptsARealAllocation(t *testing.T) {
|
||||
@@ -78,7 +80,7 @@ func TestWorkloadVerifierFromSignedTokenAcceptsARealAllocation(t *testing.T) {
|
||||
allocation := seedRealAllocation(t, db, "alloc-verify-1", "match-verify-1", now)
|
||||
|
||||
secret := []byte("integration-test-secret")
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, allocation.AllocationID, allocation.MatchID, allocation.ServerID, now, time.Minute)
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, allocation.AllocationID, now, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatalf("issue token: %v", err)
|
||||
}
|
||||
@@ -97,14 +99,14 @@ func TestWorkloadVerifierFromSignedTokenAcceptsARealAllocation(t *testing.T) {
|
||||
}
|
||||
|
||||
// TestWorkloadVerifierFromSignedTokenRejectsAnUnknownAllocation proves the
|
||||
// durable cross-check actually runs: a validly-signed, unexpired token whose
|
||||
// allocation was never recorded (e.g. superseded, or simply fabricated) must
|
||||
// still be rejected. Signature and expiry checks alone are not enough.
|
||||
// durable lookup actually runs: a validly-signed, unexpired token whose
|
||||
// allocation was never recorded (e.g. simply fabricated) must still be
|
||||
// rejected. Signature and expiry checks alone are not enough.
|
||||
func TestWorkloadVerifierFromSignedTokenRejectsAnUnknownAllocation(t *testing.T) {
|
||||
db := openIntegrationPostgres(t)
|
||||
now := time.Now().UTC()
|
||||
secret := []byte("integration-test-secret")
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, "alloc-never-recorded", "match-never-recorded", "server-never-recorded", now, time.Minute)
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, "alloc-never-recorded", now, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatalf("issue token: %v", err)
|
||||
}
|
||||
@@ -114,22 +116,44 @@ func TestWorkloadVerifierFromSignedTokenRejectsAnUnknownAllocation(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWorkloadVerifierFromSignedTokenRejectsAMismatchedTriple proves the
|
||||
// cross-check binds all three identifiers together, not each independently:
|
||||
// a real allocation's own allocation_id combined with someone else's
|
||||
// match/server must still fail.
|
||||
func TestWorkloadVerifierFromSignedTokenRejectsAMismatchedTriple(t *testing.T) {
|
||||
// TestWorkloadVerifierFromSignedTokenNeverTrustsCallerSuppliedBinding proves
|
||||
// the binding returned is entirely derived from the durable allocation row,
|
||||
// never from anything embedded in or inferable from the token: two distinct
|
||||
// allocations produce tokens that resolve to their own, and only their own,
|
||||
// match/server pairing.
|
||||
func TestWorkloadVerifierFromSignedTokenNeverTrustsCallerSuppliedBinding(t *testing.T) {
|
||||
db := openIntegrationPostgres(t)
|
||||
now := time.Now().UTC()
|
||||
allocation := seedRealAllocation(t, db, "alloc-verify-2", "match-verify-2", now)
|
||||
first := seedRealAllocation(t, db, "alloc-verify-2a", "match-verify-2a", now)
|
||||
second := seedRealAllocation(t, db, "alloc-verify-2b", "match-verify-2b", now)
|
||||
secret := []byte("integration-test-secret")
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, allocation.AllocationID, "a-different-match", allocation.ServerID, now, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatalf("issue token: %v", err)
|
||||
}
|
||||
verify := WorkloadVerifierFromSignedToken(secret, db)
|
||||
if _, err := verify(token, now.Add(time.Second)); err == nil {
|
||||
t.Fatal("expected rejection for a real allocation id paired with the wrong match id")
|
||||
|
||||
firstToken, err := workload.IssueSignedWorkloadToken(secret, first.AllocationID, now, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatalf("issue first token: %v", err)
|
||||
}
|
||||
firstBinding, err := verify(firstToken, now.Add(time.Second))
|
||||
if err != nil {
|
||||
t.Fatalf("verify first: %v", err)
|
||||
}
|
||||
if firstBinding.MatchID != first.MatchID || firstBinding.ServerID != first.ServerID {
|
||||
t.Fatalf("first binding %+v resolved to the wrong allocation", firstBinding)
|
||||
}
|
||||
|
||||
secondToken, err := workload.IssueSignedWorkloadToken(secret, second.AllocationID, now, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatalf("issue second token: %v", err)
|
||||
}
|
||||
secondBinding, err := verify(secondToken, now.Add(time.Second))
|
||||
if err != nil {
|
||||
t.Fatalf("verify second: %v", err)
|
||||
}
|
||||
if secondBinding.MatchID != second.MatchID || secondBinding.ServerID != second.ServerID {
|
||||
t.Fatalf("second binding %+v resolved to the wrong allocation", secondBinding)
|
||||
}
|
||||
if secondBinding.MatchID == firstBinding.MatchID || secondBinding.ServerID == firstBinding.ServerID {
|
||||
t.Fatalf("distinct allocations resolved to the same binding: %+v vs %+v", firstBinding, secondBinding)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -148,7 +172,7 @@ func TestWorkloadVerifierFromSignedTokenClosesTheDefaultUnwiredGap(t *testing.T)
|
||||
now := time.Now().UTC()
|
||||
allocation := seedRealAllocation(t, db, "alloc-verify-3", "match-verify-3", now)
|
||||
secret := []byte("integration-test-secret")
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, allocation.AllocationID, allocation.MatchID, allocation.ServerID, now, time.Minute)
|
||||
token, err := workload.IssueSignedWorkloadToken(secret, allocation.AllocationID, now, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatalf("issue token: %v", err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user