Add framework for parameter validation using rails_param gem

This commit is contained in:
Tom Hughes 2024-04-11 08:45:12 +01:00
parent ba90fe97ea
commit feff501b25
9 changed files with 70 additions and 0 deletions

View file

@ -62,6 +62,7 @@ gem "oauth-plugin", ">= 0.5.1"
gem "openstreetmap-deadlock_retry", ">= 1.3.1", :require => "deadlock_retry"
gem "rack-cors"
gem "rails-i18n", "~> 7.0.0"
gem "rails_param"
gem "rinku", ">= 2.0.6", :require => "rails_rinku"
gem "strong_migrations"
gem "validates_email_format_of", ">= 1.5.1"

View file

@ -459,6 +459,9 @@ GEM
rails-i18n (7.0.9)
i18n (>= 0.7, < 2)
railties (>= 6.0.0, < 8)
rails_param (1.3.1)
actionpack (>= 3.2.0)
activesupport (>= 3.2.0)
railties (7.1.3.2)
actionpack (= 7.1.3.2)
activesupport (= 7.1.3.2)
@ -665,6 +668,7 @@ DEPENDENCIES
rails (~> 7.1.0)
rails-controller-testing
rails-i18n (~> 7.0.0)
rails_param
rinku (>= 2.0.6)
rotp
rtlcss

View file

@ -10,6 +10,8 @@ class ApplicationController < ActionController::Base
rescue_from CanCan::AccessDenied, :with => :deny_access
check_authorization
rescue_from RailsParam::InvalidParameterError, :with => :invalid_parameter
before_action :fetch_body
around_action :better_errors_allow_inline, :if => proc { Rails.env.development? }
@ -306,6 +308,17 @@ class ApplicationController < ActionController::Base
end
end
def invalid_parameter(_exception)
if request.get?
respond_to do |format|
format.html { redirect_to :controller => "/errors", :action => "bad_request" }
format.any { head :bad_request }
end
else
head :bad_request
end
end
# extract authorisation credentials from headers, returns user = nil if none
def auth_data
if request.env.key? "X-HTTP_AUTHORIZATION" # where mod_rewrite might have put it

View file

@ -5,6 +5,13 @@ class ErrorsController < ApplicationController
before_action :set_locale
def bad_request
respond_to do |format|
format.html { render :status => :bad_request }
format.any { render :status => :bad_request, :plain => "" }
end
end
def forbidden
respond_to do |format|
format.html { render :status => :forbidden }

View file

@ -0,0 +1,3 @@
<h1><%= t ".title" %></h1>
<p><%= t ".description" %></p>
<%= render :partial => "contact" %>

29
config/brakeman.ignore Normal file
View file

@ -0,0 +1,29 @@
{
"ignored_warnings": [
{
"warning_type": "HTTP Verb Confusion",
"warning_code": 118,
"fingerprint": "9567bbac855c6ec5552572700ec809d7c1d77f59953e6725aeca54fee5091674",
"check_name": "VerbConfusion",
"message": "Potential HTTP verb confusion. `HEAD` is routed like `GET` but `request.get?` will return `false`",
"file": "app/controllers/application_controller.rb",
"line": 312,
"link": "https://brakemanscanner.org/docs/warning_types/http_verb_confusion/",
"code": "if request.get? then\n respond_to do\n format.html do\n redirect_to(:controller => \"/errors\", :action => \"bad_request\")\n end\n format.any do\n head(:bad_request)\n end\n end\nelse\n head(:bad_request)\nend",
"render_path": null,
"location": {
"type": "method",
"class": "ApplicationController",
"method": "invalid_parameter"
},
"user_input": "request.get?",
"confidence": "Weak",
"cwe_id": [
352
],
"note": ""
}
],
"updated": "2024-04-11 10:07:03 +0100",
"brakeman_version": "6.1.2"
}

View file

@ -634,6 +634,9 @@ en:
contact_url_title: Various contact channels explained
contact: contact
contact_the_community_html: Feel free to %{contact_link} the OpenStreetMap community if you have found a broken link / bug. Make a note of the exact URL of your request.
bad_request:
title: Bad request
description: The operation you requested on the OpenStreetMap server is not valid (HTTP 400)
forbidden:
title: Forbidden
description: The operation you requested on the OpenStreetMap server is only available to administrators (HTTP 403)

View file

@ -347,6 +347,7 @@ OpenStreetMap::Application.routes.draw do
resources :redactions
# errors
match "/400", :to => "errors#bad_request", :via => :all
match "/403", :to => "errors#forbidden", :via => :all
match "/404", :to => "errors#not_found", :via => :all
match "/500", :to => "errors#internal_server_error", :via => :all

View file

@ -2,6 +2,10 @@ require "test_helper"
class ErrorsControllerTest < ActionDispatch::IntegrationTest
def test_routes
assert_routing(
{ :path => "/400", :method => :get },
{ :controller => "errors", :action => "bad_request" }
)
assert_routing(
{ :path => "/403", :method => :get },
{ :controller => "errors", :action => "forbidden" }
@ -16,6 +20,11 @@ class ErrorsControllerTest < ActionDispatch::IntegrationTest
)
end
def test_bad_request
get "/400"
assert_response :bad_request
end
def test_forbidden
get "/403"
assert_response :forbidden