Omitting authentication tokens from the request text while logging to the trace database files #912
Conversation
… the trace databases when no_tokens_in_logs setting is set to true
| @@ -120,6 +120,7 @@ def to_dict(self, omit_request_text=None): | |||
| tags.update(self.tags) | |||
| tags.update(self.sequence_tags) | |||
| request_text = None if omit_request_text == True else self.request | |||
| request_text = logger.remove_tokens_from_logs(request_text) if Settings().no_tokens_in_logs else request_text | |||
There was a problem hiding this comment.
Hi Swamy, thanks for the fix!
The check-in tests did not run because we need to migrate them to GitHub actions (this will be done soon), so you will need to run unit tests locally. For this change, you can just run 'test_trace_database*' tests for validation.
The Settings() object can't be used directly here - could you please restructure the code as follows, which should fix the issue:
def to_dict(self, omit_request_text=None, remove_tokens_from_logs=True):
from utils.logger import remove_tokens_from_logs as remove_tokens
tags = {}
...
request_text = None if omit_request_text == True else self.request
request_text = remove_tokens(request_text) if remove_tokens_from_logs else request_text
then just one place in trace_db.py line 202 needs to be updated with the new parameter set to the setting.
There was a problem hiding this comment.
Thanks for detailed code suggestions.
Somehow I saw the usage of Settings() in the file and assumed it will work.
I did run unit tests locally and they failed, but I ran all of those and it took some time.
Now the tests are all succeeding.
Is there a place I should add verification in the unit tests to check if auth tokens are actually getting replaced or not?
|
@microsoft-github-policy-service agree company="Microsoft" |
When no_tokens_in_logs is set to true, the RESTler traditional logger detects the authorization tokens in the requests and replaces the same with OMITTED_AUTH_TOKEN. The same is not happening when logging is enabled to trace logger files using the setting use_trace_database.
The changes in this pull request aim to copy the functionality to trace logger and make trace logger also omit the auth headers during logging.