From 10af3a8b1171bef032171cc82f20402890f4ecdb Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 6 Mar 2016 20:57:33 +0100 Subject: [PATCH] fix pod table migration if someone deleted a user (owner) manually --- app/models/pod.rb | 8 +++++ db/migrate/20160124234712_extend_pods.rb | 4 +-- spec/factories.rb | 2 +- spec/lib/postzord/receiver/public_spec.rb | 2 +- spec/models/pod_spec.rb | 36 ++++++++++++++++++++--- 5 files changed, 44 insertions(+), 8 deletions(-) diff --git a/app/models/pod.rb b/app/models/pod.rb index d957d4ffb..fe0252b83 100644 --- a/app/models/pod.rb +++ b/app/models/pod.rb @@ -27,6 +27,8 @@ class Pod < ActiveRecord::Base where(arel_table[:status].gt(Pod.statuses[:no_errors])).where.not(status: Pod.statuses[:version_failed]) } + validate :not_own_pod + class << self def find_or_create_by(opts) # Rename this method to not override an AR method uri = URI.parse(opts.fetch(:url)) @@ -112,4 +114,10 @@ class Pod < ActiveRecord::Base @uri ||= (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: port) @uri.dup end + + def not_own_pod + pod_uri = AppConfig.pod_uri + pod_port = DEFAULT_PORTS.include?(pod_uri.port) ? nil : pod_uri.port + errors.add(:base, "own pod not allowed") if pod_uri.host == host && pod_port == port + end end diff --git a/db/migrate/20160124234712_extend_pods.rb b/db/migrate/20160124234712_extend_pods.rb index 825a949a5..17fb6c215 100644 --- a/db/migrate/20160124234712_extend_pods.rb +++ b/db/migrate/20160124234712_extend_pods.rb @@ -46,9 +46,9 @@ class ExtendPods < ActiveRecord::Migration add_column :people, :pod_id, :integer add_index :people, :url, length: 190 add_foreign_key :people, :pods, name: :people_pod_id_fk, on_delete: :cascade - Person.where(owner: nil).group_by {|person| person[:url] }.each do |url, _| + Person.where(owner: nil).distinct(:url).pluck(:url).each do |url| pod = Pod.find_or_create_by(url: url) - Person.where(url: url).update_all(pod_id: pod.id) + Person.where(url: url, owner_id: nil).update_all(pod_id: pod.id) if pod.persisted? end # cleanup unused pods diff --git a/spec/factories.rb b/spec/factories.rb index d1a5744b9..f0f645799 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -369,7 +369,7 @@ FactoryGirl.define do factory(:federation_person_from_webfinger, class: DiasporaFederation::Entities::Person) do sequence(:guid) { UUID.generate :compact } sequence(:diaspora_id) {|n| "bob-person-#{n}#{r_str}@example.net" } - url AppConfig.pod_uri.to_s + url "https://example.net/" exported_key OpenSSL::PKey::RSA.generate(1024).public_key.export profile { DiasporaFederation::Entities::Profile.new( diff --git a/spec/lib/postzord/receiver/public_spec.rb b/spec/lib/postzord/receiver/public_spec.rb index 8519a1d71..5ce244222 100644 --- a/spec/lib/postzord/receiver/public_spec.rb +++ b/spec/lib/postzord/receiver/public_spec.rb @@ -21,7 +21,7 @@ describe Postzord::Receiver::Public do xml = Salmon::Slap.create_by_user_and_activity(bob, comment.to_diaspora_xml).xml_for(nil) person = bob.person person.owner = nil - person.pod = Pod.find_or_create_by(url: AppConfig.pod_uri) + person.pod = Pod.find_or_create_by(url: "https://example.org/") person.save bob.destroy comment.destroy diff --git a/spec/models/pod_spec.rb b/spec/models/pod_spec.rb index f612a6152..3b9768955 100644 --- a/spec/models/pod_spec.rb +++ b/spec/models/pod_spec.rb @@ -25,18 +25,46 @@ describe Pod, type: :model do end it "updates ssl boolean if upgraded to https" do - pod = Pod.find_or_create_by(url: "http://joindiaspora.com/") + pod = Pod.find_or_create_by(url: "http://example.org/") expect(pod.ssl).to be false - pod = Pod.find_or_create_by(url: "https://joindiaspora.com/") + pod = Pod.find_or_create_by(url: "https://example.org/") expect(pod.ssl).to be true end it "does not update ssl boolean if downgraded to http" do - pod = Pod.find_or_create_by(url: "https://joindiaspora.com/") + pod = Pod.find_or_create_by(url: "https://example.org/") expect(pod.ssl).to be true - pod = Pod.find_or_create_by(url: "http://joindiaspora.com/") + pod = Pod.find_or_create_by(url: "http://example.org/") expect(pod.ssl).to be true end + + context "validation" do + it "is valid" do + pod = Pod.find_or_create_by(url: "https://example.org/") + expect(pod).to be_valid + end + + it "doesn't allow own pod" do + pod = Pod.find_or_create_by(url: AppConfig.url_to("/")) + expect(pod).not_to be_valid + end + + it "doesn't allow own pod with default port" do + uri = URI.parse("https://example.org/") + allow(AppConfig).to receive(:pod_uri).and_return(uri) + + pod = Pod.find_or_create_by(url: AppConfig.url_to("/")) + expect(pod).not_to be_valid + end + + it "doesn't allow own pod with other scheme" do + uri = URI.parse("https://example.org/") + allow(AppConfig).to receive(:pod_uri).and_return(uri) + + pod = Pod.find_or_create_by(url: "http://example.org/") + expect(pod).not_to be_valid + end + end end describe ".check_all!" do