certificate: Don't check if certificate id is a number

In phosphor-certificate-manager, only the certificate object paths
implement xyz.openbmc_project.Certs.Certificate and Delete interface.
When r

certificate: Don't check if certificate id is a number

In phosphor-certificate-manager, only the certificate object paths
implement xyz.openbmc_project.Certs.Certificate and Delete interface.
When reading with GetAll and deleting, these interfaces ensures that
the object path represents a certificate. And checking if the id in
object path is a number is based on a incorrect assumption that cert
id is always a number. This patch removes such unnecessary checks and
uses the standard filename() method to extract id from object paths.

Tested:
* Verified getting and deleting certificates only works on the URL
mapped to certificate DBus object paths, other URL will give 404
Not found error.
* Verified uploading certificates responds with correct URL.
* Redfish Service Validator passed.

Change-Id: I7484fa5602afcbe9e0fc76b17483e76e12930dee
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

certificate: Use string certId in getCertificateProperties

Current implementation assumes only the DBus paths with numbers are the
paths for certificates, so here certId is of type long int. This pa

certificate: Use string certId in getCertificateProperties

Current implementation assumes only the DBus paths with numbers are the
paths for certificates, so here certId is of type long int. This patch
changes the type to std::string for removing such incorrect assumption
in future commits as dbus paths are not limited to numbers.

Tested:
* Verified both GET Certificate and POST CertificateCollection works.
* Redfish Service Validator passed.
Change-Id: I747adf4bf955194f82650a11f9dc11a85e00d6e4
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

sdbusplus: use shorter type aliases

The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are

sdbusplus: use shorter type aliases

The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are for:
* bus_t
* exception_t
* manager_t
* match_t
* message_t
* object_t
* slot_t

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I46a5eec210002af84239af74a93c830b1d4a13f1

show more ...

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. Thi

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. This commit goes through and updates all usages.

Arguably some of these are incorrect, and instances of common error 13,
but because this is mostly a mechanical it intentionally doesn't try to
handle it.

Tested: Unit tests pass.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic4c6e5d0da90f7442693199dc691a47d2240fa4f

show more ...

Fix const correctness issues

cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.

Tested: WIP

Signed-off-by: Ed Tanous <edtanous@google.com>
Cha

Fix const correctness issues

cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.

Tested: WIP

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8f37b6353fd707923f533e1d61c5b5419282bf23

show more ...

Require explicit decorator on one arg constructors

We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implic

Require explicit decorator on one arg constructors

We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implicitly constructed an object.

This commit enables the clang-tidy check.

Tested: Code compiles, passes clang-tidy.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia428463313b075c69614fdb326e8c5c094e7adde

show more ...

certificate: Add getCertificateList for get collections

This patch adds getCertificateList (based on getCertificateLocations)
to handle all the GET CertificateCollection requests in a unified way.

certificate: Add getCertificateList for get collections

This patch adds getCertificateList (based on getCertificateLocations)
to handle all the GET CertificateCollection requests in a unified way.

Tested:
* Certificates are listed in all 3 CertificateCollection correctly
* CertificateLocations is able to list all the installed certificates
* Redfish validator passed

Change-Id: I52ec6f5e77b1f48725cecdfb8d24ecb72479a887
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

certificate: Use ObjectMapper in getCertificateLocations

Currently getCertificateLocations loops through the object paths of a
certificate service and adds those paths with numeric id to the list.
T

certificate: Use ObjectMapper in getCertificateLocations

Currently getCertificateLocations loops through the object paths of a
certificate service and adds those paths with numeric id to the list.
To get all three kinds of certificates, three calls are needed. This
patch changes to use object mapper to get all paths that implements
"xyz.openbmc_project.Certs.Certificate" interface to get all certs in
a single DBus call.

Tested:
Verified all certificates installed on BMC are listed under /redfish
/v1/CertificateService/CertificateLocations, and Redfish validator
passed.

Change-Id: I0489dc9e305f67ed7d0ef07fabda5f90fd2fdac4
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

Expose AsyncResp shared_ptr when handling response

For Redfish Aggregation, we need a common point to check the D-Bus
for satellite configs. If they are available then we perform the
aggregation op

Expose AsyncResp shared_ptr when handling response

For Redfish Aggregation, we need a common point to check the D-Bus
for satellite configs. If they are available then we perform the
aggregation operations. The functions in query.hpp are used by all
endpoints making them the logical location. The aggregation code
requires a shared_ptr to the AsyncResp so these functions need to be
able to supply that.

This patch is broken out of a future patch for routing Redfish
Aggregation requests
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310

The follow commands can be used to perform most of the replacements:
find . -type f | xargs sed -i 's/setUpRedfishRoute(app, req, asyncResp->res/setUpRedfishRoute(app, req, asyncResp/g'
find . -type f | xargs sed -i 's/setUpRedfishRouteWithDelegation(app, req, asyncResp->res/setUpRedfishRouteWithDelegation(app, req, asyncResp/g'

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I4f4f9f22cdcfb14a3bd94b9a8f3d64aae34e57bc

show more ...

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."

bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.

All changes except for the .clang-format file were made by the robot.

Tested: Code compiles, whitespace changes only.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43

show more ...

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies.

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.

This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.

Tested:
Ran redfish service validator. No new failures.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iae409b0a40ddd3ae6112cb2d52c6f6ab388595fe

show more ...

Fix certificate_service privileges

Post method:
1) /redfish/v1/CertificateService/Actions/
CertificateService.GenerateCSR/
ConfigureComponents-> ConfigureManager

This ch

Fix certificate_service privileges

Post method:
1) /redfish/v1/CertificateService/Actions/
CertificateService.GenerateCSR/
ConfigureComponents-> ConfigureManager

This change allows only Admin users to Generate CSR Certificate and
restrict Operator user.

Tested: Ran curl Post requests with Admin and Operator privileged users
Get output as expected.

Email sent to openbmc list:
https://lists.ozlabs.org/pipermail/openbmc/2021-August/027232.html

Signed-off-by: Abhishek Patel <Abhishek.Patel@ibm.com>
Change-Id: I46d505357cfc55a31911e75e8bd9948a0db90555

show more ...

CertificateService: Enhance error handling for ReplaceCertificate

Current implementation of the ReplaceCertificate API always returns
ResourceNotFound if any error occurs when calling DBus, regardle

CertificateService: Enhance error handling for ReplaceCertificate

Current implementation of the ReplaceCertificate API always returns
ResourceNotFound if any error occurs when calling DBus, regardless of
whether the certificate to be replaced exists or not. This patch checks
the error code and only return ResourceNotFound when the object path
does not exist, otherwise returns InternalError.

Tested:
* Replace HTTPS certificate at /redfish/v1/Managers/bmc/NetworkProtocol
/HTTPS/Certificates/0 (Invalid URL) returns ResourceNotFound.
* Replace HTTPS certificate at /redfish/v1/Managers/bmc/NetworkProtocol
/HTTPS/Certificates/1 with CertificateString not containing private
key returns InternalError.

Change-Id: I67f6014c3856c192b4141e6a92f173a9a8c8189e
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

Add setUpRedfishRoute to all nodes in redfish

For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a

Add setUpRedfishRoute to all nodes in redfish

For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a connection, like header checking, filtering, and other
things. In the current model, where BMCWEB_ROUTE is a common function
for all HTTP routes, this means we need to propagate this injection call
into the whole tree ahead of the requests being handled.

In a perfect world, we would invent something like a REDFISH_ROUTE
macro, but because macros are discouraged, the routes take a variadic
template of parameters, and each call to the route has a .privileges()
call in the middle, there's no good way to effect this change in a less
costly manner. This was messaged both in the prior reviews, and on
discord sourcing improvements on this pattern, to which none arose.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id29cc799e214edad41e48fc7ce6eed0521f90ecb

show more ...

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb80e80307889548c0293f

show more ...

Don't rely on operator << for object logging

In the upcoming fmt patch, we remove the use of streams, and a number of
our logging statements are relying on them. This commit changes them to
no long

Don't rely on operator << for object logging

In the upcoming fmt patch, we remove the use of streams, and a number of
our logging statements are relying on them. This commit changes them to
no longer rely on operator>> or operator+ to build their strings. This
alone isn't very useful, but in the context of the next patch makes the
automation able to do a complete conversion of all log statements
automatically.

Tested: enabled logging on local and saw log statements print to console

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I0e5dc2cf015c6924037e38d547535eda8175a6a1

show more ...

json_utils: Add support jsonRead Patch/Action

Added support for readJson for Patch and Action. The only difference is
that Patch does not allow empty json input while Action does. Action with
empty

json_utils: Add support jsonRead Patch/Action

Added support for readJson for Patch and Action. The only difference is
that Patch does not allow empty json input while Action does. Action with
empty input will use the default value based on the implementation and
return 200 OK response code.

readJsonPatch will replace the existing readJson and be used for path
requests. It will not allow empty json input and all requested
keys are required in the json input.

readJsonAction will be used for Action requests where it is possible for
all of the properties to be optional and allow empty request.
The optional properties are determined by the requested values type.

All current Action readJson are replaced with readJsonAction. It does
not change the existing behavior since it needs `std::optional`.
This will have to be updated later as we define the default behavior.

Tested:
Added unit tests and readJsonAction allows empty empty json object.

No Change to Redfish Tree.

Change-Id: Ia5e1f81695c528a20f1dc985aee19c920d8adaea
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

Enable readability-implicit-bool-conversion checks

These checks ensure that we're not implicitly converting ints or
pointers into bools, which makes the code easier to read.

Tested:
Ran series thro

Enable readability-implicit-bool-conversion checks

These checks ensure that we're not implicitly converting ints or
pointers into bools, which makes the code easier to read.

Tested:
Ran series through redfish service validator. No changes observed.
UUID failing in Qemu both before and after.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I1ca0be980d136bd4e5474341f4fd62f2f6bbdbae

show more ...

Enable readability-avoid-const-params-in-decls

This check involves explicitly declaring variables const when they're
declared auto, which helps in readability, and makes it more clear that
the varia

Enable readability-avoid-const-params-in-decls

This check involves explicitly declaring variables const when they're
declared auto, which helps in readability, and makes it more clear that
the variables are const.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I71198ea03850384a389a56ad26f2c4a48c75b148

show more ...

Enable readability-container-size-empty tests

This one is a little trivial, but it does help in readability.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5366d4eec8af2f781b3bad804131a

Enable readability-container-size-empty tests

This one is a little trivial, but it does help in readability.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5366d4eec8af2f781b3bad804131ae2eb806e3aa

show more ...

Enforce const correctness

For all async calls, we should be consistently capturing non trivial
objects by const reference. This corrects bmcweb to be consistent and
capture errors by const value, a

Enforce const correctness

For all async calls, we should be consistently capturing non trivial
objects by const reference. This corrects bmcweb to be consistent and
capture errors by const value, and objects by const reference.

Tested: Code compiles. Trivial changes.

This saves about 300 bytes on our compressed binary size.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib3e0b6edef9803a1c480701556949488406305d4

show more ...

Consistently use ManagedObjectType

Some subsystems seem to have invented their own typedefs for this stuff,
move to using the one typedef in dbus::utility so we're consistent, and
we reduce our temp

Consistently use ManagedObjectType

Some subsystems seem to have invented their own typedefs for this stuff,
move to using the one typedef in dbus::utility so we're consistent, and
we reduce our templates.

Tested: code compiles

This saves a negligible amount (104 bytes compressed) on our binary
size.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I952ea1f960aa703808d0ac80f35dc24cdd8d5027

show more ...

Move to common variant

This saves approximately 34kB in the compressed binary size of bmcweb
due to reduced template instantiations. This amounts to a 2.5%
reduction in the overall size.

Note, the

Move to common variant

This saves approximately 34kB in the compressed binary size of bmcweb
due to reduced template instantiations. This amounts to a 2.5%
reduction in the overall size.

Note, there were a few places where we broke const-correctness in the
form of pulling a non-const reference out of a const variant. This
new variant now requires const correctness, so some consts are
added where required.

Tested: Code compiles.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I6a60c8881c1268627eedb4ffddf16689dc5f6ed2

show more ...

fix the year 2038 problem in getDateTime

The existing codes cast uint64_t into time_t which is int32_t in
most 32-bit systems. It results overflow if the timestamp is larger
than INT_MAX.
time_t wil

fix the year 2038 problem in getDateTime

The existing codes cast uint64_t into time_t which is int32_t in
most 32-bit systems. It results overflow if the timestamp is larger
than INT_MAX.
time_t will be 64 bits in future releases of glibc. See
https://sourceware.org/bugzilla/show_bug.cgi?id=28182.

This change workarounds the year 2038 problem via boost's ptime.
std::chrono doesn't help since it is still 32 bits.

Tested on QEMU.
Example output for certificate:
{
"Name": "HTTPS Certificate",
"Subject": null,
"ValidNotAfter": "2106-01-28T20:40:31Z",
"ValidNotBefore": "2106-02-06T18:28:16Z"
}
Previously, the format is like "1969-12-31T12:00:00+00:00". Note
that the ending "+00:00" is the time zone, not ms.

Tested the schema on QEMU. No new Redfish Service Validator errors.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8ef0bee3d724184d96253c23f3919447828d3f82

show more ...

Update clang-format

refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format
`Don't break long string literals`

Tested: built bmcweb successfully and RedfishValidator Passed.

Sig

Update clang-format

refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format
`Don't break long string literals`

Tested: built bmcweb successfully and RedfishValidator Passed.

Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: Ib58f7c942fd3838592e043c57e0b6ffcdc3d963b

show more ...