Keep sessions across restarts and container upgrades

db.go

@@ -85,6 +85,35 @@ func dbConfigFromForm(c *gin.Context) dbConfig {
 	return cfg
 }
 
+// isDatabaseEnvKey reports whether key is one of the .env settings that
+// envFile writes, for any database type.
+func isDatabaseEnvKey(key string) bool {
+	return key == "database" || key == "sqlite_db" ||
+		strings.HasPrefix(key, "MYSQL_") || strings.HasPrefix(key, "POSTGRES_")
+}
+
+// mergedEnvFile is what the wizard's database step saves: the existing .env
+// with its database settings replaced by cfg's, and every other line kept.
+// The step used to write only envFile(), which threw away the session key
+// that startup had just put there, so the first restart after an install
+// signed everybody out (#667); it would equally have dropped SMTP or admin
+// pin settings an operator had placed in .env beforehand.
+func (cfg dbConfig) mergedEnvFile(existing string) string {
+	var kept []string
+	for _, line := range strings.Split(existing, "\n") {
+		key, _, _ := strings.Cut(strings.TrimSpace(line), "=")
+		if isDatabaseEnvKey(strings.TrimSpace(key)) {
+			continue
+		}
+		kept = append(kept, line)
+	}
+	out := strings.TrimRight(strings.Join(kept, "\n"), "\n")
+	if out != "" {
+		out += "\n"
+	}
+	return out + cfg.envFile()
+}
+
 // envFile renders the settings as the .env lines the wizard writes.
 func (cfg dbConfig) envFile() string {
 	var b strings.Builder

db_test.go

@@ -167,3 +167,23 @@ func writeTempEnv(t *testing.T, content string) string {
 	}
 	return path
 }
+
+// TestMergedEnvFile: the wizard's database step replaces the database
+// settings in .env and nothing else (#667).
+func TestMergedEnvFile(t *testing.T) {
+	existing := "SESSION_KEY=0123\n# mail\nsmtp_host=smtp.example.com\ndatabase=mysql\nMYSQL_HOST=old\nMYSQL_PASSWORD=old\nadmin_login=me\n"
+	got := dbConfig{Type: "sqlite", SQLiteFile: "blog.db"}.mergedEnvFile(existing)
+	want := "SESSION_KEY=0123\n# mail\nsmtp_host=smtp.example.com\nadmin_login=me\ndatabase=sqlite\nsqlite_db=blog.db\n"
+	if got != want {
+		t.Errorf("merged .env:\n%s\nwant:\n%s", got, want)
+	}
+	// Saving the step twice does not pile up lines.
+	if again := (dbConfig{Type: "sqlite", SQLiteFile: "blog.db"}).mergedEnvFile(got); again != want {
+		t.Errorf("merging twice:\n%s\nwant:\n%s", again, want)
+	}
+	// No .env yet, or an empty one: just the database lines.
+	cfg := dbConfig{Type: "postgres", Host: "db", Port: "5432", User: "u", Password: "p", Name: "n", SSLMode: "disable"}
+	if got := cfg.mergedEnvFile(""); got != cfg.envFile() {
+		t.Errorf("merging into nothing:\n%s\nwant:\n%s", got, cfg.envFile())
+	}
+}

goblog.go

@@ -481,10 +481,8 @@ func main() {
 	router.Use(gplugin.Middleware(registry))
 	store := cookie.NewStore([]byte(sessionKey))
 	store.Options(sessionOptions(true))
-	hostname, err := os.Hostname()
-	router.Use(sessions.Sessions(hostname, store))
+	router.Use(sessions.Sessions(sessionCookieName, store))
 	router.Use(sessionCookieSecurity(os.Getenv("SESSION_SECURE")))
-	log.Println("Hostname: ", hostname)
 	// Load templates from the active theme directory, falling back to "default".
 	// activeTheme is read by the static handler on every /theme/* request and
 	// written by loadTheme from admin requests (activate, update, settings),
@@ -602,17 +600,11 @@ func main() {
 		router.Use(static.Serve("/uploads", static.LocalFile(datadir.Path("uploads"), false)))
 	}
 	router.Use(static.Serve("/", static.LocalFile("www", false)))
-	if err != nil {
-		log.Println("Couldn't get the hostname")
-		return
-	}
-
 	if db != nil {
 		goblog.addRoutes()
 	}
 
-	err = router.Run(":7000")
-	if err != nil {
+	if err := router.Run(":7000"); err != nil {
 		log.Println("Error running goblog server: " + err.Error())
 	}
 }
@@ -720,6 +712,12 @@ func CORS() gin.HandlerFunc {
 	}
 }
 
+// sessionCookieName is the session cookie's name. It used to be the
+// machine's hostname, which Docker makes up afresh for every container, so
+// replacing the container (any upgrade) signed everybody out. Found while
+// fixing the session key that the wizard's database step dropped (#667).
+const sessionCookieName = "goblog_session"
+
 // sessionOptions returns the session cookie's attributes: HttpOnly and
 // SameSite=Lax, so a cross-site form post or fetch does not carry an admin's
 // session, and Secure as cookieSecure decides for the request.
@@ -853,7 +851,9 @@ func updateDB(c *gin.Context) {
 		fail("Invalid database type")
 		return
 	}
-	if err := os.WriteFile(datadir.Path(".env"), []byte(cfg.envFile()), 0600); err != nil {
+	// A missing .env reads as empty: the step then writes just its own lines.
+	existing, _ := os.ReadFile(datadir.Path(".env"))
+	if err := os.WriteFile(datadir.Path(".env"), []byte(cfg.mergedEnvFile(string(existing))), 0600); err != nil {
 		fail("Couldn't write the .env file: " + err.Error())
 		return
 	}

scripts/install-smoke-test.sh

@@ -245,6 +245,9 @@ else
   start_app
 fi
 check_site
+# The session key is in .env, so the browser the wizard logged in is still
+# logged in after a restart (#667).
+as_owner "GET /admin/dashboard with the session from the wizard" 200 "$BASE/admin/dashboard"
 
 step "signing in with the password"
 login "not the password" "/login?password=1&login_error=invalid"

wizard_install_only_test.go

@@ -105,12 +105,20 @@ func TestInstallOnly(t *testing.T) {
 
 	t.Run("no database yet", func(t *testing.T) {
 		t.Chdir(t.TempDir())
+		// What startup leaves in .env before the wizard runs.
+		if err := os.WriteFile(".env", []byte("SESSION_KEY=0123\n"), 0600); err != nil {
+			t.Fatal(err)
+		}
 		if w := post(app(nil), "/wizard_db"); w.Code != http.StatusSeeOther {
 			t.Errorf("POST /wizard_db before install: status %d, want 303", w.Code)
 		}
-		if got, _ := os.ReadFile(".env"); !strings.Contains(string(got), "sqlite_db=other.db") {
+		got, _ := os.ReadFile(".env")
+		if !strings.Contains(string(got), "sqlite_db=other.db") {
 			t.Errorf(".env not written by the wizard: %q", got)
 		}
+		if !strings.Contains(string(got), "SESSION_KEY=0123") {
+			t.Errorf("the database step dropped the session key from .env (#667): %q", got)
+		}
 	})
 
 	t.Run("database but no admin yet", func(t *testing.T) {