diff --git a/cmd/felis/breakglass.go b/cmd/felis/breakglass.go index 2241e35..b93fb6a 100644 --- a/cmd/felis/breakglass.go +++ b/cmd/felis/breakglass.go @@ -70,6 +70,12 @@ type ownerStore interface { // UserByUsername loads a staff login projection for credential verification. UserByUsername(ctx context.Context, username string) (*api.StaffUser, error) UpsertOwner(ctx context.Context, id, username, email, passwordHash string, mustChange bool) error + // InsertOperator mints a NEW Operator staff account. Unlike UpsertOwner it is + // insert-only: a username already taken is a conflict (api.ErrConflict), never a + // silent reset, so adding an Operator can never clobber the Owner or an existing + // Operator. The row is role=admin, identical in shape to the Owner — Felis has no + // separate operator DB role (migration 0003: staff = role=admin WITH a hash). + InsertOperator(ctx context.Context, id, username, email, passwordHash string, mustChange bool) 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 @@ -304,6 +310,41 @@ func provisionOwner(ctx context.Context, s ownerStore, username, email, password return nil } +// provisionOperator mints a NEW Operator staff account direct-to-Postgres. Like the +// Owner it is role=admin with must_change_password=true — Felis has no separate +// operator DB role, so an Operator is simply an additional staff admin (migration +// 0003). UNLIKE provisionOwner, which upserts the single Owner and resets it on a +// username conflict, this is insert-only: a username already taken returns +// api.ErrConflict rather than overwriting a live account, so adding an Operator can +// never silently clobber the Owner's or another Operator's credential. Only the +// bcrypt hash reaches the database; the plaintext never does. +func provisionOperator(ctx context.Context, s ownerStore, username, email, password string) error { + username = strings.TrimSpace(username) + if username == "" { + return errors.New("operator username is required") + } + if err := validateOwnerPassword(password); err != nil { + return err + } + id := newOwnerID() + if id == "" { + return errors.New("generate operator id: entropy source failed") + } + hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return fmt.Errorf("hash operator password: %w", err) + } + if err := s.InsertOperator(ctx, id, username, strings.TrimSpace(email), string(hash), true); err != nil { + if errors.Is(err, api.ErrConflict) { + // Wrap %w so errors.Is(err, api.ErrConflict) still holds — the TUI can render + // a "name already taken" message — while keeping a clear human string. + return fmt.Errorf("operator %q already exists: %w", username, err) + } + return fmt.Errorf("write operator: %w", err) + } + return nil +} + // enableLocalAuth flips the runtime local_auth_enabled toggle on // direct-to-Postgres. It is a load-bearing write of break-glass: without it // handleLogin returns 403 and the freshly provisioned Owner cannot log in, so a @@ -397,6 +438,64 @@ func auditBreakGlass(ctx context.Context, s ownerStore, op breakGlassOp) error { }) } +// performAddOperator mints a NEW Operator account and records a best-effort +// accountability row. It mirrors performBreakGlass — a typed password is used as-is, +// an empty one is replaced with a generated one-time password returned for one-time +// display (the common case: hand a fresh credential to the new operator) — with two +// deliberate differences. (1) It provisions insert-only (provisionOperator), so it +// can never reset an existing account the way the Owner upsert does. (2) It does NOT +// touch local_auth_enabled: adding an Operator presupposes an already-configured, +// running system (an admin is present to authorize it), so flipping the global auth +// gate as a side effect of "add a user" would be surprising — that toggle belongs to +// the Owner break-glass thread alone. The audit is best-effort and written only after +// a successful provision; a conflict mints nothing, so there is nothing to attribute. +func performAddOperator(ctx context.Context, s ownerStore, op breakGlassOp) (breakGlassOutcome, error) { + password := op.ownerPassword + generated := false + if password == "" { + p, err := generateBootstrapPassword() + if err != nil { + return breakGlassOutcome{}, err + } + password, generated = p, true + } + if err := provisionOperator(ctx, s, op.ownerUsername, op.ownerEmail, password); err != nil { + return breakGlassOutcome{}, err + } + out := breakGlassOutcome{auditErr: auditAddOperator(ctx, s, op)} + if generated { + out.displayPassword = password + } + return out, nil +} + +// auditAddOperator writes the operator-creation accountability row. It mirrors +// auditBreakGlass — same actor/source/payload shape, so one reader can tell a +// verified (recovery) add from an unverified (root_override) one — under the distinct +// break_glass.operator_create action, naming the new account under an "operator" key +// rather than "owner". +func auditAddOperator(ctx context.Context, s ownerStore, op breakGlassOp) error { + payload := map[string]any{ + "mode": op.mode, + "operator": op.ownerUsername, + "os_user": op.osUser, + "verified": op.mode == "recovery", + } + if op.attemptedAdmin != "" { + payload["admin_account"] = op.attemptedAdmin + } + blob, err := json.Marshal(payload) + if err != nil { + return err + } + return s.Audit(ctx, api.AuditEntry{ + Actor: op.accountable, + Source: "break-glass", + Action: "break_glass.operator_create", + Payload: blob, + }) +} + // breakGlassResult is what the TUI hands back to cmdBreakGlass for the durable // post-exit summary. provisioned is false on cancel. type breakGlassResult struct { diff --git a/cmd/felis/breakglass_test.go b/cmd/felis/breakglass_test.go index d2cb26d..6f562c5 100644 --- a/cmd/felis/breakglass_test.go +++ b/cmd/felis/breakglass_test.go @@ -17,12 +17,14 @@ import ( // audit) is exercised without a database or a terminal. type fakeOwnerStore struct { upserts []upsertCall + inserts []upsertCall settings map[string][]byte audits []api.AuditEntry users map[string]*api.StaffUser // keyed by username admins bool // AdminExists answer upsertErr error + insertErr error setErr error auditErr error userErr error // non-not-found error from UserByUsername @@ -59,6 +61,28 @@ func (f *fakeOwnerStore) UpsertOwner(_ context.Context, id, username, email, pas return nil } +// InsertOperator records an insert-only Operator provision. A username already in +// the users map is a conflict (api.ErrConflict), mirroring the PGRepo ON CONFLICT +// DO NOTHING + zero-RowsAffected contract; a fresh one is recorded and reflected +// into users so a later lookup — or a second insert of the same name — sees it. +func (f *fakeOwnerStore) InsertOperator(_ context.Context, id, username, email, passwordHash string, mustChange bool) error { + if f.insertErr != nil { + return f.insertErr + } + if _, taken := f.users[username]; taken { + return api.ErrConflict + } + f.inserts = append(f.inserts, upsertCall{id, username, email, passwordHash, mustChange}) + if f.users == nil { + f.users = map[string]*api.StaffUser{} + } + f.users[username] = &api.StaffUser{ + ID: id, Username: username, Email: email, + Role: "admin", PasswordHash: passwordHash, MustChangePassword: mustChange, + } + return nil +} + func (f *fakeOwnerStore) SetSetting(_ context.Context, key string, value []byte) error { if f.setErr != nil { return f.setErr @@ -507,3 +531,205 @@ func TestAccountableOSUser(t *testing.T) { } }) } + +func TestProvisionOperator(t *testing.T) { + ctx := context.Background() + + t.Run("happy path mints a must-change admin with a verifiable hash", func(t *testing.T) { + f := &fakeOwnerStore{} + const pw = "valid-test-pw" + if err := provisionOperator(ctx, f, "ops-jordan", "jordan@example.com", pw); err != nil { + t.Fatalf("provisionOperator: %v", err) + } + // Insert-only: it records an insert and never touches the Owner upsert path. + if len(f.upserts) != 0 { + t.Errorf("want 0 owner upserts, got %d — operator-add must not use the Owner path", len(f.upserts)) + } + if len(f.inserts) != 1 { + t.Fatalf("want 1 insert, got %d", len(f.inserts)) + } + got := f.inserts[0] + if got.username != "ops-jordan" { + t.Errorf("username = %q, want ops-jordan", got.username) + } + if got.email != "jordan@example.com" { + t.Errorf("email = %q, want jordan@example.com", got.email) + } + // must_change_password=true arms the API lockdown for the new operator too. + if !got.mustChange { + t.Error("mustChange = false, want true (forced first-login change)") + } + if !strings.HasPrefix(got.id, "usr-") { + t.Errorf("id = %q, want usr- prefix", got.id) + } + // Only the hash is stored; the typed plaintext must verify against it. + if bcrypt.CompareHashAndPassword([]byte(got.passwordHash), []byte(pw)) != nil { + t.Error("typed password does not verify against the stored hash") + } + if got.passwordHash == pw { + t.Error("stored hash equals plaintext — password was not hashed") + } + }) + + t.Run("a taken username is a conflict, not a silent reset", func(t *testing.T) { + // The Owner already holds this username. Operator-add must refuse rather than + // overwrite it the way UpsertOwner would. + f := &fakeOwnerStore{users: map[string]*api.StaffUser{ + "owner": {ID: "usr-owner", Username: "owner", Role: "admin", PasswordHash: "x"}, + }} + err := provisionOperator(ctx, f, "owner", "", "valid-test-pw") + if err == nil { + t.Fatal("want error when the username is already taken") + } + // The sentinel must remain matchable so the TUI can render "name already taken". + if !errors.Is(err, api.ErrConflict) { + t.Errorf("error = %v, want it to wrap api.ErrConflict", err) + } + if len(f.inserts) != 0 { + t.Errorf("want no insert on conflict, got %d", len(f.inserts)) + } + // The pre-existing account must be untouched. + if f.users["owner"].PasswordHash != "x" { + t.Error("conflicting insert clobbered the existing account's hash") + } + }) + + t.Run("trims surrounding whitespace", func(t *testing.T) { + f := &fakeOwnerStore{} + if err := provisionOperator(ctx, f, " ops ", " e@x.io ", "valid-test-pw"); err != nil { + t.Fatalf("provisionOperator: %v", err) + } + if f.inserts[0].username != "ops" || f.inserts[0].email != "e@x.io" { + t.Errorf("got username=%q email=%q, want trimmed", f.inserts[0].username, f.inserts[0].email) + } + }) + + t.Run("rejects an empty username before any write", func(t *testing.T) { + f := &fakeOwnerStore{} + if err := provisionOperator(ctx, f, " ", "", "valid-test-pw"); err == nil { + t.Fatal("want error for empty username") + } + if len(f.inserts) != 0 { + t.Errorf("want no insert on validation failure, got %d", len(f.inserts)) + } + }) + + t.Run("rejects a weak password before any write", func(t *testing.T) { + f := &fakeOwnerStore{} + if err := provisionOperator(ctx, f, "ops", "", "short"); err == nil { + t.Fatal("want error for a sub-8-byte password") + } + if len(f.inserts) != 0 { + t.Errorf("want no insert on weak password, got %d", len(f.inserts)) + } + }) + + t.Run("propagates a non-conflict store error without mislabeling it", func(t *testing.T) { + f := &fakeOwnerStore{insertErr: errors.New("boom")} + err := provisionOperator(ctx, f, "ops", "", "valid-test-pw") + if err == nil { + t.Fatal("want error when the store fails") + } + // A generic store fault must NOT be mistaken for a username conflict. + if errors.Is(err, api.ErrConflict) { + t.Error("a generic store error was misreported as a conflict") + } + }) +} + +func TestPerformAddOperator(t *testing.T) { + ctx := context.Background() + + t.Run("typed password is used as-is, never echoed, and never flips local auth", func(t *testing.T) { + f := &fakeOwnerStore{} + op := breakGlassOp{ + mode: "recovery", + accountable: "root", + osUser: "alice", + ownerUsername: "ops-jordan", + ownerEmail: "jordan@example.com", + ownerPassword: "valid-test-pw", + attemptedAdmin: "root", + } + out, err := performAddOperator(ctx, f, op) + if err != nil { + t.Fatalf("performAddOperator: %v", err) + } + // The operator's password was typed, so it must NOT be surfaced for display. + if out.displayPassword != "" { + t.Errorf("displayPassword = %q, want empty for a typed password", out.displayPassword) + } + if len(f.inserts) != 1 || bcrypt.CompareHashAndPassword([]byte(f.inserts[0].passwordHash), []byte("valid-test-pw")) != nil { + t.Error("operator was not provisioned with the typed password") + } + // Adding an Operator must NOT flip the global local-auth gate (Owner-only). + if _, ok := f.settings[api.LocalAuthEnabledKey]; ok { + t.Error("local auth was enabled — operator-add must not touch the global gate") + } + e, payload := auditOf(t, f) + if e.Actor != "root" || e.Source != "break-glass" || e.Action != "break_glass.operator_create" { + t.Errorf("audit envelope = %+v, want actor=root source=break-glass action=break_glass.operator_create", e) + } + // The new account is recorded under "operator", not "owner". + if payload["operator"] != "ops-jordan" { + t.Errorf("payload.operator = %v, want ops-jordan", payload["operator"]) + } + if _, present := payload["owner"]; present { + t.Error("payload.owner present, want the new account under the operator key") + } + if payload["verified"] != true || payload["admin_account"] != "root" { + t.Errorf("payload = %v, want verified=true admin_account=root", payload) + } + }) + + t.Run("an empty password generates a one-time credential matching the stored hash", func(t *testing.T) { + f := &fakeOwnerStore{} + op := breakGlassOp{mode: "root_override", accountable: "alice", osUser: "alice", ownerUsername: "ops", attemptedAdmin: "typo-admin"} + out, err := performAddOperator(ctx, f, op) + if err != nil { + t.Fatalf("performAddOperator: %v", err) + } + if out.displayPassword == "" { + t.Fatal("displayPassword empty, want a generated one-time password to hand off") + } + // The shown password must be the one actually stored (as a hash). + if bcrypt.CompareHashAndPassword([]byte(f.inserts[0].passwordHash), []byte(out.displayPassword)) != nil { + t.Error("displayed password does not match the stored hash") + } + _, payload := auditOf(t, f) + if payload["verified"] != false { + t.Errorf("payload.verified = %v, want false for root_override", payload["verified"]) + } + }) + + t.Run("an audit failure does not fail the operator-add", func(t *testing.T) { + f := &fakeOwnerStore{auditErr: errors.New("audit sink down")} + op := breakGlassOp{mode: "recovery", accountable: "root", osUser: "alice", ownerUsername: "ops", attemptedAdmin: "root"} + out, err := performAddOperator(ctx, f, op) + if err != nil { + t.Fatalf("performAddOperator returned %v, want nil — a dead audit sink must not fail the add", err) + } + if out.auditErr == nil { + t.Error("auditErr = nil, want the surfaced audit failure") + } + if len(f.inserts) != 1 { + t.Error("operator was not provisioned despite a recoverable audit failure") + } + }) + + t.Run("a conflict mints nothing and writes no audit row", func(t *testing.T) { + f := &fakeOwnerStore{users: map[string]*api.StaffUser{ + "owner": {ID: "usr-owner", Username: "owner", Role: "admin", PasswordHash: "x"}, + }} + op := breakGlassOp{mode: "recovery", accountable: "root", osUser: "alice", ownerUsername: "owner", ownerPassword: "valid-test-pw", attemptedAdmin: "root"} + if _, err := performAddOperator(ctx, f, op); err == nil { + t.Fatal("want error when the operator username is already taken") + } + if len(f.inserts) != 0 { + t.Error("a conflicting add should mint nothing") + } + if len(f.audits) != 0 { + t.Error("a conflicting add should write no audit row") + } + }) +} diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 5cfb024..5f38def 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -604,6 +604,32 @@ func (p *PGRepo) UpsertOwner(ctx context.Context, id, username, email, passwordH return err } +// InsertOperator mints a NEW Operator (additional staff admin) account +// direct-to-Postgres. role is forced to 'admin' — Felis has no separate operator +// role, so an Operator is an additional admin row identical in shape to the Owner +// (migration 0003). UNLIKE UpsertOwner this is insert-only: a username conflict is +// left untouched (ON CONFLICT DO NOTHING) and reported as ErrConflict via a zero +// RowsAffected, so adding an Operator can never silently reset the Owner's or +// another Operator's credential. The empty email is stored as NULL. +func (p *PGRepo) InsertOperator(ctx context.Context, id, username, email, passwordHash string, mustChange bool) error { + res, err := p.db.ExecContext(ctx, + `INSERT INTO users (id, username, email, role, password_hash, must_change_password) + VALUES ($1, $2, NULLIF($3, ''), 'admin', $4, $5) + ON CONFLICT (username) DO NOTHING`, + id, username, email, passwordHash, mustChange) + if err != nil { + return err + } + n, err := res.RowsAffected() + if err != nil { + return err + } + if n == 0 { + return ErrConflict + } + return nil +} + // SetPassword stores a new hash and clears must_change_password (the panel // change-password flow). ErrNotFound when no row matches so a stale session // cannot silently no-op the change.