From 88f78f6541ac35870832acdbca858e84911bb134 Mon Sep 17 00:00:00 2001 From: proddy Date: Tue, 9 Nov 2021 20:54:41 +0100 Subject: [PATCH] fix authentication check for GET commands that need admin - Refactor MQTT subscriptions and API calls #173 --- src/command.cpp | 14 +++++++------- src/command.h | 14 +++++++------- src/test/test.cpp | 39 +++++++++++++++++++++++++++------------ src/web/WebAPIService.cpp | 26 ++++++++++++-------------- 4 files changed, 53 insertions(+), 40 deletions(-) diff --git a/src/command.cpp b/src/command.cpp index 3a588cdba..2cf2b8739 100644 --- a/src/command.cpp +++ b/src/command.cpp @@ -30,7 +30,7 @@ std::vector Command::cmdfunctions_; // the path is leading so if duplicate keys are in the input JSON it will be ignored // the entry point will be either via the Web API (api/) or MQTT (/) // returns a return code and json output -uint8_t Command::process(const char * path, const bool authenticated, const JsonObject & input, JsonObject & output) { +uint8_t Command::process(const char * path, const bool is_admin, const JsonObject & input, JsonObject & output) { SUrlParser p; // parse URL for the path names p.parse(path); @@ -146,16 +146,16 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json // call the command based on the type uint8_t return_code = CommandRet::ERROR; if (data.is()) { - return_code = Command::call(device_type, command_p, data.as(), authenticated, id_n, output); + return_code = Command::call(device_type, command_p, data.as(), is_admin, id_n, output); } else if (data.is()) { char data_str[10]; - return_code = Command::call(device_type, command_p, Helpers::itoa(data_str, (int16_t)data.as()), authenticated, id_n, output); + return_code = Command::call(device_type, command_p, Helpers::itoa(data_str, (int16_t)data.as()), is_admin, id_n, output); } else if (data.is()) { char data_str[10]; - return_code = Command::call(device_type, command_p, Helpers::render_value(data_str, (float)data.as(), 2), authenticated, id_n, output); + return_code = Command::call(device_type, command_p, Helpers::render_value(data_str, (float)data.as(), 2), is_admin, id_n, output); } else if (data.isNull()) { // empty - return_code = Command::call(device_type, command_p, "", authenticated, id_n, output); + return_code = Command::call(device_type, command_p, "", is_admin, id_n, output); } else { // can't process output.clear(); @@ -243,7 +243,7 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * // calls a command. Takes a json object for output. // id may be used to represent a heating circuit for example // returns 0 if the command errored, 1 (TRUE) if ok, 2 if not found, 3 if error or 4 if not allowed -uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * value, bool authenticated, const int8_t id, JsonObject & output) { +uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * value, const bool is_admin, const int8_t id, JsonObject & output) { uint8_t return_code = CommandRet::OK; std::string dname = EMSdevice::device_type_2_device_name(device_type); @@ -274,7 +274,7 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * } // check permissions - if (cf->has_flags(CommandFlag::ADMIN_ONLY) && !authenticated) { + if (cf->has_flags(CommandFlag::ADMIN_ONLY) && !is_admin) { output["message"] = "authentication failed"; return CommandRet::NOT_ALLOWED; // command not allowed } diff --git a/src/command.h b/src/command.h index 682908b8e..edc62f838 100644 --- a/src/command.h +++ b/src/command.h @@ -49,11 +49,11 @@ enum CommandFlag : uint8_t { // return status after calling a Command enum CommandRet : uint8_t { - FAIL = 0, // 0 or FALSE - OK, // 1 or TRUE - NOT_FOUND, // 2 - ERROR, // 3 - NOT_ALLOWED // needs authentication + FAIL = 0, // 0 or FALSE + OK, // 1 or TRUE + NOT_FOUND, // 2 + ERROR, // 3 + NOT_ALLOWED // needs authentication }; @@ -104,7 +104,7 @@ class Command { #define add_ - static uint8_t call(const uint8_t device_type, const char * cmd, const char * value, bool authenticated, const int8_t id, JsonObject & output); + static uint8_t call(const uint8_t device_type, const char * cmd, const char * value, const bool is_admin, const int8_t id, JsonObject & output); static uint8_t call(const uint8_t device_type, const char * cmd, const char * value); // with normal call back function taking a value and id @@ -130,7 +130,7 @@ class Command { static bool list(const uint8_t device_type, JsonObject & output); - static uint8_t process(const char * path, const bool authenticated, const JsonObject & input, JsonObject & output); + static uint8_t process(const char * path, const bool is_admin, const JsonObject & input, JsonObject & output); static const char * parse_command_string(const char * command, int8_t & id); diff --git a/src/test/test.cpp b/src/test/test.cpp index fdd38e761..1e70ea8c7 100644 --- a/src/test/test.cpp +++ b/src/test/test.cpp @@ -472,7 +472,8 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { } if (command == "api") { - shell.printfln(F("Testing API with MQTT and REST")); + shell.printfln(F("Testing API with MQTT and REST, standalone")); + Mqtt::ha_enabled(true); // Mqtt::ha_enabled(false); @@ -483,17 +484,7 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { run_test("boiler"); run_test("thermostat"); - /* - EMSESP::mqtt_.incoming("ems-esp/thermostat/mode"); // empty payload, sends reponse - EMSESP::mqtt_.incoming("ems-esp/boiler/syspress"); // empty payload, sends reponse - EMSESP::mqtt_.incoming("ems-esp/thermostat/mode", "auto"); // set mode - EMSESP::mqtt_.incoming("ems-esp/thermostat/mode"); // empty payload, sends reponse - EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13"); - EMSESP::mqtt_.incoming("ems-esp/system/publish"); - EMSESP::mqtt_.incoming("ems-esp/thermostat/seltemp"); // empty payload, sends reponse - EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13"); - return; - */ +#if defined(EMSESP_STANDALONE) AsyncWebServerRequest request2; request2.method(HTTP_GET); @@ -545,11 +536,20 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { */ + /* + request2.url("/api/system/restart"); + EMSESP::webAPIService.webAPIService_get(&request2); + return; + */ + + /* request2.url("/api/dallassensor/fdfd"); EMSESP::webAPIService.webAPIService_get(&request2); emsesp::EMSESP::logger().notice("****"); request2.url("/api/dallassensor/info"); EMSESP::webAPIService.webAPIService_get(&request2); + return; + */ /* AsyncWebServerRequest request2; @@ -603,6 +603,8 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { cmd = Command::parse_command_string(command_s, id_n); shell.printfln("test cmd parse cmd=%s id=%d", cmd, id_n); +#endif + // Console tests shell.invoke_command("call thermostat entities"); shell.invoke_command("call thermostat mode auto"); @@ -622,6 +624,19 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { EMSESP::mqtt_.incoming("ems-esp/thermostat/modee", "auto"); // unknown command EMSESP::mqtt_.incoming("ems-esp/thermostat/mode/auto", "auto"); // invalid + // various MQTT checks + /* + EMSESP::mqtt_.incoming("ems-esp/thermostat/mode"); // empty payload, sends reponse + EMSESP::mqtt_.incoming("ems-esp/boiler/syspress"); // empty payload, sends reponse + EMSESP::mqtt_.incoming("ems-esp/thermostat/mode", "auto"); // set mode + EMSESP::mqtt_.incoming("ems-esp/thermostat/mode"); // empty payload, sends reponse + EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13"); + EMSESP::mqtt_.incoming("ems-esp/system/publish"); + EMSESP::mqtt_.incoming("ems-esp/thermostat/seltemp"); // empty payload, sends reponse + EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13"); + return; + */ + // check long base Mqtt::base("home/cellar/heating"); EMSESP::mqtt_.incoming("home/cellar/heating/thermostat/mode"); // empty payload, sends reponse diff --git a/src/web/WebAPIService.cpp b/src/web/WebAPIService.cpp index b01f237a7..4bec8bf85 100644 --- a/src/web/WebAPIService.cpp +++ b/src/web/WebAPIService.cpp @@ -31,17 +31,17 @@ WebAPIService::WebAPIService(AsyncWebServer * server, SecurityManager * security server->addHandler(&_apiHandler); } +// HTTP GET // GET /{device} -// GET /{device}/{name} -// GET /device={device}?cmd={name}?data={value}[?id={hc}] +// GET /{device}/{entity} void WebAPIService::webAPIService_get(AsyncWebServerRequest * request) { - // has no body JSON so create dummy as empty object + // has no body JSON so create dummy as empty input object StaticJsonDocument input_doc; JsonObject input = input_doc.to(); parse(request, input); } -// For POSTS with an optional JSON body +// For HTTP POSTS with an optional JSON body // HTTP_POST | HTTP_PUT | HTTP_PATCH // POST /{device}[/{hc|id}][/{name}] void WebAPIService::webAPIService_post(AsyncWebServerRequest * request, JsonVariant & json) { @@ -59,23 +59,21 @@ void WebAPIService::webAPIService_post(AsyncWebServerRequest * request, JsonVari // parse the URL looking for query or path parameters // reporting back any errors void WebAPIService::parse(AsyncWebServerRequest * request, JsonObject & input) { - auto method = request->method(); - bool authenticated = false; + auto method = request->method(); - // if its a POST then check authentication - if (method != HTTP_GET) { - EMSESP::webSettingsService.read([&](WebSettings & settings) { - Authentication authentication = _securityManager->authenticateRequest(request); - authenticated = settings.notoken_api | AuthenticationPredicates::IS_ADMIN(authentication); - }); - } + // see if the user has admin privelegs + bool is_admin; + EMSESP::webSettingsService.read([&](WebSettings & settings) { + Authentication authentication = _securityManager->authenticateRequest(request); + is_admin = settings.notoken_api | AuthenticationPredicates::IS_ADMIN(authentication); + }); // output json buffer PrettyAsyncJsonResponse * response = new PrettyAsyncJsonResponse(false, EMSESP_JSON_SIZE_XXLARGE_DYN); JsonObject output = response->getRoot(); // call command - uint8_t return_code = Command::process(request->url().c_str(), authenticated, input, output); + uint8_t return_code = Command::process(request->url().c_str(), is_admin, input, output); if (return_code != CommandRet::OK) { char error[100];