Kimirock/zammad
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes #6403 - HTTP log does not record requests with unparsable parameters.

Browse Source 
Approved-by: Florian Liebe <fl@zammad.com>
This commit is contained in:
Rolf Schmidt
2026-09-30 09:49:34 +02:00
parent 434370a1f2
commit 7d0d97a065
11 changed files with 156 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
app/controllers/application_controller/handles_errors.rb
View File

@@ -17,6 +17,17 @@ module ApplicationController::HandlesErrors
rescue_from Pundit::NotAuthorizedError, with: :pundit_not_authorized_error
rescue_from 'Store::Provider::S3::Error', with: :unprocessable_content
rescue_from Exceptions::MissingAttribute, Exceptions::InvalidAttribute, ActionController::ParameterMissing, with: :unprocessable_content
rescue_from ActionController::BadRequest, ActionDispatch::Http::Parameters::ParseError, with: :bad_request
end
def bad_request(e)
logger.error e
# unreadable params also hide the format default of the route
if (format = request.path_parameters[:format])
request.set_header('action_dispatch.request.formats', [Mime::Type.lookup_by_extension(format)])
end
respond_to_exception(e, :bad_request)
http_log
end
def not_found(e)
@@ -72,6 +83,13 @@ module ApplicationController::HandlesErrors
private
# Rails raises these while parsing the params, before rescue_from handlers are reachable.
def process_action(*)
super
rescue ActionController::BadRequest, ActionDispatch::Http::Parameters::ParseError => e
bad_request(e)
end
def respond_to_exception(e, status)
status_code = Rack::Utils.status_code(status)
@@ -82,7 +100,7 @@ module ApplicationController::HandlesErrors
@exception = e
@message = errors[:error_human] || errors[:error] || param[:message]
@traceback = !Rails.env.production?
file = Rails.public_path.join("#{status_code}#{'-mobile' if params[:controller] == 'mobile'}.html").open('r')
file = Rails.public_path.join("#{status_code}#{'-mobile' if request.path_parameters[:controller] == 'mobile'}.html").open('r')
render inline: file.read, status: status, content_type: 'text/html' # rubocop:disable Rails/RenderInline
end
end
@@ -115,7 +133,7 @@ module ApplicationController::HandlesErrors
elsif e.is_a?(Exceptions::InvalidCSRFToken)
data[:error_human] = data[:error]
data[:invalid_csrf_token] = true
elsif [ActionController::RoutingError, ActiveRecord::RecordNotFound, Exceptions::NotAuthorized, Exceptions::Forbidden, Store::Provider::S3::Error, Authorization::Provider::AccountError, Exceptions::MissingAttribute, ActionController::ParameterMissing].include?(e.class)
elsif [ActionController::RoutingError, ActionController::BadRequest, ActionDispatch::Http::Parameters::ParseError, ActiveRecord::RecordNotFound, Exceptions::NotAuthorized, Exceptions::Forbidden, Store::Provider::S3::Error, Authorization::Provider::AccountError, Exceptions::MissingAttribute, ActionController::ParameterMissing].include?(e.class)
data[:error_human] = data[:error]
end

16
app/controllers/application_controller/logs_http_access.rb
View File

@@ -5,17 +5,21 @@ module ApplicationController::LogsHttpAccess
included do
after_action :http_log
class_attribute :http_log_support, instance_writer: false
end
class_methods do
def http_log_config(config)
self.http_log_support = config
end
end
private
def http_log_config(config)
@http_log_support = config
end
# log http access
def http_log
return if !@http_log_support
return if !http_log_support
# request
request_data = {
@@ -57,7 +61,7 @@ module ApplicationController::LogsHttpAccess
response_data[:content] = response_data[:content].slice(0, 8000)
record = {
direction: 'in',
facility: @http_log_support[:facility],
facility: http_log_support[:facility],
url: url_for(only_path: false, overwrite_params: {}),
status: response.status,
ip: request.remote_ip,

4
app/controllers/integration/check_mk_controller.rb
View File

@@ -4,6 +4,8 @@ class Integration::CheckMkController < ApplicationController
skip_before_action :verify_csrf_token
before_action :check_configured
http_log_config facility: 'check_mk'
def update
# check params
@@ -137,8 +139,6 @@ UserAgent: #{request.env['HTTP_USER_AGENT'] || '-'}
private
def check_configured
http_log_config facility: 'check_mk'
if !Setting.get('check_mk_integration')
raise Exceptions::UnprocessableContent, __('Feature is disabled, please contact your administrator!')
end

4
app/controllers/integration/cti_controller.rb
View File

@@ -4,6 +4,8 @@ class Integration::CtiController < ApplicationController
skip_before_action :verify_csrf_token
before_action :check_configured, :check_token
http_log_config facility: 'cti'
# notify about inbound call / block inbound call
def event
local_params = ActiveSupport::HashWithIndifferentAccess.new(params.permit!.to_h)
@@ -49,8 +51,6 @@ class Integration::CtiController < ApplicationController
end
def check_configured
http_log_config facility: 'cti'
if !Setting.get('cti_integration')
response_error(__('Feature is disabled, please contact your administrator!'))
return

4
app/controllers/integration/placetel_controller.rb
View File

@@ -6,6 +6,8 @@ class Integration::PlacetelController < ApplicationController
skip_before_action :verify_csrf_token
before_action :check_configured, :check_token
http_log_config facility: 'placetel'
# notify about inbound call / block inbound call
def event
@@ -48,8 +50,6 @@ class Integration::PlacetelController < ApplicationController
end
def check_configured
http_log_config facility: 'placetel'
if !Setting.get('placetel_integration')
response_error(__('Feature is disabled, please contact your administrator!'))
return

4
app/controllers/integration/sipgate_controller.rb
View File

@@ -6,6 +6,8 @@ class Integration::SipgateController < ApplicationController
skip_before_action :verify_csrf_token
before_action :check_configured, :check_token
http_log_config facility: 'sipgate.io'
# notify about inbound call / block inbound call
def event
@@ -48,8 +50,6 @@ class Integration::SipgateController < ApplicationController
end
def check_configured
http_log_config facility: 'sipgate.io'
if !Setting.get('sipgate_integration')
xml_error(__('Feature is disabled, please contact your administrator!'))
return

2
app/controllers/tests_controller.rb
View File

@@ -32,7 +32,7 @@ class TestsController < ApplicationController
message = params.fetch(:message, 'no message provided')
# Emulate the originating controller.
params[:controller] = origin if origin
request.path_parameters[:controller] = origin if origin
klass = exception.safe_constantize

74
public/400-mobile.html Normal file
View File

@@ -0,0 +1,74 @@
<!DOCTYPE html>
<html class="dark">
<meta charset="utf-8">
<meta
name="viewport"
content="width=device-width, initial-scale=1, shrink-to-fit=no, viewport-fit=cover"
>
<title>400: Bad Request</title>
<link rel="stylesheet" href="/assets/error/style-mobile.css">
<body <% if @traceback %> class="error-message" <% end %>>
<div class="h-full min-w-full bg-black font-sans text-sm text-white antialiased">
<div class="flex min-h-screen flex-col px-4">
<header class="fixed">
<div class="grid h-16 grid-cols-3">
<div class="flex cursor-pointer items-center justify-self-start text-base">
<button
class="flex cursor-pointer items-center"
aria-label="Go home"
>
<svg
xmlns="http://www.w3.org/2000/svg"
class="icon fill-current icon-mobile-home"
width="32"
height="32"
aria-hidden="true"
viewBox="0 0 24 24"
>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M8.5 14a1.5 1.5 0 0 1 1.5-1.5h4a1.5 1.5 0 0 1 1.5 1.5v4.5h3V9.442L12 4.838 5.5 9.442V18.5h3V14Zm1.5 6H4V9.184a1 1 0 0 1 .422-.816l7-4.959a1 1 0 0 1 1.156 0l7 4.959a1 1 0 0 1 .422.816V20h-6v-6h-4v6Z"
></path>
</svg>
</button>
</div>
</div>
</header>
<main class="flex grow flex-col items-center justify-center">
<h1 class="mb-9 text-8xl font-bold">400</h1>
<% if !@traceback %>
<img alt="Error" src="/assets/error/error-403.svg">
<h2 class="mt-9 max-w-prose text-center text-xl font-medium">
The request could not be processed
</h2>
<p class="mt-4 min-h-[4rem] max-w-prose text-center text-gray">
The request was malformed, e.g. it used an invalid parameter encoding.
</p>
<% if @message.present? %>
<p class="max-w-prose text-center text-gray"><%= @message %></p>
<% end %>
<% else %>
<pre class="traceback">
<%= @exception.message %>
<% if @exception.backtrace %>
<code>
<% @exception.backtrace.each {|row| %>
<%= row %>
<% } %>
</code>
<% end %>
</pre>
<% end %>
</main>
</div>
</div>
</body>
</html>

25
public/400.html Normal file
View File

@@ -0,0 +1,25 @@
<!DOCTYPE html>
<html class="dark">
<meta charset="utf-8">
<title>400: Bad Request</title>
<link rel="stylesheet" href="/assets/error/style.css">
<body <% if @traceback %>class="error-message"<% end %>>
<h1>400: The request could not be processed.</h1>
<% if @message.present? %>
<div><%= @message %></div>
<% end %>
<% if !@traceback %>
<div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
<p>The request was malformed, e.g. it used an invalid parameter encoding.</p>
<% else %>
<% if @exception.message != @message %>
<div><%= @exception.message %></div>
<% end %>
<% if @exception.backtrace %>
<div>Traceback:</div>
<% @exception.backtrace.each {|row| %>
<%= row %><br>
<% } %>
<% end %>
<% end %>
</html>

4
spec/requests/error_spec.rb
View File

@@ -149,6 +149,7 @@ RSpec.describe 'Error handling', type: :request do
let(:user) { :agent }
include_examples 'handles exception', ActionController::BadRequest, :bad_request, '400: Bad Request', '400: The request could not be processed.'
include_examples 'handles exception', Exceptions::NotAuthorized, :unauthorized, '401: Unauthorized', '401: Unauthorized'
include_examples 'handles exception', Exceptions::Forbidden, :forbidden, '403: Forbidden', '403: Forbidden'
include_examples 'handles exception', Pundit::NotAuthorizedError, :forbidden, '403: Forbidden', '403: Forbidden', 'Not authorized'
@@ -163,6 +164,7 @@ RSpec.describe 'Error handling', type: :request do
let(:user) { :admin }
include_examples 'handles exception', ActionController::BadRequest, :bad_request, '400: Bad Request', '400: The request could not be processed.'
include_examples 'handles exception', Exceptions::NotAuthorized, :unauthorized, '401: Unauthorized', '401: Unauthorized'
include_examples 'handles exception', Exceptions::Forbidden, :forbidden, '403: Forbidden', '403: Forbidden'
include_examples 'handles exception', Pundit::NotAuthorizedError, :forbidden, '403: Forbidden', '403: Forbidden', 'Not authorized'
@@ -180,6 +182,7 @@ RSpec.describe 'Error handling', type: :request do
let(:user) { :agent }
include_examples 'handles exception', ActionController::BadRequest, :bad_request, '400: Bad Request', '400'
include_examples 'handles exception', Exceptions::NotAuthorized, :unauthorized, '401: Unauthorized', '401'
include_examples 'handles exception', Exceptions::Forbidden, :forbidden, '403: Forbidden', '403'
include_examples 'handles exception', Pundit::NotAuthorizedError, :forbidden, '403: Forbidden', '403', 'Not authorized'
@@ -193,6 +196,7 @@ RSpec.describe 'Error handling', type: :request do
let(:user) { :admin }
include_examples 'handles exception', ActionController::BadRequest, :bad_request, '400: Bad Request', '400'
include_examples 'handles exception', Exceptions::NotAuthorized, :unauthorized, '401: Unauthorized', '401'
include_examples 'handles exception', Exceptions::Forbidden, :forbidden, '403: Forbidden', '403'
include_examples 'handles exception', Pundit::NotAuthorizedError, :forbidden, '403: Forbidden', '403', 'Not authorized'

14
spec/requests/integration/check_mk_spec.rb
View File

@@ -31,6 +31,20 @@ RSpec.describe 'Integration Check MK', type: :request do
expect(json_response['error']).to eq('The provided token is invalid.')
end
it 'does fail with invalid parameter encoding and logs the request', :aggregate_failures do
post "/api/v1/integration/check_mk/#{Setting.get('check_mk_token')}", params: 'event_id=1&host=a%DFb&state=down', headers: { 'CONTENT_TYPE' => 'application/x-www-form-urlencoded' }
expect(response).to have_http_status(:bad_request)
expect(json_response['error']).to include('Invalid encoding for parameter')
expect(HttpLog.where(facility: 'check_mk', status: 400).count).to eq(1)
end
it 'does fail with malformed JSON body and logs the request', :aggregate_failures do
post "/api/v1/integration/check_mk/#{Setting.get('check_mk_token')}", params: '{"event_id":', headers: { 'CONTENT_TYPE' => 'application/json' }
expect(response).to have_http_status(:bad_request)
expect(json_response['error']).to eq('Error occurred while parsing request parameters')
expect(HttpLog.where(facility: 'check_mk', status: 400).count).to eq(1)
end
context 'service check' do
it 'does create ticket with customer email' do
params = {