Skip to content

Commit b7b72a2

Browse files
Lord-GreyLord-Grey
authored
Fix Password change to fail when wrong password is given (#2056)
1 parent e0d5fab commit b7b72a2

10 files changed

Lines changed: 98 additions & 48 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2828
- EffectFileHandler - Path traversal vulnerability when saving user-defined effects (#2011)
2929
- Effect scripts: Minor stability and style fixes in `pacman.py`, `traces.py`, `trails.py`(#2011)
3030
- WebUI - Return a valid Content-Type for static assets to prevent module loading failures
31+
- WebUI - Explain why a password change is blocked, including authorization failures and the need to change the default password before using the API
3132
- MdnsBrowser compile errors when ENABLE_MDNS is false (#2024)
3233
- LinearColorSmoothing - fix incorrect output interval calculation, fix handling for update frequency >= 1000Hz
3334
- macOS Screen Capture - Do not take a one-shot screenshot per frame. Use a continuous ScreenCaptureKit stream instead, which is established once. This removes the per-frame shareable content enumeration (WindowServer/replayd CPU) and stops the Screen Recording indicator from flickering, which pinned the macOS Control Center close to 100% CPU (#2046, #2039, #1921)

‎assets/webconfig/i18n/en.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,8 +1019,14 @@
10191019
"general_wiki_moreto": "More information to \"$1\" at our Wiki",
10201020
"InfoDialog_access_text": "Depending on settings level you could adjust more options or get access to more features. Recommended is the \"Default\" level.",
10211021
"InfoDialog_access_title": "Settings level",
1022+
"InfoDialog_authorization_failed_title": "Authorization failed!",
1023+
"InfoDialog_authorization_new_password_failed_title": "Authorization failed for new password!",
1024+
"InfoDialog_changePassword_authorization_failed": "Authorization failed. The password was not changed.",
10221025
"InfoDialog_changePassword_success": "Password successfully saved!",
1026+
"InfoDialog_changePassword_failed": "Failed to change password!",
10231027
"InfoDialog_changePassword_title": "Change Password",
1028+
"InfoDialog_defaultPasswordChangeRequired": "Password change required",
1029+
"InfoDialog_defaultPasswordChangeRequired_detail": "The default password must be changed before accessing the API.",
10241030
"infoDialog_checklist_title": "Checklist!",
10251031
"infoDialog_effconf_created_text": "The effect \"$1\" has been created successfully!",
10261032
"infoDialog_effconf_deleted_text": "The effect \"$1\" has been deleted successfully!",

‎assets/webconfig/js/content_index.js‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,28 @@ $(window.hyperion).on("ready", function (event) {
2929

3030
$(window.hyperion).on("error", function (event) {
3131
const error = event.reason;
32+
const translatedMessages = {
33+
"password change required": $.i18n('InfoDialog_defaultPasswordChangeRequired'),
34+
"Default password must be changed before accessing the API": $.i18n('InfoDialog_defaultPasswordChangeRequired_detail')
35+
};
3236

3337
//An error "No Authorization" is handled by waitForSuccessfulAuthorization
3438
if (error?.message !== "No Authorization") {
3539

36-
const errorDetails = [];
40+
const errorDetails = Array.isArray(error?.details)
41+
? error.details.map(detail => translatedMessages[detail] || detail)
42+
: [];
3743

3844
if (error?.cmd) {
3945
errorDetails.push(`Command: "${error.cmd}"`);
4046
}
4147

42-
errorDetails.push(error?.details || "No additional details.");
48+
if (errorDetails.length === 0) {
49+
errorDetails.push("No additional details.");
50+
}
4351

44-
showInfoDialog("error", "", error?.message || "Unknown error", errorDetails);
52+
const message = translatedMessages[error?.message] || error?.message || "Unknown error";
53+
showInfoDialog("error", "", message, errorDetails);
4554
}
4655
});
4756

@@ -81,15 +90,23 @@ $(window.hyperion).one("cmd-authorize-getTokenList", function (event) {
8190
$(window.hyperion).on("cmd-authorize-newPassword", async function (event) {
8291

8392
if (event.response.success === true) {
84-
showInfoDialog("success", $.i18n('InfoDialog_changePassword_success'));
93+
showInfoDialog("success", $.i18n('InfoDialog_changePassword_title'), $.i18n('InfoDialog_changePassword_success'));
8594

8695
try {
8796
// Force login with new passwort
8897
await waitForSuccessfulAuthorization();
8998
$(window.hyperion).trigger("ready");
9099
} catch (err) {
91100
console.error("Authorization failed for new password:", err);
101+
showInfoDialog("error", $.i18n('InfoDialog_authorization_new_password_failed_title'), err.message || err);
102+
}
103+
} else {
104+
event._errorHandled = true;
105+
const details = [];
106+
if (event.response.error === "No User Authorization") {
107+
details.push($.i18n('InfoDialog_changePassword_authorization_failed'));
92108
}
109+
showInfoDialog("error", $.i18n('InfoDialog_changePassword_title'), $.i18n('InfoDialog_changePassword_failed'), details);
93110
}
94111
});
95112

@@ -181,7 +198,7 @@ function waitForSuccessfulAuthorization() {
181198
let loginResolved = false;
182199

183200
const onLogin = (event) => {
184-
if (!loginResolved) {
201+
if (!loginResolved && event.response?.success === true) {
185202
loginResolved = true;
186203
$(window.hyperion).off("cmd-authorize-login", onLogin);
187204
resolve(event);
@@ -198,9 +215,13 @@ function waitForSuccessfulAuthorization() {
198215

199216
const onError = (event) => {
200217
const error = event.reason;
201-
if (error?.message === "No Authorization" && getStorage("loginToken")) {
202-
removeStorage("loginToken");
203-
requestRequiresDefaultPasswortChange(); // Retry trigger
218+
if (error?.message === "No Authorization") {
219+
if (getStorage("loginToken")) {
220+
removeStorage("loginToken");
221+
requestRequiresDefaultPasswortChange(); // Retry trigger
222+
} else {
223+
showInfoDialog("error", $.i18n('InfoDialog_authorization_failed_title'), error.message);
224+
}
204225
}
205226
};
206227

@@ -438,6 +459,11 @@ $(window.hyperion).on("cmd-sysinfo", function (event) {
438459
$(window.hyperion).on("cmd-config-setconfig", function (event) {
439460
if (event.response.success === true) {
440461
showNotification('success', $.i18n('dashboard_alert_message_confsave_success'), $.i18n('dashboard_alert_message_confsave_success_t'))
462+
} else {
463+
event._errorHandled = true;
464+
const errorData = Array.isArray(event.response.errorData) ? event.response.errorData : [];
465+
const details = errorData.map(item => item.description || "").filter(Boolean);
466+
showInfoDialog("error", $.i18n('infoDialog_writeconf_error_text'), event.response.error, details.length ? details : undefined);
441467
}
442468
});
443469

‎assets/webconfig/js/hyperion.js‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,21 @@ function initWebSocket() {
178178
const logErrorData = JSON.stringify(errorData).replace(/[\r\n\t]/g, ' ')
179179
console.error("[globalThis.websocket::onmessage] ", logError, ", Description:", logErrorData);
180180

181-
$(globalThis.hyperion).trigger({
182-
type: "error",
183-
reason: {
184-
cmd: cmd,
185-
message: error,
186-
details: errorData.map((item) => item.description || "")
187-
}
188-
});
181+
// Fire the command-specific event so its handler can respond to the failure.
182+
// If the handler sets event._errorHandled = true, the generic error dialog is suppressed.
183+
const cmdErrorEvent = $.Event("cmd-" + cmd, { response: response });
184+
$(globalThis.hyperion).trigger(cmdErrorEvent);
185+
186+
if (!cmdErrorEvent._errorHandled) {
187+
$(globalThis.hyperion).trigger({
188+
type: "error",
189+
reason: {
190+
cmd: cmd,
191+
message: error,
192+
details: errorData.map((item) => item.description || "")
193+
}
194+
});
195+
}
189196
}
190197
}
191198
}

‎assets/webconfig/js/settings.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ function changePassword() {
1414
var newPw = $('#new-password').val();
1515

1616
requestChangePassword(oldPw, newPw);
17-
history.pushState({}, "New password");
1817
});
1918

2019
$('#new-password, #current-password').off().on('input', function (e) {

‎include/api/API.h‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@
1212
// auth manager
1313
#include <hyperion/AuthManager.h>
1414

15-
16-
#include <utils/ColorRgb.h>
17-
#include <utils/ColorSys.h>
18-
1915
class JsonCallbacks;
2016
class HyperionIManager;
2117

@@ -274,32 +270,32 @@ class API : public QObject
274270
/// @brief Update the Password of Hyperion. Requires ADMIN ACCESS
275271
/// @param password Old password
276272
/// @param newPassword New password
277-
/// @return True on success else false
273+
/// @return Empty string on success, otherwise the error message
278274
///
279-
bool updateHyperionPassword(const QString &password, const QString &newPassword);
275+
QString updateHyperionPassword(const QString &password, const QString &newPassword) const;
280276

281277
///
282278
/// @brief Get a new token from AuthManager. Requires ADMIN ACCESS
283279
/// @param comment The comment of the request
284280
/// @param def The final definition
285281
/// @return Empty string on success else error message
286282
///
287-
QString createToken(const QString &comment, AuthManager::AuthDefinition &def);
283+
QString createToken(const QString &comment, AuthManager::AuthDefinition &def) const;
288284

289285
///
290286
/// @brief Rename a token by given id. Requires ADMIN ACCESS
291287
/// @param tokenId The id of the token
292288
/// @param comment The new comment
293289
/// @return Empty string on success else error message
294290
///
295-
QString renameToken(const QString &tokenId, const QString &comment);
291+
QString renameToken(const QString &tokenId, const QString &comment) const;
296292

297293
///
298294
/// @brief Delete a token by given id. Requires ADMIN ACCESS
299295
/// @param tokenId The id of the token
300296
/// @return Empty string on success else error message
301297
///
302-
QString deleteToken(const QString &tokenId);
298+
QString deleteToken(const QString &tokenId) const;
303299

304300
///
305301
/// @brief Set a new token request

‎include/hyperion/AuthManager.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class AuthManager : public QObject
2222
Q_OBJECT
2323
private:
2424
friend class HyperionDaemon;
25-
AuthManager(QObject *parent = nullptr);
25+
explicit AuthManager(QObject *parent = nullptr);
2626

2727
public:
2828
struct AuthDefinition
@@ -135,7 +135,7 @@ public slots:
135135
/// @param newPw The new password
136136
/// @return True on success else false
137137
///
138-
bool updateUserPassword(const QString &user, const QString &pw, const QString &newPw);
138+
QString updateUserPassword(const QString &user, const QString &pw, const QString &newPw);
139139

140140
///
141141
/// @brief Generate a new pending token request with the provided comment and id as identifier helper

‎libsrc/api/API.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -469,24 +469,24 @@ QString API::saveEffect(const QJsonObject &data) const
469469
}
470470
#endif
471471

472-
bool API::updateHyperionPassword(const QString &password, const QString &newPassword)
472+
QString API::updateHyperionPassword(const QString &password, const QString &newPassword) const
473473
{
474-
bool isPwUpdated {true};
475474
if (!_adminAuthorized)
476475
{
477-
isPwUpdated = false;
476+
return NO_AUTHORIZATION;
478477
}
479-
else
478+
479+
if (auto auth = _authManagerWeak.toStrongRef())
480480
{
481-
if (auto auth = _authManagerWeak.toStrongRef())
482-
{
483-
QMetaObject::invokeMethod(auth.get(), "updateUserPassword", Qt::BlockingQueuedConnection, Q_RETURN_ARG(bool, isPwUpdated), Q_ARG(QString, DEFAULT_USER), Q_ARG(QString, password), Q_ARG(QString, newPassword));
484-
}
481+
QString updateError;
482+
const bool invoked = QMetaObject::invokeMethod(auth.get(), "updateUserPassword", Qt::BlockingQueuedConnection, Q_RETURN_ARG(QString, updateError), Q_ARG(QString, DEFAULT_USER), Q_ARG(QString, password), Q_ARG(QString, newPassword));
483+
return invoked ? updateError : QString("Failed to update user password");
485484
}
486-
return isPwUpdated;
485+
486+
return "Failed to update user password";
487487
}
488488

489-
QString API::createToken(const QString &comment, AuthManager::AuthDefinition &def)
489+
QString API::createToken(const QString &comment, AuthManager::AuthDefinition &def) const
490490
{
491491
if (!_adminAuthorized)
492492
{
@@ -504,7 +504,7 @@ QString API::createToken(const QString &comment, AuthManager::AuthDefinition &de
504504
return "";
505505
}
506506

507-
QString API::renameToken(const QString &tokenId, const QString &comment)
507+
QString API::renameToken(const QString &tokenId, const QString &comment) const
508508
{
509509
if (!_adminAuthorized)
510510
{
@@ -529,7 +529,7 @@ QString API::renameToken(const QString &tokenId, const QString &comment)
529529
return (!isTokenRenamed) ? "Token does not exist" : "";
530530
}
531531

532-
QString API::deleteToken(const QString &tokenId)
532+
QString API::deleteToken(const QString &tokenId) const
533533
{
534534
if (!_adminAuthorized)
535535
{

‎libsrc/api/JsonAPI.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ void JsonAPI::handleGetImageSnapshotCommand(const QJsonObject& message, const Js
566566

567567
QObject tempContext; // context object for single-shot connection
568568
auto connection = QObject::connect(hyperion.get(), &Hyperion::currentImage, &tempContext,
569-
[&](const Image<ColorRgb>& img){
569+
[&capturedImage, &loop](const Image<ColorRgb>& img){
570570
capturedImage = img;
571571
loop.quit();
572572
});
@@ -1072,7 +1072,7 @@ void JsonAPI::handleConfigSetCommand(const QJsonObject &message, const JsonApiCo
10721072
instancesNewConfigs.insert(NO_INSTANCE_ID, JsonUtils::mergeJsonObjects(instanceZeroConfig, globalSettings));
10731073
}
10741074

1075-
QMapIterator<quint8, QJsonObject> iter (instancesNewConfigs);
1075+
QMapIterator iter (instancesNewConfigs);
10761076
while (iter.hasNext()) {
10771077
iter.next();
10781078

@@ -1505,10 +1505,11 @@ void JsonAPI::handleNewPassword(const QJsonObject &message, const JsonApiCommand
15051505
{
15061506
const QString password = message["password"].toString().trimmed();
15071507
const QString newPassword = message["newPassword"].toString().trimmed();
1508-
if (API::updateHyperionPassword(password, newPassword)) {
1508+
const QString newPasswordResult = API::updateHyperionPassword(password, newPassword);
1509+
if (newPasswordResult.isEmpty()) {
15091510
sendSuccessReply(cmd);
15101511
} else {
1511-
sendErrorReply("Failed to update user password", cmd);
1512+
sendErrorReply(newPasswordResult, cmd);
15121513
}
15131514
}
15141515

‎libsrc/hyperion/AuthManager.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111
#include <QDateTime>
1212
#include <QUuid>
1313

14+
// Constants
15+
namespace {
16+
17+
inline constexpr char NO_USER_AUTHORIZATION[] = "No User Authorization";;
18+
19+
}
20+
1421
QSharedPointer<AuthManager> AuthManager::_instance;
1522

1623
AuthManager::AuthManager(QObject *parent)
@@ -162,12 +169,19 @@ bool AuthManager::isUserTokenAuthorized(const QString &usr, const QString &token
162169
return true;
163170
}
164171

165-
bool AuthManager::updateUserPassword(const QString &user, const QString &pw, const QString &newPw)
172+
QString AuthManager::updateUserPassword(const QString &user, const QString &pw, const QString &newPw)
166173
{
167-
if (isUserAuthorized(user, pw))
168-
return _authTable->updateUserPassword(user, newPw);
174+
if (!isUserAuthorized(user, pw))
175+
{
176+
return NO_USER_AUTHORIZATION;
177+
}
169178

170-
return false;
179+
if (!_authTable->updateUserPassword(user, newPw))
180+
{
181+
return "Failed to update user password";
182+
}
183+
184+
return {};
171185
}
172186

173187
bool AuthManager::resetHyperionUser()

0 commit comments

Comments
 (0)