OWASP/NodeGoat

Let's add e2e test!

UlisesGascon opened this issue ยท 16 comments

Objective: The goal of this issue is to implement e2e test harness for the NodeGoat, so that it would be easier for project contributors to validate PRs and ensure that no unexpected side effects / defects arise due to enhancements.

====
As agreed with @ckarande I will create a PR to add e2e test for NodeGoat using Cypress.

The idea is to cover most of the UI, so we can test all the features before merging PRs.

Targets and assigneed:

@UlisesGascon You have captured its scope precisely! Thank you for opening this issue and offering to assist on it.

@ckarande just a spoiler ๐Ÿ“ฝ
cc: @lirantal @jesusprubio

ezgif-3-5fce08977a27

This is just /login behaviour. I will clean the code and left an open PR for you to check while I keep working on it. The idea is to gather early feedback.

@UlisesGascon Looks great, You rock! Yes, I will follow the open PR. Great work.

Great stuff @UlisesGascon!

We should probably describe the purpose of the e2e tests here.
Are they going to test the vulnerabilities or test that the application as-is continues to function as expected to so when we refactor we can ensure that nothing was broken? Seems like the latter but just making sure, and we should update the PR/issue with that goal.

Yes @lirantal, the purpose of these tests is to ensure nothing is broken as we update the project further. @UlisesGascon has some great ideas and graciously offered to assist on working on some long due enhancements. This issue is just a prep work to get the necessary test harness so that it would be easier to validate PRs in future.

In future, we can extend the cypress test setup to provide a test suite that could serve as a user guide allowing users to pass tests as they solve the challenges to fix vulnerabilities. But it is out of scope for this issue.

In either case, it is a good idea to add the purpose of this issue in it's description. Thanks for that suggestion.

Sounds good ๐Ÿ‘

Yeah... I agree with @ckarande. The best way is to split the test in two different PRs. The current one for general application behaviour and a separate one for test vulnerabilities. ๐Ÿ˜„

Good News! @KoolTheba will help us with the e2e test too.

๐ŸŽ‰ Welcome aboard @KoolTheba

Just bear in mind that any security regression tests written will be redundant once purpleteam is running over this project. Purpleteam requires no tests to be written, the product is smart enough to know what and how to test without tests being written.

@binarymist awesome!! Thanks for share it ๐Ÿ‘

Welcome aboard @KoolTheba ๐Ÿ‘ ๐ŸŽ‰

PR Review needed: See #143

@UlisesGascon Awesome! Great work. Thank you.
I will go over the PR and get back to you if any feedback.

@binarymist @lirantal Great inputs from you so far and feel free to suggest if any thoughts on the PR!

I think would be great to land it with a travis.yml file to make sure we're testing it and it becomes a baseline and we can work out follow-up PRs to refactor for more code clarity etc.

And of course amazing stuff @UlisesGascon! ๐ŸŒˆโœจ

This is merged into master now ^^