diff --git a/internal/api/api.go b/internal/api/api.go index db2f6bf..3170464 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -624,6 +624,18 @@ func (a *API) requireOnboarded(h http.HandlerFunc) http.HandlerFunc { } } +// setupRequired reports whether the caller still owes the forced-onboarding step, +// for the SPA to poll. It MUST stay in lockstep with requireOnboarded's unlock +// condition above: the lockdown lifts on a verified email OR an enrolled passkey. +// Keying on email PRESENCE instead of a passkey would trap a console-tier player — +// they join through the bind-code door with no email by design (no SMTP) and can +// only ever complete setup by enrolling a passkey, so any email term loops them +// forever. Passkey alone is the durable gate; email verification is a later, +// SMTP-dependent step. +func setupRequired(emailVerified, hasPasskey bool) bool { + return !emailVerified && !hasPasskey +} + // ---- request context plumbing ---- type ctxKey int diff --git a/internal/api/handlers_setup.go b/internal/api/handlers_setup.go index 6e36484..8ce8229 100644 --- a/internal/api/handlers_setup.go +++ b/internal/api/handlers_setup.go @@ -104,11 +104,8 @@ func (a *API) handleSetupRedeem(w http.ResponseWriter, r *http.Request) { "email": u.Email, "email_verified": u.EmailVerified, "has_passkey": hasPasskey, - // Setup completes on email recorded + passkey enrolled. NOT email_verified: - // the bootstrap has no SMTP, so the Owner's address is stored unverified and a - // later Settings/SMTP flow verifies it. Passkey is the Owner's only pre-SMTP - // login credential, so it — not email verification — is the durable gate. - "setup_required": u.Email == "" || !hasPasskey, + // setup_required MUST mirror requireOnboarded's unlock (api.go:615); see setupRequired. + "setup_required": setupRequired(u.EmailVerified, hasPasskey), }) } @@ -135,10 +132,7 @@ func (a *API) handleSetupStatus(w http.ResponseWriter, r *http.Request) { "email": u.Email, "email_verified": u.EmailVerified, "has_passkey": hasPasskey, - // Setup completes on email recorded + passkey enrolled. NOT email_verified: - // the bootstrap has no SMTP, so the Owner's address is stored unverified and a - // later Settings/SMTP flow verifies it. Passkey is the Owner's only pre-SMTP - // login credential, so it — not email verification — is the durable gate. - "setup_required": u.Email == "" || !hasPasskey, + // setup_required MUST mirror requireOnboarded's unlock (api.go:615); see setupRequired. + "setup_required": setupRequired(u.EmailVerified, hasPasskey), }) } diff --git a/internal/api/handlers_setup_lockdown_test.go b/internal/api/handlers_setup_lockdown_test.go new file mode 100644 index 0000000..1338556 --- /dev/null +++ b/internal/api/handlers_setup_lockdown_test.go @@ -0,0 +1,140 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" +) + +// TestAdvClaim403MechanismIsRequireOnboarded independently verifies the claimed +// root cause of the live claim-403 report. It asserts three things the claim +// implies, each of which would falsify the claim if it failed: +// +// 1. /me/servers (SetupAllowed) returns 200 for a bind-onboarded player, which is +// why the dashboard renders demo2 with a 认领 button at all. +// 2. claim, wake AND status all return 403 with code "setup_required" — i.e. the +// 403 comes from requireOnboarded (api.go:612-625) BEFORE the handler, not +// from isOwnerOrAdmin inside it (that path returns code "forbidden"). +// 3. Flipping exactly ONE bit — enrolling a passkey — lifts the 403 on all three. +// This isolates requireOnboarded as the sole cause: nothing about the server +// record, ownership, or quota changed between the failing and passing runs. +func TestAdvClaim403MechanismIsRequireOnboarded(t *testing.T) { + api, repo := seedBindAPI(t) + mintBindCode(t, api, repo, "DEMO2345", bindTestUUID, authSourceMojang) + + w := do(api.ExternalHandler(), "POST", "/api/v1/auth/bind", `{"code":"DEMO2345"}`, jsonHeader) + if w.Code != http.StatusOK { + t.Fatalf("bind = %d, want 200 (%s)", w.Code, w.Body.String()) + } + cookie := map[string]string{"Cookie": sessionCookieName + "=" + sessionCookieValue(t, w)} + userID := repo.staff[bindTestUUID].ID + + // Sanity: the bind-created player really is the lockdown-eligible state — + // unverified email (no email at all) and no passkey. + if u := repo.staff[bindTestUUID]; u.EmailVerified { + t.Fatalf("bind-created player has EmailVerified=true; lockdown premise is wrong") + } + if creds, _ := repo.PasskeyCredentialsForUser(t.Context(), userID); len(creds) != 0 { + t.Fatalf("bind-created player already has %d passkeys; lockdown premise is wrong", len(creds)) + } + + // demo2 exists, is unclaimed, and the player is linked + within quota. Every + // precondition for a SUCCESSFUL claim is satisfied, so any 403 here is the + // middleware, not the handler's own authorization. + cl := api.Cluster.(*fakeCluster) + cl.byName["demo2"] = &ServerInfo{Name: "demo2", Subdomain: "demo2", Phase: "Stopped"} + repo.claimOK["demo2"] = true + repo.quota[userID] = true + + if got := do(api.ExternalHandler(), "GET", "/api/v1/me/servers", "", cookie); got.Code != http.StatusOK { + t.Fatalf("/me/servers = %d, want 200 (%s)", got.Code, got.Body.String()) + } + + routes := []struct{ method, path string }{ + {"POST", "/api/v1/servers/demo2/claim"}, + {"POST", "/api/v1/servers/demo2/wake"}, + {"GET", "/api/v1/servers/demo2/status"}, + } + for _, tc := range routes { + got := do(api.ExternalHandler(), tc.method, tc.path, "", cookie) + if got.Code != http.StatusForbidden { + t.Fatalf("%s %s = %d, want 403 (%s)", tc.method, tc.path, got.Code, got.Body.String()) + } + // The DISCRIMINATOR: setup_required proves requireOnboarded fired. If the + // 403 came from isOwnerOrAdmin the code would be "forbidden". + if c := errCode(got.Body.Bytes()); c != "setup_required" { + t.Fatalf("%s %s 403 code = %q, want setup_required (%s)", + tc.method, tc.path, c, got.Body.String()) + } + } + + // One bit: enroll a passkey. Nothing else about the request changes. + repo.passkeyCreds["pk1"] = PasskeyCredential{ID: "pk1", UserID: userID, CredentialID: "c1"} + + // Claim now reaches its handler and succeeds. + if got := do(api.ExternalHandler(), "POST", "/api/v1/servers/demo2/claim", "", cookie); got.Code != http.StatusOK { + t.Fatalf("post-passkey claim = %d %q, want 200 (%s)", got.Code, errCode(got.Body.Bytes()), got.Body.String()) + } + // fakeRepo.ClaimServer reports success without writing ownership (the fake models + // the UPDATE's row count only), so mirror the PG write the real claim performs: + // UPDATE servers SET owner_id=$user WHERE name=$name AND owner_id IS NULL. + repo.byName["demo2"] = &ServerRecord{Name: "demo2", Subdomain: "demo2", OwnerID: userID} + + // With ownership recorded, wake and status pass authorizeWake/isOwnerOrAdmin — + // proving the earlier 403s were the lockdown, not the ownership check. + for _, tc := range routes[1:] { + got := do(api.ExternalHandler(), tc.method, tc.path, "", cookie) + if got.Code == http.StatusForbidden { + t.Fatalf("%s %s still 403 for the owner after passkey enrollment (%s)", + tc.method, tc.path, got.Body.String()) + } + } +} + +// TestAdvRequireOnboardedPredicate characterizes the ACTUAL 403-producing predicate +// (api.go:615) directly, bypassing fakeRepo.SessionUser — which, unlike the real +// PGRepo.SessionUser (pgrepo.go:927, it selects email_verified), never populates +// EmailVerified and so cannot express a verified-email session. +// +// This is the falsification test for the claim's wording: the lockdown is NOT +// unconditional on non-SetupAllowed routes. Three of the five states below sail +// straight through, so "the route is not in the exemption list" does not by itself +// yield 403 — the conjunction ViaSession && !EmailVerified && no-passkey does. +func TestAdvRequireOnboardedPredicate(t *testing.T) { + repo := newFakeRepo() + api := newTestAPI(repo, newFakeCluster()) + repo.passkeyCreds["pk-enrolled"] = PasskeyCredential{ID: "pk-enrolled", UserID: "has-pk", CredentialID: "c1"} + + const passed = http.StatusTeapot // the wrapped handler ran + h := api.requireOnboarded(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(passed) }) + + for _, tc := range []struct { + name string + p *Principal + want int + }{ + // The live demo2 player: bind-onboarded, no email, no passkey. + {"session player, unverified, no passkey", &Principal{UserID: "u1", ViaSession: true, EmailVerified: false}, http.StatusForbidden}, + {"session player, email verified", &Principal{UserID: "u1", ViaSession: true, EmailVerified: true}, passed}, + {"session player, unverified but passkey enrolled", &Principal{UserID: "has-pk", ViaSession: true, EmailVerified: false}, passed}, + // Admin Zero-Trust path carries no session flag; internal face carries no principal. + {"non-session principal", &Principal{UserID: "a1", ViaSession: false, EmailVerified: false}, passed}, + {"nil principal (internal face)", nil, passed}, + } { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRequest("POST", "/api/v1/servers/demo2/wake", nil) + if tc.p != nil { + r = r.WithContext(context.WithValue(r.Context(), ctxKeyPrincipal, tc.p)) + } + w := httptest.NewRecorder() + h(w, r) + if w.Code != tc.want { + t.Fatalf("code = %d, want %d (%s)", w.Code, tc.want, w.Body.String()) + } + if tc.want == http.StatusForbidden && errCode(w.Body.Bytes()) != "setup_required" { + t.Fatalf("403 code = %q, want setup_required", errCode(w.Body.Bytes())) + } + }) + } +} diff --git a/internal/api/handlers_setup_test.go b/internal/api/handlers_setup_test.go index 432faf5..1bb86d4 100644 --- a/internal/api/handlers_setup_test.go +++ b/internal/api/handlers_setup_test.go @@ -95,6 +95,48 @@ func TestSetEmailClearsVerified(t *testing.T) { } } +// TestSetupCompletesForNoEmailPlayer pins the console-tier player fix: a bind-code +// player joins with NO email (by design — no SMTP) and completes forced onboarding by +// enrolling a passkey alone. setup_required MUST then report false, in lockstep with +// requireOnboarded lifting (api.go:615). The OLD predicate (email == "" || !hasPasskey) +// trapped exactly this state — email is empty forever, so it looped the player. +func TestSetupCompletesForNoEmailPlayer(t *testing.T) { + repo := newFakeRepo() + // A console-tier player: role=user, no email ever, no passkey. + repo.staff["p"] = &StaffUser{ID: "p1", Username: "player", Role: "user"} + + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: &Principal{UserID: "p1", Role: "user", ViaSession: true}} + h := api.ExternalHandler() + + status := func(t *testing.T) map[string]any { + t.Helper() + w := do(h, "GET", "/api/v1/auth/setup/status", "", nil) + if w.Code != http.StatusOK { + t.Fatalf("status code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + var got map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("status body not JSON: %v", err) + } + return got + } + + // No email, no passkey → forced onboarding still owed. + if s := status(t); s["setup_required"] != true || s["email"] != "" { + t.Fatalf("fresh player status = %v, want setup_required=true email=\"\"", s) + } + + // Enroll a passkey — the ONLY step a no-email player can complete. + repo.passkeyCreds["pk1"] = PasskeyCredential{ID: "pk1", UserID: "p1", CredentialID: "cred1"} + + // Setup is now COMPLETE even though email stays empty. The OLD predicate returned + // true here (email == "") and looped the player forever. + if s := status(t); s["setup_required"] != false || s["has_passkey"] != true || s["email"] != "" { + t.Fatalf("post-passkey player status = %v, want setup_required=false has_passkey=true email=\"\"", s) + } +} + // errCode returns the error.code of a JSON error body, or "" if body is not one (a // non-failing decodeErr for cases where the response may be a success). func errCode(body []byte) string {