diff --git a/db.go b/db.go index 70dc1c7..f80e44d 100644 --- a/db.go +++ b/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 diff --git a/db_test.go b/db_test.go index 86339e6..2a3f493 100644 --- a/db_test.go +++ b/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()) + } +} diff --git a/goblog.go b/goblog.go index f9b83f3..edb55c7 100644 --- a/goblog.go +++ b/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 } diff --git a/scripts/install-smoke-test.sh b/scripts/install-smoke-test.sh index 29e6941..0dc31b6 100755 --- a/scripts/install-smoke-test.sh +++ b/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" diff --git a/wizard_install_only_test.go b/wizard_install_only_test.go index 25b29ea..7ee5c7c 100644 --- a/wizard_install_only_test.go +++ b/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) {