]> git.openstreetmap.org Git - rails.git/commitdiff
Move api error handling and timeouts to parent class
authorAndy Allan <git@gravitystorm.co.uk>
Wed, 2 Oct 2024 15:37:32 +0000 (16:37 +0100)
committerAndy Allan <git@gravitystorm.co.uk>
Wed, 2 Oct 2024 15:37:32 +0000 (16:37 +0100)
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

18 files changed:
app/controllers/api/capabilities_controller.rb
app/controllers/api/changeset_comments_controller.rb
app/controllers/api/changesets_controller.rb
app/controllers/api/map_controller.rb
app/controllers/api/messages_controller.rb
app/controllers/api/nodes_controller.rb
app/controllers/api/notes_controller.rb
app/controllers/api/old_elements_controller.rb
app/controllers/api/permissions_controller.rb
app/controllers/api/relations_controller.rb
app/controllers/api/tracepoints_controller.rb
app/controllers/api/traces_controller.rb
app/controllers/api/user_blocks_controller.rb
app/controllers/api/user_preferences_controller.rb
app/controllers/api/users_controller.rb
app/controllers/api/versions_controller.rb
app/controllers/api/ways_controller.rb
app/controllers/api_controller.rb

index cbdcace0cd80b1a7c87ea47535b2cb81328b75fa..b0600ca33517024cfddfb446bdf14f205f1d1527 100644 (file)
@@ -5,7 +5,6 @@ module Api
     authorize_resource :class => false
 
     before_action :set_request_formats
     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:
 
     # External apps that use the api are able to query the api to find out some
     # parameters of the API. It currently returns:
index 4a96ec3bbaef00087e8b03a464c7ee78e65c56c2..c180571c58b10b0184e1f3dd98ab86f4fb5d8d27 100644 (file)
@@ -6,9 +6,8 @@ module Api
     authorize_resource
 
     before_action :require_public_data, :only => [:create]
     authorize_resource
 
     before_action :require_public_data, :only => [:create]
+
     before_action :set_request_formats
     before_action :set_request_formats
-    around_action :api_call_handle_error
-    around_action :api_call_timeout
 
     ##
     # Add a comment to a changeset
 
     ##
     # Add a comment to a changeset
index 3d59eeb171e4232c70d5fe1b68dc5252a6d4de2b..9111bb609d27d91753d3b6b46a6ee7ae59954eae 100644 (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]
 
     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
 
     # Helper methods for checking consistency
     include ConsistencyValidations
index 6d4a9feb6c3d8062cf141fb375eca35a42da018e..da8138597ae7b269fbb983a7eda8a4347c8a9ae7 100644 (file)
@@ -2,8 +2,6 @@ module Api
   class MapController < ApiController
     authorize_resource :class => false
 
   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
     before_action :set_request_formats
 
     # This is probably the most common call of all. It is used for getting the
index 074f8739871094318a7939783512dd7532652669..886922bff1f8918ad7d58b4f38b1b8246be372e8 100644 (file)
@@ -9,8 +9,6 @@ module Api
 
     authorize_resource
 
 
     authorize_resource
 
-    around_action :api_call_handle_error, :api_call_timeout
-
     before_action :set_request_formats
 
     def inbox
     before_action :set_request_formats
 
     def inbox
index 5aad78dbffb91bfc99c36f36adcbb20893b2d5dc..6477271d4e890788f3c260775cd27a3c3ad53a88 100644 (file)
@@ -8,8 +8,6 @@ module Api
     authorize_resource
 
     before_action :require_public_data, :only => [:create, :update, :delete]
     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]
 
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 
index 3352c1f69b591c121fcf582d2ec01f6307185bac..9a00814f502d48d4c0df921458897a7a05a1dd4b 100644 (file)
@@ -7,7 +7,6 @@ module Api
     authorize_resource
 
     before_action :set_locale
     authorize_resource
 
     before_action :set_locale
-    around_action :api_call_handle_error, :api_call_timeout
     before_action :set_request_formats, :except => [:feed]
 
     ##
     before_action :set_request_formats, :except => [:feed]
 
     ##
index 2343252dbb0a63243ece6e1c90c9de0875dfee4c..73e57c1f82355415baeef6a4ee044e3a741b942c 100644 (file)
@@ -9,7 +9,6 @@ module Api
 
     authorize_resource
 
 
     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]
 
     before_action :lookup_old_element, :except => [:history]
     before_action :lookup_old_element_versions, :only => [:history]
 
index 637aa36a0342caf017254f3baa3203c7b0d235e0..75bfe6b107a2f01203b0cb1d0fc0bfe7a9faeae6 100644 (file)
@@ -4,7 +4,6 @@ module Api
 
     before_action :setup_user_auth
     before_action :set_request_formats
 
     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:
 
     # 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:
index 5fb99dbd18a390e1e234308bcf78b90f05a36939..b237f8bf85e09bec86fb1fcbfc6658e765b126f6 100644 (file)
@@ -6,8 +6,6 @@ module Api
     authorize_resource
 
     before_action :require_public_data, :only => [:create, :update, :delete]
     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]
 
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 
index d8d9da98b680f62729b503085a5ab36c40e318cc..e45b5968c213d94917cc690fd160bf7ca11f9514 100644 (file)
@@ -2,8 +2,6 @@ module Api
   class TracepointsController < ApiController
     authorize_resource
 
   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
     # 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
index 738642fff7dc56531bec8d550c3a5f31dcc414b6..76dfb3a2dc1bd4b6d57728b17b3f074894ef88cd 100644 (file)
@@ -7,7 +7,7 @@ module Api
     authorize_resource
 
     before_action :offline_error, :only => [:create, :destroy, :data]
     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])
 
     def show
       @trace = Trace.visible.find(params[:id])
index 6c285e14a253d50e45daa31ef57134071560a708..51f0d26d3e641ee3762357a20a8b085eea505e32 100644 (file)
@@ -2,7 +2,6 @@ module Api
   class UserBlocksController < ApiController
     authorize_resource
 
   class UserBlocksController < ApiController
     authorize_resource
 
-    around_action :api_call_handle_error, :api_call_timeout
     before_action :set_request_formats
 
     def show
     before_action :set_request_formats
 
     def show
index cb852ce881aa10909a267e776aa884c406e388b4..d1bd6d6242bc5713ce28054630e26341f711bc28 100644 (file)
@@ -6,8 +6,6 @@ module Api
 
     authorize_resource
 
 
     authorize_resource
 
-    around_action :api_call_handle_error
-
     before_action :set_request_formats
 
     ##
     before_action :set_request_formats
 
     ##
index 5ff275ee9dc52f268b250d52f445dd71a3295b1c..e9f42e12b1444787260dad22a18bf5ee97fdb368 100644 (file)
@@ -6,7 +6,6 @@ module Api
 
     authorize_resource
 
 
     authorize_resource
 
-    around_action :api_call_handle_error
     load_resource :only => :show
 
     before_action :set_request_formats, :except => [:gpx_files]
     load_resource :only => :show
 
     before_action :set_request_formats, :except => [:gpx_files]
index d311a18d24f14a1b3304bc3b8c7325272a805323..15cde9d6d606254168d41aed2fcca14da90f8f51 100644 (file)
@@ -4,7 +4,6 @@ module Api
     authorize_resource :class => false
 
     before_action :set_request_formats
     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.
 
     # Show the list of available API versions. This will replace the global
     # unversioned capabilities call in due course.
index 4099e16763f15554e7b33de5f35cffb7eb8885f9..27c4a1fcc7a319ce21e5e629aeeebbe9f81dc765 100644 (file)
@@ -6,8 +6,6 @@ module Api
     authorize_resource
 
     before_action :require_public_data, :only => [:create, :update, :delete]
     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]
 
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 
index 5b264db970aeb19a8b7a4efd6eeec31a787e78e9..6ce8a0ef2d18045309e36784db5cb07d0f53483e 100644 (file)
@@ -3,6 +3,8 @@ class ApiController < ApplicationController
 
   before_action :check_api_readable
 
 
   before_action :check_api_readable
 
+  around_action :api_call_handle_error, :api_call_timeout
+
   private
 
   ##
   private
 
   ##
@@ -132,7 +134,7 @@ class ApiController < ApplicationController
     report_error message, :bad_request
   rescue OSM::APIError => e
     report_error e.message, e.status
     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}")
     raise
   rescue StandardError => e
     logger.info("API threw unexpected #{e.class} exception: #{e.message}")