Coverage for remote follows (#2694)

* Add coverage for create with empty acct value

* Add coverage for create with webfinger failure

* Add coverage for create with webfinger providing bad values

* Add coverage for create when webfinger is good

* Add coverage for session[:remote_follow] having data

* Simplify how remote follow pulls acct from session

* Remote follow behaves more like model

* Move the discovery portions of remote follow out of controller

* Check for suspended accounts

app/controllers/remote_follow_controller.rb

@@ -4,34 +4,21 @@ class RemoteFollowController < ApplicationController
   layout 'public'
 
   before_action :set_account
-  before_action :check_account_suspension
+  before_action :gone, if: -> { @account.suspended? }
 
   def new
-    @remote_follow = RemoteFollow.new
+    @remote_follow = RemoteFollow.new(session_params)
-    @remote_follow.acct = session[:remote_follow] if session.key?(:remote_follow)
   end
 
   def create
     @remote_follow = RemoteFollow.new(resource_params)
 
     if @remote_follow.valid?
-      resource          = Goldfinger.finger("acct:#{@remote_follow.acct}")
-      redirect_url_link = resource&.link('http://ostatus.org/schema/1.0/subscribe')
-
-      if redirect_url_link.nil? || redirect_url_link.template.nil?
-        @remote_follow.errors.add(:acct, I18n.t('remote_follow.missing_resource'))
-        render(:new) && return
-      end
-
       session[:remote_follow] = @remote_follow.acct
-
+      redirect_to @remote_follow.subscribe_address_for(@account)
-      redirect_to Addressable::Template.new(redirect_url_link.template).expand(uri: @account.to_webfinger_s).to_s
     else
       render :new
     end
-  rescue Goldfinger::Error
-    @remote_follow.errors.add(:acct, I18n.t('remote_follow.missing_resource'))
-    render :new
   end
 
   private
@@ -40,11 +27,11 @@ class RemoteFollowController < ApplicationController
     params.require(:remote_follow).permit(:acct)
   end
 
-  def set_account
+  def session_params
-    @account = Account.find_local!(params[:account_username])
+    { acct: session[:remote_follow] }
   end
 
-  def check_account_suspension
+  def set_account
-    head 410 if @account.suspended?
+    @account = Account.find_local!(params[:account_username])
   end
 end

app/models/remote_follow.rb

@@ -3,11 +3,47 @@
 class RemoteFollow
   include ActiveModel::Validations
 
-  attr_accessor :acct
+  attr_accessor :acct, :addressable_template
-
-  validates :acct, presence: true
 
   def initialize(attrs = {})
     @acct = attrs[:acct].strip unless attrs[:acct].nil?
   end
+
+  def valid?
+    populate_template
+    errors.empty?
+  end
+
+  def subscribe_address_for(account)
+    addressable_template.expand(uri: account.to_webfinger_s).to_s
+  end
+
+  private
+
+  def populate_template
+    if acct.blank? || redirect_url_link.nil? || redirect_url_link.template.nil?
+      missing_resource_error
+    else
+      @addressable_template = Addressable::Template.new(redirect_uri_template)
+    end
+  end
+
+  def redirect_uri_template
+    redirect_url_link.template
+  end
+
+  def redirect_url_link
+    acct_resource&.link('http://ostatus.org/schema/1.0/subscribe')
+  end
+
+  def acct_resource
+    @_acct_resource ||= Goldfinger.finger("acct:#{acct}")
+  rescue Goldfinger::Error
+    missing_resource_error
+    nil
+  end
+
+  def missing_resource_error
+    errors.add(:acct, I18n.t('remote_follow.missing_resource'))
+  end
 end

spec/controllers/remote_follow_controller_spec.rb

@@ -6,11 +6,105 @@ describe RemoteFollowController do
   render_views
 
   describe '#new' do
-    it 'returns a success' do
+    it 'returns success when session is empty' do
       account = Fabricate(:account)
       get :new, params: { account_username: account.to_param }
 
       expect(response).to have_http_status(:success)
+      expect(response).to render_template(:new)
+      expect(assigns(:remote_follow).acct).to be_nil
+    end
+
+    it 'populates the remote follow with session data when session exists' do
+      session[:remote_follow] = 'user@example.com'
+      account = Fabricate(:account)
+      get :new, params: { account_username: account.to_param }
+
+      expect(response).to have_http_status(:success)
+      expect(response).to render_template(:new)
+      expect(assigns(:remote_follow).acct).to eq 'user@example.com'
+    end
+  end
+
+  describe '#create' do
+    before do
+      @account = Fabricate(:account, username: 'test_user')
+    end
+
+    context 'with a valid acct' do
+      context 'when webfinger values are wrong' do
+        it 'renders new when redirect url is nil' do
+          resource_with_nil_link = double(link: nil)
+          allow(Goldfinger).to receive(:finger).with('acct:user@example.com').and_return(resource_with_nil_link)
+          post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
+
+          expect(response).to render_template(:new)
+          expect(response.body).to include(I18n.t('remote_follow.missing_resource'))
+        end
+
+        it 'renders new when template is nil' do
+          link_with_nil_template = double(template: nil)
+          resource_with_link = double(link: link_with_nil_template)
+          allow(Goldfinger).to receive(:finger).with('acct:user@example.com').and_return(resource_with_link)
+          post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
+
+          expect(response).to render_template(:new)
+          expect(response.body).to include(I18n.t('remote_follow.missing_resource'))
+        end
+      end
+
+      context 'when webfinger values are good' do
+        before do
+          link_with_template = double(template: 'http://example.com/follow_me?acct={uri}')
+          resource_with_link = double(link: link_with_template)
+          allow(Goldfinger).to receive(:finger).with('acct:user@example.com').and_return(resource_with_link)
+          post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
+        end
+
+        it 'saves the session' do
+          expect(session[:remote_follow]).to eq 'user@example.com'
+        end
+
+        it 'redirects to the remote location' do
+          address = "http://example.com/follow_me?acct=acct%3Atest_user%40#{Rails.configuration.x.local_domain}"
+
+          expect(response).to redirect_to(address)
+        end
+      end
+    end
+
+    context 'with an invalid acct' do
+      it 'renders new when acct is missing' do
+        post :create, params: { account_username: @account.to_param, remote_follow: { acct: '' } }
+
+        expect(response).to render_template(:new)
+      end
+
+      it 'renders new with error when goldfinger fails' do
+        allow(Goldfinger).to receive(:finger).with('acct:user@example.com').and_raise(Goldfinger::Error)
+        post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } }
+
+        expect(response).to render_template(:new)
+        expect(response.body).to include(I18n.t('remote_follow.missing_resource'))
+      end
+    end
+  end
+
+  describe 'with a suspended account' do
+    before do
+      @account = Fabricate(:account, suspended: true)
+    end
+
+    it 'returns 410 gone on GET to #new' do
+      get :new, params: { account_username: @account.to_param }
+
+      expect(response).to have_http_status(:gone)
+    end
+
+    it 'returns 410 gone on POST to #create' do
+      post :create, params: { account_username: @account.to_param }
+
+      expect(response).to have_http_status(:gone)
     end
   end
 end