]> cat aescling's git repositories - mastodon.git/commitdiff
cat aescling's git repositories / mastodon.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 49b182c)
Fix link verification for remote accounts (#8868)
authorEugen Rochko <eugen@zeonfederated.com>
Thu, 4 Oct 2018 13:47:03 +0000 (15:47 +0200)
committerGitHub <noreply@github.com>
Thu, 4 Oct 2018 13:47:03 +0000 (15:47 +0200)
app/models/account.rb patch | blob | history
app/serializers/rest/account_serializer.rb patch | blob | history
app/services/verify_link_service.rb patch | blob | history
spec/services/verify_link_service_spec.rb patch | blob | history

diff --git a/app/models/account.rb b/app/models/account.rb
index d8e5c7340534831817d98b6445a8c799e5ed602e..44963f3e636905070807aad43f7146c4607b2eb5 100644 (file)
--- 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 d84b48afb1cbae7e99c1c10716af45e6d637663d..12adc971cb76a6894338b2dcf342d34026b287a3 100644 (file)
--- 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 7d53bc255a666d6de0a7f52b1f931b92027aee2d..3453b54c5f7fdec5173a3dee251a7c40c2f69515 100644 (file)
--- 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 9b04d613642e30be92201c76375a12dacbd9649a..2edcdb75f12494e499a0b1f31a67b75ae9a3e50e 100644 (file)
--- 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 <a> back' do
-    let(:html) do
-      <<-HTML
-        <!doctype html>
-        <body>
-          <a href="#{ActivityPub::TagManager.instance.url_for(account)}" rel="me">Follow me on Mastodon</a>
-        </body>
-      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 <a> back' do
+      let(:html) do
+        <<-HTML
+          <!doctype html>
+          <body>
+            <a href="#{ActivityPub::TagManager.instance.url_for(account)}" rel="me">Follow me on Mastodon</a>
+          </body>
+        HTML
+      end
+
+      it 'marks the field as verified' do
+        expect(field.verified?).to be true
+      end
     end
-  end
 
-  context 'when a link contains an <a rel="noopener"> back' do
-    let(:html) do
-      <<-HTML
-        <!doctype html>
-        <body>
-          <a href="#{ActivityPub::TagManager.instance.url_for(account)}" rel="noopener me" target="_blank">Follow me on Mastodon</a>
-        </body>
-      HTML
+    context 'when a link contains an <a rel="noopener"> back' do
+      let(:html) do
+        <<-HTML
+          <!doctype html>
+          <body>
+            <a href="#{ActivityPub::TagManager.instance.url_for(account)}" rel="noopener me" target="_blank">Follow me on Mastodon</a>
+          </body>
+        HTML
+      end
+
+      it 'marks the field as verified' do
+        expect(field.verified?).to be true
+      end
     end
 
-    it 'marks the field as verified' do
-      expect(field.verified?).to be true
+    context 'when a link contains a <link> back' do
+      let(:html) do
+        <<-HTML
+          <!doctype html>
+          <head>
+            <link type="text/html" href="#{ActivityPub::TagManager.instance.url_for(account)}" rel="me" />
+          </head>
+        HTML
+      end
+
+      it 'marks the field as verified' do
+        expect(field.verified?).to be true
+      end
     end
-  end
 
-  context 'when a link contains a <link> back' do
-    let(:html) do
-      <<-HTML
-        <!doctype html>
-        <head>
-          <link type="text/html" href="#{ActivityPub::TagManager.instance.url_for(account)}" rel="me" />
-        </head>
-      HTML
+    context 'when a link goes through a redirect back' do
+      let(:html) do
+        <<-HTML
+          <!doctype html>
+          <head>
+            <link type="text/html" href="https://redirect.me/abc" rel="me" />
+          </head>
+        HTML
+      end
+
+      it 'marks the field as verified' do
+        expect(field.verified?).to be true
+      end
     end
 
-    it 'marks the field as verified' do
-      expect(field.verified?).to be true
+    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 goes through a redirect back' do
-    let(:html) do
-      <<-HTML
-        <!doctype html>
-        <head>
-          <link type="text/html" href="https://redirect.me/abc" rel="me" />
-        </head>
-      HTML
-    end
+  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' => '<a href="http://example.com" rel="me"><span class="invisible">http://</span><span class="">example.com</span><span class="invisible"></span></a>') }
 
-    it 'marks the field as verified' do
-      expect(field.verified?).to be true
+    before do
+      stub_request(:get, 'http://example.com').to_return(status: 200, body: html)
+      subject.call(field)
     end
-  end
 
-  context 'when a link does not contain a link back' do
-    let(:html) { '' }
+    context 'when a link contains an <a> back' do
+      let(:html) do
+        <<-HTML
+          <!doctype html>
+          <body>
+            <a href="https://profile.example.com/alice" rel="me">Follow me on Mastodon</a>
+          </body>
+        HTML
+      end
 
-    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
source code fur Kibicat Mastodon