Age | Commit message (Collapse) | Author | Files | Lines |
|
Clang has new checks for std::move/std::forward correctness, which
catches quite a few "wrong" things where we were making copies of
callback handlers.
Unfortunately, the lambda syntax of
callback{std::forward<Callback>(callback)}
in a capture confuses it, so change usages to
callback = std::forward<Callback>(callback)
to be consistent.
Tested: Redfish service validator passes.
Change-Id: I7a111ec00cf78ecb7d5f5b102c786c1c14d74384
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
PATCH may cause bmcweb to coredump depending on timing
of `validatePrivilege` execution. It is because `req' is captured as
reference, and it may be cleared-up before async-call method completes.
(This problem can be seen more frequently by enabling debug mode).
This commit is to keep `req` during to async-method execution.
Tested:
- Create a ReadOnly user - here, called as `readonly`
- Using `redfishtool`, run PATCH on `readonly` user role.
```
$ redfishtool -vvvvv raw -r ${bmc}:18080 -u ${user} -p ${password} -S Always PATCH /redfish/v1/AccountService/Accounts/readonly --data='{"RoleId":"Administrator"}'
...
This sometimes fails because bmcweb coredump
```
After:
```
$ redfishtool raw -r ${bmc}:18080 -u ${user} -p ${password} -S Always PATCH /redfish/v1/AccountService/Accounts/readonly --data='{"RoleId":"Administrator"}'
{
"@odata.id": "/redfish/v1/AccountService/Accounts/readonly",
"@odata.type": "#ManagerAccount.v1_7_0.ManagerAccount",
...
}
```
Change-Id: I2a28d1743cfc0fbd9239f69dec5584b34c7ebe43
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
|
|
- No need to set error code in asyncResp since caller already does that.
Then we can remove the asyncResp param altogether.
- Check if session is valid before unpacking properties to avoid
unnecessary work.
- Use std::optional instead of pointers for slighter cleaner code.
- Enforce required properties for local users based on D-Bus interface
documentation (UserGroups must be provided for local users).
Change-Id: I770d3556a0d62182b6abd72bfa3f8d62e2a105d1
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
|
|
clang-format-17 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest .clang-format from the docs repository and reformat the
repository.
Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
The user's groups were getting re-appended in the session object on
every request, resulting in a small memory leak (that does clear away
when the session is ended). Fix by just overwriting instead of
appending.
Tested: Added debug prints in ~UserSession to check contents of
userGroups, then ran multiple GETs to /redfish/v1 via token auth, then
destroyed session via WebUI and confirmed userGroups contained correct
set of groups.
Change-Id: I7c04a18437f69a97f138ff1f9aeee2731952ae8b
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
|
|
This reverts commit 8ed41c35a314580bb794fa0fff2e01b0bf7efcf7.
In discord, it was posted 2 systems are hitting 403 Forbidden for all
endpoints.
Reverting fixed the problem, until time is given to dive into this,
just revert.
One of the things wrong is this is missing an After/Want
xyz.openbmc_project.User.Manager.service.
Change-Id: I1766a6ec2dbc9fb52da3940b07ac002a1a6d269a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
There is an async call within the router that leads to a small, but
pervasive performance issue for all queries. Removing that call from the
router has the potential to increase the performance of every
authenticated query, and significantly reduce our dbus traffic for
"simple" operations.
This commit re-implements the role cache in session object that existed
previously many years ago. Each users role is fetched during
authentication and persisted in session object. Each successive request
can then be matched against the privilege which is there in the
in-memory session object.
This was discussed on below commit
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39756
Tested by:
```
POST /redfish/v1/SessionService/Sessions {"UserName":"root", "Password": “0penBmc”}
```
Followed by redfish queries
Get /redfish/v1/AccountService
Tested user role persistency
Redfish service validator passes.
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I575599c29358e32849446ce6ee7f62c8eb3885f6
|
|
These were found by inspection, and should be cases that aren't
possible, but we should be consistent. Check the pointers for null
before dereferencing.
Tested: Inspection only. Condition theoretically not possible to hit.
Change-Id: I1423bb5bae5445d2b4b0cee2f3315b3ddd1c3836
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
std::format is a much more modern logging solution, and gives us a lot
more flexibility, and better compile times when doing logging.
Unfortunately, given its level of compile time checks, it needs to be a
method, instead of the stream style logging we had before. This
requires a pretty substantial change. Fortunately, this change can be
largely automated, via the script included in this commit under
scripts/replace_logs.py. This is to aid people in moving their
patchsets over to the new form in the short period where old patches
will be based on the old logging. The intention is that this script
eventually goes away.
The old style logging (stream based) looked like.
BMCWEB_LOG_DEBUG << "Foo " << foo;
The new equivalent of the above would be:
BMCWEB_LOG_DEBUG("Foo {}", foo);
In the course of doing this, this also cleans up several ignored linter
errors, including macro usage, and array to pointer deconstruction.
Note, This patchset does remove the timestamp from the log message. In
practice, this was duplicated between journald and bmcweb, and there's
no need for both to exist.
One design decision of note is the addition of logPtr. Because the
compiler can't disambiguate between const char* and const MyThing*, it's
necessary to add an explicit cast to void*. This is identical to how
fmt handled it.
Tested: compiled with logging meson_option enabled, and launched bmcweb
Saw the usual logging, similar to what was present before:
```
[Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled
[Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800
[Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist
[Info src/webserver_main.cpp:59] Starting webserver on port 18080
[Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file.
[Info src/webserver_main.cpp:137] Start Hostname Monitor Service...
```
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
|
|
The router is a giant behemoth. Start breaking it down into pieces.
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9d04f53a58ffce3ecbd88dded1aa6e9648d2a762
|