-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added check to hide password in html reports #4054
base: main
Are you sure you want to change the base?
Added check to hide password in html reports #4054
Conversation
@garg3133 you can review this. |
Status
|
Hey @dikwickley the changes look good. However can you think of a better way to hide it. Right now if you see the command in verbose logs it would show the passkey passed to |
This would also appear in the "Raw HTTP log" tab of the report. |
// Masking passwords so they don't appear as plain text in reports. | ||
if (command.name === 'setPassword') { | ||
const newArgs = [command.args[0], '*'.repeat(command.args[1].length)]; | ||
command.args = newArgs; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't add fix for a very specific case in a general method like this, it bloats these methods. We should always try to make our solution general so that we can easily expand it to other places as well (think if more commands were to use the redaction functionality, how the solution we're adding could help them as well without hard coding the command names).
See here how this functionality was approached earlier and try to make changes along the same lines and using the existing code: #2672
The above solution no longer works because while earlier we used to call TransportActions.post()
to send HTTP requests directly, we now use Selenium methods (element.sendKeys()
here) to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am looking into this only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sure, I just posted a message there so that other people can also try and find a solution to this, if they have no other issues to work on.
Converted this to draft since it still requires work. Please feel free to mark it as ready for review when the PR is ready. |
fixes: #3935
Thanks in advance for your contribution. Please follow the below steps in submitting a pull request, as it will help us with reviewing it quicker.
Final result
before:
after:
features/my-new-feature
orissue/123-my-bugfix
);