From 7fe137d2f7792ed735be11eaca6d87fbc114043a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 4 Oct 2018 15:47:03 +0200 Subject: [PATCH] Fix link verification for remote accounts (#8868) --- app/models/account.rb | 26 +++- app/serializers/rest/account_serializer.rb | 6 +- app/services/verify_link_service.rb | 2 +- spec/services/verify_link_service_spec.rb | 157 ++++++++++++--------- 4 files changed, 117 insertions(+), 74 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index d8e5c73405..44963f3e63 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -312,8 +312,8 @@ class Account < ApplicationRecord def initialize(account, attributes) @account = account @attributes = attributes - @name = attributes['name'].strip[0, 255] - @value = attributes['value'].strip[0, 255] + @name = attributes['name'].strip[0, string_limit] + @value = attributes['value'].strip[0, string_limit] @verified_at = attributes['verified_at']&.to_datetime @errors = {} end @@ -322,8 +322,18 @@ class Account < ApplicationRecord verified_at.present? end + def value_for_verification + @value_for_verification ||= begin + if account.local? + value + else + ActionController::Base.helpers.strip_tags(value) + end + end + end + def verifiable? - value.present? && value.start_with?('http://', 'https://') + value_for_verification.present? && value_for_verification.start_with?('http://', 'https://') end def mark_verified! @@ -334,6 +344,16 @@ class Account < ApplicationRecord def to_h { name: @name, value: @value, verified_at: @verified_at } end + + private + + def string_limit + if account.local? + 255 + else + 2047 + end + end end class << self diff --git a/app/serializers/rest/account_serializer.rb b/app/serializers/rest/account_serializer.rb index d84b48afb1..12adc971cb 100644 --- a/app/serializers/rest/account_serializer.rb +++ b/app/serializers/rest/account_serializer.rb @@ -11,11 +11,7 @@ class REST::AccountSerializer < ActiveModel::Serializer has_many :emojis, serializer: REST::CustomEmojiSerializer class FieldSerializer < ActiveModel::Serializer - attributes :name, :value - - attribute :verified_at, if: :verifiable? - - delegate :verifiable?, to: :object + attributes :name, :value, :verified_at def value Formatter.instance.format_field(object.account, object.value) diff --git a/app/services/verify_link_service.rb b/app/services/verify_link_service.rb index 7d53bc255a..3453b54c5f 100644 --- a/app/services/verify_link_service.rb +++ b/app/services/verify_link_service.rb @@ -3,7 +3,7 @@ class VerifyLinkService < BaseService def call(field) @link_back = ActivityPub::TagManager.instance.url_for(field.account) - @url = field.value + @url = field.value_for_verification perform_request! diff --git a/spec/services/verify_link_service_spec.rb b/spec/services/verify_link_service_spec.rb index 9b04d61364..2edcdb75f1 100644 --- a/spec/services/verify_link_service_spec.rb +++ b/spec/services/verify_link_service_spec.rb @@ -3,80 +3,107 @@ require 'rails_helper' RSpec.describe VerifyLinkService, type: :service do subject { described_class.new } - let(:account) { Fabricate(:account, username: 'alice') } - let(:field) { Account::Field.new(account, 'name' => 'Website', 'value' => 'http://example.com') } + context 'given a local account' do + let(:account) { Fabricate(:account, username: 'alice') } + let(:field) { Account::Field.new(account, 'name' => 'Website', 'value' => 'http://example.com') } - before do - stub_request(:head, 'https://redirect.me/abc').to_return(status: 301, headers: { 'Location' => ActivityPub::TagManager.instance.url_for(account) }) - stub_request(:get, 'http://example.com').to_return(status: 200, body: html) - subject.call(field) - end - - context 'when a link contains an back' do - let(:html) do - <<-HTML - - - Follow me on Mastodon - - HTML + before do + stub_request(:head, 'https://redirect.me/abc').to_return(status: 301, headers: { 'Location' => ActivityPub::TagManager.instance.url_for(account) }) + stub_request(:get, 'http://example.com').to_return(status: 200, body: html) + subject.call(field) end - it 'marks the field as verified' do - expect(field.verified?).to be true + context 'when a link contains an back' do + let(:html) do + <<-HTML + + + Follow me on Mastodon + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end + end + + context 'when a link contains an back' do + let(:html) do + <<-HTML + + + Follow me on Mastodon + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end + end + + context 'when a link contains a back' do + let(:html) do + <<-HTML + + + + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end + end + + context 'when a link goes through a redirect back' do + let(:html) do + <<-HTML + + + + + HTML + end + + it 'marks the field as verified' do + expect(field.verified?).to be true + end + end + + context 'when a link does not contain a link back' do + let(:html) { '' } + + it 'marks the field as verified' do + expect(field.verified?).to be false + end end end - context 'when a link contains an back' do - let(:html) do - <<-HTML - - - Follow me on Mastodon - - HTML + context 'given a remote account' do + let(:account) { Fabricate(:account, username: 'alice', domain: 'example.com', url: 'https://profile.example.com/alice') } + let(:field) { Account::Field.new(account, 'name' => 'Website', 'value' => 'example.com') } + + before do + stub_request(:get, 'http://example.com').to_return(status: 200, body: html) + subject.call(field) end - it 'marks the field as verified' do - expect(field.verified?).to be true - end - end + context 'when a link contains an back' do + let(:html) do + <<-HTML + + + Follow me on Mastodon + + HTML + end - context 'when a link contains a back' do - let(:html) do - <<-HTML - - - - - HTML - end - - it 'marks the field as verified' do - expect(field.verified?).to be true - end - end - - context 'when a link goes through a redirect back' do - let(:html) do - <<-HTML - - - - - HTML - end - - it 'marks the field as verified' do - expect(field.verified?).to be true - end - end - - context 'when a link does not contain a link back' do - let(:html) { '' } - - it 'marks the field as verified' do - expect(field.verified?).to be false + it 'marks the field as verified' do + expect(field.verified?).to be true + end end end end