Feature/multiple account domain server - #2343
Conversation
Check overte-org#1350 for up to date list.
This will allow the user to save an updated password without poking a different setting to unlock the save button.
…th the Overte client.
JulianGro
added
server
vatral
left a comment
vatral
left a comment
There was a problem hiding this comment.
In addition to the above, attention needs to be paid to the wizard, I think it may need a change.
| return false; | ||
| } | ||
|
|
||
| if (passwordVerify && password == passwordVerify) { |
| QJsonObject accountListObject; | ||
|
|
||
| accountListObject.insert("http_username", httpUsernameValue->toString()); | ||
| accountListObject.insert("http_password", httpPasswordValue->toString()); |
There was a problem hiding this comment.
I believe this sends the hashed password to the UI of the domain server.
This means that anyone with access can read the hashes of other users, and potentially break them. This might be used for nefarious ends. For instance, a current admin could obtain password hashes, crack them, and then log in as another user even after having their access removed.
| // we've pulled a username and password - now check if there is a match in our basic auth hash | ||
| QString settingsUsername = _settingsManager.valueForKeyPath(BASIC_AUTH_USERNAME_KEY_PATH).toString(); | ||
| QVariant settingsPasswordVariant = _settingsManager.valueForKeyPath(BASIC_AUTH_PASSWORD_KEY_PATH); | ||
| QVariant allAccounts = _settingsManager.valueForKeyPath(BASIC_AUTH_MULTI_PATH); |
There was a problem hiding this comment.
Got to check whether you can lock yourself out -- I think it may be possible to delete every account.
| delete formJSON["security"]["http_authentication"][loginIndex]["http_password_verify"]; | ||
|
|
||
| // Set the form password value to the new hashed value of that password | ||
| formJSON["security"]["http_authentication"][loginIndex]["http_password"] = password; |
There was a problem hiding this comment.
If password doesn't verify, I believe we leave the password in clear text
3 participants
This is #1350 just rebased (and with some trailing whitespaces removed). There were actually no conflicts whatsoever during rebase, so I would consider this to still be code reviewed.