From dfaa219f8820224d37cd060d253a507111c63460 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 3 Oct 2017 23:21:19 +0200 Subject: [PATCH] Fix HTTP responses for salmon and ActivityPub inbox processing (#5200) * Return sensible HTTP status for ActivityPub inbox processing * Return sensible HTTP status for salmon slap processing * Return additional information to debug signature verification failures --- app/controllers/activitypub/inboxes_controller.rb | 4 ++-- app/controllers/api/salmon_controller.rb | 6 ++++-- app/controllers/concerns/signature_verification.rb | 9 +++++++++ spec/controllers/api/salmon_controller_spec.rb | 4 ++-- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index d0f8073ed..76553a162 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -9,9 +9,9 @@ class ActivityPub::InboxesController < Api::BaseController if signed_request_account upgrade_account process_payload - head 201 - else head 202 + else + [signature_verification_failure_reason, 401] end end diff --git a/app/controllers/api/salmon_controller.rb b/app/controllers/api/salmon_controller.rb index e9e700b18..143e9d3cd 100644 --- a/app/controllers/api/salmon_controller.rb +++ b/app/controllers/api/salmon_controller.rb @@ -7,9 +7,11 @@ class Api::SalmonController < Api::BaseController def update if verify_payload? process_salmon - head 201 - else head 202 + elsif payload.present? + [signature_verification_failure_reason, 401] + else + head 400 end end diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 52a9cf290..dc2d9a678 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -9,10 +9,15 @@ module SignatureVerification request.headers['Signature'].present? end + def signature_verification_failure_reason + return @signature_verification_failure_reason if defined?(@signature_verification_failure_reason) + end + def signed_request_account return @signed_request_account if defined?(@signed_request_account) unless signed_request? + @signature_verification_failure_reason = 'Request not signed' @signed_request_account = nil return end @@ -27,6 +32,7 @@ module SignatureVerification end if incompatible_signature?(signature_params) + @signature_verification_failure_reason = 'Incompatible request signature' @signed_request_account = nil return end @@ -34,6 +40,7 @@ module SignatureVerification account = account_from_key_id(signature_params['keyId']) if account.nil? + @signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}" @signed_request_account = nil return end @@ -51,9 +58,11 @@ module SignatureVerification @signed_request_account = account @signed_request_account else + @signed_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}" @signed_request_account = nil end else + @signed_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}" @signed_request_account = nil end end diff --git a/spec/controllers/api/salmon_controller_spec.rb b/spec/controllers/api/salmon_controller_spec.rb index 3e4686200..323d85b61 100644 --- a/spec/controllers/api/salmon_controller_spec.rb +++ b/spec/controllers/api/salmon_controller_spec.rb @@ -46,8 +46,8 @@ RSpec.describe Api::SalmonController, type: :controller do post :update, params: { id: account.id } end - it 'returns http success' do - expect(response).to have_http_status(202) + it 'returns http client error' do + expect(response).to have_http_status(400) end end end