Merge remote-tracking branch 'upstream/pull/2186'

diff --git a/app/controllers/api/amf_controller.rb b/app/controllers/api/amf_controller.rb
index 509cf1d77c7847a77f1b8d6f849b9aae6116850d..0cf511d76333cb7ba9d0975c9019fd3fabbc91b3 100644 (file)
--- a/app/controllers/api/amf_controller.rb
+++ b/app/controllers/api/amf_controller.rb
@@ -36,10 +36,9 @@
 # * version conflict when POIs and ways are reverted
 
 module Api
-  class AmfController < ApplicationController
+  class AmfController < ApiController
     include Potlatch
 
-    skip_before_action :verify_authenticity_token
     before_action :check_api_writable
 
     # AMF Controller implements its own authentication and authorization checks

diff --git a/app/controllers/api/capabilities_controller.rb b/app/controllers/api/capabilities_controller.rb
index 8337bc8091ee6d1229f5727fca88cb42cc41f54e..7f91557f85849b74bf749daf590dbe77aa3f582d 100644 (file)
--- a/app/controllers/api/capabilities_controller.rb
+++ b/app/controllers/api/capabilities_controller.rb
@@ -1,8 +1,5 @@
 module Api
-  class CapabilitiesController < ApplicationController
-    skip_before_action :verify_authenticity_token
-    before_action :api_deny_access_handler
-
+  class CapabilitiesController < ApiController
     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 c9195e1d9b97f6335a34a82f6f00d16e4724deca..7170e15622468037a6c2a210d7cdce6e7091e36c 100644 (file)
--- a/app/controllers/api/changes_controller.rb
+++ b/app/controllers/api/changes_controller.rb
@@ -1,8 +1,5 @@
 module Api
-  class ChangesController < ApplicationController
-    skip_before_action :verify_authenticity_token
-    before_action :api_deny_access_handler
-
+  class ChangesController < ApiController
     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 0bebce433666f5d7d0fbbfa4854517c3b281efdf..21c854139c9bf9d684183461650be60af8c9ee12 100644 (file)
--- a/app/controllers/api/changeset_comments_controller.rb
+++ b/app/controllers/api/changeset_comments_controller.rb
@@ -1,8 +1,6 @@
 module Api
-  class ChangesetCommentsController < ApplicationController
-    skip_before_action :verify_authenticity_token
+  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 dd43f7ed0943e59829d3902ee2142717eb200bb8..fb6523f4e0065a1dd9c508fc2c9e490e68cbfaaa 100644 (file)
--- a/app/controllers/api/changesets_controller.rb
+++ b/app/controllers/api/changesets_controller.rb
@@ -1,20 +1,17 @@
 # The ChangesetController is the RESTful interface to Changeset objects
 
 module Api
-  class ChangesetsController < ApplicationController
+  class ChangesetsController < ApiController
     layout "site"
     require "xml/libxml"
 
-    skip_before_action :verify_authenticity_token
     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
 
     before_action :require_public_data, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
     before_action :check_api_writable, :only => [:create, :update, :upload, :subscribe, :unsubscribe]
     before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :subscribe, :unsubscribe]
-    before_action(:only => [:index, :feed]) { |c| c.check_database_readable(true) }
     around_action :api_call_handle_error
     around_action :api_call_timeout, :except => [:upload]
 

diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb
index e8d36c8ec375e5e69eb25b290d1c524695d9112c..27d6f3667410b13504851e0298e1eb67257c4154 100644 (file)
--- a/app/controllers/api/map_controller.rb
+++ b/app/controllers/api/map_controller.rb
@@ -1,8 +1,5 @@
 module Api
-  class MapController < ApplicationController
-    skip_before_action :verify_authenticity_token
-    before_action :api_deny_access_handler
-
+  class MapController < ApiController
     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 4e46b38d56461c94f50f162169623f1f2ff8a05a..5218159c16097f61b921e07d63d0ea7cee1ff601 100644 (file)
--- a/app/controllers/api/nodes_controller.rb
+++ b/app/controllers/api/nodes_controller.rb
@@ -1,12 +1,10 @@
 # The NodeController is the RESTful interface to Node objects
 
 module Api
-  class NodesController < ApplicationController
+  class NodesController < ApiController
     require "xml/libxml"
 
-    skip_before_action :verify_authenticity_token
     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 686e76b147d0141d960950309d485aa78a6f6c8f..20a24ce997bbe10a6aaf546d2f0131b91a4abb47 100644 (file)
--- a/app/controllers/api/notes_controller.rb
+++ b/app/controllers/api/notes_controller.rb
@@ -1,12 +1,10 @@
 module Api
-  class NotesController < ApplicationController
+  class NotesController < ApiController
     layout "site", :only => [:mine]
 
-    skip_before_action :verify_authenticity_token
     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 9a86bded590ea8c143634480a01023514f1d2be5..fa2b5814e49a400a5b22fd95d5d287812bb4fb29 100644 (file)
--- a/app/controllers/api/old_controller.rb
+++ b/app/controllers/api/old_controller.rb
@@ -2,12 +2,10 @@
 # into one place. as it turns out, the API methods for historical
 # nodes, ways and relations are basically identical.
 module Api
-  class OldController < ApplicationController
+  class OldController < ApiController
     require "xml/libxml"
 
-    skip_before_action :verify_authenticity_token
     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 b24aca776dcfef6afeb0d0c476aaaea6bd46d5b4..9b168e04b568e5c1f880f0f2a035a11f1dc74c44 100644 (file)
--- a/app/controllers/api/permissions_controller.rb
+++ b/app/controllers/api/permissions_controller.rb
@@ -1,8 +1,5 @@
 module Api
-  class PermissionsController < ApplicationController
-    skip_before_action :verify_authenticity_token
-    before_action :api_deny_access_handler
-
+  class PermissionsController < ApiController
     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 a0740b382819f720d59013a3b70157834428c758..b7d990c3d3570321d21614afc1ef94293d39055b 100644 (file)
--- a/app/controllers/api/relations_controller.rb
+++ b/app/controllers/api/relations_controller.rb
@@ -1,10 +1,8 @@
 module Api
-  class RelationsController < ApplicationController
+  class RelationsController < ApiController
     require "xml/libxml"
 
-    skip_before_action :verify_authenticity_token
     before_action :authorize, :only => [:create, :update, :delete]
-    before_action :api_deny_access_handler
 
     authorize_resource
 

diff --git a/app/controllers/api/search_controller.rb b/app/controllers/api/search_controller.rb
index 0afbbf8e2a40990947c8bf9eeb88b883bd98f721..feb487ac0e4d44369a7d49b5296b28afd93aa548 100644 (file)
--- a/app/controllers/api/search_controller.rb
+++ b/app/controllers/api/search_controller.rb
@@ -1,9 +1,8 @@
 module Api
-  class SearchController < ApplicationController
+  class SearchController < ApiController
     # Support searching for nodes, ways, or all
     # Can search by tag k, v, or both (type->k,value->v)
     # Can search by name (k=name,v=....)
-    skip_before_action :verify_authenticity_token
     authorize_resource :class => false
 
     def search_all

diff --git a/app/controllers/api/swf_controller.rb b/app/controllers/api/swf_controller.rb
index d48731b7023404cf72c7c388a6affd988502e718..2f8a5392da459274b423468d52664db6ad51b270 100644 (file)
--- a/app/controllers/api/swf_controller.rb
+++ b/app/controllers/api/swf_controller.rb
@@ -1,6 +1,5 @@
 module Api
-  class SwfController < ApplicationController
-    skip_before_action :verify_authenticity_token
+  class SwfController < ApiController
     before_action :check_api_readable
     authorize_resource :class => false
 

diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb
index c71b5a3e9a06d7e6c5e7e1d3fea08282ae4db29f..b22bcfaeaff994508a4a7d2272035157e0867473 100644 (file)
--- a/app/controllers/api/tracepoints_controller.rb
+++ b/app/controllers/api/tracepoints_controller.rb
@@ -1,8 +1,5 @@
 module Api
-  class TracepointsController < ApplicationController
-    skip_before_action :verify_authenticity_token
-    before_action :api_deny_access_handler
-
+  class TracepointsController < ApiController
     authorize_resource
 
     before_action :check_api_readable

diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb
index d7f2f043a3182b28cd15710e05ab69f66e339cbf..47dd152a3fa6bc53bb38e8929865789fc37074a4 100644 (file)
--- a/app/controllers/api/traces_controller.rb
+++ b/app/controllers/api/traces_controller.rb
@@ -1,12 +1,10 @@
 module Api
-  class TracesController < ApplicationController
+  class TracesController < ApiController
     layout "site", :except => :georss
 
-    skip_before_action :verify_authenticity_token
     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/user_preferences_controller.rb b/app/controllers/api/user_preferences_controller.rb
index 82f6c6a4d78255d6fdc8b17087888679f54ed4f6..39e0dff300d83078e3fffcb54b5b4d04d29a74cf 100644 (file)
--- a/app/controllers/api/user_preferences_controller.rb
+++ b/app/controllers/api/user_preferences_controller.rb
@@ -1,7 +1,6 @@
 # Update and read user preferences, which are arbitrayr key/val pairs
 module Api
-  class UserPreferencesController < ApplicationController
-    skip_before_action :verify_authenticity_token
+  class UserPreferencesController < ApiController
     before_action :authorize
 
     authorize_resource

diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index 70ad93f65e4862c33f73bd40d8a52f302f7317c6..f24d50cf101d2525c8054a642cf6b8e5d68df858 100644 (file)
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -1,11 +1,9 @@
 module Api
-  class UsersController < ApplicationController
+  class UsersController < ApiController
     layout "site", :except => [:api_details]
 
-    skip_before_action :verify_authenticity_token
     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 8684c5cfb13a9ea3575af5838b6c89df3887aeae..5e3e5e11ab1d511dea0882b5c455fa59c520f25b 100644 (file)
--- a/app/controllers/api/ways_controller.rb
+++ b/app/controllers/api/ways_controller.rb
@@ -1,10 +1,8 @@
 module Api
-  class WaysController < ApplicationController
+  class WaysController < ApiController
     require "xml/libxml"
 
-    skip_before_action :verify_authenticity_token
     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
new file mode 100644 (file)
index 0000000..579af27
--- /dev/null
+++ b/app/controllers/api_controller.rb
@@ -0,0 +1,79 @@
+class ApiController < ApplicationController
+  skip_before_action :verify_authenticity_token
+
+  private
+
+  def authorize(realm = "Web Password", errormessage = "Couldn't authenticate you")
+    # make the current_user object from any auth sources we have
+    setup_user_auth
+
+    # handle authenticate pass/fail
+    unless current_user
+      # no auth, the user does not exist or the password was wrong
+      response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\""
+      render :plain => errormessage, :status => :unauthorized
+      return false
+    end
+  end
+
+  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
+
+  def gpx_status
+    status = database_status
+    status = "offline" if status == "online" && Settings.status == "gpx_offline"
+    status
+  end
+
+  ##
+  # sets up the current_user for use by other methods. this is mostly called
+  # from the authorize method, but can be called elsewhere if authorisation
+  # is optional.
+  def setup_user_auth
+    # try and setup using OAuth
+    unless Authenticator.new(self, [:token]).allow?
+      username, passwd = get_auth_data # parse from headers
+      # authenticate per-scheme
+      self.current_user = if username.nil?
+                            nil # no authentication provided - perhaps first connect (client should retry after 401)
+                          elsif username == "token"
+                            User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth
+                          else
+                            User.authenticate(:username => username, :password => passwd) # basic auth
+                          end
+    end
+
+    # have we identified the user?
+    if current_user
+      # check if the user has been banned
+      user_block = current_user.blocks.active.take
+      unless user_block.nil?
+        set_locale
+        if user_block.zero_hour?
+          report_error t("application.setup_user_auth.blocked_zero_hour"), :forbidden
+        else
+          report_error t("application.setup_user_auth.blocked"), :forbidden
+        end
+      end
+
+      # if the user hasn't seen the contributor terms then don't
+      # allow editing - they have to go to the web site and see
+      # (but can decline) the CTs to continue.
+      if !current_user.terms_seen && flash[:skip_terms].nil?
+        set_locale
+        report_error t("application.setup_user_auth.need_to_see_terms"), :forbidden
+      end
+    end
+  end
+end

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index fb561423640b723372f63f6f4d864b88225577ff..3ab09b63d805bc67899c6e54dabeac86f7d5fa69 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -12,6 +12,8 @@ class ApplicationController < ActionController::Base
   attr_accessor :current_user
   helper_method :current_user
 
+  private
+
   def authorize_web
     if session[:user]
       self.current_user = User.where(:id => session[:user]).where("status IN ('active', 'confirmed', 'suspended')").first
@@ -71,60 +73,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  ##
-  # sets up the current_user for use by other methods. this is mostly called
-  # from the authorize method, but can be called elsewhere if authorisation
-  # is optional.
-  def setup_user_auth
-    # try and setup using OAuth
-    unless Authenticator.new(self, [:token]).allow?
-      username, passwd = get_auth_data # parse from headers
-      # authenticate per-scheme
-      self.current_user = if username.nil?
-                            nil # no authentication provided - perhaps first connect (client should retry after 401)
-                          elsif username == "token"
-                            User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth
-                          else
-                            User.authenticate(:username => username, :password => passwd) # basic auth
-                          end
-    end
-
-    # have we identified the user?
-    if current_user
-      # check if the user has been banned
-      user_block = current_user.blocks.active.take
-      unless user_block.nil?
-        set_locale
-        if user_block.zero_hour?
-          report_error t("application.setup_user_auth.blocked_zero_hour"), :forbidden
-        else
-          report_error t("application.setup_user_auth.blocked"), :forbidden
-        end
-      end
-
-      # if the user hasn't seen the contributor terms then don't
-      # allow editing - they have to go to the web site and see
-      # (but can decline) the CTs to continue.
-      if !current_user.terms_seen && flash[:skip_terms].nil?
-        set_locale
-        report_error t("application.setup_user_auth.need_to_see_terms"), :forbidden
-      end
-    end
-  end
-
-  def authorize(realm = "Web Password", errormessage = "Couldn't authenticate you")
-    # make the current_user object from any auth sources we have
-    setup_user_auth
-
-    # handle authenticate pass/fail
-    unless current_user
-      # no auth, the user does not exist or the password was wrong
-      response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\""
-      render :plain => errormessage, :status => :unauthorized
-      return false
-    end
-  end
-
   def check_database_readable(need_api = false)
     if Settings.status == "database_offline" || (need_api && Settings.status == "api_offline")
       if request.xhr?
@@ -182,12 +130,6 @@ class ApplicationController < ActionController::Base
     status
   end
 
-  def gpx_status
-    status = database_status
-    status = "offline" if status == "online" && Settings.status == "gpx_offline"
-    status
-  end
-
   def require_public_data
     unless current_user.data_public?
       report_error "You must make your edits public to upload new data", :forbidden
@@ -395,15 +337,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,28 +357,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
   def get_auth_data
     if request.env.key? "X-HTTP_AUTHORIZATION" # where mod_rewrite might have put it

diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb
index 0fccbb506bb2a19dae6db4087853a64b8f0933cf..ebdd2cd80fbbb2dfc812ae155477639215069e01 100644 (file)
--- a/app/controllers/browse_controller.rb
+++ b/app/controllers/browse_controller.rb
@@ -3,7 +3,7 @@ class BrowseController < ApplicationController
 
   before_action :authorize_web
   before_action :set_locale
-  before_action(:except => [:query]) { |c| c.check_database_readable(true) }
+  before_action -> { check_database_readable(true) }
   before_action :require_oauth
   around_action :web_timeout
   authorize_resource :class => false

diff --git a/app/controllers/changeset_comments_controller.rb b/app/controllers/changeset_comments_controller.rb
index 05b28eacf01c9806759bcafa8138f33b5958ce44..4abffb90efafac72a7a1d662c5b06fd1d97eb78a 100644 (file)
--- a/app/controllers/changeset_comments_controller.rb
+++ b/app/controllers/changeset_comments_controller.rb
@@ -4,7 +4,7 @@ class ChangesetCommentsController < ApplicationController
 
   authorize_resource
 
-  before_action(:only => [:index]) { |c| c.check_database_readable(true) }
+  before_action -> { check_database_readable(true) }
   around_action :web_timeout
 
   ##

diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb
index fff9f543bf7f7dc3e95d0c49fa6f77e6267db466..32d9fd73314f53e731717081af5c383550298f8b 100644 (file)
--- a/app/controllers/changesets_controller.rb
+++ b/app/controllers/changesets_controller.rb
@@ -7,7 +7,7 @@ class ChangesetsController < ApplicationController
   skip_before_action :verify_authenticity_token, :except => [:index]
   before_action :authorize_web
   before_action :set_locale
-  before_action(:only => [:index, :feed]) { |c| c.check_database_readable(true) }
+  before_action -> { check_database_readable(true) }, :only => [:index, :feed]
 
   authorize_resource