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

allow to configure proxy via env var #761

Merged
merged 6 commits into from Dec 23, 2018
Merged

allow to configure proxy via env var #761

merged 6 commits into from Dec 23, 2018

Conversation

omerlh
Copy link
Contributor

@omerlh omerlh commented Dec 16, 2018

As part of a session I'm doing, I wanted to demo Zap capabilities as DAST solution. The best test case is, of course, juice shop. So I added a small change that will allow to run the existing e2e tests and scan them with Zap. All that is now required is to run:

http_proxy=http://localhost:8080 yarn run e2e

And the e2e will be proxied via Zap (assuming http://localhost:8080 is Zap's proxy endpoint).
Is this something that you would like to merge? Do you want me to add a small docker compose file that will run everything in compose?
BTW I tried to proxy the API tests, but I couldn't find how to control firsby proxy behavior. I managed to do it only by changing the tests code, but this is pretty ugly and not something I think should merged.
Soluto/webdriverio-zap-proxy/issues/6

@bkimminich
Copy link
Member

I like the idea, but http_proxy is kind of a standard system variable name, that it might be set to e.g. some corporate proxy - and then this would mess with the tests, right?

@omerlh
Copy link
Contributor Author

omerlh commented Dec 17, 2018

That's a tricky question. I choose http_proxy because it's so common, so I thought it's better to use the regular environment variable that controls proxy settings.
In the case of a corporate proxy - it just means that the tests will run using this proxy - which might be the expected behavior. E.g. - sometime the proxy is the only way to reach the internet.

@omerlh
Copy link
Contributor Author

omerlh commented Dec 22, 2018

Ok, now the tests should finally pass.

@omerlh
Copy link
Contributor Author

omerlh commented Dec 22, 2018

Do you know why travis test failed although it look like a success?

Protractor exited with code 0 (SUCCESS)
The command "node_version=$(node -v); if [ ${node_version:1:2} = 10 ]; then NODE_ENV=ctf npm run protractor; else echo "Skipping Protractor e2e tests on $node_version"; fi" exited with 0.

@bkimminich
Copy link
Member

bkimminich commented Dec 22, 2018

It failed already at running the code style checks:


standard: Use JavaScript Standard Style (https://standardjs.com)
standard: Run `standard --fix` to automatically fix some problems.
/home/travis/build/bkimminich/juice-shop/protractor.conf.js:5:2: Extra semicolon.
/home/travis/build/bkimminich/juice-shop/protractor.conf.js:7:28: Expected '!==' and instead saw '!='.
npm ERR! Test failed. See above for more details.
The command "npm test" exited with 1.

Travis-CI runs all tests regardsless of prior suites failing. That can be confusing to spot the actual failure, but it avoids chasing from one failing suite to the next.

@omerlh
Copy link
Contributor Author

omerlh commented Dec 23, 2018

Ohh yes, I missed that. Thanks! Hope now it will pass...

@omerlh
Copy link
Contributor Author

omerlh commented Dec 23, 2018

Now it finally passed, let me know if you want me to change the name of the env var :)

@bkimminich
Copy link
Member

Looks good, will try the behavior in proxied corp. environment after merging! 👍

If you want to try your luck with API tests, here might be a solution: vlucas/frisby#363

@bkimminich bkimminich changed the base branch from master to develop December 23, 2018 10:55
@bkimminich bkimminich merged commit 9b9d28e into juice-shop:develop Dec 23, 2018
bkimminich added a commit to juice-shop/pwning-juice-shop that referenced this pull request Dec 23, 2018
@omerlh
Copy link
Contributor Author

omerlh commented Dec 23, 2018

By looking on the issue, we either need to that per file, or create frisby in one file, and reuse the same instance in all tests. Am I right?

crispy-peppers pushed a commit to crispy-peppers/juice-shop that referenced this pull request Nov 12, 2019
* Fix /user page for users without teams (e.g. user mode)
@lock
Copy link

lock bot commented Dec 23, 2019

This thread has been automatically locked because it has not had recent activity after it was closed. 🔒 Please open a new issue for regressions or related bugs.

@lock lock bot locked and limited conversation to collaborators Dec 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants