Add more specific error messages to HTTP signature verification (#21617)

* Return specific error on failure to parse Date header

* Add error message when preferredUsername is not set

* Change error report to be JSON and include more details

* Change error report to differentiate unknown account and failed refresh

* Add tests

2 files changed, 9 insertions, 8 deletions

diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb
index 4502da698..a9950d21f 100644
--- a/app/controllers/concerns/signature_verification.rb
+++ b/app/controllers/concerns/signature_verification.rb
@@ -46,11 +46,11 @@ module SignatureVerification
   end
 
   def require_account_signature!
-    render plain: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_account
+    render json: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_account
   end
 
   def require_actor_signature!
-    render plain: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_actor
+    render json: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_actor
   end
 
   def signed_request?
@@ -97,11 +97,11 @@ module SignatureVerification
 
     actor = stoplight_wrap_request { actor_refresh_key!(actor) }
 
-    raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if actor.nil?
+    raise SignatureVerificationError, "Could not refresh public key #{signature_params['keyId']}" if actor.nil?
 
     return actor unless verify_signature(actor, signature, compare_signed_string).nil?
 
-    fail_with! "Verification failed for #{actor.to_log_human_identifier} #{actor.uri} using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)"
+    fail_with! "Verification failed for #{actor.to_log_human_identifier} #{actor.uri} using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)", signed_string: compare_signed_string, signature: signature_params['signature']
   rescue SignatureVerificationError => e
     fail_with! e.message
   rescue HTTP::Error, OpenSSL::SSL::SSLError => e
@@ -118,8 +118,8 @@ module SignatureVerification
 
   private
 
-  def fail_with!(message)
-    @signature_verification_failure_reason = message
+  def fail_with!(message, **options)
+    @signature_verification_failure_reason = { error: message }.merge(options)
     @signed_request_actor = nil
   end
 
@@ -209,8 +209,8 @@ module SignatureVerification
       end
 
       expires_time = Time.at(signature_params['expires'].to_i).utc if signature_params['expires'].present?
-    rescue ArgumentError
-      return false
+    rescue ArgumentError => e
+      raise SignatureVerificationError, "Invalid Date header: #{e.message}"
     end
 
     expires_time ||= created_time + 5.minutes unless created_time.nil?
diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb
index a25fa54c4..4f60ea5e8 100644
--- a/app/services/activitypub/fetch_remote_actor_service.rb
+++ b/app/services/activitypub/fetch_remote_actor_service.rb
@@ -28,6 +28,7 @@ class ActivityPub::FetchRemoteActorService < BaseService
     raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context?
     raise Error, "Unexpected object type for actor #{uri} (expected any of: #{SUPPORTED_TYPES})" unless expected_type?
     raise Error, "Actor #{uri} has moved to #{@json['movedTo']}" if break_on_redirect && @json['movedTo'].present?
+    raise Error, "Actor #{uri} has no 'preferredUsername', which is a requirement for Mastodon compatibility" unless @json['preferredUsername'].present?
 
     @uri      = @json['id']
     @username = @json['preferredUsername']