fix(confighttp): do not return 200 on errors

docs/api.md

@@ -14,7 +14,7 @@ basic authentication with the admin username and password.
 ## POST /api/apps
 @copydoc confighttp::saveApp()
 
-## DELETE /api/apps{index}
+## DELETE /api/apps/{index}
 @copydoc confighttp::deleteApp()
 
 ## POST /api/covers/upload

src/confighttp.cpp

@@ -141,8 +141,13 @@
     return true;
   }
 
+  /**
+   * @brief Send a 404 Not Found response.
+   * @param response The original response object.
+   * @param request The original request object.
+   */
   void
-  not_found(resp_https_t response, req_https_t request) {
+  not_found(resp_https_t response, [[maybe_unused]] req_https_t request) {
     pt::ptree tree;
     tree.put("root.<xmlattr>.status_code", 404);
 
@@ -155,6 +160,27 @@
               << data.str();
   }
 
+  /**
+   * @brief Send a 400 Bad Request response.
+   * @param response The original response object.
+   * @param request The original request object.
+   * @param error_message The error message to include in the response.
+   */
+  void
+  bad_request(resp_https_t response, [[maybe_unused]] req_https_t request, const std::string &error_message) {
+    pt::ptree tree;
+    tree.put("root.<xmlattr>.status_code", 400);
+    tree.put("root.error", error_message);
+
+    std::ostringstream data;
+    pt::write_xml(data, tree);
+
+    SimpleWeb::CaseInsensitiveMultimap headers;
+    headers.emplace("Content-Type", "application/xml");
+
+    response->write(SimpleWeb::StatusCode::client_error_bad_request, data.str(), headers);
+  }
+
   /**
    * @todo combine these functions into a single function that accepts the page, i.e "index", "pin", "apps"
    */
@@ -359,7 +385,7 @@
   }
 
   /**
-   * @brief Save an application. If the application already exists, it will be updated, otherwise it will be added.
+   * @brief Save an application. To save a new application the index must be `-1`. To update an existing application, you must provide the current index of the application.
    * @param response The HTTP response object.
    * @param request The HTTP request object.
    * The body for the post request should be JSON serialized in the following format:
@@ -384,7 +410,7 @@
    *   "detached": [
    *     "Detached command"
    *   ],
-   *   "image-path": "Full path to the application image. Must be a png file.",
+   *   "image-path": "Full path to the application image. Must be a png file."
    * }
    * @endcode
    */
@@ -467,8 +493,7 @@
     catch (std::exception &e) {
       BOOST_LOG(warning) << "SaveApp: "sv << e.what();
 
-      outputTree.put("status", "false");
-      outputTree.put("error", "Invalid Input JSON");
+      bad_request(response, request, "Invalid Input JSON");
       return;
     }
 
@@ -501,8 +526,7 @@
       int index = stoi(request->path_match[1]);
 
       if (index < 0) {
-        outputTree.put("status", "false");
-        outputTree.put("error", "Invalid Index");
+        bad_request(response, request, "Invalid Index");
         return;
       }
       else {
@@ -521,8 +545,7 @@
     }
     catch (std::exception &e) {
       BOOST_LOG(warning) << "DeleteApp: "sv << e.what();
-      outputTree.put("status", "false");
-      outputTree.put("error", "Invalid File JSON");
+      bad_request(response, request, "Invalid File JSON");
       return;
     }
 
@@ -538,7 +561,7 @@
    * @code{.json}
    * {
    *   "key": "igdb_<game_id>",
-   *   "url": "https://images.igdb.com/igdb/image/upload/t_cover_big_2x/<slug>.png",
+   *   "url": "https://images.igdb.com/igdb/image/upload/t_cover_big_2x/<slug>.png"
    * }
    * @endcode
    */
@@ -567,8 +590,7 @@
     }
     catch (std::exception &e) {
       BOOST_LOG(warning) << "UploadCover: "sv << e.what();
-      outputTree.put("status", "false");
-      outputTree.put("error", e.what());
+      bad_request(response, request, e.what());
       return;
     }
 
@@ -698,8 +720,7 @@
     }
     catch (std::exception &e) {
       BOOST_LOG(warning) << "SaveConfig: "sv << e.what();
-      outputTree.put("status", "false");
-      outputTree.put("error", e.what());
+      bad_request(response, request, e.what());
       return;
     }
   }
@@ -740,6 +761,7 @@
 
     print_req(request);
 
+    std::vector<std::string> errors = {};
     std::stringstream ss;
     std::stringstream configStream;
     ss << request->content.rdbuf();
@@ -762,15 +784,13 @@
       auto confirmPassword = inputTree.count("confirmNewPassword") > 0 ? inputTree.get<std::string>("confirmNewPassword") : "";
       if (newUsername.length() == 0) newUsername = username;
       if (newUsername.length() == 0) {
-        outputTree.put("status", false);
-        outputTree.put("error", "Invalid Username");
+        errors.emplace_back("Invalid Username");
       }
       else {
         auto hash = util::hex(crypto::hash(password + config::sunshine.salt)).to_string();
         if (config::sunshine.username.empty() || (boost::iequals(username, config::sunshine.username) && hash == config::sunshine.password)) {
           if (newPassword.empty() || newPassword != confirmPassword) {
-            outputTree.put("status", false);
-            outputTree.put("error", "Password Mismatch");
+            errors.emplace_back("Password Mismatch");
           }
           else {
             http::save_user_creds(config::sunshine.credentials_file, newUsername, newPassword);
@@ -779,15 +799,22 @@
           }
         }
         else {
-          outputTree.put("status", false);
-          outputTree.put("error", "Invalid Current Credentials");
+          errors.emplace_back("Invalid Current Credentials");
         }
       }
+
+      if (!errors.empty()) {
+        // join the errors array
+        std::string error = std::accumulate(errors.begin(), errors.end(), std::string(),
+          [](const std::string &a, const std::string &b) {
+            return a.empty() ? b : a + ", " + b;
+          });
+        bad_request(response, request, error);
+      }
     }
     catch (std::exception &e) {
       BOOST_LOG(warning) << "SavePassword: "sv << e.what();
-      outputTree.put("status", false);
-      outputTree.put("error", e.what());
+      bad_request(response, request, e.what());
       return;
     }
   }
@@ -830,8 +857,7 @@
     }
     catch (std::exception &e) {
       BOOST_LOG(warning) << "SavePin: "sv << e.what();
-      outputTree.put("status", false);
-      outputTree.put("error", e.what());
+      bad_request(response, request, e.what());
       return;
     }
   }
@@ -895,8 +921,7 @@
     }
     catch (std::exception &e) {
       BOOST_LOG(warning) << "Unpair: "sv << e.what();
-      outputTree.put("status", false);
-      outputTree.put("error", e.what());
+      bad_request(response, request, e.what());
       return;
     }
   }