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

diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb
index d0f8073edf33c382b1bfb1253b5c783ec156265c..76553a162a24b06a5331cf8daf051b81a0c59a0c 100644 (file)
--- 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 e9e700b18de5c731bbdc77594393493322becf28..143e9d3cdc6b8b30e072ea352613f4f3ecdfa205 100644 (file)
--- 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 52a9cf2905ce8caf49cb3fa26036e6a939551196..dc2d9a6782537afc8dd54891a86b7f1c21785436 100644 (file)
--- 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 3e4686200b6e31a0e5fef535dc63143553868de6..323d85b615e95efaecbfc21869138b2f4df1cf12 100644 (file)
--- 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