From: Andy Allan Date: Wed, 20 Mar 2019 13:27:05 +0000 (+0100) Subject: Simplify deny_access handling X-Git-Tag: live~3227^2~4 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/742291a840ea9dd741ef439e8678c50d1537973b?ds=sidebyside;hp=-c Simplify deny_access handling Now that we have all api controllers inheriting from a common base, it's easier to override the deny_access handler without having to switch between both. Fixes #2064 --- 742291a840ea9dd741ef439e8678c50d1537973b diff --git a/app/controllers/api/capabilities_controller.rb b/app/controllers/api/capabilities_controller.rb index 68968d107..7f91557f8 100644 --- a/app/controllers/api/capabilities_controller.rb +++ b/app/controllers/api/capabilities_controller.rb @@ -1,7 +1,5 @@ module Api class CapabilitiesController < ApiController - before_action :api_deny_access_handler - authorize_resource :class => false around_action :api_call_handle_error, :api_call_timeout diff --git a/app/controllers/api/changes_controller.rb b/app/controllers/api/changes_controller.rb index 97ddc1763..7170e1562 100644 --- a/app/controllers/api/changes_controller.rb +++ b/app/controllers/api/changes_controller.rb @@ -1,7 +1,5 @@ module Api class ChangesController < ApiController - before_action :api_deny_access_handler - authorize_resource :class => false before_action :check_api_readable diff --git a/app/controllers/api/changeset_comments_controller.rb b/app/controllers/api/changeset_comments_controller.rb index db90dcbe3..21c854139 100644 --- a/app/controllers/api/changeset_comments_controller.rb +++ b/app/controllers/api/changeset_comments_controller.rb @@ -1,7 +1,6 @@ module Api class ChangesetCommentsController < ApiController before_action :authorize - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb index 0f016c902..8841d4b3b 100644 --- a/app/controllers/api/changesets_controller.rb +++ b/app/controllers/api/changesets_controller.rb @@ -6,7 +6,6 @@ module Api require "xml/libxml" before_action :authorize, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe] - before_action :api_deny_access_handler, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox] authorize_resource diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb index 107366071..27d6f3667 100644 --- a/app/controllers/api/map_controller.rb +++ b/app/controllers/api/map_controller.rb @@ -1,7 +1,5 @@ module Api class MapController < ApiController - before_action :api_deny_access_handler - authorize_resource :class => false before_action :check_api_readable diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb index d081b28bb..5218159c1 100644 --- a/app/controllers/api/nodes_controller.rb +++ b/app/controllers/api/nodes_controller.rb @@ -5,7 +5,6 @@ module Api require "xml/libxml" before_action :authorize, :only => [:create, :update, :delete] - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api/notes_controller.rb b/app/controllers/api/notes_controller.rb index 92b68bd46..20a24ce99 100644 --- a/app/controllers/api/notes_controller.rb +++ b/app/controllers/api/notes_controller.rb @@ -5,7 +5,6 @@ module Api before_action :check_api_readable before_action :setup_user_auth, :only => [:create, :comment, :show] before_action :authorize, :only => [:close, :reopen, :destroy] - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api/old_controller.rb b/app/controllers/api/old_controller.rb index 9d9f2fabc..fa2b5814e 100644 --- a/app/controllers/api/old_controller.rb +++ b/app/controllers/api/old_controller.rb @@ -6,7 +6,6 @@ module Api require "xml/libxml" before_action :setup_user_auth, :only => [:history, :version] - before_action :api_deny_access_handler before_action :authorize, :only => [:redact] authorize_resource diff --git a/app/controllers/api/permissions_controller.rb b/app/controllers/api/permissions_controller.rb index 15f381267..9b168e04b 100644 --- a/app/controllers/api/permissions_controller.rb +++ b/app/controllers/api/permissions_controller.rb @@ -1,7 +1,5 @@ module Api class PermissionsController < ApiController - before_action :api_deny_access_handler - authorize_resource :class => false before_action :check_api_readable diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index 6f52f2f94..b7d990c3d 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -3,7 +3,6 @@ module Api require "xml/libxml" before_action :authorize, :only => [:create, :update, :delete] - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb index 7799de266..b22bcfaea 100644 --- a/app/controllers/api/tracepoints_controller.rb +++ b/app/controllers/api/tracepoints_controller.rb @@ -1,7 +1,5 @@ module Api class TracepointsController < ApiController - before_action :api_deny_access_handler - authorize_resource before_action :check_api_readable diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb index 88b6edc67..47dd152a3 100644 --- a/app/controllers/api/traces_controller.rb +++ b/app/controllers/api/traces_controller.rb @@ -5,7 +5,6 @@ module Api before_action :authorize_web before_action :set_locale before_action :authorize - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb index 5c3a6cb8e..f24d50cf1 100644 --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@ -4,7 +4,6 @@ module Api before_action :disable_terms_redirect, :only => [:api_details] before_action :authorize, :only => [:api_details, :api_gpx_files] - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index 2de2d619b..5e3e5e11a 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -3,7 +3,6 @@ module Api require "xml/libxml" before_action :authorize, :only => [:create, :update, :delete] - before_action :api_deny_access_handler authorize_resource diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index fb3717b2a..8ddb7242f 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,3 +1,17 @@ class ApiController < ApplicationController skip_before_action :verify_authenticity_token + + def deny_access(_exception) + if current_token + set_locale + report_error t("oauth.permissions.missing"), :forbidden + elsif current_user + head :forbidden + else + realm = "Web Password" + errormessage = "Couldn't authenticate you" + response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\"" + render :plain => errormessage, :status => :unauthorized + end + end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8d9ee11f9..485c30b21 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -395,15 +395,7 @@ class ApplicationController < ActionController::Base end end - def deny_access(exception) - if @api_deny_access_handling - api_deny_access(exception) - else - web_deny_access(exception) - end - end - - def web_deny_access(_exception) + def deny_access(_exception) if current_token set_locale report_error t("oauth.permissions.missing"), :forbidden @@ -423,26 +415,6 @@ class ApplicationController < ActionController::Base end end - def api_deny_access(_exception) - if current_token - set_locale - report_error t("oauth.permissions.missing"), :forbidden - elsif current_user - head :forbidden - else - realm = "Web Password" - errormessage = "Couldn't authenticate you" - response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\"" - render :plain => errormessage, :status => :unauthorized - end - end - - attr_accessor :api_access_handling - - def api_deny_access_handler - @api_deny_access_handling = true - end - private # extract authorisation credentials from headers, returns user = nil if none