Merge pull request #7764 from code-dot-org/revert-7688-sinatra_csrf2

Revert "Add CSRF protection for the AJAX section update actions in pegasus"

aws/cloudformation/cloudfrontProperties.js

@@ -1,6 +1,6 @@
 // Lambda function for missing CloudFront properties.
 // TODO remove once CloudFormation adds support for CloudFront ACM.
-exports.handler = function (event, context) {
+exports.handler = function(event, context) {
   var response = require('cfn-response');
   console.log('REQUEST RECEIVED:\\n', JSON.stringify(event));
   var props = event.ResourceProperties;
@@ -15,11 +15,12 @@ exports.handler = function (event, context) {
     oldCert['CloudFrontDefaultCertificate'] = JSON.parse(oldCert['CloudFrontDefaultCertificate']);
     var AWS = require('aws-sdk');
     var cloudfront = new AWS.CloudFront();
-    cloudfront.getDistributionConfig({Id: distributionId}, function (err, data) {
+    cloudfront.getDistributionConfig({Id: distributionId}, function(err, data) {
       if (err) {
         console.log('getDistributionConfig failed:\\n', err);
         response.send(event, context, response.FAILED, responseData);
-      } else {
+      }
+      else {
         var etag = data.ETag;
         var config = data.DistributionConfig;
         config['ViewerCertificate'] = event.RequestType == 'Delete' ? oldCert : cert;
@@ -29,7 +30,7 @@ exports.handler = function (event, context) {
           Id: distributionId,
           DistributionConfig: config,
           IfMatch: etag
-        }, function (err, data) {
+        }, function(err, data) {
           if (err) {
             console.log('updateDistribution call failed:\\n', err);
             response.send(event, context, response.FAILED, responseData);

cookbooks/Berksfile.lock

@@ -89,7 +89,7 @@ GRAPH
   cdo-solr (0.1.0)
     cdo-java-7 (>= 0.0.0)
   cdo-users (0.1.0)
-  cdo-varnish (0.3.11)
+  cdo-varnish (0.3.10)
     apt (>= 0.0.0)
   chef-client (4.2.4)
     cron (>= 1.2.0)

cookbooks/cdo-varnish/metadata.rb

@@ -4,6 +4,6 @@
 license          'All rights reserved'
 description      'Installs/Configures cdo-varnish'
 long_description IO.read(File.join(File.dirname(__FILE__), 'README.md'))
-version          '0.3.11'
+version          '0.3.10'
 
 depends 'apt'

pegasus/config.ru

@@ -2,16 +2,7 @@ require File.expand_path('../router', __FILE__)
 
 require 'rack/csrf'
 use Rack::Session::Cookie, secret: (CDO.sinatra_session_secret || 'dev_mode')
-
-# The following routes are protected against CSRF by including
-# an unguessable session token as a form parameter or AJAX
-# request header.
-CSRF_PROTECTED_ROUTES = [
-  'POST:/v2/poste/send-message',
-  'POST:/v2/sections',
-  'POST:/v2/sections/.*'
-]
-use Rack::Csrf, check_only: CSRF_PROTECTED_ROUTES
+use Rack::Csrf, check_only: ['POST:/v2/poste/send-message']
 
 require 'rack/ssl-enforcer'
 use Rack::SslEnforcer,

pegasus/helpers.rb

@@ -82,11 +82,7 @@ def unsupported_media_type!()
 end
 
 def csrf_token
-  if env['rack.session']
-    Rack::Csrf.csrf_token(env)
-  else
-    ''  # In some tests, there is no session defined, so don't use a CSRF token.
-  end
+  Rack::Csrf.csrf_token(env)
 end
 
 def csrf_tag

pegasus/sites.v3/code.org/public/teacher-dashboard/index.haml

@@ -116,8 +116,6 @@ title: <%= I18n.t('dashboard_landing_title').inspect %>
          assessments: {method:'GET', url:'/dashboardapi/section_assessments/:id', isArray: true},
       });
     }]).config(['$httpProvider', function($httpProvider) {
-      $httpProvider.defaults.headers.common["X_CSRF_TOKEN"] = '#{csrf_token}';
-
       // X-Requested-With header required for CSRF requests protected by Rack::Protection::JsonCsrf included by Sinatra.
       // Angular originally set this, but removed it in a breaking change in v1.4 because it is "rarely used in practice":
       // https://github.com/angular/angular.js/commit/3a75b1124d062f64093a90b26630938558909e8d

pegasus/sites.v3/code.org/views/header.haml

@@ -18,7 +18,7 @@
   }
 
   #headerlinks {
-    float: right;
+    float: right; 
     position: relative;
   }
 
@@ -27,15 +27,15 @@
   }
 
   #learnoptions {
-    position: absolute;
-    top: 45px;
-    left: 10px;
-    background-color: rgb(88, 103, 113);
-    color: white;
-    padding: 10px;
-    text-align: left;
-    margin-top: 10px;
-    border-radius: 4px;
+    position: absolute; 
+    top: 45px; 
+    left: 10px; 
+    background-color: rgb(88, 103, 113); 
+    color: white; 
+    padding: 10px; 
+    text-align: left; 
+    margin-top: 10px; 
+    border-radius: 4px; 
     width: 560px;
     display: none;
   }
@@ -57,7 +57,7 @@
     border-color: rgba(54, 72, 84, 0);
     border-top-color: rgb(255, 128, 0);
     border-width: 8px;
-    margin-left: 0px;
+    margin-left: 0px;  
   }
 #pageheader-wrapper
   - pageheader_class = request.path_info == "/" ? "pageheader_translucent" : ""
@@ -97,13 +97,6 @@
   var headerSetup = false;
 
   $ = jQuery;
-
-  $.ajaxSetup({
-    beforeSend: function(xhr) {
-      xhr.setRequestHeader('X_CSRF_TOKEN', '#{csrf_token}');
-    }
-  });
-
   $(document).ready(function() {
     if (headerSetup) {
       return;

pegasus/test/test_csrf.rb

@@ -7,8 +7,6 @@
 require 'mocha/mini_test'
 require 'webmock/minitest'
 
-FAKE_CSRF_TOKEN = 'fake_token'
-
 # Test for the router CSRF logic.
 class CsrfTest < Minitest::Test
   include Rack::Test::Methods
@@ -17,69 +15,30 @@ def app
     Rack::Builder.parse_file(File.absolute_path('../config.ru', __dir__)).first
   end
 
-  def test_poste_send_message_without_csrf_token
-    as_admin_user
+  def test_poste_send_message_csrf
+    # Pretend to be an admin user
+    fake_admin = FakeAdminUserHelper.new
+    ::Documents.any_instance.stubs(:dashboard_user_helper).returns(fake_admin)
+
+    # Make sure that a post without the CSRF token is denied
     header 'host', 'code.org'
     params = {
       template: '2-11-recruitment-dallas',
       recipients: 'example@example.com'
     }
     response = post('/v2/poste/send-message', params)
     assert_equal 403, response.status
-  end
 
-  def test_poste_send_message_with_csrf_token
-    as_admin_user
-    env = {'rack.session' => session_with_csrf_token}
-    params_with_token = {
-      template: '2-11-recruitment-dallas',
-      recipients: 'example@example.com',
-      _csrf: FAKE_CSRF_TOKEN
-    }
-    response = post('/v2/poste/send-message', params_with_token, env)
+    # Set a fake CSR token in the environment and verify a post with that
+    # token succeeds.
+    fake_csrf_token = 'fake_token'
+    fake_session = {Rack::Csrf.key => fake_csrf_token}
+    params_with_token = params.merge(_csrf: fake_csrf_token)
+    response = post('/v2/poste/send-message',
+                    params_with_token,
+                    {'rack.session' => fake_session})
     assert_equal 200, response.status
   end
-
-  def set_xhr_headers
-    header 'host', 'code.org'
-    header 'X-REQUESTED-WITH', 'XMLHttpRequest'
-    header 'Content-Type', 'application/json; charset=utf-8'
-  end
-
-  def test_section_api_reject_requests_without_csrf_token
-    as_admin_user
-    set_xhr_headers
-    env = {}
-    response = post('/v2/sections', '{}', env)
-    assert_equal 403, response.status
-  end
-
-  def test_section_api_reject_requests_with_invalid_csrf_token
-    as_admin_user
-    set_xhr_headers
-    header 'X-CSRF-TOKEN', 'bad_token'
-    env = {'rack.session' => session_with_csrf_token}
-    response = post('/v2/sections', '{}', env)
-    assert_equal 403, response.status
-  end
-
-  def test_section_api_csrf_with_valid_csrf_token
-    set_xhr_headers
-    header 'X-CSRF-TOKEN', FAKE_CSRF_TOKEN
-    env = {'rack.session' => session_with_csrf_token}
-    DashboardSection.stubs(:create).returns(1)
-    response = post('/v2/sections', '{}', env)
-    assert_equal 302, response.status
-  end
-end
-
-def session_with_csrf_token
-  {Rack::Csrf.key => FAKE_CSRF_TOKEN}
-end
-
-# Stubs the Pegasus app to pretend that an admin is logged in.
-def as_admin_user
-  ::Documents.any_instance.stubs(:dashboard_user_helper).returns(FakeAdminUserHelper.new)
 end
 
 # A fake dashboard user helper which always claims to be an admin.