search

OWASP / railsgoat

A vulnerable version of Rails that follows the OWASP Top 10
railsgoat.cktricky.com
MIT License
858 stars 666 forks source link

Clean up specs #372

Closed rifkinni closed 4 years ago

rifkinni commented 4 years ago

I'm putting together a training using RailsGoat and noticed a number of issues in the specs that made it difficult or impossible to get a fully passing test suite. In addition, a couple of the tutorial links were out of date.

There are a number of changes in this pull request that are true bug fixes and one or two changes that I would consider small improvements to the specs. I'm open to any and all feedback!

forced-request commented 4 years ago

Hey @rifkinni thanks for this Pull Request. It's been a while since I've actually used Railsgoat 😊. I had created a similar PR a year ago, which we never merged. There was some confusion around the best way to test for the CSRF case. Did you run into any issues with getting that test to pass/fail as expected?

Side note: I'll close my other PR in favor of yours. It was created from an account I no longer have access to anyway :)

rifkinni commented 4 years ago

https://github.com/OWASP/railsgoat/commit/190a0bd6b83e6c17b192ae95e74a08ccb5cebd3b

This was my solution to the CSRF vulnerability - I think I solved for the issue you ran into by including the conditional if current_user

Thanks for taking a look at this PR @forced-request!

rifkinni commented 4 years ago

I hadn't thought to do this originally https://github.com/OWASP/railsgoat/pull/320/files#diff-a4d8cf95af68ed26ffd6385fe0ce6182R23 but I've just added it to mine as well!

cktricky commented 4 years ago

Thanks @rifkinni ! We'll go ahead and merge. Sorry for the delay! I spoke with @forced-request today and we're 👍 to merge.