Merge branch 'bugfix/send-406-on-unsupported-format' into 'develop'

Bugfix/send 406 on unsupported format



See merge request !5

22 files changed, 71 insertions, 20 deletions

diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb
index ee7cca4..42e8983 100644
--- a/app/controllers/account_controller.rb
+++ b/app/controllers/account_controller.rb
@@ -3,6 +3,8 @@ class AccountController < ApplicationController
   before_filter :require_registration_allowed
   before_filter :redirect_if_logged_in
 
+  respond_to :html
+
   def new
     @user = User.new
   end
diff --git a/app/controllers/account_settings_controller.rb b/app/controllers/account_settings_controller.rb
deleted file mode 100644
index e69de29..0000000
--- a/app/controllers/account_settings_controller.rb
+++ /dev/null
diff --git a/app/controllers/api/identities_controller.rb b/app/controllers/api/identities_controller.rb
index ab2ac00..de4910a 100644
--- a/app/controllers/api/identities_controller.rb
+++ b/app/controllers/api/identities_controller.rb
@@ -3,6 +3,8 @@ module Api
     before_filter :token_authenticate
     before_filter :require_monitor
 
+    respond_to :json
+
     def show
       @identity = Identity.find_by_address(params[:id])
       if @identity
diff --git a/app/controllers/api/services_controller.rb b/app/controllers/api/services_controller.rb
index da2774b..58e129c 100644
--- a/app/controllers/api/services_controller.rb
+++ b/app/controllers/api/services_controller.rb
@@ -2,6 +2,8 @@ class Api::ServicesController < ApiController
 
   before_filter :require_login, :unless => :anonymous_access_allowed?
 
+  respond_to :json
+
   def show
     respond_with current_user.effective_service_level
   end
diff --git a/app/controllers/api/sessions_controller.rb b/app/controllers/api/sessions_controller.rb
index c8deb7a..178f86e 100644
--- a/app/controllers/api/sessions_controller.rb
+++ b/app/controllers/api/sessions_controller.rb
@@ -2,6 +2,7 @@ module Api
   class SessionsController < ApiController
 
     before_filter :require_login, only: :destroy
+    respond_to :json
 
     def new
       @session = Session.new
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 70b3cac..95c8f57 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -1,7 +1,6 @@
 class ApiController < ApplicationController
 
   skip_before_filter :verify_authenticity_token
-  respond_to :json
 
   protected
 
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2af2f29..8d08a2c 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -4,6 +4,12 @@ class ApplicationController < ActionController::Base
   before_filter :no_cache_header
   before_filter :no_frame_header
   before_filter :language_header
+
+  # UPGRADE: this won't be needed in Rails 5 anymore as it's the default
+  # behavior if a template is present but a different format would be
+  # rendered and that template is not present
+  before_filter :verify_request_format!, if: :mime_types_specified
+
   rescue_from StandardError, :with => :default_error_handler
   rescue_from CouchRest::Exception, :with => :default_error_handler
 
@@ -11,6 +17,11 @@ class ApplicationController < ActionController::Base
 
   protected
 
+  def mime_types_specified
+    mimes = collect_mimes_from_class_level
+    mimes.present?
+  end
+
   def default_error_handler(exc)
     respond_to do |format|
       format.json { render_json_error(exc) }
diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb
index d869ab5..80c270f 100644
--- a/app/controllers/errors_controller.rb
+++ b/app/controllers/errors_controller.rb
@@ -1,5 +1,7 @@
 # We render http errors ourselves so we can customize them
 class ErrorsController < ApplicationController
+  respond_to :html
+
   # 404
   def not_found
     render status: 404
diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb
index 1d62178..86c36e9 100644
--- a/app/controllers/home_controller.rb
+++ b/app/controllers/home_controller.rb
@@ -1,6 +1,8 @@
 class HomeController < ApplicationController
   layout 'home'
 
+  respond_to :html
+
   def index
     if logged_in?
       redirect_to current_user
diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb
index e0f39e3..b9c601a 100644
--- a/app/controllers/pages_controller.rb
+++ b/app/controllers/pages_controller.rb
@@ -2,7 +2,9 @@
 # Render static pages
 #
 
+
 class PagesController < ApplicationController
+  respond_to :html
 
   def show
     @show_navigation = false
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 34d4f53..18e5216 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -1,6 +1,7 @@
 class SessionsController < ApplicationController
 
   before_filter :redirect_if_logged_in, :only => [:new]
+  respond_to :html, :json
 
   def new
     @session = Session.new
@@ -16,7 +17,7 @@ class SessionsController < ApplicationController
   end
 
   #
-  # Warden will catch all 401s and run this instead:
+  # Warden will catch all 401s and triggers this action:
   #
   def unauthenticated
     login_required
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 4d198b9..0a0f551 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -7,7 +7,7 @@ class UsersController < ApplicationController
 
   before_filter :require_login
   before_filter :require_admin, :only => [:index, :deactivate, :enable]
-  before_filter :fetch_user, :only => [:show, :edit, :destroy, :deactivate, :enable]
+  before_filter :fetch_user, :except => [:index]
 
   respond_to :html
 
diff --git a/engines/billing/app/controllers/billing_admin_controller.rb b/engines/billing/app/controllers/billing_admin_controller.rb
index 23740d6..7a1de30 100644
--- a/engines/billing/app/controllers/billing_admin_controller.rb
+++ b/engines/billing/app/controllers/billing_admin_controller.rb
@@ -1,5 +1,6 @@
 class BillingAdminController < BillingBaseController
   before_filter :require_admin
+  respond_to :html
 
   #not sure if this controller is still needed. Admin can easly acess
   #braintree's dashboard and check subscriptions. Don't know if everything
diff --git a/engines/billing/app/controllers/billing_base_controller.rb b/engines/billing/app/controllers/billing_base_controller.rb
index c343938..39d41e4 100644
--- a/engines/billing/app/controllers/billing_base_controller.rb
+++ b/engines/billing/app/controllers/billing_base_controller.rb
@@ -3,6 +3,8 @@ class BillingBaseController < ApplicationController
 
   helper 'billing'
 
+  protected
+
   # required for navigation to work.
   def assign_user
     if params[:user_id]
diff --git a/engines/billing/app/controllers/payments_controller.rb b/engines/billing/app/controllers/payments_controller.rb
index 871f1b4..4f93aa6 100644
--- a/engines/billing/app/controllers/payments_controller.rb
+++ b/engines/billing/app/controllers/payments_controller.rb
@@ -1,6 +1,8 @@
 class PaymentsController < BillingBaseController
   before_filter :require_login, :only => [:index]
 
+  respond_to :html
+
   def new
     if current_user.has_payment_info?
       @client_token = Braintree::ClientToken.generate(customer_id: current_user.braintree_customer_id)
diff --git a/engines/billing/app/controllers/subscriptions_controller.rb b/engines/billing/app/controllers/subscriptions_controller.rb
index 1d29cac..9df4ecd 100644
--- a/engines/billing/app/controllers/subscriptions_controller.rb
+++ b/engines/billing/app/controllers/subscriptions_controller.rb
@@ -1,4 +1,7 @@
 class SubscriptionsController < BillingBaseController
+
+  respond_to :html
+
   before_filter :require_login
   before_filter :assign_user
   before_filter :confirm_cancel_subscription, only: [:destroy]
diff --git a/test/functional/api/certs_controller_test.rb b/test/functional/api/certs_controller_test.rb
index f23b4c8..25ceb8e 100644
--- a/test/functional/api/certs_controller_test.rb
+++ b/test/functional/api/certs_controller_test.rb
@@ -57,4 +57,10 @@ class Api::CertsControllerTest < ApiControllerTest
       returns(cert)
     return cert
   end
+
+  # overwrite defaults from ApiController because we don't do json here.
+  def add_api_defaults(args)
+    add_defaults args, version: '2'
+  end
+
 end
diff --git a/test/functional/api/sessions_controller_test.rb b/test/functional/api/sessions_controller_test.rb
index 03a1ef9..06a3c22 100644
--- a/test/functional/api/sessions_controller_test.rb
+++ b/test/functional/api/sessions_controller_test.rb
@@ -44,7 +44,8 @@ class Api::SessionsControllerTest < ApiControllerTest
 
     api_post :update, :id => @user.login, :client_auth => @client_hex
 
-    assert_nil session[:handshake]
+    assert_nil session[:handshake],
+      'session should be cleared to prevent session fixation attacks'
     assert_response :success
     assert json_response.keys.include?("id")
     assert json_response.keys.include?("token")
diff --git a/test/functional/home_controller_test.rb b/test/functional/home_controller_test.rb
new file mode 100644
index 0000000..cafaac5
--- /dev/null
+++ b/test/functional/home_controller_test.rb
@@ -0,0 +1,16 @@
+require 'test_helper'
+
+class HomeControllerTest < ActionController::TestCase
+
+  def test_renders_okay
+    get :index
+    assert_response :success
+  end
+
+  def test_other_formats_trigger_406
+    assert_raises ActionController::UnknownFormat do
+      get :index, format: :xml
+    end
+  end
+
+end
diff --git a/test/integration/api/smtp_cert_test.rb b/test/integration/api/smtp_cert_test.rb
index 53382c1..3adddfd 100644
--- a/test/integration/api/smtp_cert_test.rb
+++ b/test/integration/api/smtp_cert_test.rb
@@ -3,13 +3,8 @@ require 'openssl'
 
 class SmtpCertTest < ApiIntegrationTest
 
-  setup do
-    @testcode = InviteCode.new
-    @testcode.save!
-  end
-
   test "retrieve smtp cert" do
-    @user = FactoryGirl.create :user, effective_service_level_code: 2, :invite_code => @testcode.invite_code
+    @user = create_invited_user effective_service_level_code: 2
     login
     post smtp_cert_url, {}, RACK_ENV
     assert_text_response
@@ -20,7 +15,7 @@ class SmtpCertTest < ApiIntegrationTest
   end
 
   test "cert and key" do
-    @user = FactoryGirl.create :user, effective_service_level_code: 2, :invite_code => @testcode.invite_code
+    @user = create_invited_user effective_service_level_code: 2
     login
     post smtp_cert_url, {}, RACK_ENV
     assert_text_response
@@ -32,7 +27,7 @@ class SmtpCertTest < ApiIntegrationTest
   end
 
   test "fingerprint is stored with identity" do
-    @user = FactoryGirl.create :user, effective_service_level_code: 2, :invite_code => @testcode.invite_code
+    @user = create_invited_user effective_service_level_code: 2
     login
     post smtp_cert_url, {}, RACK_ENV
     assert_text_response
@@ -46,7 +41,6 @@ class SmtpCertTest < ApiIntegrationTest
   end
 
   test "fetching smtp certs requires email account" do
-
     login
     post smtp_cert_url, {}, RACK_ENV
     assert_access_denied
diff --git a/test/support/api_controller_test.rb b/test/support/api_controller_test.rb
index 06cb46a..97d86fc 100644
--- a/test/support/api_controller_test.rb
+++ b/test/support/api_controller_test.rb
@@ -17,7 +17,7 @@ class ApiControllerTest < ActionController::TestCase
   end
 
   def add_api_defaults(args)
-    add_defaults args, version: '2'
+    add_defaults args, version: '2', format: :json
   end
 
   def add_defaults(args, defaults)
diff --git a/test/support/api_integration_test.rb b/test/support/api_integration_test.rb
index cea480c..7942558 100644
--- a/test/support/api_integration_test.rb
+++ b/test/support/api_integration_test.rb
@@ -7,13 +7,8 @@ class ApiIntegrationTest < ActionDispatch::IntegrationTest
     2
   end
 
-  setup do
-    @testcode = InviteCode.new
-    @testcode.save!
-  end
-
   def login(user = nil)
-    @user ||= user ||= FactoryGirl.create(:user, :invite_code => @testcode.invite_code)
+    @user ||= user ||= create_invited_user
     # DUMMY_TOKEN will be frozen. So let's use a dup
     @token ||= DUMMY_TOKEN.dup
     # make sure @token is up to date if it already exists
@@ -23,6 +18,13 @@ class ApiIntegrationTest < ActionDispatch::IntegrationTest
     @token.save
   end
 
+  def create_invited_user(options = {})
+    @testcode = InviteCode.new
+    @testcode.save!
+    options.reverse_merge! invite_code: @testcode.invite_code
+    FactoryGirl.create :user, options
+  end
+
   teardown do
     if @user && @user.persisted?
       @user.destroy_identities