From a4859446abea166ae55806169425646c1bd42f85 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 1 May 2017 18:44:23 -0400 Subject: [PATCH] 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 | 27 ++---- app/models/remote_follow.rb | 42 +++++++- .../remote_follow_controller_spec.rb | 96 ++++++++++++++++++- 3 files changed, 141 insertions(+), 24 deletions(-) diff --git a/app/controllers/remote_follow_controller.rb b/app/controllers/remote_follow_controller.rb index 22e376836..625ce1488 100644 --- a/app/controllers/remote_follow_controller.rb +++ b/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.acct = session[:remote_follow] if session.key?(:remote_follow) + @remote_follow = RemoteFollow.new(session_params) 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 Addressable::Template.new(redirect_url_link.template).expand(uri: @account.to_webfinger_s).to_s + redirect_to @remote_follow.subscribe_address_for(@account) 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 session_params + { acct: session[:remote_follow] } + end + def set_account @account = Account.find_local!(params[:account_username]) end - - def check_account_suspension - head 410 if @account.suspended? - end end diff --git a/app/models/remote_follow.rb b/app/models/remote_follow.rb index 6c291e22c..c226cdb14 100644 --- a/app/models/remote_follow.rb +++ b/app/models/remote_follow.rb @@ -3,11 +3,47 @@ class RemoteFollow include ActiveModel::Validations - attr_accessor :acct - - validates :acct, presence: true + attr_accessor :acct, :addressable_template 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 diff --git a/spec/controllers/remote_follow_controller_spec.rb b/spec/controllers/remote_follow_controller_spec.rb index 1c9e6a4b6..ce04e2c43 100644 --- a/spec/controllers/remote_follow_controller_spec.rb +++ b/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