From fbad882075e745ab7afbe5f89c67544fb3c607c3 Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Thu, 18 Aug 2016 11:00:16 +0200
Subject: respond_to on a per controller basis

If you inherit respond to and call it again in your controller
it will not overwrite the previous but add to it.

Since we always have some exceptions from the rules it's probably
easiest to be explicit in the controllers that require it themselves.
---
 app/controllers/account_controller.rb              |  2 ++
 app/controllers/account_settings_controller.rb     |  0
 app/controllers/api/identities_controller.rb       |  2 ++
 app/controllers/api/services_controller.rb         |  2 ++
 app/controllers/api/sessions_controller.rb         |  1 +
 app/controllers/api_controller.rb                  |  1 -
 app/controllers/application_controller.rb          | 26 ++++++++--------------
 app/controllers/errors_controller.rb               |  2 ++
 app/controllers/home_controller.rb                 |  2 ++
 app/controllers/pages_controller.rb                |  2 ++
 app/controllers/sessions_controller.rb             |  3 ++-
 app/controllers/users_controller.rb                |  2 +-
 .../app/controllers/billing_admin_controller.rb    |  1 +
 .../app/controllers/billing_base_controller.rb     |  2 ++
 .../billing/app/controllers/payments_controller.rb |  2 ++
 .../app/controllers/subscriptions_controller.rb    |  3 +++
 test/functional/api/certs_controller_test.rb       |  6 +++++
 test/functional/api/sessions_controller_test.rb    |  3 ++-
 test/integration/api/smtp_cert_test.rb             | 12 +++-------
 test/support/api_controller_test.rb                |  2 +-
 test/support/api_integration_test.rb               | 14 +++++++-----
 21 files changed, 53 insertions(+), 37 deletions(-)
 delete mode 100644 app/controllers/account_settings_controller.rb

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
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 61ced21..8d08a2c 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,33 +1,25 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
-  before_action :check_mime_types
   before_filter :set_locale
   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
 
   ActiveSupport.run_load_hooks(:application_controller, self)
 
-  # by default we only respond to html.
-  # If you want to respond with json you are probably working on
-  # an ApiController.
-  respond_to :html
-
   protected
 
-  # 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
-  def check_mime_types
-    mimes = collect_mimes_from_class_level()
-    return if mimes.empty?
-
-    collector = ActionController::MimeResponds::Collector.new(mimes, request.variant)
-    unless collector.negotiate_format(request)
-      raise ActionController::UnknownFormat
-    end
+  def mime_types_specified
+    mimes = collect_mimes_from_class_level
+    mimes.present?
   end
 
   def default_error_handler(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/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
-- 
cgit v1.2.3