From 83425edd8da6a01047702cbb3ac8642f3ef452fa Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 2 Oct 2024 16:37:32 +0100 Subject: [PATCH 1/1] 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 --- app/controllers/api/capabilities_controller.rb | 1 - app/controllers/api/changeset_comments_controller.rb | 3 +-- app/controllers/api/changesets_controller.rb | 3 +-- app/controllers/api/map_controller.rb | 2 -- app/controllers/api/messages_controller.rb | 2 -- app/controllers/api/nodes_controller.rb | 2 -- app/controllers/api/notes_controller.rb | 1 - app/controllers/api/old_elements_controller.rb | 1 - app/controllers/api/permissions_controller.rb | 1 - app/controllers/api/relations_controller.rb | 2 -- app/controllers/api/tracepoints_controller.rb | 2 -- app/controllers/api/traces_controller.rb | 2 +- app/controllers/api/user_blocks_controller.rb | 1 - app/controllers/api/user_preferences_controller.rb | 2 -- app/controllers/api/users_controller.rb | 1 - app/controllers/api/versions_controller.rb | 1 - app/controllers/api/ways_controller.rb | 2 -- app/controllers/api_controller.rb | 4 +++- 18 files changed, 6 insertions(+), 27 deletions(-) diff --git a/app/controllers/api/capabilities_controller.rb b/app/controllers/api/capabilities_controller.rb index cbdcace0c..b0600ca33 100644 --- a/app/controllers/api/capabilities_controller.rb +++ b/app/controllers/api/capabilities_controller.rb @@ -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: diff --git a/app/controllers/api/changeset_comments_controller.rb b/app/controllers/api/changeset_comments_controller.rb index 4a96ec3bb..c180571c5 100644 --- a/app/controllers/api/changeset_comments_controller.rb +++ b/app/controllers/api/changeset_comments_controller.rb @@ -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 diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb index 3d59eeb17..9111bb609 100644 --- a/app/controllers/api/changesets_controller.rb +++ b/app/controllers/api/changesets_controller.rb @@ -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 diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb index 6d4a9feb6..da8138597 100644 --- a/app/controllers/api/map_controller.rb +++ b/app/controllers/api/map_controller.rb @@ -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 diff --git a/app/controllers/api/messages_controller.rb b/app/controllers/api/messages_controller.rb index 074f87398..886922bff 100644 --- a/app/controllers/api/messages_controller.rb +++ b/app/controllers/api/messages_controller.rb @@ -9,8 +9,6 @@ module Api authorize_resource - around_action :api_call_handle_error, :api_call_timeout - before_action :set_request_formats def inbox diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb index 5aad78dbf..6477271d4 100644 --- a/app/controllers/api/nodes_controller.rb +++ b/app/controllers/api/nodes_controller.rb @@ -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] diff --git a/app/controllers/api/notes_controller.rb b/app/controllers/api/notes_controller.rb index 3352c1f69..9a00814f5 100644 --- a/app/controllers/api/notes_controller.rb +++ b/app/controllers/api/notes_controller.rb @@ -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] ## diff --git a/app/controllers/api/old_elements_controller.rb b/app/controllers/api/old_elements_controller.rb index 2343252db..73e57c1f8 100644 --- a/app/controllers/api/old_elements_controller.rb +++ b/app/controllers/api/old_elements_controller.rb @@ -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] diff --git a/app/controllers/api/permissions_controller.rb b/app/controllers/api/permissions_controller.rb index 637aa36a0..75bfe6b10 100644 --- a/app/controllers/api/permissions_controller.rb +++ b/app/controllers/api/permissions_controller.rb @@ -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: diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index 5fb99dbd1..b237f8bf8 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -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] diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb index d8d9da98b..e45b5968c 100644 --- a/app/controllers/api/tracepoints_controller.rb +++ b/app/controllers/api/tracepoints_controller.rb @@ -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 diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb index 738642fff..76dfb3a2d 100644 --- a/app/controllers/api/traces_controller.rb +++ b/app/controllers/api/traces_controller.rb @@ -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]) diff --git a/app/controllers/api/user_blocks_controller.rb b/app/controllers/api/user_blocks_controller.rb index 6c285e14a..51f0d26d3 100644 --- a/app/controllers/api/user_blocks_controller.rb +++ b/app/controllers/api/user_blocks_controller.rb @@ -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 diff --git a/app/controllers/api/user_preferences_controller.rb b/app/controllers/api/user_preferences_controller.rb index cb852ce88..d1bd6d624 100644 --- a/app/controllers/api/user_preferences_controller.rb +++ b/app/controllers/api/user_preferences_controller.rb @@ -6,8 +6,6 @@ module Api authorize_resource - around_action :api_call_handle_error - before_action :set_request_formats ## diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb index 5ff275ee9..e9f42e12b 100644 --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@ -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] diff --git a/app/controllers/api/versions_controller.rb b/app/controllers/api/versions_controller.rb index d311a18d2..15cde9d6d 100644 --- a/app/controllers/api/versions_controller.rb +++ b/app/controllers/api/versions_controller.rb @@ -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. diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index 4099e1676..27c4a1fcc 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -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] diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 5b264db97..6ce8a0ef2 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -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}") -- 2.39.5