diff --git a/api/globalconfig/types.go b/api/globalconfig/types.go index 03365666d..0cc6d5ae5 100644 --- a/api/globalconfig/types.go +++ b/api/globalconfig/types.go @@ -7,7 +7,6 @@ import ( "time" "github.com/evcc-io/evcc/api" - "github.com/evcc-io/evcc/plugin/mqtt" "github.com/evcc-io/evcc/push" "github.com/evcc-io/evcc/server/eebus" "github.com/evcc-io/evcc/util/config" @@ -72,26 +71,47 @@ func (c Hems) Redacted() any { var _ api.Redactor = (*Mqtt)(nil) +func masked(s any) string { + if s != "" { + return "***" + } + return "" +} + type Mqtt struct { - mqtt.Config `mapstructure:",squash"` - Topic string `json:"topic"` + Broker string `json:"broker"` + Topic string `json:"topic"` + User string `json:"user"` + ClientID string `json:"clientID"` + Insecure bool `json:"insecure"` + Password string `json:"password"` + CaCert string `json:"caCert"` + ClientCert string `json:"clientCert"` + ClientKey string `json:"clientKey"` } // Redacted implements the redactor interface used by the tee publisher func (m Mqtt) Redacted() any { - // TODO add masked password return struct { - Broker string `json:"broker"` - Topic string `json:"topic"` - User string `json:"user,omitempty"` - ClientID string `json:"clientID,omitempty"` - Insecure bool `json:"insecure,omitempty"` + Broker string `json:"broker"` + Topic string `json:"topic"` + User string `json:"user,omitempty"` + ClientID string `json:"clientID,omitempty"` + Insecure bool `json:"insecure,omitempty"` + Password string `json:"password,omitempty"` + CaCert string `json:"caCert,omitempty"` + ClientCert string `json:"clientCert,omitempty"` + ClientKey string `json:"clientKey,omitempty"` }{ - Broker: m.Broker, - Topic: m.Topic, - User: m.User, - ClientID: m.ClientID, - Insecure: m.Insecure, + Broker: m.Broker, + Topic: m.Topic, + User: m.User, + ClientID: m.ClientID, + Insecure: m.Insecure, + Password: masked(m.Password), + CaCert: masked(m.CaCert), + ClientCert: masked(m.ClientCert), + ClientKey: masked(m.ClientKey), } } @@ -108,19 +128,22 @@ type Influx struct { // Redacted implements the redactor interface used by the tee publisher func (c Influx) Redacted() any { - // TODO add masked password return struct { URL string `json:"url"` Database string `json:"database"` Org string `json:"org"` User string `json:"user"` Insecure bool `json:"insecure"` + Password string `json:"password,omitempty"` + Token string `json:"token,omitempty"` }{ URL: c.URL, Database: c.Database, Org: c.Org, User: c.User, Insecure: c.Insecure, + Password: masked(c.Password), + Token: masked(c.Token), } } diff --git a/server/http.go b/server/http.go index 468353b21..eb7339333 100644 --- a/server/http.go +++ b/server/http.go @@ -271,8 +271,7 @@ func (s *HTTPd) RegisterSystemHandler(site *core.Site, valueChan chan<- util.Par keys.Mqtt: func() any { return new(globalconfig.Mqtt) }, // has default keys.Influx: func() any { return new(globalconfig.Influx) }, } { - // routes[key] = route{Method: "GET", Pattern: "/" + key, HandlerFunc: settingsGetJsonHandler(key, fun())} - routes["update"+key] = route{Method: "POST", Pattern: "/" + key, HandlerFunc: settingsSetJsonHandler(key, valueChan, fun())} + routes["update"+key] = route{Method: "POST", Pattern: "/" + key, HandlerFunc: settingsSetJsonHandler(key, valueChan, fun)} routes["delete"+key] = route{Method: "DELETE", Pattern: "/" + key, HandlerFunc: settingsDeleteJsonHandler(key, valueChan, fun())} } diff --git a/server/http_global_settings_handler.go b/server/http_global_settings_handler.go index 7c46a623e..9a1788739 100644 --- a/server/http_global_settings_handler.go +++ b/server/http_global_settings_handler.go @@ -9,8 +9,11 @@ import ( "strings" "time" + "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/server/db/settings" "github.com/evcc-io/evcc/util" + "github.com/fatih/structs" + "github.com/go-viper/mapstructure/v2" "github.com/gorilla/mux" "gopkg.in/yaml.v3" ) @@ -72,19 +75,9 @@ func settingsSetYamlHandler(key string, other, struc any) http.HandlerFunc { } } -// func settingsGetJsonHandler(key string, struc any) http.HandlerFunc { -// return func(w http.ResponseWriter, r *http.Request) { -// if err := settings.Json(key, &struc); err != nil && err != settings.ErrNotFound { -// jsonError(w, http.StatusInternalServerError, err) -// return -// } - -// jsonResult(w, struc) -// } -// } - -func settingsSetJsonHandler(key string, valueChan chan<- util.Param, struc any) http.HandlerFunc { +func settingsSetJsonHandler(key string, valueChan chan<- util.Param, newStruc func() any) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + struc := newStruc() dec := json.NewDecoder(r.Body) dec.DisallowUnknownFields() if err := dec.Decode(&struc); err != nil { @@ -92,6 +85,14 @@ func settingsSetJsonHandler(key string, valueChan chan<- util.Param, struc any) return } + oldStruc := newStruc() + if err := settings.Json(key, &oldStruc); err == nil { + if err := mergeSettings(oldStruc, struc); err != nil { + jsonError(w, http.StatusInternalServerError, err) + return + } + } + settings.SetJson(key, struc) setConfigDirty() @@ -111,3 +112,24 @@ func settingsDeleteJsonHandler(key string, valueChan chan<- util.Param, struc an jsonResult(w, true) } } + +func mergeSettings(old any, new any) error { + redactable, ok := old.(api.Redactor) + if !ok { + return nil + } + + newMap := structs.Map(new) + oldMap := structs.Map(old) + redactedMap := structs.Map(redactable.Redacted()) + + for k, v := range newMap { + if rv, ok := redactedMap[k]; ok && v == rv { + if ov, ok := oldMap[k]; ok { + newMap[k] = ov + } + } + } + + return mapstructure.Decode(newMap, &new) +} diff --git a/server/http_global_settings_handler_test.go b/server/http_global_settings_handler_test.go new file mode 100644 index 000000000..84f3ccd38 --- /dev/null +++ b/server/http_global_settings_handler_test.go @@ -0,0 +1,62 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeSettings(t *testing.T) { + tests := []struct { + old any + new *RedactedStruct + expected *RedactedStruct + }{ + { + old: nil, + new: &RedactedStruct{"newValue1", 42}, + expected: &RedactedStruct{"newValue1", 42}, + }, + { + old: &TestStruct{"oldValue1", 24}, + new: &RedactedStruct{"newValue1", 42}, + expected: &RedactedStruct{"newValue1", 42}, + }, + { + old: &RedactedStruct{"oldValue1", 24}, + new: &RedactedStruct{"redacted", 42}, + expected: &RedactedStruct{"oldValue1", 42}, + }, + { + old: &RedactedStruct{"oldValue1", 24}, + new: &RedactedStruct{"newValue1", 42}, + expected: &RedactedStruct{"newValue1", 42}, + }, + } + + for _, tc := range tests { + mergeSettings(tc.old, tc.new) + assert.Equal(t, tc.expected.Field1, tc.new.Field1) + assert.Equal(t, tc.expected.Field2, tc.new.Field2) + } +} + +type TestStruct struct { + Field1 string + Field2 int +} + +type RedactedStruct struct { + Field1 string + Field2 int +} + +func (t *RedactedStruct) Redacted() any { + return struct { + Field1 string + Field2 int + }{ + Field1: "redacted", + Field2: t.Field2, + } +} diff --git a/tests/config-mqtt.spec.js b/tests/config-mqtt.spec.js index 98f169293..47c107b57 100644 --- a/tests/config-mqtt.spec.js +++ b/tests/config-mqtt.spec.js @@ -16,6 +16,14 @@ test.beforeEach(async ({ page }) => { test.afterEach(async () => { await stop(); }); + +const VALID_BROKER = "test.mosquitto.org:1884"; +const INVALID_BROKER = "unknown.example.org"; +const VALID_TOPIC = "my-topic"; +const VALID_CLIENT_ID = "my-client-id"; +const VALID_USERNAME = "rw"; +const VALID_PASSWORD = "readwrite"; + test.describe("mqtt", async () => { test("mqtt not configured", async ({ page }) => { await expect(page.getByTestId("mqtt")).toBeVisible(); @@ -26,10 +34,12 @@ test.describe("mqtt", async () => { await page.getByTestId("mqtt").getByRole("button", { name: "edit" }).click(); const modal = await page.getByTestId("mqtt-modal"); - await modal.getByLabel("Broker").fill("unknown.example.org"); - await modal.getByLabel("Topic").fill(" my-topic "); // whitespace should be trimmed - await modal.getByLabel("Client ID").fill("my-client-id"); - + // setup with invalid broker + await modal.getByLabel("Broker").fill(INVALID_BROKER); + await modal.getByLabel("Topic").fill(" " + VALID_TOPIC + " "); // whitespace should be trimmed + await modal.getByLabel("Client ID").fill(VALID_CLIENT_ID); + await modal.getByLabel("Username").fill(VALID_USERNAME); + await modal.getByLabel("Password").fill(VALID_PASSWORD); await page.getByRole("button", { name: "Save" }).click(); await expect(modal.getByTestId("error")).not.toBeVisible(); await expect(modal).not.toBeVisible(); @@ -46,17 +56,29 @@ test.describe("mqtt", async () => { await expect(restartButton).not.toBeVisible(); await expect(page.getByTestId("mqtt")).toHaveClass(/round-box--error/); await expect(page.getByTestId("mqtt")).toContainText( - ["Broker", "unknown.example.org", "Topic", "my-topic"].join("") + ["Broker", INVALID_BROKER, "Topic", VALID_TOPIC].join("") ); - await expect(page.getByTestId("bottom-banner")).toContainText("failed configuring mqtt"); + await expect(page.getByTestId("fatal-error")).toContainText("failed configuring mqtt"); await page.getByTestId("mqtt").getByRole("button", { name: "edit" }).click(); - await page.getByRole("button", { name: "Remove" }).click(); - await expect(page.getByTestId("mqtt")).toContainText(["Configured", "no"].join("")); - await expect(restartButton).toBeVisible(); + await expect(modal.getByLabel("Broker")).toHaveValue(INVALID_BROKER); + await expect(modal.getByLabel("Topic")).toHaveValue(VALID_TOPIC); // whitespace has been trimmed + await expect(modal.getByLabel("Client ID")).toHaveValue(VALID_CLIENT_ID); + await expect(modal.getByLabel("Username")).toHaveValue(VALID_USERNAME); + await expect(modal.getByLabel("Password")).toHaveValue("***"); + + // use valid broker + await modal.getByLabel("Broker").fill(VALID_BROKER); + await modal.getByRole("button", { name: "Save" }).click(); + await expect(page.getByTestId("mqtt")).toContainText( + ["Broker", VALID_BROKER, "Topic", VALID_TOPIC].join("") + ); await restart(CONFIG); - await expect(restartButton).not.toBeVisible(); + + await expect(page.getByTestId("fatal-error")).not.toBeVisible(); await expect(page.getByTestId("mqtt")).not.toHaveClass(/round-box--error/); - await expect(page.getByTestId("mqtt")).toContainText(["Configured", "no"].join("")); + await expect(page.getByTestId("mqtt")).toContainText( + ["Broker", VALID_BROKER, "Topic", VALID_TOPIC].join("") + ); }); });