Browse Source

Improved error handling for json requests.

Martin Edenhofer 8 years ago
parent
commit
9fe709f9b7

+ 67 - 51
app/controllers/application_controller.rb

@@ -1,4 +1,5 @@
 # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/
+require 'exceptions'
 
 class ApplicationController < ActionController::Base
   #  http_basic_authenticate_with :name => "test", :password => "ttt"
@@ -18,6 +19,13 @@ class ApplicationController < ActionController::Base
   before_action :set_user, :session_update, :user_device_check, :cors_preflight_check
   after_action  :trigger_events, :http_log, :set_access_control_headers
 
+  rescue_from StandardError, with: :server_error
+  rescue_from ExecJS::RuntimeError, with: :server_error
+  rescue_from ActiveRecord::RecordNotFound, with: :not_found
+  rescue_from ArgumentError, with: :unprocessable_entity
+  rescue_from Exceptions::UnprocessableEntity, with: :unprocessable_entity
+  rescue_from Exceptions::NotAuthorized, with: :unauthorized
+
   # For all responses in this controller, return the CORS access control headers.
   def set_access_control_headers
     headers['Access-Control-Allow-Origin']      = '*'
@@ -192,8 +200,7 @@ class ApplicationController < ActionController::Base
     # for sessions we need the fingperprint
     if type == 'session'
       if !session[:user_device_updated_at] && !params[:fingerprint] && !session[:user_device_fingerprint]
-        render json: { error: 'Need fingerprint param!' }, status: :unprocessable_entity
-        return false
+        raise Exceptions::UnprocessableEntity, 'Need fingerprint param!'
       end
       if params[:fingerprint]
         session[:user_device_fingerprint] = params[:fingerprint]
@@ -310,13 +317,7 @@ class ApplicationController < ActionController::Base
 
     # return auth not ok
     if result[:auth] == false
-      render(
-        json: {
-          error: result[:message],
-        },
-        status: :unauthorized
-      )
-      return false
+      raise Exceptions::NotAuthorized, result[:message]
     end
 
     # return auth ok
@@ -330,35 +331,27 @@ class ApplicationController < ActionController::Base
 
   def ticket_permission(ticket)
     return true if ticket.permission(current_user: current_user)
-    response_access_deny
-    false
+    raise Exceptions::NotAuthorized
   end
 
   def article_permission(article)
     ticket = Ticket.lookup(id: article.ticket_id)
     return true if ticket.permission(current_user: current_user)
-    response_access_deny
-    false
+    raise Exceptions::NotAuthorized
   end
 
   def deny_if_not_role(role_name)
     return false if role?(role_name)
-    response_access_deny
-    true
+    raise Exceptions::NotAuthorized
   end
 
   def valid_session_with_user
     return true if current_user
-    render json: { message: 'No session user!' }, status: :unprocessable_entity
-    false
+    raise Exceptions::UnprocessableEntity, 'No session user!'
   end
 
   def response_access_deny
-    render(
-      json: {},
-      status: :unauthorized
-    )
-    false
+    raise Exceptions::NotAuthorized
   end
 
   def config_frontend
@@ -401,10 +394,6 @@ class ApplicationController < ActionController::Base
     end
 
     model_create_render_item(generic_object)
-  rescue => e
-    logger.error e.message
-    logger.error e.backtrace.inspect
-    render json: model_match_error(e.message), status: :unprocessable_entity
   end
 
   def model_create_render_item(generic_object)
@@ -431,10 +420,6 @@ class ApplicationController < ActionController::Base
     end
 
     model_update_render_item(generic_object)
-  rescue => e
-    logger.error e.message
-    logger.error e.backtrace.inspect
-    render json: model_match_error(e.message), status: :unprocessable_entity
   end
 
   def model_update_render_item(generic_object)
@@ -445,17 +430,13 @@ class ApplicationController < ActionController::Base
     generic_object = object.find(params[:id])
     generic_object.destroy
     model_destory_render_item()
-  rescue => e
-    logger.error e.message
-    logger.error e.backtrace.inspect
-    render json: model_match_error(e.message), status: :unprocessable_entity
   end
 
   def model_destory_render_item ()
     render json: {}, status: :ok
   end
 
-  def model_show_render (object, params)
+  def model_show_render(object, params)
 
     if params[:expand]
       generic_object = object.find(params[:id])
@@ -471,10 +452,6 @@ class ApplicationController < ActionController::Base
 
     generic_object = object.find(params[:id])
     model_show_render_item(generic_object)
-  rescue => e
-    logger.error e.message
-    logger.error e.backtrace.inspect
-    render json: model_match_error(e.message), status: :unprocessable_entity
   end
 
   def model_show_render_item(generic_object)
@@ -522,10 +499,6 @@ class ApplicationController < ActionController::Base
       generic_objects_with_associations.push item.attributes_with_associations
     }
     model_index_render_result(generic_objects_with_associations)
-  rescue => e
-    logger.error e.message
-    logger.error e.backtrace.inspect
-    render json: model_match_error(e.message), status: :unprocessable_entity
   end
 
   def model_index_render_result(generic_objects)
@@ -546,18 +519,62 @@ class ApplicationController < ActionController::Base
     generic_object = object.find(params[:id])
     result = Models.references(object, generic_object.id)
     return false if result.empty?
-    render json: { error: 'Can\'t delete, object has references.' }, status: :unprocessable_entity
-    true
+    raise Exceptions::UnprocessableEntity, 'Can\'t delete, object has references.'
   rescue => e
+    raise Exceptions::UnprocessableEntity, e
+  end
+
+  def not_found(e)
     logger.error e.message
     logger.error e.backtrace.inspect
-    render json: model_match_error(e.message), status: :unprocessable_entity
+    respond_to do |format|
+      format.json { render json: model_match_error(e.message), status: :not_found }
+      format.any {
+        @exception = e
+        @traceback = !Rails.env.production?
+        file = File.open(Rails.root.join('public', '404.html'), 'r')
+        render inline: file.read, status: :not_found
+      }
+    end
   end
 
-  def not_found(e)
+  def unprocessable_entity(e)
+    logger.error e.message
+    logger.error e.backtrace.inspect
     respond_to do |format|
-      format.json { render json: { error: e.message }, status: :not_found }
-      format.any { render text: "Error: #{e.message}", status: :not_found }
+      format.json { render json: model_match_error(e.message), status: :unprocessable_entity }
+      format.any {
+        @exception = e
+        @traceback = !Rails.env.production?
+        file = File.open(Rails.root.join('public', '422.html'), 'r')
+        render inline: file.read, status: :unprocessable_entity
+      }
+    end
+  end
+
+  def server_error(e)
+    logger.error e.message
+    logger.error e.backtrace.inspect
+    respond_to do |format|
+      format.json { render json: model_match_error(e.message), status: 500 }
+      format.any {
+        @exception = e
+        @traceback = !Rails.env.production?
+        file = File.open(Rails.root.join('public', '500.html'), 'r')
+        render inline: file.read, status: 500
+      }
+    end
+  end
+
+  def unauthorized(e)
+    respond_to do |format|
+      format.json { render json: model_match_error(e.message), status: :unauthorized }
+      format.any {
+        @exception = e
+        @traceback = !Rails.env.production?
+        file = File.open(Rails.root.join('public', '401.html'), 'r')
+        render inline: file.read, status: :unauthorized
+      }
     end
   end
 
@@ -571,8 +588,7 @@ class ApplicationController < ActionController::Base
 
   def check_maintenance(user)
     return false if !check_maintenance_only(user)
-    render json: { error: 'Maintenance mode enabled!' }, status: :unauthorized
-    true
+    raise Exceptions::NotAuthorized, 'Maintenance mode enabled!'
   end
 
 end

+ 5 - 5
app/controllers/calendars_controller.rb

@@ -4,7 +4,7 @@ class CalendarsController < ApplicationController
   before_action :authentication_check
 
   def index
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     # calendars
     assets = {}
@@ -25,22 +25,22 @@ class CalendarsController < ApplicationController
   end
 
   def show
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_show_render(Calendar, params)
   end
 
   def create
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_create_render(Calendar, params)
   end
 
   def update
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(Calendar, params)
   end
 
   def destroy
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_destory_render(Calendar, params)
   end
 end

+ 15 - 17
app/controllers/channels_controller.rb

@@ -17,8 +17,8 @@ curl http://localhost/api/v1/group/channels.json -v -u #{login}:#{password} -H "
 =end
 
   def group_update
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
-    return if !check_access
+    deny_if_not_role(Z_ROLENAME_ADMIN)
+    check_access
 
     channel = Channel.find(params[:id])
     channel.group_id = params[:group_id]
@@ -40,8 +40,8 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
 =end
 
   def destroy
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
-    return if !check_access
+    deny_if_not_role(Z_ROLENAME_ADMIN)
+    check_access
     model_destory_render(Channel, params)
   end
 
@@ -64,7 +64,7 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
   end
 
   def twitter_verify
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(Channel, params)
   end
 
@@ -87,12 +87,12 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
   end
 
   def facebook_verify
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(Channel, params)
   end
 
   def email_index
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     system_online_service = Setting.get('system_online_service')
     account_channel_ids = []
     notification_channel_ids = []
@@ -143,7 +143,7 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
   def email_probe
 
     # check admin permissions
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     # probe settings based on email and password
     result = EmailHelper::Probe.full(
@@ -163,7 +163,7 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
   def email_outbound
 
     # check admin permissions
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     # verify access
     return if params[:channel_id] && !check_access(params[:channel_id])
@@ -175,7 +175,7 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
   def email_inbound
 
     # check admin permissions
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     # verify access
     return if params[:channel_id] && !check_access(params[:channel_id])
@@ -192,7 +192,7 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
   def email_verify
 
     # check admin permissions
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     email = params[:email] || params[:meta][:email]
     email = email.downcase
@@ -284,10 +284,10 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
 
   def email_notification
 
-    return if !check_online_service
+    check_online_service
 
     # check admin permissions
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     adapter = params[:adapter].downcase
 
@@ -341,8 +341,7 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
 
   def check_online_service
     return true if !Setting.get('system_online_service')
-    response_access_deny
-    false
+    raise Exceptions::NotAuthorized
   end
 
   def check_access(id = nil)
@@ -354,7 +353,6 @@ curl http://localhost/api/v1/channels.json -v -u #{login}:#{password} -H "Conten
     channel = Channel.find(id)
     return true if channel.preferences && !channel.preferences[:online_service_disable]
 
-    response_access_deny
-    false
+    raise Exceptions::NotAuthorized
   end
 end

+ 5 - 5
app/controllers/chats_controller.rb

@@ -4,7 +4,7 @@ class ChatsController < ApplicationController
   before_action :authentication_check
 
   def index
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     chat_ids = []
     assets = {}
     Chat.order(:id).each {|chat|
@@ -20,22 +20,22 @@ class ChatsController < ApplicationController
   end
 
   def show
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_show_render(Chat, params)
   end
 
   def create
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_create_render(Chat, params)
   end
 
   def update
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(Chat, params)
   end
 
   def destroy
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_destory_render(Chat, params)
   end
 end

+ 2 - 2
app/controllers/cti_controller.rb

@@ -5,7 +5,7 @@ class CtiController < ApplicationController
 
   # list current caller log
   def index
-    return if deny_if_not_role('CTI')
+    deny_if_not_role('CTI')
 
     backends = [
       {
@@ -22,7 +22,7 @@ class CtiController < ApplicationController
 
   # set caller log to done
   def done
-    return if deny_if_not_role('CTI')
+    deny_if_not_role('CTI')
     log = Cti::Log.find(params['id'])
     log.done = params['done']
     log.save

+ 3 - 3
app/controllers/email_addresses_controller.rb

@@ -97,7 +97,7 @@ curl http://localhost/api/v1/email_addresses.json -v -u #{login}:#{password} -H
 =end
 
   def create
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_create_render(EmailAddress, params)
   end
 
@@ -128,7 +128,7 @@ curl http://localhost/api/v1/email_addresses/#{id}.json -v -u #{login}:#{passwor
 =end
 
   def update
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(EmailAddress, params)
   end
 
@@ -146,7 +146,7 @@ curl http://localhost/api/v1/email_addresses/#{id}.json -v -u #{login}:#{passwor
 =end
 
   def destroy
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_destory_render(EmailAddress, params)
   end
 end

+ 5 - 0
app/controllers/errors_controller.rb

@@ -0,0 +1,5 @@
+class ErrorsController < ApplicationController
+  def routing
+    not_found(ActionController::RoutingError.new("No route matches [#{request.method}] #{request.path}"))
+  end
+end

+ 7 - 7
app/controllers/external_credentials_controller.rb

@@ -4,27 +4,27 @@ class ExternalCredentialsController < ApplicationController
   before_action :authentication_check
 
   def index
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_index_render(ExternalCredential, params)
   end
 
   def show
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_show_render(ExternalCredential, params)
   end
 
   def create
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_create_render(ExternalCredential, params)
   end
 
   def update
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(ExternalCredential, params)
   end
 
   def destroy
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_destory_render(ExternalCredential, params)
   end
 
@@ -37,7 +37,7 @@ class ExternalCredentialsController < ApplicationController
   end
 
   def link_account
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     provider = params[:provider].downcase
     attributes = ExternalCredential.request_account_to_link(provider)
     session[:request_token] = attributes[:request_token]
@@ -45,7 +45,7 @@ class ExternalCredentialsController < ApplicationController
   end
 
   def callback
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     provider = params[:provider].downcase
     channel = ExternalCredential.link_account(provider, session[:request_token], params)
     session[:request_token] = nil

+ 1 - 1
app/controllers/getting_started_controller.rb

@@ -111,7 +111,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password}
   def base
 
     # check admin permissions
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
 
     # validate url
     messages = {}

+ 3 - 3
app/controllers/groups_controller.rb

@@ -101,7 +101,7 @@ curl http://localhost/api/v1/groups -v -u #{login}:#{password} -H "Content-Type:
 =end
 
   def create
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_create_render(Group, params)
   end
 
@@ -133,7 +133,7 @@ curl http://localhost/api/v1/groups -v -u #{login}:#{password} -H "Content-Type:
 =end
 
   def update
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_update_render(Group, params)
   end
 
@@ -151,7 +151,7 @@ curl http://localhost/api/v1/groups/{id} -v -u #{login}:#{password} -H "Content-
 =end
 
   def destroy
-    return if deny_if_not_role(Z_ROLENAME_ADMIN)
+    deny_if_not_role(Z_ROLENAME_ADMIN)
     model_destory_render(Group, params)
   end
 end

Some files were not shown because too many files changed in this diff