about summary refs log tree commit diff
diff options
context:
space:
mode:
authorThibG <thib@sitedethib.com>2020-09-14 13:04:29 +0200
committerGitHub <noreply@github.com>2020-09-14 13:04:29 +0200
commitcd4ec7cd74c0975c7ff9aa832ed7e1bb10966439 (patch)
tree7f8c77f396e375b5ea02afe2ca54de3b484f164d
parent42c4322ce72f33a12bffdc42c7ffe27a08dcba44 (diff)
Do not serve account actors at all in limited federation mode (#14800)
* Do not serve account actors at all in limited federation mode

When an account is fetched without a signature from an allowed instance,
return an error.

This isn't really an improvement in security, as the only information that was
previously returned was required protocol-level info, and the only personal bit
was the existence of the account. The existence of the account can still be
checked by issuing a webfinger query, as those are accepted without signatures.

However, this change makes it so that unallowed instances won't create account
records on their end when they find a reference to an unknown account.

The previous behavior of rendering a limited list of fields, instead of not
rendering the actor at all, was in order to prevent situations in which two
instances in Authorized Fetch mode or Limited Federation mode would fail to
reach each other because resolving an account would require a signed query…
from an account which can only be fetched with a signed query itself. However,
this should now be fine as fetching accounts is done by signing on behalf of
the special instance actor, which does not require any kind of valid signature
to be fetched.

* Fix tests
-rw-r--r--app/controllers/accounts_controller.rb11
-rw-r--r--spec/controllers/accounts_controller_spec.rb20
2 files changed, 4 insertions, 27 deletions
diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
index d97d88fd9..6d711afd0 100644
--- a/app/controllers/accounts_controller.rb
+++ b/app/controllers/accounts_controller.rb
@@ -7,6 +7,7 @@ class AccountsController < ApplicationController
   include AccountControllerConcern
   include SignatureAuthentication
 
+  before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
   before_action :set_cache_headers
   before_action :set_body_classes
 
@@ -48,7 +49,7 @@ class AccountsController < ApplicationController
 
       format.json do
         expires_in 3.minutes, public: !(authorized_fetch_mode? && signed_request_account.present?)
-        render_with_cache json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter, fields: restrict_fields_to
+        render_with_cache json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter
       end
     end
   end
@@ -153,12 +154,4 @@ class AccountsController < ApplicationController
   def params_slice(*keys)
     params.slice(*keys).permit(*keys)
   end
-
-  def restrict_fields_to
-    if signed_request_account.present? || public_fetch_mode?
-      # Return all fields
-    else
-      %i(id type preferred_username inbox public_key endpoints)
-    end
-  end
 end
diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb
index 93bf2c83f..b04f4650b 100644
--- a/spec/controllers/accounts_controller_spec.rb
+++ b/spec/controllers/accounts_controller_spec.rb
@@ -348,24 +348,8 @@ RSpec.describe AccountsController, type: :controller do
         context 'in authorized fetch mode' do
           let(:authorized_fetch_mode) { true }
 
-          it 'returns http success' do
-            expect(response).to have_http_status(200)
-          end
-
-          it 'returns application/activity+json' do
-            expect(response.content_type).to eq 'application/activity+json'
-          end
-
-          it_behaves_like 'cachable response'
-
-          it 'returns Vary header with Signature' do
-            expect(response.headers['Vary']).to include 'Signature'
-          end
-
-          it 'renders bare minimum account' do
-            json = body_as_json
-            expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey)
-            expect(json).to_not include(:name, :summary)
+          it 'returns http unauthorized' do
+            expect(response).to have_http_status(401)
           end
         end
       end