Why do we need integration tests? We've been suffering a lot of regressions in the Mephisto UI, because our functional tests don't reach high enough up towards the browser, and whole classes of bugs manage to slip through. What is Webrat? Webrat is a "browser simulator" written in Ruby. It generates a DOM and allows us to fill in forms as though an actual user were interacting with the site. Why Webrat, and not Selenium, Watir, etc? Webrat is recommended by the Cucumber project as the default way to write user stories; it's very fast; and it has a reasonable API. Plus, Webrat actively maintained, and very recent versions of Webrat can be used as a front-end to Selenium. Why Rails integration tests, and not Cucumber stories? Since the people contributing to Mephisto will largely be programmers, I decided to write integration tests using a Ruby-based DSL. Cucumber stories look really interesting, but with no actual clients in the loop, the text-based format is slightly less useful and has a steeper learning curve for programmers. Since we're switching to a new integration testing tool, I moved a bunch of code out of test_helper.rb and put it into our only existing integration test, caching_test.rb. I also switched blueprints.rb to set up user passwords using 'password' and 'password_confirm' (instead of crypted and salted values) to make it easier for tests to override
"Plain HTML" is not a very reasonable comment format for end-users, who probably want simple paragraph breaks to do the right thing. So I'm defaulting this to Textile for new sites. I also considered defaulting comments to Markdown, but I didn't like the way that Markdown handled underscores_in_identifiers or * versus _. For very simple stuff, Textile seems very slightly more intuitive than Markdown, though neither is perfect.
J.C. Zhu reported an InvalidAuthenticityToken error when trying to change themes. This was one of several bugs in the theme switching code left over from the Rails 2.2 porting process and the security audit. We needed to convince Ajax.Request to use a GET request when displaying the theme tools, and we needed to properly escape some text in the _tools template. I've also added a test case to make sure we actually render the _tools view successfully.
We've been installing safe_erb in all ERB templates, which breaks script/generate and lots of other important stuff. But before we can fix this bug in our custom-hacked safe_erb, we need to narrow our Mephisto unit tests down so that they only test ActionView::Template. A safe_erb update will be along shortly.
Some Rails authentication systems have suffered from a vulnerability involving nil or blank login tokens: http://www.rorsecurity.info/2007/10/28/restful_authentication-login-security/ This patch includes a bunch of test cases testing for possible attacks along these lines, and some sanity-checking code in our authentication methods. Note that the tests and the code don't really "line up" here--most of the test methods passed already, and most of the sanity-checking code is probably unnecessary. But again, better safe than sorry.
The W3C makes a clear distinction between GET and POST requests. GET requests should only cause "safe" actions, and the user should never be held accountable for making GET requests. See the following for an overview: http://www.w3.org/2001/tag/doc/whenToUseGet.html The Rails 'protect_against_forgery' function (and possibly some web browsers) rely on the distinction between GET and POST to provide protection against CSRF attacks. See: http://en.wikipedia.org/wiki/Cross-site_request_forgery http://guides.rubyonrails.org/security.html#_csrf_countermeasures Unfortunately, enforcing these rules in rather difficult, especially in a large application with lots of controllers and plugins. So this patch applies a rather heavy-handed fix: We globally block database writes during GET requests, and specifically override that policy in one or two places. All of our current overrides invoke User#reset_token!. I haven't performed a full security analysis of allowing User#reset_token! (or updates to session[:user] based on our "remember me" token) in a GET request. For now, I'm going to go ahead and allow this activity--if we actually have some sort of vulnerability here, it affects a wide range of web applications. Note that this patch may break some part of the /admin interface. I've tried posting articles and other basic stuff, but I haven't used the lesser-known corners of /admin since making these changes. Please report any problems.
The comment form is most-exposed attack point in all of Mephisto, because it doesn't require an authenticity_token and can be used by anybody on the internet. In the interests of paranoia, this patch removes bulk assignment from the comment-posting code. I don't see any way to exploit the previous code (several attr_accessible fields looked vulnerable, but don't actually exist any more). But better safe than sorry.
This bug was introduced in 3414a37, and was reported by "barontick" on #mephisto. Changes introduced while cleaning up unit test failures on the way to Rails 2.2 prevented users from setting the site's timezone. Some notes on this patch. 1) We want to keep using 'tzinfo', and not Rail's built-in time zone classes, because the former supports daylight savings time. 2) It's easier to just add a virtual timezone_name accessor instead of trying to do conversions in Site#timezone=. 3) We can re-enable some old specs for Site, because there's no longer any danger of deleting site themes. I really wish we had a better way of testing that HTML forms could be submitted back to the database successfully, closing the loop between two different sets of test cases on output and input.
The Rails protect_from_forgery function helps protect against cross-site request forgery attacks, as described on Wikipedia: http://en.wikipedia.org/wiki/Cross-site_request_forgery These attacks involve a hostile site sending requests to a site where the user is logged in, exploiting the user's session cookie to do various bad things. The protect_from_forgery function works by requiring all POST (and PUT and UPDATE) requests to have an authenticity_token parameter that corresponds to a value in the user's session. This is automatically included in generated forms by the various form helpers, and checked in the controller. However, we still need to deal with some cases (specifically Ajax.Request) manually. We make several types of changes to get everything working again: - Some POST requests were changed to GET requests, when appropriate. - The token was added manually to other POST requests. This was done using the new init_mephisto_authenticity_token. - Forgery protection was disabled in the test environment. Note that we still need to review the authentication controller closely, and eliminate various XSS attacks against our application before this protection will do much good. I tested this code by manually using the admin/ interface, editing articles, adding users, and working with assets. There's probably still some breakage somewhere that I missed, so let me know if you have problems. I also updated the TODO list for Rails 2.2 and added security-auditing notes.
Thank you to Isaac Kearse for finding this bug and proposing a fix. His patch made sure to delete the extra 'themes/site-4' and 'themes/site-5' directories created by running these unit tests. However, I'm still concerned about clobbering pre-existing site-* directories that were created by a user. So I'm going with a different, stop-gap solution until we can fix this problem in general throughout the unit tests.
admin/themes controller: change_to and export changed from get to post import changed from post to get added test verifying that trying to get admin/themes/change_to will fail and be redirected admin/base controller: changed protect_action method to be more generic and to give a helpful flash[:error] [Notes from Eric: I squashed three patches together and made some tweaks. See the patch changelog below.] v2: Squashed a patch series into one patch Fix themes_controller_spec to route :post request, not :get Added comment on ThemesController#import about GET handling
…for design controller
This will avoid adding 'absolute_url' to our global namespace when running other tests.
With this patch, our tests now run at 100%! Our remaining spec failures all involved model_stubbing, an old library by Rick. According to Rick, he's no longer maintaining model_stubbing, and he recommends that we use machinist to generate test data instead. This patch rips out model_stubbing, and replaces it with a combination of machinist and faker. In most cases, the new code is quite a bit shorter, simpler and easier to understand. A few minor things worth noting: - Several rspec blocks have been combined or nested. - Unnecessary setup code has been removed. - Two of the three copies of the code in site_spec.rb have been deleted. - An extra test case has been added for membership_spec.rb, to better document some interesting behavior I discovered while debugging. The machinist plugin is version 17985ba55aff6420caadb70ef698dd93aef5a26b from http://github.com/notahat/machinist/tree/master.
As in 16335c0, these tests appear to rely on a missing plugin. I hope to re-enable these once we have a useable sample plugin in the source tree. Here are some links from Sven Fuchs explaining how to write new-style Mephisto plugins. http://jamescrisp.org/2008/08/25/migrating-mephisto-plugins-to-drax-08/ http://groups.google.com/group/MephistoBlog/browse_thread/thread/ec7cc25e5720ec9c/8d8b867c8f020557?lnk=gst#8d8b867c8f020557 http://groups.google.com/group/MephistoBlog/browse_thread/thread/7fd7d1198ef8345e/c000f0e1ea8c490e?lnk=gst#c000f0e1ea8c490e
git-svn-id: http://svn.techno-weenie.net/projects/mephisto/trunk@3056 567b1171-46fb-0310-a4c9-b4bef9110e78
git-svn-id: http://svn.techno-weenie.net/projects/mephisto/trunk@3055 567b1171-46fb-0310-a4c9-b4bef9110e78
git-svn-id: http://svn.techno-weenie.net/projects/mephisto/trunk@3054 567b1171-46fb-0310-a4c9-b4bef9110e78
git-svn-id: http://svn.techno-weenie.net/projects/mephisto/trunk@3053 567b1171-46fb-0310-a4c9-b4bef9110e78
git-svn-id: http://svn.techno-weenie.net/projects/mephisto/trunk@3051 567b1171-46fb-0310-a4c9-b4bef9110e78