Move api error handling and timeouts to parent class

Fixes #4861

Since the around_action is defined before authorize_resource is called,
the handler needs to pass on the CanCan::AccessDenied exception.

I've added the timeouts where I think they were missing (e.g. UserPreferencesController)
but I've kept the exception for changeset#upload and traces#create
This commit is contained in:
Andy Allan 2024-10-02 16:37:32 +01:00
parent e8da505518
commit 83425edd8d
18 changed files with 6 additions and 27 deletions

View file

@ -5,7 +5,6 @@ module Api
authorize_resource :class => false
before_action :set_request_formats
around_action :api_call_handle_error, :api_call_timeout
# External apps that use the api are able to query the api to find out some
# parameters of the API. It currently returns:

View file

@ -6,9 +6,8 @@ module Api
authorize_resource
before_action :require_public_data, :only => [:create]
before_action :set_request_formats
around_action :api_call_handle_error
around_action :api_call_timeout
##
# Add a comment to a changeset

View file

@ -11,8 +11,7 @@ module Api
before_action :require_public_data, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
before_action :set_request_formats, :except => [:create, :close, :upload]
around_action :api_call_handle_error
around_action :api_call_timeout, :except => [:upload]
skip_around_action :api_call_timeout, :only => [:upload]
# Helper methods for checking consistency
include ConsistencyValidations

View file

@ -2,8 +2,6 @@ module Api
class MapController < ApiController
authorize_resource :class => false
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats
# This is probably the most common call of all. It is used for getting the

View file

@ -9,8 +9,6 @@ module Api
authorize_resource
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats
def inbox

View file

@ -8,8 +8,6 @@ module Api
authorize_resource
before_action :require_public_data, :only => [:create, :update, :delete]
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats, :except => [:create, :update, :delete]
before_action :check_rate_limit, :only => [:create, :update, :delete]

View file

@ -7,7 +7,6 @@ module Api
authorize_resource
before_action :set_locale
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats, :except => [:feed]
##

View file

@ -9,7 +9,6 @@ module Api
authorize_resource
around_action :api_call_handle_error, :api_call_timeout
before_action :lookup_old_element, :except => [:history]
before_action :lookup_old_element_versions, :only => [:history]

View file

@ -4,7 +4,6 @@ module Api
before_action :setup_user_auth
before_action :set_request_formats
around_action :api_call_handle_error, :api_call_timeout
# External apps that use the api are able to query which permissions
# they have. This currently returns a list of permissions granted to the current user:

View file

@ -6,8 +6,6 @@ module Api
authorize_resource
before_action :require_public_data, :only => [:create, :update, :delete]
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats, :except => [:create, :update, :delete]
before_action :check_rate_limit, :only => [:create, :update, :delete]

View file

@ -2,8 +2,6 @@ module Api
class TracepointsController < ApiController
authorize_resource
around_action :api_call_handle_error, :api_call_timeout
# Get an XML response containing a list of tracepoints that have been uploaded
# within the specified bounding box, and in the specified page.
def index

View file

@ -7,7 +7,7 @@ module Api
authorize_resource
before_action :offline_error, :only => [:create, :destroy, :data]
around_action :api_call_handle_error
skip_around_action :api_call_timeout, :only => :create
def show
@trace = Trace.visible.find(params[:id])

View file

@ -2,7 +2,6 @@ module Api
class UserBlocksController < ApiController
authorize_resource
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats
def show

View file

@ -6,8 +6,6 @@ module Api
authorize_resource
around_action :api_call_handle_error
before_action :set_request_formats
##

View file

@ -6,7 +6,6 @@ module Api
authorize_resource
around_action :api_call_handle_error
load_resource :only => :show
before_action :set_request_formats, :except => [:gpx_files]

View file

@ -4,7 +4,6 @@ module Api
authorize_resource :class => false
before_action :set_request_formats
around_action :api_call_handle_error, :api_call_timeout
# Show the list of available API versions. This will replace the global
# unversioned capabilities call in due course.

View file

@ -6,8 +6,6 @@ module Api
authorize_resource
before_action :require_public_data, :only => [:create, :update, :delete]
around_action :api_call_handle_error, :api_call_timeout
before_action :set_request_formats, :except => [:create, :update, :delete]
before_action :check_rate_limit, :only => [:create, :update, :delete]

View file

@ -3,6 +3,8 @@ class ApiController < ApplicationController
before_action :check_api_readable
around_action :api_call_handle_error, :api_call_timeout
private
##
@ -132,7 +134,7 @@ class ApiController < ApplicationController
report_error message, :bad_request
rescue OSM::APIError => e
report_error e.message, e.status
rescue AbstractController::ActionNotFound => e
rescue AbstractController::ActionNotFound, CanCan::AccessDenied => e
raise
rescue StandardError => e
logger.info("API threw unexpected #{e.class} exception: #{e.message}")