Skip to content
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

password visible in html output #3935

Open
karlfock opened this issue Oct 16, 2023 · 6 comments · May be fixed by #4054
Open

password visible in html output #3935

karlfock opened this issue Oct 16, 2023 · 6 comments · May be fixed by #4054

Comments

@karlfock
Copy link

karlfock commented Oct 16, 2023

Description of the bug/issue

When using setValue in tests, and also ´setPassword`, the field is visible in the html, json and xml reports.

I found an old issue regarding this that was closed with a reference to a Browserstack configuration. But as far as I understand, that is not applicable if you're not using Browserstack.

Here is the issue I refer to:
#758

Our solution to this was to do a string replace in the final report. We found it simpler than creating custom reporter since we wanted all the features of the standrad html report. Maybe there is a simpler more standard way of doing this? But ideally, Nightwatch shouldn't put values in the report if setPassword is used.

Steps to reproduce

  1. Use setValue or setPassword etc in a test
  2. Run the test with the html reporter
  3. Check the report for where the password field is set, it will display something like:
setValue('<selector>,<password in cleartext>)

Sample test

this.navigate()
        .isVisible('@userNameField')
        .setValue('@userNameField', username)
        .setPassword('@passWordField', password)
        .click('@signInButton')

Command to run

nightwatch --reporter html

Verbose Output

No response

Nightwatch Configuration

No response

Nightwatch.js Version

3.2.1

Node Version

19.9.0

Browser

Chrome 117.0.5938

Operating System

MacOS Ventura

Additional Information

No response

@github-actions
Copy link

Thank you for setting this as an enhancement. One of the product folk will triage this again to help see when we can fit this in an upcoming sprint.

@dikwickley dikwickley linked a pull request Feb 26, 2024 that will close this issue
8 tasks
@dikwickley
Copy link
Contributor

okay so hiding the password field in the raw HTTP response is not that straight forward.
As per this issue, there is some support for this kind of thing in appium (appium/appium#14239)
but nothing like this for selenium (SeleniumHQ/selenium#9339)

We can either

Just keep the password masking till test detail level (which is already done in above PR).
or
Somehow find a way to scrub out any sensitive information in the final HTTP logs.

@garg3133 can you suggest how I should proceed

@garg3133
Copy link
Member

@dikwickley The requests are logged here:

logRequest() {

So, if we can somehow know here that we want to redact the request args for the current command, we can do that here. The only thing to think about is how would we know that.

@garg3133
Copy link
Member

This issue was earlier solved in #2672 but the solution present there no longer works because while earlier we used to call TransportActions.post() to send HTTP requests directly (with a redacted parameter) but now, we use Selenium methods (element.sendKeys() in this case) to do so, for which we cannot directly set a redacted parameter.

@dikwickley
Copy link
Contributor

Here is what I have found till now.

There is a provision to redact a HTTPRequest

this.redact = options.redact || false;

But the problem is setting this this.redact option in HttpRequest.
The issue is we don't have the control over the HttpClient (which in turn makes this HttpRequest)
This HttpClient.send is invoked by selenium-webdriver package, so there are no ways to pass in extra stuff without or telling selenium that we want the value redacted.

As you can see in the Call stack trace, there is no way to propagate information from setElementValueRedacted to the HttpClient as we are bound by the Selenium API.
Screenshot 2024-02-27 at 10 48 20 PM

Moreover, there is a feature request at SeleniumHQ/selenium#12043 that addresses just this. But it's seems like it's not happening anytime soon. Meanwhile, I have found a very weird way to work around this. Will raise a PR for it soon.

@garg3133
Copy link
Member

With the above linked PR merged now, we longer show the password text in raw HTTP logs but the password is still visible in the JSON and HTML reports, which is the only thing left to be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants