Improved checking for sensitive properties in logs#2118
Open
andy-maier wants to merge 1 commit intomasterfrom
Open
Improved checking for sensitive properties in logs#2118andy-maier wants to merge 1 commit intomasterfrom
andy-maier wants to merge 1 commit intomasterfrom
Conversation
andy-maier
force-pushed
the
andy/blank-client-secret
branch
from
efd84b7 to
80ef752
Compare
Collaborator
andy-maier
force-pushed
the
andy/blank-client-secret
branch
2 times, most recently
from
b1957a5 to
f45b6fa
Compare
andy-maier
force-pushed
the
andy/blank-client-secret
branch
from
f45b6fa to
1ceb19d
Compare
The following sensitive data is now blanked out that was not blanked out before:
In the HTTP operation logging:
- "Create SSO Server Definition" operation:
- `client-secret` request field - OIDC client secret for the SSO server
In the API call logging:
- SSOServerDefinitionManager.create():
- `client-secret` property - OIDC client secret for the SSO server
Note that the support for SSO server definitions is new and has not yet
been released. So this is not an externally documented fix.
Details:
* Changed the approach for blanking out sensitive properties in logs for
API calls and return values: Instead of using the blanked_properties
argument of the logged_api_call decorator to point to specific properties,
the blanking out is now done when the string representation of the
argument or return value has been created, by matching certain
property names using a regexp in the repr() string of the arguments or
return values.
The blanked_properties argument in the logged_api_call decorator still
exists in case it is needed in the future for additional properties, but
it is not currently used.
* Changed the approach for blanking out sensitive properties in logs for
HTTP requests and responses: Instead of checking for a set of specific
sensitive property names in the dict representation of the request and
response bodies, the sensitive properties are now detected by matching
certain property names using a regexp in the JSON string of the body.
* In the test_logging.py unit test module, changed the use of .msg for
accessing the expanded log message to using .getMessage(). It is not
clear why that worked before, but getMessage() is the official way to
get at the expanded log message.
* Improved performance in _log_http_request() and _log_http_response()
by not executing the code when debug logging is disabled.
Signed-off-by: Andreas Maier <maiera@de.ibm.com>
andy-maier
force-pushed
the
andy/blank-client-secret
branch
from
1ceb19d to
a2ffa49
Compare
| @@ -0,0 +1,3 @@ | |||
| Added a `stop_event` parameter to the :meth:`zhmcclient.NotificationReceiver.notifications` | |||
Contributor
There was a problem hiding this comment.
This change log entry looks unrelated to the change.
|
|
||
| #: Regexp pattern for properties in the JSON or dict representation string that | ||
| #: contains sensitive data and will be blanked out in any logs. | ||
| BLANKED_OUT_PROPERTY_PATTERN = re.compile( |
Contributor
There was a problem hiding this comment.
Could you add a unit test that checks that the existing sensitive properties are correctly captured by the regex to prevent regressions?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For details, see the commit message. It is helpful to first read the commit message before reviewing the code.
Does not need to be rolled back to 1.25 because the only newly blanked-out property is
client-secretinSSOServerDefinitionwhich has not yet been released.