Enhance settings management with persistence and explicit saving, including SAN updates and unit tests

cmd/soundtouch-service/main.go

@@ -136,9 +136,37 @@ func main() {
 		Action: func(c *cli.Context) error {
 			config := loadConfig(c)
 			ds := initDataStore(config.dataDir)
+
+			// Load settings from datastore
+			persisted, _ := ds.GetSettings()
+			if persisted.ServerURL != "" {
+				config.serverURL = persisted.ServerURL
+			}
+
+			if persisted.ProxyURL != "" {
+				config.targetURL = persisted.ProxyURL
+			}
+
+			if persisted.HTTPServerURL != "" {
+				config.httpsServerURL = persisted.HTTPServerURL
+			}
+
+			config.redact = persisted.RedactLogs || config.redact
+			config.logBody = persisted.LogBodies || config.logBody
+			config.record = persisted.RecordInteractions || config.record
+
+			// Recalculate domains if settings changed
+			hostname, _ := os.Hostname()
+			if hostname == "" {
+				hostname = "localhost"
+			}
+
+			config.domains = getDomains(config.serverURL, config.httpsServerURL, hostname)
+
 			cm := initCertificateManager(config.dataDir)
 			sm := setup.NewManager(config.serverURL, ds, cm)
 			server := handlers.NewServer(ds, sm, config.serverURL, config.redact, config.logBody, config.record)
+			server.SetHTTPServerURL(config.httpsServerURL)
 
 			recorder := proxy.NewRecorder(config.dataDir)
 			recorder.Redact = config.redact
@@ -426,6 +454,7 @@ func setupRouter(server *handlers.Server, pyProxy *httputil.ReverseProxy) *chi.M
 		r.Post("/discover", server.HandleTriggerDiscovery)
 		r.Get("/discovery-status", server.HandleGetDiscoveryStatus)
 		r.Get("/settings", server.HandleGetSettings)
+		r.Post("/settings", server.HandleUpdateSettings)
 		r.Get("/info/{deviceIP}", server.HandleGetDeviceInfo)
 		r.Get("/summary/{deviceIP}", server.HandleGetMigrationSummary)
 		r.Post("/migrate/{deviceIP}", server.HandleMigrateDevice)

data/.gitignore

@@ -1,3 +1,4 @@
 certs/
 default/
 interactions/
+settings.json

docs/guides/SOUNDTOUCH-SERVICE.md

@@ -136,7 +136,17 @@ Use the web interface or API to migrate devices from Bose cloud services to your
 
 ## Configuration
 
-The service can be configured via environment variables or command-line flags:
+### Configuration Precedence
+
+The service supports multiple ways to configure its behavior. When multiple sources provide the same setting, the following precedence rules apply (highest to lowest):
+
+1.  **`settings.json`**: Settings saved via the Web UI (stored in the data directory) take the highest precedence. This ensures that changes made in the browser persist across service restarts even if environment variables or flags change.
+2.  **Environment Variables / CLI Flags**: If a setting is not present in `settings.json`, environment variables and flags are used.
+3.  **Default Values**: If no configuration is provided, the service uses its built-in defaults.
+
+> **Tip**: If you find that changes to environment variables are not taking effect, check the **Settings** tab in the Web UI or inspect the `settings.json` file in your data directory, as it might be overriding your manual configuration.
+
+### Configuration Options
 
 | Variable                           | Flag                       | Description                                      | Default                   |
 |------------------------------------|----------------------------|--------------------------------------------------|---------------------------|

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/go-chi/chi/v5 v5.2.5
 	github.com/gorilla/websocket v1.5.3
 	github.com/hashicorp/mdns v1.0.6
+	github.com/russross/blackfriday/v2 v2.1.0
 	github.com/urfave/cli/v2 v2.27.7
 	golang.org/x/crypto v0.47.0
 )
@@ -13,7 +14,6 @@ require (
 require (
 	github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect
 	github.com/miekg/dns v1.1.72 // indirect
-	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 // indirect
 	golang.org/x/mod v0.32.0 // indirect
 	golang.org/x/net v0.49.0 // indirect

pkg/service/datastore/datastore.go

@@ -706,6 +706,60 @@ func (ds *DataStore) GetETagForAccount(account, device string) int64 {
 	return maxETag
 }
 
+// Settings represents the global service settings.
+type Settings struct {
+	ServerURL          string `json:"server_url"`
+	ProxyURL           string `json:"proxy_url"`
+	HTTPServerURL      string `json:"https_server_url,omitempty"`
+	RedactLogs         bool   `json:"redact_logs"`
+	LogBodies          bool   `json:"log_bodies"`
+	RecordInteractions bool   `json:"record_interactions"`
+}
+
+// GetSettings retrieves the global service settings.
+func (ds *DataStore) GetSettings() (Settings, error) {
+	if ds == nil || ds.DataDir == "" {
+		return Settings{}, nil
+	}
+
+	path := filepath.Join(ds.DataDir, "settings.json")
+	if !exists(path) {
+		return Settings{}, nil
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return Settings{}, err
+	}
+
+	var settings Settings
+	if err := json.Unmarshal(data, &settings); err != nil {
+		return Settings{}, err
+	}
+
+	return settings, nil
+}
+
+// SaveSettings saves the global service settings.
+func (ds *DataStore) SaveSettings(settings Settings) error {
+	if ds == nil || ds.DataDir == "" {
+		return nil
+	}
+
+	if err := os.MkdirAll(ds.DataDir, 0755); err != nil {
+		return fmt.Errorf("failed to create data directory: %w", err)
+	}
+
+	path := filepath.Join(ds.DataDir, "settings.json")
+
+	data, err := json.MarshalIndent(settings, "", "  ")
+	if err != nil {
+		return err
+	}
+
+	return os.WriteFile(path, data, 0644)
+}
+
 // SaveUsageStats saves usage statistics to the datastore.
 func (ds *DataStore) SaveUsageStats(stats models.UsageStats) error {
 	dir := filepath.Join(ds.DataDir, "stats", "usage")

pkg/service/datastore/datastore_test.go

@@ -365,3 +365,36 @@ func TestConfiguredSources(t *testing.T) {
 		t.Error("Expected auto-assigned ID for source with empty ID")
 	}
 }
+
+func TestSettingsPersistence(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "settings-test-*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	ds := NewDataStore(tempDir)
+
+	settings := Settings{
+		ServerURL: "http://myserver:8000",
+		ProxyURL:  "http://myproxy:8001",
+		LogBodies: true,
+	}
+
+	err = ds.SaveSettings(settings)
+	if err != nil {
+		t.Fatalf("SaveSettings failed: %v", err)
+	}
+
+	loaded, err := ds.GetSettings()
+	if err != nil {
+		t.Fatalf("GetSettings failed: %v", err)
+	}
+
+	if loaded.ServerURL != settings.ServerURL {
+		t.Errorf("Expected ServerURL %s, got %s", settings.ServerURL, loaded.ServerURL)
+	}
+	if loaded.LogBodies != settings.LogBodies {
+		t.Errorf("Expected LogBodies %v, got %v", settings.LogBodies, loaded.LogBodies)
+	}
+}

pkg/service/handlers/handlers_setup.go

@@ -3,10 +3,12 @@ package handlers
 import (
 	"context"
 	"encoding/json"
+	"log"
 	"net/http"
 	"os"
 
 	"github.com/gesellix/bose-soundtouch/pkg/models"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 	"github.com/gesellix/bose-soundtouch/pkg/service/setup"
 	"github.com/go-chi/chi/v5"
 )
@@ -93,15 +95,68 @@ func (s *Server) HandleGetDiscoveryStatus(w http.ResponseWriter, _ *http.Request
 func (s *Server) HandleGetSettings(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 
+	serverURL, proxyURL, httpsServerURL := s.GetSettings()
+
 	if err := json.NewEncoder(w).Encode(map[string]string{
-		"server_url": s.serverURL,
-		"proxy_url":  s.proxyURL,
+		"server_url":       serverURL,
+		"proxy_url":        proxyURL,
+		"https_server_url": httpsServerURL,
 	}); err != nil {
 		http.Error(w, "Failed to encode response", http.StatusInternalServerError)
 		return
 	}
 }
 
+// HandleUpdateSettings updates the service settings.
+func (s *Server) HandleUpdateSettings(w http.ResponseWriter, r *http.Request) {
+	var settings struct {
+		ServerURL string `json:"server_url"`
+		ProxyURL  string `json:"proxy_url"`
+	}
+	if err := json.NewDecoder(r.Body).Decode(&settings); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	s.mu.Lock()
+	s.serverURL = settings.ServerURL
+	s.proxyURL = settings.ProxyURL
+
+	if s.sm != nil {
+		s.sm.ServerURL = settings.ServerURL
+	}
+
+	// Persist to datastore
+	// Access fields directly since we already hold the lock
+	currentRedact := s.proxyRedact
+	currentLogBody := s.proxyLogBody
+	currentRecord := s.recordEnabled
+	currentHTTPS := s.httpsServerURL
+
+	log.Printf("Saving updated settings to %s/settings.json", s.ds.DataDir)
+	err := s.ds.SaveSettings(datastore.Settings{
+		ServerURL:          s.serverURL,
+		ProxyURL:           s.proxyURL,
+		HTTPServerURL:      currentHTTPS,
+		RedactLogs:         currentRedact,
+		LogBodies:          currentLogBody,
+		RecordInteractions: currentRecord,
+	})
+	s.mu.Unlock()
+
+	if err != nil {
+		http.Error(w, "Failed to save settings: "+err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+
+	if err := json.NewEncoder(w).Encode(map[string]interface{}{"ok": true, "message": "Settings updated"}); err != nil {
+		http.Error(w, "Failed to encode response", http.StatusInternalServerError)
+		return
+	}
+}
+
 // HandleGetDeviceInfo returns live information for a device.
 func (s *Server) HandleGetDeviceInfo(w http.ResponseWriter, r *http.Request) {
 	deviceIP := chi.URLParam(r, "deviceIP")
@@ -389,10 +444,12 @@ func (s *Server) HandleBackupConfig(w http.ResponseWriter, r *http.Request) {
 func (s *Server) HandleGetProxySettings(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 
+	redact, logBody, record := s.GetProxySettings()
+
 	if err := json.NewEncoder(w).Encode(map[string]bool{
-		"redact":   s.proxyRedact,
-		"log_body": s.proxyLogBody,
-		"record":   s.recordEnabled,
+		"redact":   redact,
+		"log_body": logBody,
+		"record":   record,
 	}); err != nil {
 		http.Error(w, "Failed to encode response", http.StatusInternalServerError)
 		return
@@ -426,10 +483,31 @@ func (s *Server) HandleUpdateProxySettings(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
+	s.mu.Lock()
 	s.proxyRedact = settings.Redact
 	s.proxyLogBody = settings.LogBody
 	s.recordEnabled = settings.Record
 
+	// Persist to datastore
+	// Access fields directly since we already hold the lock
+	serverURL, proxyURL, httpsServerURL := s.serverURL, s.proxyURL, s.httpsServerURL
+
+	log.Printf("Saving updated proxy settings to %s/settings.json", s.ds.DataDir)
+	err := s.ds.SaveSettings(datastore.Settings{
+		ServerURL:          serverURL,
+		ProxyURL:           proxyURL,
+		HTTPServerURL:      httpsServerURL,
+		RedactLogs:         s.proxyRedact,
+		LogBodies:          s.proxyLogBody,
+		RecordInteractions: s.recordEnabled,
+	})
+	s.mu.Unlock()
+
+	if err != nil {
+		http.Error(w, "Failed to save settings: "+err.Error(), http.StatusInternalServerError)
+		return
+	}
+
 	w.Header().Set("Content-Type", "application/json")
 
 	if err := json.NewEncoder(w).Encode(map[string]interface{}{"ok": true, "message": "Proxy settings updated"}); err != nil {

pkg/service/handlers/handlers_setup_test.go

@@ -15,7 +15,16 @@ import (
 )
 
 func TestProxySettingsAPI(t *testing.T) {
-	r, server := setupRouter("http://localhost:8001", nil)
+	tempDir, err := os.MkdirTemp("", "proxy-settings-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	ds := datastore.NewDataStore(tempDir)
+	_ = ds.Initialize()
+
+	r, server := setupRouter("http://localhost:8001", ds)
 
 	ts := httptest.NewServer(r)
 	defer ts.Close()
@@ -86,6 +95,34 @@ func TestProxySettingsAPI(t *testing.T) {
 	if settings["redact"] != false || settings["log_body"] != true {
 		t.Errorf("GET (after update): Unexpected settings: %+v", settings)
 	}
+
+	// 3. Test System Settings POST
+	sysUpdate := map[string]string{
+		"server_url": "http://new-server:8000",
+		"proxy_url":  "http://new-proxy:8001",
+	}
+
+	sysBody, err := json.Marshal(sysUpdate)
+	if err != nil {
+		t.Fatalf("Failed to marshal system settings data: %v", err)
+	}
+
+	res, err = http.Post(ts.URL+"/setup/settings", "application/json", bytes.NewBuffer(sysBody))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() { _ = res.Body.Close() }()
+
+	if res.StatusCode != http.StatusOK {
+		t.Errorf("POST /setup/settings: Expected status OK, got %v", res.Status)
+	}
+
+	// Verify server state
+	sURL, pURL, _ := server.GetSettings()
+	if sURL != "http://new-server:8000" || pURL != "http://new-proxy:8001" {
+		t.Errorf("POST /setup/settings: Server state did not update: serverURL=%s, proxyURL=%s", sURL, pURL)
+	}
 }
 
 func TestMigrationAndCA(t *testing.T) {

pkg/service/handlers/main_test.go

@@ -47,6 +47,8 @@ func setupRouter(targetURL string, ds *datastore.DataStore) (*chi.Mux, *Server)
 
 	// Setup Setup for tests
 	r.Route("/setup", func(r chi.Router) {
+		r.Get("/settings", server.HandleGetSettings)
+		r.Post("/settings", server.HandleUpdateSettings)
 		r.Get("/proxy-settings", server.HandleGetProxySettings)
 		r.Post("/proxy-settings", server.HandleUpdateProxySettings)
 		r.Post("/ensure-remote-services/{deviceIP}", server.HandleEnsureRemoteServices)

pkg/service/handlers/server.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"context"
 	"log"
+	"sync"
 	"time"
 
 	"github.com/gesellix/bose-soundtouch/pkg/discovery"
@@ -14,15 +15,17 @@ import (
 
 // Server handles HTTP requests for the SoundTouch service.
 type Server struct {
-	ds            *datastore.DataStore
-	sm            *setup.Manager
-	serverURL     string
-	proxyURL      string
-	discovering   bool
-	proxyRedact   bool
-	proxyLogBody  bool
-	recordEnabled bool
-	recorder      *proxy.Recorder
+	ds             *datastore.DataStore
+	sm             *setup.Manager
+	mu             sync.RWMutex
+	serverURL      string
+	proxyURL       string
+	httpsServerURL string
+	discovering    bool
+	proxyRedact    bool
+	proxyLogBody   bool
+	recordEnabled  bool
+	recorder       *proxy.Recorder
 }
 
 // NewServer creates a new SoundTouch service server.
@@ -38,6 +41,14 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, pro
 	}
 }
 
+// SetHTTPServerURL sets the external HTTPS URL of the service.
+func (s *Server) SetHTTPServerURL(url string) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.httpsServerURL = url
+}
+
 // SetRecorder sets the recorder for the server.
 func (s *Server) SetRecorder(r *proxy.Recorder) {
 	s.recorder = r
@@ -45,9 +56,28 @@ func (s *Server) SetRecorder(r *proxy.Recorder) {
 
 // GetRecordEnabled returns whether recording is enabled.
 func (s *Server) GetRecordEnabled() bool {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
 	return s.recordEnabled
 }
 
+// GetSettings returns the current server settings.
+func (s *Server) GetSettings() (string, string, string) {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	return s.serverURL, s.proxyURL, s.httpsServerURL
+}
+
+// GetProxySettings returns the current proxy settings.
+func (s *Server) GetProxySettings() (bool, bool, bool) {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	return s.proxyRedact, s.proxyLogBody, s.recordEnabled
+}
+
 // DiscoverDevices starts a background device discovery process.
 //
 //nolint:contextcheck

pkg/service/handlers/web/index.html

@@ -89,6 +89,10 @@
                 <input type="text" id="proxy-domain" placeholder="http://192.168.x.x:8000" style="width: 300px;">
                 <span style="font-size: 0.8em; color: #666;">(Upstream proxy URL)</span>
             </div>
+            <div style="margin-bottom: 20px;">
+                <button onclick="updateSettings()">Save Settings</button>
+                <span id="settings-status" style="margin-left: 10px; font-size: 0.9em;"></span>
+            </div>
             <div style="margin-bottom: 20px;">
                 Proxy Logging:
                 <label><input type="checkbox" id="proxy-redact" onchange="updateProxySettings()"> Redact Sensitive Headers</label>

pkg/service/handlers/web/js/script.js

@@ -43,6 +43,36 @@ async function updateProxySettings() {
     }
 }
 
+async function updateSettings() {
+    const settings = {
+        server_url: document.getElementById('target-domain').value,
+        proxy_url: document.getElementById('proxy-domain').value
+    };
+    const status = document.getElementById('settings-status');
+    status.innerText = 'Saving...';
+    status.style.color = 'blue';
+
+    try {
+        const response = await fetch('/setup/settings', {
+            method: 'POST',
+            headers: { 'Content-Type': 'application/json' },
+            body: JSON.stringify(settings)
+        });
+        if (response.ok) {
+            status.innerText = '✅ Settings saved. Restart service to apply all changes (like certificate SANs).';
+            status.style.color = 'green';
+            setTimeout(() => fetchSettings(), 500); // Give backend a moment to settle
+        } else {
+            const err = await response.text();
+            status.innerText = '❌ Failed: ' + err;
+            status.style.color = 'red';
+        }
+    } catch (error) {
+        status.innerText = '❌ Error: ' + error.message;
+        status.style.color = 'red';
+    }
+}
+
 async function fetchDevices() {
     try {
         const response = await fetch('/setup/devices');

pkg/service/setup/setup_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/gesellix/bose-soundtouch/pkg/service/certmanager"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 )
 
 type mockSSH struct {
@@ -762,6 +763,97 @@ func TestReboot(t *testing.T) {
 	}
 }
 
+func TestBackupConfigOffDevice(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "backup-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	ds := datastore.NewDataStore(tempDir)
+	_ = ds.Initialize()
+
+	m := NewManager("http://localhost:8000", ds, nil)
+
+	serial := "08DF1F0BA325"
+
+	// Mock info server
+	infoServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		fmt.Fprintf(w, `<info deviceID="%s"><name>Test</name><components><component><componentCategory>SCM</componentCategory><serialNumber>%s</serialNumber></component></components></info>`, serial, serial)
+	}))
+	defer infoServer.Close()
+
+	// Extract IP and port
+	deviceIP := infoServer.Listener.Addr().String()
+
+	// Mock SSH to return some config and hosts content
+	m.NewSSH = func(host string) SSHClient {
+		return &mockSSH{
+			runFunc: func(command string) (string, error) {
+				if strings.Contains(command, SoundTouchSdkPrivateCfgPath) {
+					return "<SoundTouchSdkPrivateCfg><margeServerUrl>http://original</margeServerUrl></SoundTouchSdkPrivateCfg>", nil
+				}
+				if strings.Contains(command, "/etc/hosts") {
+					return "127.0.0.1 localhost\n192.168.1.1 bmx.bose.com", nil
+				}
+				return "", nil
+			},
+		}
+	}
+
+	err = m.BackupConfigOffDevice(deviceIP)
+	if err != nil {
+		t.Fatalf("BackupConfigOffDevice failed: %v", err)
+	}
+
+	// Verify files were created in datastore
+	deviceDir := m.DataStore.AccountDeviceDir("default", serial)
+	configPath := filepath.Join(deviceDir, "SoundTouchSdkPrivateCfg.xml.bak")
+	hostsPath := filepath.Join(deviceDir, "hosts.bak")
+
+	if _, err := os.Stat(configPath); os.IsNotExist(err) {
+		t.Errorf("Expected config backup at %s, but it doesn't exist", configPath)
+	}
+
+	if _, err := os.Stat(hostsPath); os.IsNotExist(err) {
+		t.Errorf("Expected hosts backup at %s, but it doesn't exist", hostsPath)
+	}
+
+	// Verify content
+	configContent, _ := os.ReadFile(configPath)
+	if !strings.Contains(string(configContent), "http://original") {
+		t.Errorf("Unexpected config backup content: %s", string(configContent))
+	}
+
+	hostsContent, _ := os.ReadFile(hostsPath)
+	if !strings.Contains(string(hostsContent), "bmx.bose.com") {
+		t.Errorf("Unexpected hosts backup content: %s", string(hostsContent))
+	}
+}
+
+func TestMigrateSpeaker_PreFlightFailure(t *testing.T) {
+	m := NewManager("http://localhost:8000", nil, nil)
+	m.NewSSH = func(host string) SSHClient {
+		return &mockSSH{
+			runFunc: func(command string) (string, error) {
+				if strings.Contains(command, "mount -o remount,rw /") {
+					return "mount: / is read-only", fmt.Errorf("remount failed")
+				}
+				return "", nil
+			},
+		}
+	}
+
+	_, err := m.MigrateSpeaker("192.168.1.10", "", "", nil, MigrationMethodXML)
+	if err == nil {
+		t.Errorf("Expected error during pre-flight write check, got nil")
+	}
+	if !strings.Contains(err.Error(), "pre-flight check failed") {
+		t.Errorf("Expected pre-flight error message, got: %v", err)
+	}
+}
+
 func contains(s, substr string) bool {
 	return strings.Contains(s, substr)
 }