improvements for first-time setup

internal/identity/identity.go

@@ -9,6 +9,7 @@
 package identity
 
 import (
+	"crypto/subtle"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -19,6 +20,29 @@ import (
 const (
 	metaFileName = "identity.json"
 	iconBaseName = "icon"
+	// setupPendingFileName marks an install as not yet through the
+	// first-run wizard. Deliberately a sentinel file next to identity.json,
+	// not a field inside it: identity.json's own content changes *during*
+	// the wizard -- the Identity step saves a name well before Done is ever
+	// reached -- so a signal derived from that content (e.g. "name is set")
+	// flips to "done" the moment that one step is saved, not when the
+	// wizard actually finishes. This file is created once, by quickstart's
+	// bootstrap_env() the moment it creates a fresh .env (see
+	// tui-panel/server-panel.py), and removed once, by MarkSetupComplete --
+	// nothing in between (including a server restart mid-wizard) touches
+	// it, so "still pending" survives every step until Done really is
+	// reached.
+	setupPendingFileName = ".setup_pending"
+	// passwordTemporaryFileName holds the exact value of the password
+	// quickstart's bootstrap_env() generated for the very first login on a
+	// fresh install (see tui-panel/server-panel.py) -- created alongside
+	// setupPendingFileName, never on its own. Storing the value itself
+	// (rather than just the file's existence) is what lets
+	// TemporaryPasswordMatches tell "still the generated one" apart from
+	// "an operator has since set their own", however that happened -- a
+	// manually typed .env edit included, since that never goes through this
+	// package at all.
+	passwordTemporaryFileName = ".admin_password_temporary"
 )
 
 // Info is the editable identity shown to clients.
@@ -71,6 +95,49 @@ func (s *Store) iconPath(ext string) string {
 	return filepath.Join(s.dir, iconBaseName+ext)
 }
 
+func (s *Store) setupPendingPath() string {
+	return filepath.Join(s.dir, setupPendingFileName)
+}
+
+func (s *Store) passwordTemporaryPath() string {
+	return filepath.Join(s.dir, passwordTemporaryFileName)
+}
+
+// SetupPending reports whether the first-run wizard still has work to do.
+// A deployment that predates this feature (upgraded from an older admin
+// binary, or one that was never bootstrapped through quickstart at all)
+// never had this file created for it, so it reads as "not pending" --
+// already done, no wizard -- regardless of what its identity.json happens
+// to contain. A nil Store (a minimal test fixture, say) reads the same way
+// -- "not pending" is the answer that costs nothing if it's wrong.
+func (s *Store) SetupPending() bool {
+	if s == nil {
+		return false
+	}
+	_, err := os.Stat(s.setupPendingPath())
+	return err == nil
+}
+
+// TemporaryPasswordMatches reports whether password is exactly the value
+// quickstart auto-generated for the very first login. cmd/telesrv-admin's
+// validSecret pairs this with SetupPending: the generated password
+// authenticates only until the wizard finishes, so a string that was
+// printed once to a terminal and never chosen by anyone doesn't go on
+// being a standing credential forever. It never matches a password an
+// operator set themselves, at any point -- there's no file to fool it
+// with, only an exact value comparison. A nil Store never matches, same
+// reasoning as SetupPending.
+func (s *Store) TemporaryPasswordMatches(password string) bool {
+	if s == nil || password == "" {
+		return false
+	}
+	stored, err := os.ReadFile(s.passwordTemporaryPath())
+	if err != nil {
+		return false
+	}
+	return subtle.ConstantTimeCompare(stored, []byte(password)) == 1
+}
+
 // Get reads the current identity. A missing file is not an error -- it just
 // means nothing has been configured yet, so Info{} (all empty) is returned.
 func (s *Store) Get() (Info, error) {
@@ -136,6 +203,25 @@ func (s *Store) SetLoginCodeMessageTemplate(template string) error {
 	return s.save(info)
 }
 
+// MarkSetupComplete removes the pending marker so SetupPending reads false
+// from here on. Idempotent -- calling it again once the marker is already
+// gone is a no-op, not an error.
+//
+// Deliberately leaves the temporary-password marker in place: validSecret
+// needs TemporaryPasswordMatches to keep recognizing that exact value
+// *after* setup completes, which is the whole mechanism that retires it --
+// deleting the marker here would make that check quietly stop matching and
+// the password would keep working forever, the opposite of the point.
+// Nothing about leaving it costs anything: it never matches a different
+// password (an operator's real one, whenever they set it), and this
+// package's only reader of it is that one comparison.
+func (s *Store) MarkSetupComplete() error {
+	if err := os.Remove(s.setupPendingPath()); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("identity: remove setup-pending marker: %w", err)
+	}
+	return nil
+}
+
 // SetIcon replaces the icon file (removing any previous one under a
 // different extension) and records its extension in identity.json.
 // ext must include the leading dot (e.g. ".png").

internal/identity/identity_test.go

@@ -197,3 +197,98 @@ func TestStoreWelcomeMessageTemplatesPreservedAcrossTextEdits(t *testing.T) {
 		t.Fatalf("welcome message templates lost after unrelated SetText: %+v", info)
 	}
 }
+
+// TestStoreSetupPendingSurvivesIdentityWrites is the regression case for the
+// bug where "setup complete" was inferred from identity.json's own content
+// (a non-empty name): the wizard's own Identity step calls SetText well
+// before Done is ever reached, which made that content-based check flip to
+// "done" mid-wizard -- a restart-and-reload partway through skipped
+// straight to the normal shell. SetupPending must stay true across any
+// number of unrelated identity writes and clear only via MarkSetupComplete.
+func TestStoreSetupPendingSurvivesIdentityWrites(t *testing.T) {
+	dir := t.TempDir()
+	s := NewStore(dir)
+
+	// Not pending at all until something (quickstart's bootstrap_env, in
+	// production) creates the marker -- an install this store never saw
+	// bootstrapped is treated as predating the wizard, not as mid-wizard.
+	if s.SetupPending() {
+		t.Fatal("expected SetupPending() == false before the marker file exists")
+	}
+
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, setupPendingFileName), nil, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if !s.SetupPending() {
+		t.Fatal("expected SetupPending() == true once the marker file exists")
+	}
+
+	// The Identity step's save, and everything else short of Done.
+	if err := s.SetText("Demo Server", "A test server"); err != nil {
+		t.Fatal(err)
+	}
+	if err := s.SetIcon([]byte{1, 2, 3}, ".png"); err != nil {
+		t.Fatal(err)
+	}
+	if !s.SetupPending() {
+		t.Fatal("expected SetupPending() to stay true after unrelated identity writes")
+	}
+
+	if err := s.MarkSetupComplete(); err != nil {
+		t.Fatal(err)
+	}
+	if s.SetupPending() {
+		t.Fatal("expected SetupPending() == false after MarkSetupComplete")
+	}
+
+	// Idempotent: calling it again once already gone is not an error.
+	if err := s.MarkSetupComplete(); err != nil {
+		t.Fatalf("MarkSetupComplete should be idempotent, got: %v", err)
+	}
+}
+
+// TestStoreTemporaryPasswordMatches covers the one-time-login password
+// quickstart generates: it must match only its own exact value, never an
+// operator-chosen one. MarkSetupComplete deliberately does NOT erase this
+// marker (see that method's doc comment) -- retiring the password is
+// validSecret's job, combining this with SetupPending; on its own,
+// TemporaryPasswordMatches keeps recognizing the same stored value even
+// after the wizard finishes, which is exactly what lets that combination
+// work at every login from then on, not just the first one after Done.
+func TestStoreTemporaryPasswordMatches(t *testing.T) {
+	dir := t.TempDir()
+	s := NewStore(dir)
+
+	if s.TemporaryPasswordMatches("anything") {
+		t.Fatal("expected no match before the marker file exists")
+	}
+	if s.TemporaryPasswordMatches("") {
+		t.Fatal("an empty password must never match")
+	}
+
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, passwordTemporaryFileName), []byte("generated-pw-123"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if !s.TemporaryPasswordMatches("generated-pw-123") {
+		t.Fatal("expected a match against the exact stored value")
+	}
+	if s.TemporaryPasswordMatches("something-an-operator-typed") {
+		t.Fatal("a different password must never match the marker")
+	}
+
+	if err := s.MarkSetupComplete(); err != nil {
+		t.Fatal(err)
+	}
+	if !s.TemporaryPasswordMatches("generated-pw-123") {
+		t.Fatal("expected the match to survive MarkSetupComplete -- see its doc comment for why")
+	}
+	if s.SetupPending() {
+		t.Fatal("expected SetupPending() == false after MarkSetupComplete regardless")
+	}
+}