diff --git a/cmd/felis/breakglass.go b/cmd/felis/breakglass.go index 410a124..ce7c869 100644 --- a/cmd/felis/breakglass.go +++ b/cmd/felis/breakglass.go @@ -83,14 +83,11 @@ type ownerStore interface { // Operator. The row is role=admin, identical in shape to the Owner — Felis has no // separate operator DB role (migration 0003: staff = role=admin). InsertOperator(ctx context.Context, id, username, email string) error - // RedeemLinkCodeForOwner consumes an in-game link code and creates-or-promotes - // the bound user to role='admin' (Owner). It is the `felis setup` MC-bind path: - // the operator enters limbo, runs /link, types the code here, and the bound - // account becomes the passwordless Owner. Unlike RedeemPlayerBindCode it does NOT - // refuse staff — setup deliberately elevates the bound account. - RedeemLinkCodeForOwner(ctx context.Context, newUserID, code string, now time.Time) (userID, mcUUID, authSource string, err error) - // CreateSetupToken mints a one-time setup token for first-web-login bootstrap. - CreateSetupToken(ctx context.Context, tokenHash, userID string, expiresAt time.Time) error + // CompleteOwnerSetup atomically consumes the in-game link code, creates or + // promotes the bound Owner, enables local auth, and stores the one-time setup + // token. A failure rolls all four writes back so setup is always retryable. + CompleteOwnerSetup(ctx context.Context, newUserID, code string, now time.Time, + tokenHash string, tokenExpiresAt time.Time) (userID, mcUUID, authSource string, err error) SetSetting(ctx context.Context, key string, value []byte) error // Audit records the break-glass accountability row. Audit(ctx context.Context, e api.AuditEntry) error @@ -347,6 +344,7 @@ type breakGlassOp struct { // breakGlassOutcome is what performBreakGlass reports back to the TUI. type breakGlassOutcome struct { setupTokenURL string // non-empty when setup minted a one-time first-login URL + ownerIdentity string // verified Minecraft UUID for the setup Owner-bind path auditErr error // non-nil if the accountability row could not be written } @@ -387,11 +385,17 @@ func newSetupToken() (raw, hash string, err error) { } // performSetupMCBind is the `felis setup` Owner-establishment path: the operator -// binds their Minecraft account via an in-game /link code, the bound user is -// promoted to role='admin' (passwordless Owner), and a one-time setup URL is -// minted for the first web login where the Owner verifies email / enrolls a -// passkey. adminHostname is the op.console host the URL points at. -func performSetupMCBind(ctx context.Context, s ownerStore, code, adminHostname string) (breakGlassOutcome, error) { +// binds their Minecraft account via a one-time link code the login gate handed +// them in-game, the bound user is promoted to role='admin' (passwordless Owner), +// local auth is enabled, and a one-time setup URL is minted for the first web +// login where the Owner verifies email / enrolls a passkey. adminHostname is the +// op.console host the URL points at; osUser is recorded as the accountable actor. +// +// Local auth is as load-bearing here as it is in break-glass, and for a sharper +// reason: an MC-bound Owner has no password AND no email, so the setup token is +// their ONLY door. CompleteOwnerSetup therefore commits the identity bind, auth +// toggle, and token together; any failed write leaves the link code retryable. +func performSetupMCBind(ctx context.Context, s ownerStore, code, adminHostname, osUser string) (breakGlassOutcome, error) { code = strings.TrimSpace(strings.ToUpper(code)) if code == "" { return breakGlassOutcome{}, errors.New("link code is required") @@ -400,23 +404,51 @@ func performSetupMCBind(ctx context.Context, s ownerStore, code, adminHostname s if newID == "" { return breakGlassOutcome{}, errors.New("generate owner id: entropy source failed") } - userID, _, _, err := s.RedeemLinkCodeForOwner(ctx, newID, code, time.Now()) - if err != nil { - return breakGlassOutcome{}, fmt.Errorf("bind minecraft account: %w", err) - } raw, hash, err := newSetupToken() if err != nil { return breakGlassOutcome{}, err } - if err := s.CreateSetupToken(ctx, hash, userID, time.Now().Add(setupTokenTTL)); err != nil { - return breakGlassOutcome{}, fmt.Errorf("mint setup token: %w", err) + now := time.Now() + _, mcUUID, authSource, err := s.CompleteOwnerSetup( + ctx, newID, code, now, hash, now.Add(setupTokenTTL)) + if err != nil { + return breakGlassOutcome{}, fmt.Errorf("complete owner setup: %w", err) + } + // The load-bearing writes committed together above. Accountability remains + // best-effort: an unhappy audit sink never costs the operator their install. + out := breakGlassOutcome{ + ownerIdentity: mcUUID, + auditErr: auditSetupMCBind(ctx, s, osUser, mcUUID, authSource), } host := strings.TrimSpace(adminHostname) if host == "" { host = "op.console.localhost" } - url := "https://" + host + "/setup?token=" + raw - return breakGlassOutcome{setupTokenURL: url}, nil + out.setupTokenURL = "https://" + host + "/setup?token=" + raw + return out, nil +} + +// auditSetupMCBind records who claimed the Owner seat at setup. It carries the +// Minecraft identity rather than a username because that IS the evidence: the +// login gate only issues a link code to a player it authenticated, so mc_uuid + +// auth_source say which account was verified and by whom. Actor is the OS user who +// ran `felis setup` — honest attribution, not proof (root can edit the row). +func auditSetupMCBind(ctx context.Context, s ownerStore, osUser, mcUUID, authSource string) error { + blob, err := json.Marshal(map[string]any{ + "mode": "setup", + "os_user": osUser, + "mc_uuid": mcUUID, + "auth_source": authSource, + }) + if err != nil { + return err + } + return s.Audit(ctx, api.AuditEntry{ + Actor: osUser, + Source: "setup", + Action: "setup.owner_bind", + Payload: blob, + }) } // auditBreakGlass writes the break-glass accountability row. The actor is the @@ -494,7 +526,11 @@ func auditAddOperator(ctx context.Context, s ownerStore, op breakGlassOp) error // breakGlassResult is what the TUI hands back to cmdBreakGlass for the durable // post-exit summary. provisioned is false on cancel. type breakGlassResult struct { - provisioned bool + provisioned bool + // alreadySetUp marks the re-run landing (the status screen): setup ran, found an + // Owner, and deliberately changed nothing. Without it a re-run is indistinguishable + // from a cancel and reports itself as one. + alreadySetUp bool isOperator bool // an Operator was added rather than the Owner provisioned mode string accountable string diff --git a/cmd/felis/breakglass_test.go b/cmd/felis/breakglass_test.go index c2940cc..ddb81f8 100644 --- a/cmd/felis/breakglass_test.go +++ b/cmd/felis/breakglass_test.go @@ -28,7 +28,7 @@ type fakeOwnerStore struct { users map[string]*api.StaffUser // keyed by username admins bool // AdminExists answer - // RedeemLinkCodeForOwner's success result. redeemUserID defaults to the fresh id + // CompleteOwnerSetup's success result. redeemUserID defaults to the fresh id // the caller passes (the unlinked-UUID case) when left empty. redeemUserID string redeemMCUUID string @@ -50,14 +50,14 @@ type upsertCall struct { id, username, email string } -// setupTokenCall is a recorded CreateSetupToken write. Only the hash is persisted. +// setupTokenCall is a recorded setup-token write. Only the hash is persisted. type setupTokenCall struct { tokenHash string userID string expiresAt time.Time } -// redeemCall records the inputs RedeemLinkCodeForOwner was called with. +// redeemCall records the inputs CompleteOwnerSetup was called with. type redeemCall struct { newUserID string code string @@ -107,29 +107,30 @@ func (f *fakeOwnerStore) InsertOperator(_ context.Context, id, username, email s return nil } -// RedeemLinkCodeForOwner records the call and returns the configured Owner identity -// (or the injected error). The real method consumes a link code and promotes the -// bound account; the fake models only its inputs and outputs. -func (f *fakeOwnerStore) RedeemLinkCodeForOwner(_ context.Context, newUserID, code string, _ time.Time) (string, string, string, error) { +// CompleteOwnerSetup models the real all-or-nothing transaction: injected failures +// record none of the redeem, auth-toggle, or setup-token writes. +func (f *fakeOwnerStore) CompleteOwnerSetup(_ context.Context, newUserID, code string, _ time.Time, + tokenHash string, expiresAt time.Time) (string, string, string, error) { if f.redeemErr != nil { return "", "", "", f.redeemErr } + if f.setErr != nil { + return "", "", "", f.setErr + } + if f.createTokenErr != nil { + return "", "", "", f.createTokenErr + } f.redeems = append(f.redeems, redeemCall{newUserID, code}) userID := f.redeemUserID if userID == "" { userID = newUserID // unlinked UUID → the fresh id becomes the Owner } - return userID, f.redeemMCUUID, f.redeemAuthSource, nil -} - -// CreateSetupToken records a minted setup token (hash only), or fails with the -// injected error without recording it. -func (f *fakeOwnerStore) CreateSetupToken(_ context.Context, tokenHash, userID string, expiresAt time.Time) error { - if f.createTokenErr != nil { - return f.createTokenErr + if f.settings == nil { + f.settings = map[string][]byte{} } + f.settings[api.LocalAuthEnabledKey] = []byte("true") f.tokens = append(f.tokens, setupTokenCall{tokenHash, userID, expiresAt}) - return nil + return userID, f.redeemMCUUID, f.redeemAuthSource, nil } func (f *fakeOwnerStore) SetSetting(_ context.Context, key string, value []byte) error { @@ -648,11 +649,29 @@ func TestPerformSetupMCBind(t *testing.T) { ctx := context.Background() t.Run("binds the owner and mints a setup URL whose token hash is what is stored", func(t *testing.T) { - f := &fakeOwnerStore{redeemUserID: "usr-owner-1"} - out, err := performSetupMCBind(ctx, f, " abc-123 ", "op.console.example.com") + f := &fakeOwnerStore{redeemUserID: "usr-owner-1", redeemMCUUID: "mc-uuid-1", redeemAuthSource: "mojang"} + out, err := performSetupMCBind(ctx, f, " abc-123 ", "op.console.example.com", "deploybot") if err != nil { t.Fatalf("performSetupMCBind: %v", err) } + // The Owner this mints has no password and no email, so the setup token is the + // only door — and handleSetupRedeem is gated on local_auth_enabled. A bind that + // leaves the toggle off hands back a URL that answers 403. + if _, ok := f.settings[api.LocalAuthEnabledKey]; !ok { + t.Error("local auth was not enabled — the setup URL would 403 local_auth_disabled") + } + // The bind is attributed by Minecraft identity, because that is what the login + // gate verified; a username would be the one thing nobody checked. + e, payload := auditOf(t, f) + if e.Actor != "deploybot" || e.Source != "setup" || e.Action != "setup.owner_bind" { + t.Errorf("audit = %+v, want actor=deploybot source=setup action=setup.owner_bind", e) + } + if payload["mc_uuid"] != "mc-uuid-1" || payload["auth_source"] != "mojang" { + t.Errorf("audit payload = %v, want the redeemed mc_uuid + auth_source", payload) + } + if out.ownerIdentity != "mc-uuid-1" { + t.Errorf("owner identity = %q, want the verified Minecraft UUID", out.ownerIdentity) + } const prefix = "https://op.console.example.com/setup?token=" if !strings.HasPrefix(out.setupTokenURL, prefix) { t.Fatalf("setup URL = %q, want prefix %q", out.setupTokenURL, prefix) @@ -692,40 +711,76 @@ func TestPerformSetupMCBind(t *testing.T) { t.Run("an empty link code mints nothing", func(t *testing.T) { f := &fakeOwnerStore{} - if _, err := performSetupMCBind(ctx, f, " ", "op.console.example.com"); err == nil { + if _, err := performSetupMCBind(ctx, f, " ", "op.console.example.com", "root"); err == nil { t.Fatal("want error for an empty link code") } if len(f.redeems) != 0 || len(f.tokens) != 0 { t.Errorf("want no redeem/token on an empty code, got redeems=%d tokens=%d", len(f.redeems), len(f.tokens)) } + if _, ok := f.settings[api.LocalAuthEnabledKey]; ok { + t.Error("local auth was enabled without an owner — the gate must not open on a failed bind") + } }) t.Run("a link-code redemption failure mints no token", func(t *testing.T) { f := &fakeOwnerStore{redeemErr: errors.New("code expired")} - if _, err := performSetupMCBind(ctx, f, "abc-123", "op.console.example.com"); err == nil { + if _, err := performSetupMCBind(ctx, f, "abc-123", "op.console.example.com", "root"); err == nil { t.Fatal("want error when the link code cannot be redeemed") } if len(f.tokens) != 0 { t.Errorf("want no token minted on a redeem failure, got %d", len(f.tokens)) } + if _, ok := f.settings[api.LocalAuthEnabledKey]; ok { + t.Error("local auth was enabled without an owner — the gate must not open on a failed redeem") + } }) - t.Run("a token-store failure surfaces after the bind", func(t *testing.T) { + t.Run("a local-auth failure fails the bind rather than minting an unredeemable URL", func(t *testing.T) { + f := &fakeOwnerStore{redeemUserID: "usr-owner-1", setErr: errors.New("db down")} + if _, err := performSetupMCBind(ctx, f, "abc-123", "op.console.example.com", "root"); err == nil { + t.Fatal("want error when local auth cannot be enabled") + } + if len(f.redeems) != 0 || len(f.tokens) != 0 { + t.Errorf("atomic setup was partially recorded: redeems=%d tokens=%d", len(f.redeems), len(f.tokens)) + } + }) + + t.Run("a token-store failure rolls the bind back", func(t *testing.T) { f := &fakeOwnerStore{redeemUserID: "usr-owner-1", createTokenErr: errors.New("db down")} - if _, err := performSetupMCBind(ctx, f, "abc-123", "op.console.example.com"); err == nil { + if _, err := performSetupMCBind(ctx, f, "abc-123", "op.console.example.com", "root"); err == nil { t.Fatal("want error when the setup token cannot be stored") } - if len(f.redeems) != 1 { - t.Errorf("want the redeem to have happened before the token write, got %d", len(f.redeems)) + if len(f.redeems) != 0 { + t.Errorf("link code was consumed despite token failure, got %d redeems", len(f.redeems)) } if len(f.tokens) != 0 { t.Errorf("want no recorded token when the store fails, got %d", len(f.tokens)) } + if _, ok := f.settings[api.LocalAuthEnabledKey]; ok { + t.Error("local auth stayed enabled despite transaction rollback") + } + }) + + t.Run("an audit failure does not cost the operator their install", func(t *testing.T) { + f := &fakeOwnerStore{redeemUserID: "usr-owner-1", auditErr: errors.New("audit sink down")} + out, err := performSetupMCBind(ctx, f, "abc-123", "op.console.example.com", "root") + if err != nil { + t.Fatalf("an audit failure must not fail the bind: %v", err) + } + if out.auditErr == nil { + t.Error("the audit failure was swallowed instead of surfaced on the outcome") + } + if out.setupTokenURL == "" { + t.Error("no setup URL minted despite a recoverable audit failure") + } + if _, ok := f.settings[api.LocalAuthEnabledKey]; !ok { + t.Error("local auth was not enabled despite a recoverable audit failure") + } }) t.Run("defaults the op.console host when adminHostname is empty", func(t *testing.T) { f := &fakeOwnerStore{redeemUserID: "usr-owner-1"} - out, err := performSetupMCBind(ctx, f, "abc-123", " ") + out, err := performSetupMCBind(ctx, f, "abc-123", " ", "root") if err != nil { t.Fatalf("performSetupMCBind: %v", err) } diff --git a/cmd/felis/setup.go b/cmd/felis/setup.go index ac30edc..4a31d71 100644 --- a/cmd/felis/setup.go +++ b/cmd/felis/setup.go @@ -11,7 +11,9 @@ import ( "time" "felis.lolicon.best/internal/api" + "felis.lolicon.best/internal/apis/felis/v1alpha1" "felis.lolicon.best/internal/config" + "felis.lolicon.best/internal/naming" "felis.lolicon.best/internal/platform" "felis.lolicon.best/internal/store" ) @@ -102,6 +104,18 @@ func cmdSetup(args []string, stdout, stderr io.Writer) int { } defer setup.drv.Close() + // The wizard's first screen asks the operator to join the server and run /link: + // the Owner IS the Minecraft account, so the login gate must be UP before we ask + // for a link code. This used to run after the wizard, which is why setup asked + // for a code from a server that had never been started. On a re-run the Owner + // already exists, so provisioning stays best-effort and never blocks the + // operator from reaching the status screen. + if err := provisionSystemServers(ctx, setup.cfg, stdout, !setup.adminExists); err != nil { + fmt.Fprintf(stderr, "felis setup: %v\n", err) + fmt.Fprintln(stderr, "The Owner is bound by joining the login gate in-game, so setup cannot continue without it.") + return 1 + } + res, err := runSetupTUI(ctx, setup.repo, setup.cfg.Database.URL, setup.cfg.Server.RootDomain, setup.cfg.Auth.AdminHostname, setup.cfg.Auth.PanelHostname, setup.cfg.Auth.AccessJWTAud, setup.cfg.K8s.Namespace, accountableOSUser(), setup.adminExists) if err != nil { fmt.Fprintf(stderr, "felis setup: %v\n", err) @@ -121,7 +135,13 @@ func cmdSetup(args []string, stdout, stderr io.Writer) int { } return 0 } - fmt.Fprintln(stdout, "felis setup: cancelled — no changes made.") + // A re-run lands on the status screen, which changes nothing by design — + // reporting that as "cancelled" reads as a failure the operator did not cause. + msg := "felis setup: cancelled — no changes made." + if res.alreadySetUp { + msg = "felis setup: already set up — nothing to change." + } + fmt.Fprintln(stdout, msg) if panelURL != "" { fmt.Fprintf(stdout, "Panel: %s\n", panelURL) } @@ -161,31 +181,42 @@ func cmdSetup(args []string, stdout, stderr io.Writer) int { } } - // After a real setup pass (Owner provisioned and/or edge configured), make - // sure the always-on login/lobby system services exist. This is idempotent - // and best-effort — it never fails the setup that got this far. - if res.provisioned || res.connectConfigured { - provisionSystemServers(ctx, setup.cfg, stdout) - } return 0 } -// provisionSystemServers ensures the login limbo and lobby system services exist -// after setup, then prints the off-cluster Velocity wiring the operator must -// apply by hand (Felis never writes the off-cluster proxy config). It is -// best-effort: unconfigured images or an unreachable cluster degrade to guidance -// rather than failing setup. -func provisionSystemServers(ctx context.Context, cfg *config.Config, out io.Writer) { +// provisionSystemServers ensures the login limbo and lobby system services exist, +// then prints the login-first Velocity wiring. deploy/bootstrap.sh writes this +// configuration for its host proxy; operators only need to mirror it when they +// deliberately run Velocity elsewhere. +// +// required is set on a first run, where the next screen asks the operator to join +// the server and run /link. There a gate that never comes up is not a degraded +// install, it is an impossible one — so every soft landing below becomes a hard +// error and we block until the gate reports Ready. On a re-run the Owner already +// exists and nothing downstream needs the gate, so unconfigured images or an +// unreachable cluster degrade to printed guidance exactly as before. +func provisionSystemServers(ctx context.Context, cfg *config.Config, out io.Writer, required bool) error { + // fail is the one place the two modes diverge: fatal on a first run, guidance + // on a re-run. + fail := func(format string, args ...any) error { + if required { + return fmt.Errorf(format, args...) + } + fmt.Fprintf(out, "\nfelis setup: "+format+"\n", args...) + return nil + } if cfg.Velocity.LoginImage == "" && cfg.Velocity.LobbyImage == "" { - fmt.Fprintln(out, "\nfelis setup: login/lobby system servers NOT provisioned — set [velocity] login_image "+ - "and lobby_image in felis.toml (build them from deploy/limbo and deploy/lobby), then re-run `sudo felis setup`.") - return + return fail("login/lobby system servers NOT provisioned — set [velocity] login_image " + + "and lobby_image in felis.toml (build them from deploy/limbo and deploy/lobby), then re-run `sudo felis setup`") + } + if required && cfg.Velocity.LoginImage == "" { + return errors.New("the Owner binds by joining the login gate, but [velocity] login_image is not set in felis.toml " + + "(build it from deploy/limbo), then re-run `sudo felis setup`") } cl, err := buildSystemServerClient() if err != nil { - fmt.Fprintf(out, "\nfelis setup: could not reach the cluster to provision the login/lobby system servers: %v\n"+ - "Re-run `sudo felis setup` on the control-plane host once the cluster is reachable.\n", err) - return + return fail("could not reach the cluster to provision the login/lobby system servers: %v\n"+ + "Re-run `sudo felis setup` on the control-plane host once the cluster is reachable", err) } // The login limbo authenticates to the felis-api INTERNAL face, so it needs the // internal base URL, the root domain (to link players at the console), and the @@ -197,9 +228,19 @@ func provisionSystemServers(ctx context.Context, cfg *config.Config, out io.Writ // renamed it must replicate the Secret by hand. controlNS := platform.DefaultControlNamespace apiBaseURL := platform.InternalAPIBaseURL(controlNS) - tokenOutcome := ensureServiceTokenReplica(ctx, cl, controlNS, cfg.K8s.Namespace) + // Both Secrets must land in the minecraft namespace before the pods that mount + // them are created: the service token (login authenticates to felis-api with it) + // and the Velocity forwarding secret (every backend verifies the proxy's signed + // handshake with it — without it the login gate would derive an OFFLINE UUID and + // the Owner would bind the wrong Minecraft identity). + secretOutcomes := []systemServerOutcome{ + ensureSecretReplica(ctx, cl, controlNS, cfg.K8s.Namespace, + naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token"), + ensureSecretReplica(ctx, cl, controlNS, cfg.K8s.Namespace, + naming.ForwardingSecretName, naming.ForwardingSecretKey, "forwarding-secret"), + } outcomes := ensureSystemServers(ctx, cl, cfg.K8s.Namespace, cfg.Velocity.LoginImage, cfg.Velocity.LobbyImage, apiBaseURL, cfg.Server.RootDomain) - outcomes = append([]systemServerOutcome{tokenOutcome}, outcomes...) + outcomes = append(secretOutcomes, outcomes...) fmt.Fprintln(out, "\nfelis setup: login/lobby system servers (always-on, reaper-exempt):") for _, o := range outcomes { switch { @@ -211,30 +252,44 @@ func provisionSystemServers(ctx context.Context, cfg *config.Config, out io.Writ fmt.Fprintf(out, " - %s: skipped (%s)\n", o.name, o.skipped) } } + if required { + if err := requiredProvisioningError(outcomes); err != nil { + return fmt.Errorf("required Minecraft provisioning failed: %w", err) + } + fmt.Fprintln(out, "\nfelis setup: waiting for the login gate to accept players…") + err := awaitLoginGateReady(ctx, cl, cfg.K8s.Namespace, loginGateReadyTimeout, loginGatePollInterval, func(p v1alpha1.Phase) { + fmt.Fprintf(out, " login: %s\n", phaseOrPending(p)) + }) + if err != nil { + return err + } + fmt.Fprintln(out, " login: Ready") + } printVelocityWiringGuidance(out, cfg.Server.RootDomain) + return nil } -// printVelocityWiringGuidance emits the manual off-cluster Velocity config that -// enforces the login-first topology. Felis auto-registers login/lobby as dynamic -// backends via /api/v1/servers, but the proxy's DEFAULT landing and waiting-park -// target live in velocity.toml on the off-cluster Java host, which Felis never -// writes. The one invariant: the default landing and the initial wait-park are -// BOTH the login gate — never the lobby — so no connection reaches the lobby (or -// any backend) without passing authentication first. The Paper lobby is reached -// only when the login gate transfers an authenticated player onward. +// printVelocityWiringGuidance records the login-first topology bootstrap applies to +// its host proxy and an external proxy must mirror. Felis auto-registers login/lobby +// as dynamic backends via /api/v1/servers, while velocity.toml owns the static +// login-only fallback. The invariant is stateful: every fresh connection lands on +// login; only login may release a linked player to the lobby; and the proxy may then +// redirect that release to the originally requested backend or park it in the lobby +// while the backend wakes. func printVelocityWiringGuidance(out io.Writer, rootDomain string) { - fmt.Fprintln(out, "\nfelis setup: finish the login topology on the off-cluster Velocity host (velocity.toml):") + fmt.Fprintln(out, "\nfelis setup: Velocity login topology (bootstrap configured the host proxy automatically):") + fmt.Fprintln(out, " If Velocity runs on another host, mirror these settings there:") fmt.Fprintln(out, " 1. Set the DEFAULT landing server to \"login\" so every fresh connection hits the") fmt.Fprintln(out, " auth gate first (try = [\"login\"] under [servers], and the default forced-host).") - fmt.Fprintln(out, " 2. Point the waiting-park target at the gate, NOT the lobby:") - fmt.Fprintln(out, " set FELIS_LOBBY_SERVER=login (or lobby-server=login). The limbo holds waiters") - fmt.Fprintln(out, " while their backend wakes, and a player is never parked past authentication.") - fmt.Fprintln(out, " 3. Leave the Paper \"lobby\" OUT of the default/fallback paths — it is reached only") - fmt.Fprintln(out, " when the login gate transfers an authenticated player onward.") + fmt.Fprintln(out, " 2. Keep the gate and post-auth lobby distinct:") + fmt.Fprintln(out, " set FELIS_LOGIN_SERVER=login and FELIS_LOBBY_SERVER=lobby") + fmt.Fprintln(out, " (or login-server=login / lobby-server=lobby).") + fmt.Fprintln(out, " 3. Leave the Paper \"lobby\" OUT of every default/fallback path. The proxy accepts") + fmt.Fprintln(out, " it only as login's authenticated release target, then restores the requested route.") fmt.Fprintln(out, " Rationale: rather refuse a connection when login is down than route a player past") fmt.Fprintln(out, " the gate. Felis already refuses to give any server a fallback of \"lobby\".") if rootDomain != "" { - fmt.Fprintf(out, " (login is the front door for %s; per-server subdomains fall back to login while waking.)\n", rootDomain) + fmt.Fprintf(out, " (login is the front door for %s; linked players wait in lobby while a target wakes.)\n", rootDomain) } } diff --git a/cmd/felis/setup_test.go b/cmd/felis/setup_test.go index 4a6922c..04f8b48 100644 --- a/cmd/felis/setup_test.go +++ b/cmd/felis/setup_test.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -27,6 +28,9 @@ func TestSetupConfigPathPrefersGeneratedHostConfig(t *testing.T) { } func TestEnsureDefaultConfigLinkBacksUpStaleDefault(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("creating symbolic links requires an optional Windows privilege") + } dir := t.TempDir() target := filepath.Join(dir, "felis.toml") host := filepath.Join(dir, "felis.host.toml") @@ -61,6 +65,9 @@ func TestEnsureDefaultConfigLinkBacksUpStaleDefault(t *testing.T) { } func TestHostBootstrapReadyRequiresMarkerAndArtifacts(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows files do not expose Unix executable mode bits") + } dir := t.TempDir() marker := filepath.Join(dir, "bootstrap.done") hostConfig := filepath.Join(dir, "felis.host.toml") diff --git a/cmd/felis/systemservers.go b/cmd/felis/systemservers.go index 42233ab..f39d13f 100644 --- a/cmd/felis/systemservers.go +++ b/cmd/felis/systemservers.go @@ -3,11 +3,13 @@ package main import ( "context" "fmt" + "time" "felis.lolicon.best/internal/apis/felis/v1alpha1" "felis.lolicon.best/internal/naming" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -121,6 +123,9 @@ func buildSystemServer(in systemServerSpec, namespace string) (*v1alpha1.Minecra ObjectMeta: metav1.ObjectMeta{ Name: in.name, Namespace: namespace, + Labels: map[string]string{ + v1alpha1.LabelSystemRole: in.name, + }, }, Spec: v1alpha1.MinecraftServerSpec{ Subdomain: in.subdomain, @@ -214,10 +219,11 @@ func buildSystemServerClient() (client.Client, error) { // systemServerOutcome records what ensureSystemServers did with one service so // setup can report it without the provisioner deciding on the output format. type systemServerOutcome struct { - name string - created bool // true = we created it this run - skipped string // non-empty = why it was skipped (image unset / already exists) - err error // non-nil = create failed + name string + created bool // true = we created it this run + available bool // true = the required object now exists + skipped string // non-empty = why it was skipped (image unset / already exists) + err error // non-nil = create failed } // ensureSystemServers idempotently creates the login and lobby system services. @@ -252,11 +258,22 @@ func ensureSystemServers(ctx context.Context, cl client.Client, namespace, login continue } // Create-if-absent: check first so an existing service is reported as a - // deliberate skip rather than an AlreadyExists error. + // deliberate skip rather than an AlreadyExists error. Never adopt a + // legacy user server that happens to occupy a reserved system name. var existing v1alpha1.MinecraftServer getErr := cl.Get(ctx, client.ObjectKeyFromObject(ms), &existing) if getErr == nil { - outcomes = append(outcomes, systemServerOutcome{name: p.name, skipped: "already exists"}) + if existing.Labels[v1alpha1.LabelSystemRole] != p.name { + outcomes = append(outcomes, systemServerOutcome{ + name: p.name, + err: fmt.Errorf( + "existing MinecraftServer %s/%s is not marked as the Felis %q system role; remove or rename it, then rerun setup", + namespace, p.name, p.name, + ), + }) + continue + } + outcomes = append(outcomes, systemServerOutcome{name: p.name, available: true, skipped: "already exists"}) continue } if !apierrors.IsNotFound(getErr) { @@ -265,64 +282,199 @@ func ensureSystemServers(ctx context.Context, cl client.Client, namespace, login } if err := cl.Create(ctx, ms); err != nil { if apierrors.IsAlreadyExists(err) { - outcomes = append(outcomes, systemServerOutcome{name: p.name, skipped: "already exists"}) + // Close the Get/Create race without trusting the object that won it. + var raced v1alpha1.MinecraftServer + if getErr := cl.Get(ctx, client.ObjectKeyFromObject(ms), &raced); getErr != nil { + outcomes = append(outcomes, systemServerOutcome{name: p.name, err: getErr}) + continue + } + if raced.Labels[v1alpha1.LabelSystemRole] != p.name { + outcomes = append(outcomes, systemServerOutcome{ + name: p.name, + err: fmt.Errorf( + "concurrent MinecraftServer %s/%s is not marked as the Felis %q system role; refusing to adopt it", + namespace, p.name, p.name, + ), + }) + continue + } + outcomes = append(outcomes, systemServerOutcome{name: p.name, available: true, skipped: "already exists"}) continue } outcomes = append(outcomes, systemServerOutcome{name: p.name, err: err}) continue } - outcomes = append(outcomes, systemServerOutcome{name: p.name, created: true}) + outcomes = append(outcomes, systemServerOutcome{name: p.name, created: true, available: true}) } return outcomes } -// ensureServiceTokenReplica copies the internal-API service-token Secret from the -// control namespace into the minecraft namespace so the login system server's pod -// can mount it via secretKeyRef. A secretKeyRef is namespace-local, but the login -// pod runs in the minecraft namespace while the source Secret lives beside the -// control plane — so without this replica the operator's injected secretKeyRef -// would dangle and wedge the login pod in CreateContainerConfigError. It is -// create-if-absent: an existing replica is left untouched so a hand-rotated token -// in the minecraft namespace is never clobbered (to rotate, delete the replica and -// re-run setup). Best-effort like the rest of the provisioner: a missing source or +// The login gate is a hard prerequisite of the Owner bind, so setup waits for it +// rather than racing it. The ceiling covers a cold image pull on a fresh node; +// the poll is fast enough that a warm start feels immediate. +const ( + loginGateReadyTimeout = 5 * time.Minute + loginGatePollInterval = 3 * time.Second +) + +// awaitLoginGateReady blocks until the login system server reports status.ready. +// +// The Owner claims their seat by JOINING the game and running /link, so the gate +// being up is not a nicety — it is the precondition for the very next thing setup +// asks of the operator. progress is called on each phase change so the caller can +// show movement during a cold image pull; it may be nil. +func awaitLoginGateReady(ctx context.Context, cl client.Client, namespace string, timeout, poll time.Duration, progress func(v1alpha1.Phase)) error { + key := client.ObjectKey{Namespace: namespace, Name: naming.SystemLoginServer} + deadline := time.Now().Add(timeout) + last := v1alpha1.Phase("") + for { + var ms v1alpha1.MinecraftServer + switch err := cl.Get(ctx, key, &ms); { + case err == nil: + if ms.Status.Ready { + return nil + } + if ms.Status.Phase != last { + last = ms.Status.Phase + if progress != nil { + progress(last) + } + } + // The operator only marks Failed once its OWN startup deadline has already + // elapsed, so Failed is a settled verdict rather than a transient — sitting + // out the rest of our timeout on top of it would only hide the reason. + if ms.Status.Phase == v1alpha1.PhaseFailed { + return fmt.Errorf("the login gate failed to start: %s", readyConditionMessage(&ms)) + } + case !apierrors.IsNotFound(err): + return err + } + if !time.Now().Before(deadline) { + return fmt.Errorf("timed out after %s waiting for the login gate to become ready (last phase: %s)", timeout, phaseOrPending(last)) + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(poll): + } + } +} + +// readyConditionMessage is the operator's own account of why the gate is not +// ready — far more useful to an operator than "phase: Failed". +func readyConditionMessage(ms *v1alpha1.MinecraftServer) string { + if c := meta.FindStatusCondition(ms.Status.Conditions, v1alpha1.ConditionReady); c != nil && c.Message != "" { + return c.Message + } + return "no Ready condition was reported" +} + +// phaseOrPending names the empty phase, which means the operator has not +// reconciled the server yet (commonly: the operator itself is not running). +func phaseOrPending(p v1alpha1.Phase) string { + if p == "" { + return "not yet reconciled — is the felis operator running?" + } + return string(p) +} + +// ensureSecretReplica copies one Secret from the control namespace into the minecraft +// namespace so a backend pod can mount it via secretKeyRef. A secretKeyRef is +// namespace-local, but the backends run in the minecraft namespace while the sources +// of truth live beside the control plane — so without this replica the operator's +// injected secretKeyRef would dangle and wedge the pod in CreateContainerConfigError. +// +// Two Secrets need it, for different reasons: the service token (login only — it +// authenticates the limbo plugin to the felis-api internal face) and the Velocity +// modern-forwarding secret (every backend — it is how a backend knows a login really +// came from the proxy, and so that the player's UUID is Mojang-verified rather than +// offline-derived). +// +// It is create-if-absent: an existing replica is left untouched so a hand-rotated +// value in the minecraft namespace is never clobbered (to rotate, delete the replica +// and re-run setup). Best-effort like the rest of the provisioner: a missing source or // a create failure degrades to a reported outcome, never a hard setup failure. It // copies only Type and Data — never labels/annotations/ownerRefs — so the replica // carries no accidental GC owner or managed-by lineage. -func ensureServiceTokenReplica(ctx context.Context, cl client.Client, controlNamespace, minecraftNamespace string) systemServerOutcome { - const name = "service-token (minecraft ns)" - if controlNamespace == minecraftNamespace { - // Same namespace — the operator's secretKeyRef already resolves in place. - return systemServerOutcome{name: name, skipped: "control and minecraft namespaces coincide"} +func ensureSecretReplica(ctx context.Context, cl client.Client, controlNamespace, minecraftNamespace, secretName, secretKey, label string) systemServerOutcome { + name := label + " (minecraft ns)" + validate := func(secret *corev1.Secret, location, skipped string) systemServerOutcome { + if len(secret.Data[secretKey]) == 0 { + return systemServerOutcome{name: name, skipped: fmt.Sprintf( + "Secret %s/%s has no non-empty %q key", location, secretName, secretKey)} + } + return systemServerOutcome{name: name, available: true, skipped: skipped} } - // Never overwrite an existing replica (it may hold a rotated token). + if controlNamespace == minecraftNamespace { + // Same namespace needs no replica, but the source still has to exist. + var existing corev1.Secret + err := cl.Get(ctx, client.ObjectKey{Namespace: minecraftNamespace, Name: secretName}, &existing) + if err == nil { + return validate(&existing, minecraftNamespace, "control and minecraft namespaces coincide") + } + if apierrors.IsNotFound(err) { + return systemServerOutcome{name: name, skipped: fmt.Sprintf( + "source Secret %s/%s not found — provision it (deploy/bootstrap.sh), then re-run setup", + controlNamespace, secretName)} + } + return systemServerOutcome{name: name, err: err} + } + // Never overwrite an existing replica (it may hold a rotated value). var existing corev1.Secret - getErr := cl.Get(ctx, client.ObjectKey{Namespace: minecraftNamespace, Name: naming.ServiceTokenSecretName}, &existing) + getErr := cl.Get(ctx, client.ObjectKey{Namespace: minecraftNamespace, Name: secretName}, &existing) if getErr == nil { - return systemServerOutcome{name: name, skipped: "already exists"} + return validate(&existing, minecraftNamespace, "already exists") } if !apierrors.IsNotFound(getErr) { return systemServerOutcome{name: name, err: getErr} } // Read the source of truth from the control namespace. var src corev1.Secret - if err := cl.Get(ctx, client.ObjectKey{Namespace: controlNamespace, Name: naming.ServiceTokenSecretName}, &src); err != nil { + if err := cl.Get(ctx, client.ObjectKey{Namespace: controlNamespace, Name: secretName}, &src); err != nil { if apierrors.IsNotFound(err) { return systemServerOutcome{name: name, skipped: fmt.Sprintf( "source Secret %s/%s not found — provision it (deploy/bootstrap.sh), then re-run setup", - controlNamespace, naming.ServiceTokenSecretName)} + controlNamespace, secretName)} } return systemServerOutcome{name: name, err: err} } + if out := validate(&src, controlNamespace, ""); !out.available { + return out + } replica := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: naming.ServiceTokenSecretName, Namespace: minecraftNamespace}, + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: minecraftNamespace}, Type: src.Type, Data: src.Data, } if err := cl.Create(ctx, replica); err != nil { if apierrors.IsAlreadyExists(err) { - return systemServerOutcome{name: name, skipped: "already exists"} + if getErr := cl.Get(ctx, client.ObjectKey{Namespace: minecraftNamespace, Name: secretName}, &existing); getErr != nil { + return systemServerOutcome{name: name, err: getErr} + } + return validate(&existing, minecraftNamespace, "already exists") } return systemServerOutcome{name: name, err: err} } - return systemServerOutcome{name: name, created: true} + return systemServerOutcome{name: name, created: true, available: true} +} + +func requiredProvisioningError(outcomes []systemServerOutcome) error { + required := map[string]struct{}{ + "service-token (minecraft ns)": {}, + "forwarding-secret (minecraft ns)": {}, + naming.SystemLoginServer: {}, + } + for _, o := range outcomes { + if o.err != nil { + return fmt.Errorf("%s: %w", o.name, o.err) + } + if _, ok := required[o.name]; ok && !o.available { + reason := o.skipped + if reason == "" { + reason = "object was not created" + } + return fmt.Errorf("%s unavailable: %s", o.name, reason) + } + } + return nil } diff --git a/cmd/felis/systemservers_test.go b/cmd/felis/systemservers_test.go index cac7709..5d44d3c 100644 --- a/cmd/felis/systemservers_test.go +++ b/cmd/felis/systemservers_test.go @@ -2,7 +2,9 @@ package main import ( "context" + "strings" "testing" + "time" "felis.lolicon.best/internal/apis/felis/v1alpha1" "felis.lolicon.best/internal/naming" @@ -134,11 +136,12 @@ func TestLoginSystemServerEnv(t *testing.T) { } } -// ensureServiceTokenReplica copies the token Secret from the control namespace into -// the minecraft namespace (create-if-absent), so the operator's secretKeyRef on the -// login pod resolves. It must not overwrite an existing replica, and must degrade -// gracefully when the source is missing or the namespaces coincide. -func TestEnsureServiceTokenReplica(t *testing.T) { +// ensureSecretReplica copies a Secret from the control namespace into the minecraft +// namespace (create-if-absent), so the operator's secretKeyRef on the backend pod +// resolves. It must not overwrite an existing replica, and must degrade gracefully +// when the source is missing or the namespaces coincide. Exercised here with the +// service token; setup runs it a second time for the Velocity forwarding secret. +func TestEnsureSecretReplica(t *testing.T) { scheme := newSystemServerScheme(t) ctx := context.Background() @@ -149,11 +152,15 @@ func TestEnsureServiceTokenReplica(t *testing.T) { Data: map[string][]byte{naming.ServiceTokenSecretKey: []byte("s3cr3t")}, } } + replicate := func(cl client.Client, controlNS, mcNS string) systemServerOutcome { + return ensureSecretReplica(ctx, cl, controlNS, mcNS, + naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token") + } t.Run("replicates when absent", func(t *testing.T) { cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(srcSecret()).Build() - out := ensureServiceTokenReplica(ctx, cl, "felis", "minecraft") - if out.err != nil || !out.created { + out := replicate(cl, "felis", "minecraft") + if out.err != nil || !out.created || !out.available { t.Fatalf("outcome = %+v, want created", out) } var replica corev1.Secret @@ -172,8 +179,8 @@ func TestEnsureServiceTokenReplica(t *testing.T) { Data: map[string][]byte{naming.ServiceTokenSecretKey: []byte("rotated")}, } cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(srcSecret(), existing).Build() - out := ensureServiceTokenReplica(ctx, cl, "felis", "minecraft") - if out.created || out.skipped == "" { + out := replicate(cl, "felis", "minecraft") + if out.created || !out.available || out.skipped == "" { t.Fatalf("outcome = %+v, want skipped (not clobbered)", out) } var replica corev1.Secret @@ -187,19 +194,143 @@ func TestEnsureServiceTokenReplica(t *testing.T) { t.Run("skips when source missing", func(t *testing.T) { cl := fake.NewClientBuilder().WithScheme(scheme).Build() - out := ensureServiceTokenReplica(ctx, cl, "felis", "minecraft") - if out.err != nil || out.created || out.skipped == "" { + out := replicate(cl, "felis", "minecraft") + if out.err != nil || out.created || out.available || out.skipped == "" { t.Fatalf("outcome = %+v, want skipped (source absent)", out) } }) + t.Run("rejects a source with an empty required key", func(t *testing.T) { + bad := srcSecret() + bad.Data[naming.ServiceTokenSecretKey] = nil + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(bad).Build() + out := replicate(cl, "felis", "minecraft") + if out.err != nil || out.created || out.available || !strings.Contains(out.skipped, naming.ServiceTokenSecretKey) { + t.Fatalf("outcome = %+v, want unavailable required key", out) + } + }) + + t.Run("rejects an existing replica with an empty required key", func(t *testing.T) { + bad := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: naming.ServiceTokenSecretName, Namespace: "minecraft"}, + Data: map[string][]byte{}, + } + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(srcSecret(), bad).Build() + out := replicate(cl, "felis", "minecraft") + if out.err != nil || out.created || out.available || !strings.Contains(out.skipped, naming.ServiceTokenSecretKey) { + t.Fatalf("outcome = %+v, want unavailable existing replica", out) + } + }) + t.Run("no-op when namespaces coincide", func(t *testing.T) { - cl := fake.NewClientBuilder().WithScheme(scheme).Build() - out := ensureServiceTokenReplica(ctx, cl, "felis", "felis") - if out.err != nil || out.created { + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(srcSecret()).Build() + out := replicate(cl, "felis", "felis") + if out.err != nil || out.created || !out.available { t.Fatalf("outcome = %+v, want skipped no-op", out) } }) + + t.Run("same namespace still requires source", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + out := replicate(cl, "felis", "felis") + if out.err != nil || out.available || out.skipped == "" { + t.Fatalf("outcome = %+v, want unavailable source", out) + } + }) + + t.Run("same namespace still requires the key", func(t *testing.T) { + bad := srcSecret() + bad.Data = nil + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(bad).Build() + out := replicate(cl, "felis", "felis") + if out.err != nil || out.available || !strings.Contains(out.skipped, naming.ServiceTokenSecretKey) { + t.Fatalf("outcome = %+v, want unavailable required key", out) + } + }) +} + +func TestRequiredProvisioningError(t *testing.T) { + ready := []systemServerOutcome{ + {name: "service-token (minecraft ns)", available: true}, + {name: "forwarding-secret (minecraft ns)", available: true}, + {name: naming.SystemLoginServer, available: true}, + {name: naming.SystemLobbyServer, skipped: "image not configured"}, + } + if err := requiredProvisioningError(ready); err != nil { + t.Fatalf("ready outcomes: %v", err) + } + + missing := append([]systemServerOutcome(nil), ready...) + missing[1] = systemServerOutcome{name: "forwarding-secret (minecraft ns)", skipped: "source missing"} + if err := requiredProvisioningError(missing); err == nil || !strings.Contains(err.Error(), "forwarding-secret") { + t.Fatalf("missing forwarding secret = %v, want named error", err) + } + + failed := append([]systemServerOutcome(nil), ready...) + failed[3] = systemServerOutcome{name: naming.SystemLobbyServer, err: context.DeadlineExceeded} + if err := requiredProvisioningError(failed); err == nil || !strings.Contains(err.Error(), naming.SystemLobbyServer) { + t.Fatalf("lobby create failure = %v, want immediate named error", err) + } +} + +// The Owner binds by joining the game, so setup blocks on the login gate rather +// than racing it. What matters is that each ending is distinguishable: Ready +// proceeds, Failed reports the operator's own reason instead of waiting out the +// clock, and a gate that never appears (no operator reconciling it) times out +// saying so rather than dropping the operator on a bind screen that cannot work. +func TestAwaitLoginGateReady(t *testing.T) { + scheme := newSystemServerScheme(t) + ctx := context.Background() + + gate := func(mut func(*v1alpha1.MinecraftServer)) *v1alpha1.MinecraftServer { + ms := &v1alpha1.MinecraftServer{ + ObjectMeta: metav1.ObjectMeta{Name: naming.SystemLoginServer, Namespace: "minecraft"}, + } + mut(ms) + return ms + } + + t.Run("returns once the gate is ready", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gate(func(ms *v1alpha1.MinecraftServer) { + ms.Status.Phase = v1alpha1.PhaseRunning + ms.Status.Ready = true + })).Build() + if err := awaitLoginGateReady(ctx, cl, "minecraft", time.Second, 10*time.Millisecond, nil); err != nil { + t.Fatalf("await: %v", err) + } + }) + + t.Run("fails fast on Failed, carrying the operator's reason", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(gate(func(ms *v1alpha1.MinecraftServer) { + ms.Status.Phase = v1alpha1.PhaseFailed + ms.Status.Conditions = []metav1.Condition{{ + Type: v1alpha1.ConditionReady, + Status: metav1.ConditionFalse, + Reason: "StartupTimeout", + Message: "pod never became ready: ImagePullBackOff", + LastTransitionTime: metav1.Now(), + }} + })).Build() + start := time.Now() + err := awaitLoginGateReady(ctx, cl, "minecraft", time.Minute, 10*time.Millisecond, nil) + if err == nil { + t.Fatal("await: nil error, want failure") + } + if !strings.Contains(err.Error(), "ImagePullBackOff") { + t.Errorf("error = %q, want the operator's Ready-condition message", err) + } + if time.Since(start) > 5*time.Second { + t.Error("await sat out the full timeout on a settled Failed verdict") + } + }) + + t.Run("times out when nothing ever reconciles the gate", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + err := awaitLoginGateReady(ctx, cl, "minecraft", 30*time.Millisecond, 10*time.Millisecond, nil) + if err == nil || !strings.Contains(err.Error(), "timed out") { + t.Fatalf("await = %v, want a timeout", err) + } + }) } func newSystemServerScheme(t *testing.T) *runtime.Scheme { @@ -229,7 +360,7 @@ func TestEnsureSystemServersIdempotent(t *testing.T) { if o.err != nil { t.Fatalf("%s: unexpected error: %v", o.name, o.err) } - if !o.created { + if !o.created || !o.available { t.Errorf("%s: created = false on fresh cluster (skipped=%q)", o.name, o.skipped) } } @@ -242,6 +373,9 @@ func TestEnsureSystemServersIdempotent(t *testing.T) { if login.Spec.DesiredState != v1alpha1.DesiredRunning || !login.Spec.ReaperExempt { t.Errorf("login spec = {desired=%q exempt=%v}, want {Running true}", login.Spec.DesiredState, login.Spec.ReaperExempt) } + if got := login.Labels[v1alpha1.LabelSystemRole]; got != naming.SystemLoginServer { + t.Errorf("login system-role label = %q, want %q", got, naming.SystemLoginServer) + } // Re-run: both already exist → skipped, nothing created, no error. second := ensureSystemServers(ctx, cl, "minecraft", "reg/limbo:1", "reg/lobby:1", "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net") @@ -252,12 +386,39 @@ func TestEnsureSystemServersIdempotent(t *testing.T) { if o.created { t.Errorf("%s: created = true on re-run, want skipped", o.name) } + if !o.available { + t.Errorf("%s: available = false on re-run", o.name) + } if o.skipped == "" { t.Errorf("%s: skipped reason empty on re-run", o.name) } } } +func TestEnsureSystemServersRejectsLegacyLoginNameCollision(t *testing.T) { + scheme := newSystemServerScheme(t) + legacy := &v1alpha1.MinecraftServer{ObjectMeta: metav1.ObjectMeta{ + Name: naming.SystemLoginServer, + Namespace: "minecraft", + }} + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(legacy).Build() + + out := ensureSystemServers( + context.Background(), cl, "minecraft", "reg/limbo:1", "", + "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net", + ) + if len(out) != 2 { + t.Fatalf("outcomes = %d, want 2", len(out)) + } + login := out[0] + if login.err == nil || !strings.Contains(login.err.Error(), "not marked") { + t.Fatalf("login error = %v, want an unmarked-name collision error", login.err) + } + if login.available || login.created { + t.Fatalf("login outcome = %+v, want unavailable and not created", login) + } +} + func TestEnsureSystemServersSkipsUnsetImage(t *testing.T) { scheme := newSystemServerScheme(t) cl := fake.NewClientBuilder().WithScheme(scheme).Build() diff --git a/cmd/felis/tui_mc_bind.go b/cmd/felis/tui_mc_bind.go index 6e84afc..07963fe 100644 --- a/cmd/felis/tui_mc_bind.go +++ b/cmd/felis/tui_mc_bind.go @@ -1,202 +1,219 @@ -package main - -import ( - "context" - "strings" - - "github.com/charmbracelet/bubbles/spinner" - tea "github.com/charmbracelet/bubbletea" - "github.com/charmbracelet/huh" -) - -// mcBindModel is the `felis setup` Owner-establishment screen: the operator -// joins the server, runs /link to get a one-time code, and types it here. The -// bound Minecraft account is promoted to the passwordless Owner, and a one-time -// setup URL is minted for the first web login. It replaces the old ownerModel -// bootstrap form in setup mode — no username/email/password is typed here, the -// MC identity is the root of trust. -type mcBindModel struct { - ctx context.Context - store ownerStore - adminHost string - osUser string - - step mcBindStep - form *huh.Form - sp spinner.Model - working string - - linkCode string - setupTokenURL string - - width, height int -} - -type mcBindStep int - -const ( - mcBindForm mcBindStep = iota - mcBindWorking - mcBindDone -) - -type mcBindMsg struct { - outcome breakGlassOutcome - err error -} - -func newMCBindModel(ctx context.Context, store ownerStore, adminHost, osUser string) *mcBindModel { - sp := spinner.New() - sp.Spinner = spinner.Dot - sp.Style = tuiLabel - m := &mcBindModel{ - ctx: ctx, - store: store, - adminHost: adminHost, - osUser: osUser, - sp: sp, - step: mcBindForm, - } - m.form = m.buildForm() - return m -} - -func (m *mcBindModel) buildForm() *huh.Form { - return m.sized(newFelisForm(huh.NewGroup( - huh.NewNote(). - Title("Bind your Minecraft account"). - Description("Join the server and run /link to get a one-time code,\nthen type it here. Your bound account becomes the\npasswordless Owner."), - huh.NewInput(). - Title("Link code"). - Placeholder("ABCD12"). - Value(&m.linkCode). - Validate(requiredField("link code")), - ))) -} - -func (m *mcBindModel) sized(f *huh.Form) *huh.Form { - if m.width > 0 { - return f.WithWidth(m.width).WithHeight(m.height) - } - return f -} - -func (m *mcBindModel) setSize(w, h int) { - m.width, m.height = w, h - if m.form != nil { - m.form = m.form.WithWidth(w).WithHeight(h) - } -} - -func (m *mcBindModel) Init() tea.Cmd { return m.form.Init() } - -func (m *mcBindModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - switch msg := msg.(type) { - case mcBindMsg: - if msg.err != nil { - return m, m.failCmd(msg.err) - } - m.step = mcBindDone - m.setupTokenURL = msg.outcome.setupTokenURL - return m, nil - - case spinner.TickMsg: - if m.step == mcBindWorking { - var cmd tea.Cmd - m.sp, cmd = m.sp.Update(msg) - return m, cmd - } - return m, nil - - case tea.KeyMsg: - switch m.step { - case mcBindDone: - switch msg.String() { - case "ctrl+c", "esc", "enter": - return m, m.resultCmd() - } - return m, nil - case mcBindWorking: - if msg.String() == "ctrl+c" { - return m, tea.Quit - } - return m, nil - default: - switch msg.String() { - case "ctrl+c", "esc": - return m, tea.Quit - } - } - } - - if m.step == mcBindForm && m.form != nil { - form, cmd := m.form.Update(msg) - if f, ok := form.(*huh.Form); ok { - m.form = f - } - switch m.form.State { - case huh.StateCompleted: - return m.onFormComplete() - case huh.StateAborted: - return m, tea.Quit - } - return m, cmd - } - return m, nil -} - -func (m *mcBindModel) onFormComplete() (tea.Model, tea.Cmd) { - m.step = mcBindWorking - m.working = "Binding Minecraft account…" - code := strings.TrimSpace(strings.ToUpper(m.linkCode)) - return m, tea.Batch(m.sp.Tick, func() tea.Msg { - out, err := performSetupMCBind(m.ctx, m.store, code, m.adminHost) - return mcBindMsg{outcome: out, err: err} - }) -} - -func (m *mcBindModel) failCmd(err error) tea.Cmd { - return func() tea.Msg { return ownerResultMsg{err: err} } -} - -func (m *mcBindModel) resultCmd() tea.Cmd { - return func() tea.Msg { - return ownerResultMsg{ - setupTokenURL: m.setupTokenURL, - mode: "setup", - accountable: m.osUser, - } - } -} - -func (m *mcBindModel) View() string { - switch m.step { - case mcBindWorking: - msg := m.working - if msg == "" { - msg = "Working…" - } - return " " + m.sp.View() + " " + tuiHint.Render(msg) + "\n" - case mcBindDone: - return m.doneView() - default: - if m.form == nil { - return "" - } - return m.form.View() - } -} - -func (m *mcBindModel) doneView() string { - var b strings.Builder - b.WriteString(tuiSuccessBanner("Owner account is ready.") + "\n\n") - - var box strings.Builder - if m.setupTokenURL != "" { - box.WriteString(tuiLabel.Render("setup URL ") + "\n" + tuiPassword.Render(m.setupTokenURL) + "\n\n") - box.WriteString(tuiWarn.Render("Open this URL to complete passwordless login setup.\nIt is shown only once.")) - } - b.WriteString(tuiCardStyle.Render(box.String()) + "\n\n") - b.WriteString(tuiAction("enter", "continue")) - return b.String() -} +package main + +import ( + "context" + "strings" + + "github.com/charmbracelet/bubbles/spinner" + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/huh" +) + +// mcBindModel is the `felis setup` Owner-establishment screen: the operator +// joins the server, runs /link to get a one-time code, and types it here. The +// bound Minecraft account is promoted to the passwordless Owner, and a one-time +// setup URL is minted for the first web login. It replaces the old ownerModel +// bootstrap form in setup mode — no username/email/password is typed here, the +// MC identity is the root of trust. +type mcBindModel struct { + ctx context.Context + store ownerStore + adminHost string + osUser string + + step mcBindStep + form *huh.Form + sp spinner.Model + working string + + linkCode string + ownerIdentity string + setupTokenURL string + auditWarning string + + width, height int +} + +type mcBindStep int + +const ( + mcBindForm mcBindStep = iota + mcBindWorking + mcBindDone +) + +type mcBindMsg struct { + outcome breakGlassOutcome + err error +} + +func newMCBindModel(ctx context.Context, store ownerStore, adminHost, osUser string) *mcBindModel { + sp := spinner.New() + sp.Spinner = spinner.Dot + sp.Style = tuiLabel + m := &mcBindModel{ + ctx: ctx, + store: store, + adminHost: adminHost, + osUser: osUser, + sp: sp, + step: mcBindForm, + } + m.form = m.buildForm() + return m +} + +func (m *mcBindModel) buildForm() *huh.Form { + return m.sized(newFelisForm(huh.NewGroup( + huh.NewNote(). + Title("Bind your Minecraft account"). + Description("Join the server and run /link to get a one-time code,\nthen type it here. Your bound account becomes the\npasswordless Owner."), + huh.NewInput(). + Title("Link code"). + Placeholder("ABCD12"). + Value(&m.linkCode). + Validate(requiredField("link code")), + ))) +} + +func (m *mcBindModel) sized(f *huh.Form) *huh.Form { + if m.width > 0 { + return f.WithWidth(m.width).WithHeight(m.height) + } + return f +} + +func (m *mcBindModel) setSize(w, h int) { + m.width, m.height = w, h + if m.form != nil { + m.form = m.form.WithWidth(w).WithHeight(h) + } +} + +func (m *mcBindModel) Init() tea.Cmd { return m.form.Init() } + +func (m *mcBindModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case mcBindMsg: + if msg.err != nil { + return m, m.failCmd(msg.err) + } + m.step = mcBindDone + m.ownerIdentity = msg.outcome.ownerIdentity + m.setupTokenURL = msg.outcome.setupTokenURL + if msg.outcome.auditErr != nil { + m.auditWarning = msg.outcome.auditErr.Error() + } + return m, nil + + case spinner.TickMsg: + if m.step == mcBindWorking { + var cmd tea.Cmd + m.sp, cmd = m.sp.Update(msg) + return m, cmd + } + return m, nil + + case tea.KeyMsg: + switch m.step { + case mcBindDone: + switch msg.String() { + case "ctrl+c", "esc", "enter": + return m, m.resultCmd() + } + return m, nil + case mcBindWorking: + if msg.String() == "ctrl+c" { + return m, tea.Quit + } + return m, nil + default: + switch msg.String() { + case "ctrl+c", "esc": + return m, tea.Quit + } + } + } + + if m.step == mcBindForm && m.form != nil { + form, cmd := m.form.Update(msg) + if f, ok := form.(*huh.Form); ok { + m.form = f + } + switch m.form.State { + case huh.StateCompleted: + return m.onFormComplete() + case huh.StateAborted: + return m, tea.Quit + } + return m, cmd + } + return m, nil +} + +func (m *mcBindModel) onFormComplete() (tea.Model, tea.Cmd) { + m.step = mcBindWorking + m.working = "Binding Minecraft account…" + code := strings.TrimSpace(strings.ToUpper(m.linkCode)) + return m, tea.Batch(m.sp.Tick, func() tea.Msg { + out, err := performSetupMCBind(m.ctx, m.store, code, m.adminHost, m.osUser) + return mcBindMsg{outcome: out, err: err} + }) +} + +func (m *mcBindModel) failCmd(err error) tea.Cmd { + return func() tea.Msg { return ownerResultMsg{err: err} } +} + +func (m *mcBindModel) resultCmd() tea.Cmd { + return func() tea.Msg { + return ownerResultMsg{ + username: m.ownerIdentity, + setupTokenURL: m.setupTokenURL, + mode: "setup", + accountable: m.osUser, + auditWarning: m.auditWarning, + } + } +} + +func (m *mcBindModel) View() string { + switch m.step { + case mcBindWorking: + msg := m.working + if msg == "" { + msg = "Working…" + } + return " " + m.sp.View() + " " + tuiHint.Render(msg) + "\n" + case mcBindDone: + return m.doneView() + default: + if m.form == nil { + return "" + } + return m.form.View() + } +} + +func (m *mcBindModel) doneView() string { + var b strings.Builder + b.WriteString(tuiSuccessBanner("Owner account is ready.") + "\n\n") + + var box strings.Builder + if m.ownerIdentity != "" { + box.WriteString(tuiLabel.Render("minecraft ") + m.ownerIdentity + "\n") + } + if m.setupTokenURL != "" { + if box.Len() > 0 { + box.WriteString("\n") + } + box.WriteString(tuiLabel.Render("setup URL ") + "\n" + tuiPassword.Render(m.setupTokenURL) + "\n\n") + box.WriteString(tuiWarn.Render("Open this URL to complete passwordless login setup.\nIt is shown only once.")) + } + if m.auditWarning != "" { + box.WriteString("\n\n" + tuiWarn.Render("Audit warning: "+m.auditWarning)) + } + b.WriteString(tuiCardStyle.Render(box.String()) + "\n\n") + b.WriteString(tuiAction("enter", "continue")) + return b.String() +} diff --git a/cmd/felis/tui_menu_test.go b/cmd/felis/tui_menu_test.go index b925321..a2bbcfd 100644 --- a/cmd/felis/tui_menu_test.go +++ b/cmd/felis/tui_menu_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" "felis.lolicon.best/internal/api" @@ -183,6 +184,26 @@ func TestOwnerResultCmdCarriesIsOperator(t *testing.T) { } } +func TestMCBindCarriesAuditWarning(t *testing.T) { + m := newMCBindModel(context.Background(), &fakeOwnerStore{}, "op.console.example.com", "root") + next, _ := m.Update(mcBindMsg{outcome: breakGlassOutcome{ + ownerIdentity: "mc-uuid-1", + setupTokenURL: "https://op.console.example.com/setup?token=t0ken", + auditErr: errors.New("audit insert failed"), + }}) + bound := next.(*mcBindModel) + if !strings.Contains(bound.doneView(), "audit insert failed") { + t.Fatalf("done view did not surface audit warning:\n%s", bound.doneView()) + } + res := bound.resultCmd()().(ownerResultMsg) + if res.username != "mc-uuid-1" { + t.Fatalf("result username = %q, want verified Minecraft UUID", res.username) + } + if res.auditWarning != "audit insert failed" { + t.Fatalf("result audit warning = %q", res.auditWarning) + } +} + func TestRootBreakGlassOpensMenuWhenAdminExists(t *testing.T) { m := newTestRoot(true, consoleModeBreakGlass, "") if m.stage != stageMenu { diff --git a/cmd/felis/tui_root.go b/cmd/felis/tui_root.go index 2ce4c54..478e9d0 100644 --- a/cmd/felis/tui_root.go +++ b/cmd/felis/tui_root.go @@ -548,6 +548,7 @@ func (m *rootModel) showSummary() (tea.Model, tea.Cmd) { // operator at the panel without forcing any reconfiguration. func (m *rootModel) showStatus() (tea.Model, tea.Cmd) { m.stage = stageSummary + m.result.alreadySetUp = true method := connectLocal accessLabel := "configured (manage in panel)" if m.accessAud != "" { diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index cc2f3c3..662db11 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -84,7 +84,9 @@ func (p *PGRepo) VerifyLinkCode(ctx context.Context, userID, code string, now ti var mcUUID, authSource string switch err := tx.QueryRowContext(ctx, - `SELECT mc_uuid, auth_source FROM account_link_codes WHERE code = $1 AND expires_at > $2`, + `SELECT mc_uuid, auth_source FROM account_link_codes + WHERE code = $1 AND expires_at > $2 + FOR UPDATE`, code, now).Scan(&mcUUID, &authSource); { case errors.Is(err, sql.ErrNoRows): return "", "", ErrLinkCodeInvalid @@ -191,8 +193,9 @@ func (p *PGRepo) RedeemPlayerBindCode(ctx context.Context, newUserID, code strin return userID, mcUUID, authSource, nil } -// RedeemLinkCodeForOwner consumes an in-game link code and creates-or-promotes the -// bound account to the passwordless Owner (role='admin'). It is the `felis setup` +// CompleteOwnerSetup consumes an in-game link code, creates-or-promotes the bound +// account to the passwordless Owner (role='admin'), enables local auth, and stores +// the one-time first-login token in one transaction. It is the `felis setup` // MC-bind path: the operator enters limbo, runs /link, and types the code here. // Unlike RedeemPlayerBindCode — which refuses an already-staff account so a game // login can never self-elevate — this DELIBERATELY elevates: an unlinked UUID is @@ -201,8 +204,10 @@ func (p *PGRepo) RedeemPlayerBindCode(ctx context.Context, newUserID, code strin // survive. The elevation is gated by the caller's local-root break-glass // authority, not by anything in-band. Returns the Owner's (userID, mcUUID, // authSource); an absent or expired code is ErrLinkCodeInvalid and consumes -// nothing. -func (p *PGRepo) RedeemLinkCodeForOwner(ctx context.Context, newUserID, code string, now time.Time) (string, string, string, error) { +// nothing. Any failure in the auth-toggle or token writes rolls the elevation and +// code consumption back, leaving the operator able to retry setup. +func (p *PGRepo) CompleteOwnerSetup(ctx context.Context, newUserID, code string, now time.Time, + tokenHash string, tokenExpiresAt time.Time) (string, string, string, error) { tx, err := p.db.BeginTx(ctx, nil) if err != nil { return "", "", "", err @@ -248,6 +253,18 @@ func (p *PGRepo) RedeemLinkCodeForOwner(ctx context.Context, newUserID, code str } } + if _, err := tx.ExecContext(ctx, + `INSERT INTO platform_settings (key, value) VALUES ($1, $2::jsonb) + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`, + LocalAuthEnabledKey, "true"); err != nil { + return "", "", "", fmt.Errorf("enable local auth: %w", err) + } + if _, err := tx.ExecContext(ctx, + `INSERT INTO setup_tokens (token_hash, user_id, expires_at) VALUES ($1, $2, $3)`, + tokenHash, userID, tokenExpiresAt); err != nil { + return "", "", "", fmt.Errorf("mint setup token: %w", err) + } + if _, err := tx.ExecContext(ctx, `DELETE FROM account_link_codes WHERE code = $1`, code); err != nil { return "", "", "", fmt.Errorf("consume link code: %w", err) @@ -1995,12 +2012,11 @@ func (p *PGRepo) ConsumeSetupToken(ctx context.Context, tokenHash string, now ti return userID, nil } -// CreateSetupToken persists a one-time setup token for the first-web-login -// bootstrap, storing only its hash (the raw value rides in the /setup?token=... -// URL). `felis setup` mints it after binding the Owner's Minecraft account; it is -// redeemed exactly once by ConsumeSetupToken. The caller supplies a 256-bit -// random token, so a token_hash collision is not a case worth special-handling — -// any insert error (including an unknown user_id) surfaces to the caller. +// CreateSetupToken persists a one-time first-web-login token, storing only its +// hash (the raw value rides in the /setup?token=... URL). The setup Owner-bind +// path uses CompleteOwnerSetup so identity binding, local auth, and this token +// commit atomically; this lower-level helper remains for callers that already +// established the user. The token is redeemed exactly once by ConsumeSetupToken. func (p *PGRepo) CreateSetupToken(ctx context.Context, tokenHash, userID string, expiresAt time.Time) error { _, err := p.db.ExecContext(ctx, `INSERT INTO setup_tokens (token_hash, user_id, expires_at) VALUES ($1, $2, $3)`,