aaribaud/ESPSomfy-RTS
0
0
Fork 0
mirror of https://github.com/rstrouse/ESPSomfy-RTS.git synced 2026-10-31 13:52:15 +01:00
Code Issues Projects Releases Packages Wiki Activity

fix: keep stored secrets when an older UI sends them empty

Browse source 
Firmware and UI files update separately, so an old index.js can run
against firmware that no longer returns secrets. It sends "" for the
field it can't fill in, which cleared the WiFi passphrase (reboot into
AP mode) or the MQTT password.

The firmware now decides: "" keeps the stored secret, unless the same
request changes the SSID or MQTT username, then it clears it (open
network). /setNetwork compares the settings after applying them, so a
kept passphrase no longer triggers a reboot. The UI cache-buster moves
to 3.0.14d.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
cjkas 2026-10-18 20:07:31 +02:00
parent 1fb4c4cfcd
commit 9383ebec2e
6 changed files with 77 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

8
data-src/index.html
View file

@ -8,9 +8,9 @@
<meta name="apple-mobile-web-app-title" content="ESPSomfy RTS App">
<meta name="apple-mobile-web-app-status-bar-style" content="black">
<link rel="stylesheet" href="main.css?v=3.0.14c" type="text/css" />
<link rel="stylesheet" href="widgets.css?v=3.0.14c" type="text/css" />
<link rel="stylesheet" href="icons.css?v=3.0.14c" type="text/css" />
<link rel="stylesheet" href="main.css?v=3.0.14d" type="text/css" />
<link rel="stylesheet" href="widgets.css?v=3.0.14d" type="text/css" />
<link rel="stylesheet" href="icons.css?v=3.0.14d" type="text/css" />
<link rel="icon" type="image/png" href="favicon.png" />
<!-- iPad retina icon -->
@ -114,7 +114,7 @@
rel="apple-touch-startup-image">
<script type="text/javascript" src="index.js?v=3.0.14c"></script>
<script type="text/javascript" src="index.js?v=3.0.14d"></script>
</head>
<body>
<div id="divContainer" class="container main" data-auth="false">

19
docs/KNOWN_ISSUES.md
View file

@ -110,13 +110,21 @@ It needs a group whose flags are out of date at the moment a client reads it, fo
**Status:** open. A workaround exists (below).
**Introduced:** 2026-10-18, when the device stopped sending stored secrets back to clients.
**Location:** `keepSavedPassphrase` in `Wifi` and `connectMQTT` in `MQTT`, both in [data-src/index.js](../data-src/index.js). The firmware side is `applySecret` in [src/Secrets.h](../src/Secrets.h).
**Location:** `applySecret` in [src/Secrets.h](../src/Secrets.h), called from `WifiSettings::fromJSON` and `MQTTSettings::fromJSON` in [src/ConfigSettings.cpp](../src/ConfigSettings.cpp) and from `/connectwifi`. The UI side is `keepSavedPassphrase` in `Wifi` and `connectMQTT` in `MQTT`, in [data-src/index.js](../data-src/index.js).
### Summary
The settings routes report only whether a secret is stored (`hasPassphrase`, `hasPassword`), so the UI loads every secret field empty. When the field is still blank on save, the UI treats it as "keep the saved value" and leaves the key out of the request, and `applySecret` keeps a value whose key is absent. Without that rule, saving any other setting on the page would wipe the secret.
The settings routes report only whether a secret is stored (`hasPassphrase`, `hasPassword`), so the UI loads every secret field empty. The firmware decides what an empty secret in a request means, so that every UI version is safe, including one older than this change, which sends `""` for the field it can no longer fill in:
The cost: a blank field can't mean "no password" while the other half of the login stays the same. That is the SSID for WiFi and the username for MQTT. When it does change, a blank field is sent as an empty value and clears the secret, so switching to a different open network works.
| Secret | Key absent | `""` | A value |
|---|---|---|---|
| WiFi passphrase | keep | keep, or clear if the same request changes the SSID | replace |
| MQTT password | keep | keep, or clear if the same request changes the username | replace |
| Login password, PIN | keep | keep | replace |
Without "empty keeps", saving any other setting from an old UI would wipe the secret: for WiFi the device would reboot and drop into AP mode.
The cost: an empty value can't mean "no password" while the SSID or username stays the same. Switching to a different open network works.
The login password and PIN aren't affected. Neither can be empty, and security is turned off with the type selector.
@ -127,8 +135,9 @@ The login password and PIN aren't affected. Neither can be empty, and security i
### Workaround
Save once with a different SSID or MQTT username, then change it back and save again with the secret field blank.
- **MQTT:** save once with a different username, then change it back and save again with the password blank.
- **WiFi:** there's no safe workaround. Saving a different SSID makes the device reboot and try to join that network.
### Fix options
- **(a) "Clear saved password" checkbox** next to the WiFi passphrase and MQTT password fields, shown only when a value is stored. Ticking it sends an explicit empty value, which `applySecret` already treats as "clear" (covered by `test_applySecret_empty_value_clears_stored`). This is a UI-only change.
- **(a) "Clear saved password" checkbox** next to the WiFi passphrase and MQTT password fields, shown only when a value is stored. Since `""` means keep, ticking it sends an explicit flag (`clearPassphrase` / `clearPassword`), which `fromJSON` turns into `applySecret(..., identityChanged = true)`.

10
src/ConfigSettings.cpp
View file

@ -312,8 +312,11 @@ bool MQTTSettings::fromJSON(JsonObject &obj) {
if(!obj["pubDisco"].isNull()) this->pubDisco = obj["pubDisco"];
this->parseValueString(obj, "protocol", this->protocol, sizeof(this->protocol));
this->parseValueString(obj, "hostname", this->hostname, sizeof(this->hostname));
// An empty password clears the stored one only alongside a new username.
const char *username = obj["username"].as<const char *>();
bool newUsername = username && strcmp(username, this->username) != 0;
this->parseValueString(obj, "username", this->username, sizeof(this->username));
applySecret(obj["password"].as<const char *>(), this->password, sizeof(this->password));
applySecret(obj["password"].as<const char *>(), this->password, sizeof(this->password), newUsername);
this->parseValueString(obj, "rootTopic", this->rootTopic, sizeof(this->rootTopic));
this->parseValueString(obj, "discoTopic", this->discoTopic, sizeof(this->discoTopic));
if(!obj["port"].isNull()) this->port = obj["port"];
@ -526,8 +529,11 @@ bool WifiSettings::begin() {
return true;
}
bool WifiSettings::fromJSON(JsonObject &obj) {
// An empty passphrase clears the stored one only alongside a new SSID (an open network).
const char *ssid = obj["ssid"].as<const char *>();
bool newSsid = ssid && strcmp(ssid, this->ssid) != 0;
this->parseValueString(obj, "ssid", this->ssid, sizeof(this->ssid));
applySecret(obj["passphrase"].as<const char *>(), this->passphrase, sizeof(this->passphrase));
applySecret(obj["passphrase"].as<const char *>(), this->passphrase, sizeof(this->passphrase), newSsid);
if(!obj["roaming"].isNull()) this->roaming = obj["roaming"];
if(!obj["hidden"].isNull()) this->hidden = obj["hidden"];
return true;

13
src/Secrets.h
View file

@ -9,12 +9,17 @@ inline bool hasSecret(const char *s) { return s && s[0] != '\0'; }
inline const char *maskSecret(const char *s) { return hasSecret(s) ? "<set>" : "<empty>"; }
// Updates dest from an incoming value, truncating to size. A null incoming
// value means the client left the field out, so the stored secret is kept.
// Pass obj["key"].as<const char *>(), which is null when the key is absent.
// Updates dest from an incoming value, truncating to size. Pass
// obj["key"].as<const char *>(), which is null when the key is absent.
// absent -> keep the stored secret.
// "" -> keep it too, unless identityChanged (a new SSID or username in
// the same request), then clear it. UIs older than write-only
// secrets send "" for a field the device no longer fills in.
// value -> replace it.
// Returns true when dest changed.
inline bool applySecret(const char *incoming, char *dest, size_t size) {
inline bool applySecret(const char *incoming, char *dest, size_t size, bool identityChanged = false) {
if(!incoming || size == 0) return false;
if(incoming[0] == '\0' && !identityChanged) return false;
size_t len = strlen(incoming);
if(len >= size) len = size - 1;
if(strncmp(dest, incoming, len) == 0 && dest[len] == '\0') return false;

28
src/entryPoints/WebUiRoutes.cpp
View file

@ -6,6 +6,7 @@
#include "ConfigSettings.h"
#include "ConfigFile.h"
#include "Utils.h"
#include "Secrets.h"
#include "SSDP.h"
#include "Somfy.h"
#include "WResp.h"
@ -781,16 +782,13 @@ void registerWebUiRoutes(AsyncWebServer &server) {
}
if(!obj["wifi"].isNull()) {
JsonObject objWifi = obj["wifi"];
if(settings.connType == conn_types_t::wifi) {
if(!objWifi["ssid"].isNull() && objWifi["ssid"].as<String>().compareTo(settings.WIFI.ssid) != 0) {
if(WiFi.softAPgetStationNum() == 0) reboot = true;
}
if(!objWifi["passphrase"].isNull() && objWifi["passphrase"].as<String>().compareTo(settings.WIFI.passphrase) != 0) {
if(WiFi.softAPgetStationNum() == 0) reboot = true;
}
}
// Compared after fromJSON, which decides whether an empty passphrase keeps the stored one.
WifiSettings before = settings.WIFI;
settings.WIFI.fromJSON(objWifi);
settings.WIFI.save();
if(settings.connType == conn_types_t::wifi && WiFi.softAPgetStationNum() == 0 &&
(strcmp(before.ssid, settings.WIFI.ssid) != 0 || strcmp(before.passphrase, settings.WIFI.passphrase) != 0))
reboot = true;
}
if(!obj["ethernet"].isNull()) {
JsonObject objEth = obj["ethernet"];
@ -833,19 +831,19 @@ void registerWebUiRoutes(AsyncWebServer &server) {
JsonObject obj = json.as<JsonObject>();
ESP_LOGD(TAG, "Settings WIFI connection...");
String ssid = "";
// Clients never receive the stored passphrase, so an omitted one keeps it.
String passphrase = settings.WIFI.passphrase;
if(!obj["ssid"].isNull()) ssid = obj["ssid"].as<String>();
if(!obj["passphrase"].isNull()) passphrase = obj["passphrase"].as<String>();
bool reboot = false;
if(ssid.compareTo(settings.WIFI.ssid) != 0) reboot = true;
if(passphrase.compareTo(settings.WIFI.passphrase) != 0) reboot = true;
bool newSsid = ssid.compareTo(settings.WIFI.ssid) != 0;
// Clients never receive the stored passphrase: an omitted or empty one keeps it unless the SSID changes.
char passphrase[sizeof(settings.WIFI.passphrase)];
SETCHARPROP(passphrase, settings.WIFI.passphrase, sizeof(passphrase));
bool newPassphrase = applySecret(obj["passphrase"].as<const char *>(), passphrase, sizeof(passphrase), newSsid);
bool reboot = newSsid || newPassphrase;
if(!settings.WIFI.ssidExists(ssid.c_str()) && ssid.length() > 0) {
request->send(400, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"WiFi Network Does not exist\"}");
}
else {
SETCHARPROP(settings.WIFI.ssid, ssid.c_str(), sizeof(settings.WIFI.ssid));
SETCHARPROP(settings.WIFI.passphrase, passphrase.c_str(), sizeof(settings.WIFI.passphrase));
SETCHARPROP(settings.WIFI.passphrase, passphrase, sizeof(settings.WIFI.passphrase));
settings.WIFI.save();
settings.WIFI.print();
request->send(201, _encoding_json, "{\"status\":\"OK\",\"desc\":\"Successfully set server connection\"}");

33
test/test_secrets/test_secrets.cpp
View file

@ -44,12 +44,33 @@ static void test_applySecret_prefix_of_stored_is_a_change() {
TEST_ASSERT_TRUE(applySecret("sto", dest, sizeof(dest)));
TEST_ASSERT_EQUAL_STRING("sto", dest);
}
// Sending an empty value explicitly clears it, e.g. an open WiFi network.
static void test_applySecret_empty_value_clears_stored() {
// An older UI sends "" for a field the device no longer fills in.
static void test_applySecret_empty_value_keeps_stored() {
char dest[16] = "stored";
TEST_ASSERT_TRUE(applySecret("", dest, sizeof(dest)));
TEST_ASSERT_FALSE(applySecret("", dest, sizeof(dest)));
TEST_ASSERT_EQUAL_STRING("stored", dest);
}
// A new SSID or username with an empty secret, e.g. an open WiFi network.
static void test_applySecret_empty_value_with_new_identity_clears_stored() {
char dest[16] = "stored";
TEST_ASSERT_TRUE(applySecret("", dest, sizeof(dest), true));
TEST_ASSERT_EQUAL_STRING("", dest);
}
static void test_applySecret_empty_value_with_new_identity_and_nothing_stored() {
char dest[16] = "";
TEST_ASSERT_FALSE(applySecret("", dest, sizeof(dest), true));
TEST_ASSERT_EQUAL_STRING("", dest);
}
static void test_applySecret_new_identity_still_takes_a_value() {
char dest[16] = "stored";
TEST_ASSERT_TRUE(applySecret("fresh", dest, sizeof(dest), true));
TEST_ASSERT_EQUAL_STRING("fresh", dest);
}
static void test_applySecret_absent_keeps_stored_even_with_new_identity() {
char dest[16] = "stored";
TEST_ASSERT_FALSE(applySecret(nullptr, dest, sizeof(dest), true));
TEST_ASSERT_EQUAL_STRING("stored", dest);
}
static void test_applySecret_truncates_and_terminates() {
char storage[8];
memset(storage, 'X', sizeof(storage));
@ -74,7 +95,11 @@ int main(int argc, char **argv) {
RUN_TEST(test_applySecret_new_value_replaces_stored);
RUN_TEST(test_applySecret_same_value_is_no_change);
RUN_TEST(test_applySecret_prefix_of_stored_is_a_change);
RUN_TEST(test_applySecret_empty_value_clears_stored);
RUN_TEST(test_applySecret_empty_value_keeps_stored);
RUN_TEST(test_applySecret_empty_value_with_new_identity_clears_stored);
RUN_TEST(test_applySecret_empty_value_with_new_identity_and_nothing_stored);
RUN_TEST(test_applySecret_new_identity_still_takes_a_value);
RUN_TEST(test_applySecret_absent_keeps_stored_even_with_new_identity);
RUN_TEST(test_applySecret_truncates_and_terminates);
RUN_TEST(test_applySecret_truncated_same_value_is_no_change);
return UNITY_END();