Remove sessions on user password update
When a user's password is changed, existing Redfish sessions for that
user, created with the old password, continue to work.
As per OWASP session management, "The session ID must be renewed or
regenerated by the web application after any privilege level change
within the associated user session... Common scenarios to consider
include; password changes, permission changes, or switching from a
regular user role to an administrator role within the web application."
[1] https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html
This commit removes existing user sessions when the user's password is
changed. This commit leaves the current session in place though a new
removeSessionsByUsernameExceptSession().
This commit doesn't completely get us fully to what owasp says but is a
start.
Tested:
Create some users:
```
curl -k -v -X POST -H "Content-Type: application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/ -d \
'{"UserName":"testadminuser","Password":"<password>","RoleId":"Administrator","Enabled":true}'
```
Using basic auth was able to update own password and another user's
password.
Using token auth, verified the current session did not get deleted but
other sessions from that user did.
```
curl -k -H "Content-Type: application/json" -X POST -D headers.txt \
https://${bmc}/redfish/v1/SessionService/Sessions -d \
'{"UserName":"testadminuser", "Password":"<password>"}'
```
```
curl -k -v -X PATCH -H "X-Auth-Token: $token" \
-H "Content-Type:application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/testadminuser \
-d '{"Password":"<password>"}'
```
Verified when changing another user's password all sessions were
dropped.
Change-Id: I4de60b84964a6b29c021dc3a2bece9ed4bc09eac
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 1d3ef41..4fe1857 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -1073,16 +1073,18 @@
const std::optional<std::string>& password,
const std::optional<bool>& enabled,
const std::optional<std::string>& roleId, const std::optional<bool>& locked,
- std::optional<std::vector<std::string>> accountTypes, bool userSelf)
+ std::optional<std::vector<std::string>> accountTypes, bool userSelf,
+ const std::shared_ptr<persistent_data::UserSession>& session)
{
sdbusplus::message::object_path tempObjPath(rootUserDbusPath);
tempObjPath /= username;
std::string dbusObjectPath(tempObjPath);
dbus::utility::checkDbusPathExists(
- dbusObjectPath, [dbusObjectPath, username, password, roleId, enabled,
- locked, accountTypes(std::move(accountTypes)),
- userSelf, asyncResp{std::move(asyncResp)}](int rc) {
+ dbusObjectPath,
+ [dbusObjectPath, username, password, roleId, enabled, locked,
+ accountTypes(std::move(accountTypes)), userSelf, session,
+ asyncResp{std::move(asyncResp)}](int rc) {
if (rc <= 0)
{
messages::resourceNotFound(asyncResp->res, "ManagerAccount",
@@ -1113,6 +1115,9 @@
}
else
{
+ // Remove existing sessions of the user when password changed
+ persistent_data::SessionStore::getInstance()
+ .removeSessionsByUsernameExceptSession(username, session);
messages::success(asyncResp->res);
}
}
@@ -2082,13 +2087,13 @@
if (!newUserName || (newUserName.value() == username))
{
updateUserProperties(asyncResp, username, password, enabled, roleId,
- locked, accountTypes, userSelf);
+ locked, accountTypes, userSelf, req.session);
return;
}
crow::connections::systemBus->async_method_call(
[asyncResp, username, password(std::move(password)),
roleId(std::move(roleId)), enabled, newUser{std::string(*newUserName)},
- locked, userSelf, accountTypes(std::move(accountTypes))](
+ locked, userSelf, req, accountTypes(std::move(accountTypes))](
const boost::system::error_code& ec, sdbusplus::message_t& m) {
if (ec)
{
@@ -2098,7 +2103,7 @@
}
updateUserProperties(asyncResp, newUser, password, enabled, roleId,
- locked, accountTypes, userSelf);
+ locked, accountTypes, userSelf, req.session);
},
"xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
"xyz.openbmc_project.User.Manager", "RenameUser", username,