diff --git a/Changelog.md b/Changelog.md index 78f92fbb9..024d76d13 100644 --- a/Changelog.md +++ b/Changelog.md @@ -91,6 +91,7 @@ Contributions are very welcome, the hard work is done! * Dropped `parent_author_signature` from relayables [#6586](https://github.com/diaspora/diaspora/pull/6586) * Attached ShareVisibilities to the User, not the Contact [#6723](https://github.com/diaspora/diaspora/pull/6723) * Refactor mentions input, now based on typeahead.js [#6728](https://github.com/diaspora/diaspora/pull/6728) +* Optimized the pod up checks [#6727](https://github.com/diaspora/diaspora/pull/6727) ## Bug fixes * Destroy Participation when removing interactions with a post [#5852](https://github.com/diaspora/diaspora/pull/5852) diff --git a/app/assets/javascripts/app/pages/admin_pods.js b/app/assets/javascripts/app/pages/admin_pods.js index 4c4b14dd7..a4880354e 100644 --- a/app/assets/javascripts/app/pages/admin_pods.js +++ b/app/assets/javascripts/app/pages/admin_pods.js @@ -34,6 +34,11 @@ app.pages.AdminPods = app.views.Base.extend({ .append(Diaspora.I18n.t("admin.pods.unchecked", {count: gon.uncheckedCount})); msgs.appendChild(unchecked[0]); } + if( gon.versionFailedCount && gon.versionFailedCount > 0 ) { + var versionFailed = $("
") + .append(Diaspora.I18n.t("admin.pods.version_failed", {count: gon.versionFailedCount})); + msgs.appendChild(versionFailed[0]); + } if( gon.errorCount && gon.errorCount > 0 ) { var errors = $("
") .append(Diaspora.I18n.t("admin.pods.errors", {count: gon.errorCount})); diff --git a/app/assets/javascripts/app/views/pod_entry_view.js b/app/assets/javascripts/app/views/pod_entry_view.js index c8c402fb8..112b6708f 100644 --- a/app/assets/javascripts/app/views/pod_entry_view.js +++ b/app/assets/javascripts/app/views/pod_entry_view.js @@ -31,7 +31,8 @@ app.views.PodEntry = app.views.Base.extend({ has_no_errors: (this.model.get("status")==="no_errors"), has_errors: (this.model.get("status")!=="no_errors"), status_text: Diaspora.I18n.t("admin.pods.states."+this.model.get("status")), - pod_url: (this.model.get("ssl") ? "https" : "http") + "://" + this.model.get("host"), + pod_url: (this.model.get("ssl") ? "https" : "http") + "://" + this.model.get("host") + + (this.model.get("port") ? ":" + this.model.get("port") : ""), response_time_fmt: this._fmtResponseTime() /* jshint camelcase: true */ }); diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss index 1859d1c06..ea933e6e6 100644 --- a/app/assets/stylesheets/admin.scss +++ b/app/assets/stylesheets/admin.scss @@ -39,6 +39,12 @@ /** pod list **/ #pod-list { + .pod-title { + max-width: 200px; + overflow: hidden; + text-overflow: ellipsis; + } + th.added, td.added, td.actions { white-space: nowrap; } diff --git a/app/assets/templates/pod_table_entry_tpl.jst.hbs b/app/assets/templates/pod_table_entry_tpl.jst.hbs index 9daa5b331..e43ee6c6f 100644 --- a/app/assets/templates/pod_table_entry_tpl.jst.hbs +++ b/app/assets/templates/pod_table_entry_tpl.jst.hbs @@ -7,7 +7,7 @@ {{/if}} -{{host}} +{{host}} diff --git a/app/controllers/admin/pods_controller.rb b/app/controllers/admin/pods_controller.rb index 3f81dd582..1b4d2bf2f 100644 --- a/app/controllers/admin/pods_controller.rb +++ b/app/controllers/admin/pods_controller.rb @@ -10,6 +10,7 @@ module Admin format.html do gon.preloads[:pods] = pods_json gon.unchecked_count = Pod.unchecked.count + gon.version_failed_count = Pod.version_failed.count gon.error_count = Pod.check_failed.count render "admins/pods" diff --git a/app/models/person.rb b/app/models/person.rb index f4c2c49a7..4182c7a4b 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -50,15 +50,14 @@ class Person < ActiveRecord::Base has_many :roles belongs_to :owner, :class_name => 'User' + belongs_to :pod has_many :notification_actors has_many :notifications, :through => :notification_actors has_many :mentions, :dependent => :destroy - before_validation :clean_url - - validates :url, :presence => true + validate :owner_xor_pod validates :profile, :presence => true validates :serialized_public_key, :presence => true validates :diaspora_handle, :uniqueness => true @@ -205,8 +204,6 @@ class Person < ActiveRecord::Base def url url_to "/" - rescue - self[:url] end def profile_url @@ -290,23 +287,6 @@ class Person < ActiveRecord::Base json end - # Update an array of people given a url, and set it as the new destination_url - # @param people [Array] - # @param url [String] - def self.url_batch_update(people, url) - people.each do |person| - person.update_url(url) - end - end - - # @param person [Person] - # @param url [String] - def update_url(url) - @uri = URI.parse(url) - @uri.path = "/" - update_attributes(:url => @uri.to_s) - end - def lock_access! self.closed_account = true self.save @@ -317,27 +297,12 @@ class Person < ActiveRecord::Base self end - protected - - def clean_url - if self.url - self.url = 'http://' + self.url unless self.url.match(/https?:\/\//) - self.url = self.url + '/' if self.url[-1, 1] != '/' - end - end - private - # @return [URI] - def uri - @uri ||= URI.parse(self[:url]) - @uri.dup - end - # @param path [String] # @return [String] def url_to(path) - uri.tap {|uri| uri.path = path }.to_s + local? ? AppConfig.url_to(path) : pod.url_to(path) end def fix_profile @@ -345,4 +310,8 @@ class Person < ActiveRecord::Base DiasporaFederation::Discovery::Discovery.new(diaspora_handle).fetch_and_save reload end + + def owner_xor_pod + errors.add(:base, "Specify an owner or a pod, not both") unless owner.blank? ^ pod.blank? + end end diff --git a/app/models/pod.rb b/app/models/pod.rb index c835fbce7..d957d4ffb 100644 --- a/app/models/pod.rb +++ b/app/models/pod.rb @@ -19,18 +19,21 @@ class Pod < ActiveRecord::Base ConnectionTester::NodeInfoFailure => :version_failed } + DEFAULT_PORTS = [URI::HTTP::DEFAULT_PORT, URI::HTTPS::DEFAULT_PORT] + + has_many :people + scope :check_failed, lambda { - where(arel_table[:status].gt(Pod.statuses[:no_errors])) + where(arel_table[:status].gt(Pod.statuses[:no_errors])).where.not(status: Pod.statuses[:version_failed]) } class << self def find_or_create_by(opts) # Rename this method to not override an AR method - u = URI.parse(opts.fetch(:url)) - find_or_initialize_by(host: u.host).tap do |pod| - unless pod.persisted? - pod.ssl = (u.scheme == "https") - pod.save - end + uri = URI.parse(opts.fetch(:url)) + port = DEFAULT_PORTS.include?(uri.port) ? nil : uri.port + find_or_initialize_by(host: uri.host, port: port).tap do |pod| + pod.ssl ||= (uri.scheme == "https") + pod.save end end @@ -57,15 +60,20 @@ class Pod < ActiveRecord::Base end def test_connection! - url = "#{ssl ? 'https' : 'http'}://#{host}" - result = ConnectionTester.check url - logger.info "testing pod: '#{url}' - #{result.inspect}" + result = ConnectionTester.check uri.to_s + logger.debug "tested pod: '#{uri}' - #{result.inspect}" transaction do update_from_result(result) end end + # @param path [String] + # @return [String] + def url_to(path) + uri.tap {|uri| uri.path = path }.to_s + end + private def update_from_result(result) @@ -85,6 +93,7 @@ class Pod < ActiveRecord::Base end def attributes_from_result(result) + self.ssl ||= result.ssl self.error = result.failure_message[0..254] if result.error? self.software = result.software_version[0..254] if result.software_version.present? self.response_time = result.rt @@ -97,4 +106,10 @@ class Pod < ActiveRecord::Base :no_errors end end + + # @return [URI] + def uri + @uri ||= (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: port) + @uri.dup + end end diff --git a/app/models/user.rb b/app/models/user.rb index 598ec8cb6..f6e2cefba 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,7 +39,7 @@ class User < ActiveRecord::Base serialize :hidden_shareables, Hash - has_one :person, :foreign_key => :owner_id + has_one :person, inverse_of: :owner, foreign_key: :owner_id has_one :profile, through: :person delegate :guid, :public_key, :posts, :photos, :owns?, :image_url, @@ -82,8 +82,7 @@ class User < ActiveRecord::Base has_many :share_visibilities - before_save :guard_unconfirmed_email, - :save_person! + before_save :guard_unconfirmed_email def self.all_sharing_with_person(person) User.joins(:contacts).where(:contacts => {:person_id => person.id}) @@ -460,7 +459,6 @@ class User < ActiveRecord::Base end def set_person(person) - person.url = AppConfig.pod_uri.to_s person.diaspora_handle = "#{self.username}#{User.diaspora_id_host}" self.person = person end @@ -539,16 +537,6 @@ class User < ActiveRecord::Base end end - # Sometimes we access the person in a strange way and need to do this - # @note we should make this method depricated. - # - # @return [Person] - def save_person! - self.person.save if self.person && self.person.changed? - self.person - end - - def no_person_with_same_username diaspora_id = "#{self.username}#{User.diaspora_id_host}" if self.username_changed? && Person.exists?(:diaspora_handle => diaspora_id) diff --git a/app/presenters/pod_presenter.rb b/app/presenters/pod_presenter.rb index c59823c39..7f08b3770 100644 --- a/app/presenters/pod_presenter.rb +++ b/app/presenters/pod_presenter.rb @@ -3,6 +3,7 @@ class PodPresenter < BasePresenter { id: id, host: host, + port: port, ssl: ssl, status: status, checked_at: checked_at, diff --git a/config/initializers/diaspora_federation.rb b/config/initializers/diaspora_federation.rb index 4d63aebd2..1fa4b18e6 100644 --- a/config/initializers/diaspora_federation.rb +++ b/config/initializers/diaspora_federation.rb @@ -47,7 +47,7 @@ DiasporaFederation.configure do |config| # find existing person or create a new one person_entity = Person.find_by(diaspora_handle: person.diaspora_id) || Person.new(diaspora_handle: person.diaspora_id, guid: person.guid, - serialized_public_key: person.exported_key, url: person.url) + serialized_public_key: person.exported_key, pod: Pod.find_or_create_by(url: person.url)) profile = person.profile profile_entity = person_entity.profile ||= Profile.new diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index 17ae35f06..b9b5dc3d4 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -72,6 +72,9 @@ en: unchecked: one: "There is still one pod that hasn't been checked at all." other: "There are still <%= count %> pods that haven't been checked at all." + version_failed: + one: "There is one pod that has no version (old pod, no NodeInfo)." + other: "There are <%= count %> pods that have no version (old pods, no NodeInfo)." errors: one: "The connection test returned an error for one pod." other: "The connection test returned an error for <%= count %> pods." diff --git a/db/migrate/20160124234712_extend_pods.rb b/db/migrate/20160124234712_extend_pods.rb new file mode 100644 index 000000000..825a949a5 --- /dev/null +++ b/db/migrate/20160124234712_extend_pods.rb @@ -0,0 +1,77 @@ +class ExtendPods < ActiveRecord::Migration + class Pod < ActiveRecord::Base + has_many :people + + DEFAULT_PORTS = [URI::HTTP::DEFAULT_PORT, URI::HTTPS::DEFAULT_PORT] + + def self.find_or_create_by(opts) + uri = URI.parse(opts.fetch(:url)) + port = DEFAULT_PORTS.include?(uri.port) ? nil : uri.port + find_or_initialize_by(host: uri.host, port: port).tap do |pod| + pod.ssl ||= (uri.scheme == "https") + pod.save + end + end + + def url + (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: port, path: "/") + end + end + + class Person < ActiveRecord::Base + belongs_to :owner, class_name: "User" + belongs_to :pod + + def url + owner_id.nil? ? pod.url.to_s : AppConfig.url_to("/") + end + end + + class User < ActiveRecord::Base + has_one :person, inverse_of: :owner, foreign_key: :owner_id + end + + def up + remove_index :pods, :host + + # add port + add_column :pods, :port, :integer + add_index :pods, %i(host port), unique: true, length: {host: 190, port: nil}, using: :btree + + add_column :pods, :blocked, :boolean, default: false + + Pod.reset_column_information + + # link people with pod + 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, _| + pod = Pod.find_or_create_by(url: url) + Person.where(url: url).update_all(pod_id: pod.id) + end + + # cleanup unused pods + Pod.joins("LEFT OUTER JOIN people ON pods.id = people.pod_id").delete_all("people.id is NULL") + + remove_column :people, :url + end + + def down + # restore url + add_column :people, :url, :text + Person.all.group_by(&:pod_id).each do |pod_id, persons| + Person.where(pod_id: pod_id).update_all(url: persons.first.url) + end + change_column :people, :url, :text, null: false + remove_foreign_key :people, :pods + remove_column :people, :pod_id + + # remove pods with port + Pod.where.not(port: nil).delete_all + + remove_index :pods, column: %i(host port) + remove_columns :pods, :port, :blocked + add_index :pods, :host, unique: true, length: 190 + end +end diff --git a/db/schema.rb b/db/schema.rb index 04230193d..ed02fdb92 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -327,7 +327,6 @@ ActiveRecord::Schema.define(version: 20160225232049) do create_table "people", force: :cascade do |t| t.string "guid", limit: 255, null: false - t.text "url", limit: 65535, null: false t.string "diaspora_handle", limit: 255, null: false t.text "serialized_public_key", limit: 65535, null: false t.integer "owner_id", limit: 4 @@ -335,11 +334,13 @@ ActiveRecord::Schema.define(version: 20160225232049) do t.datetime "updated_at", null: false t.boolean "closed_account", default: false t.integer "fetch_status", limit: 4, default: 0 + t.integer "pod_id", limit: 4 end add_index "people", ["diaspora_handle"], name: "index_people_on_diaspora_handle", unique: true, length: {"diaspora_handle"=>191}, using: :btree add_index "people", ["guid"], name: "index_people_on_guid", unique: true, length: {"guid"=>191}, using: :btree add_index "people", ["owner_id"], name: "index_people_on_owner_id", unique: true, using: :btree + add_index "people", ["pod_id"], name: "people_pod_id_fk", using: :btree create_table "photos", force: :cascade do |t| t.integer "tmp_old_id", limit: 4 @@ -375,10 +376,12 @@ ActiveRecord::Schema.define(version: 20160225232049) do t.integer "response_time", limit: 4, default: -1 t.string "software", limit: 255 t.string "error", limit: 255 + t.integer "port", limit: 4 + t.boolean "blocked", default: false end add_index "pods", ["checked_at"], name: "index_pods_on_checked_at", using: :btree - add_index "pods", ["host"], name: "index_pods_on_host", unique: true, length: {"host"=>190}, using: :btree + add_index "pods", ["host", "port"], name: "index_pods_on_host_and_port", unique: true, length: {"host"=>190, "port"=>nil}, using: :btree add_index "pods", ["offline_since"], name: "index_pods_on_offline_since", using: :btree add_index "pods", ["status"], name: "index_pods_on_status", using: :btree @@ -672,6 +675,7 @@ ActiveRecord::Schema.define(version: 20160225232049) do add_foreign_key "notification_actors", "notifications", name: "notification_actors_notification_id_fk", on_delete: :cascade add_foreign_key "o_auth_access_tokens", "authorizations" add_foreign_key "o_auth_applications", "users" + add_foreign_key "people", "pods", name: "people_pod_id_fk", on_delete: :cascade add_foreign_key "posts", "people", column: "author_id", name: "posts_author_id_fk", on_delete: :cascade add_foreign_key "ppid", "o_auth_applications" add_foreign_key "ppid", "users" diff --git a/lib/account_deleter.rb b/lib/account_deleter.rb index 7e8fee278..58707810f 100644 --- a/lib/account_deleter.rb +++ b/lib/account_deleter.rb @@ -104,11 +104,11 @@ class AccountDeleter end def normal_ar_person_associates_to_delete - [:posts, :photos, :mentions, :participations, :roles] + %i(posts photos mentions participations roles) end def ignored_or_special_ar_person_associations - [:comments, :contacts, :notification_actors, :notifications, :owner, :profile, :conversation_visibilities] + %i(comments contacts notification_actors notifications owner profile conversation_visibilities pod) end def mark_account_deletion_complete diff --git a/lib/connection_tester.rb b/lib/connection_tester.rb index 9b36e5e47..0783b3a93 100644 --- a/lib/connection_tester.rb +++ b/lib/connection_tester.rb @@ -17,11 +17,10 @@ class ConnectionTester # # @api This is the entry point you're supposed to use for testing # connections to other diaspora-compatible servers. - # @param [String] server URL + # @param [String] url URL # @return [Result] result object containing information about the # server and to what point the connection was successful def check(url) - url = "http://#{url}" unless url.include?("://") result = Result.new begin @@ -54,7 +53,7 @@ class ConnectionTester result.reachable = false when SSLFailure result.reachable = true - result.ssl_status = false + result.ssl = false when HTTPFailure result.reachable = true when NodeInfoFailure @@ -70,8 +69,6 @@ class ConnectionTester @uri ||= URI.parse(@url) raise AddressFailure, "invalid protocol: '#{@uri.scheme.upcase}'" unless http_uri?(@uri) - - result.hostname = @uri.host rescue AddressFailure => e raise e rescue URI::InvalidURIError => e @@ -83,11 +80,8 @@ class ConnectionTester # Perform the DNS query, the IP address will be stored in the result # @raise [DNSFailure] caused by a failure to resolve or a timeout def resolve - with_dns_resolver do |dns| - addr = dns.getaddress(@uri.host) - @result.ip = addr.to_s - end - rescue Resolv::ResolvError, Resolv::ResolvTimeout => e + @result.ip = IPSocket.getaddress(@uri.host) + rescue SocketError => e raise DNSFailure, "'#{@uri.host}' - #{e.message}" rescue StandardError => e raise Failure, e.inspect @@ -99,13 +93,15 @@ class ConnectionTester # * is the SSL certificate valid (only on HTTPS) # * does the server return a successful HTTP status code # * is there a reasonable amount of redirects (3 by default) + # * is there a /.well-known/host-meta (this is needed to work, this can be replaced with a mandatory NodeInfo later) # (can't do a HEAD request, since that's not a defined route in the app) # # @raise [NetFailure, SSLFailure, HTTPFailure] if any of the checks fail # @return [Integer] HTTP status code def request with_http_connection do |http| - response = capture_response_time { http.get("/") } + capture_response_time { http.get("/") } + response = http.get("/.well-known/host-meta") handle_http_response(response) end rescue HTTPFailure => e @@ -132,7 +128,7 @@ class ConnectionTester find_software_version(nd_resp.body) end rescue Faraday::ResourceNotFound, JSON::JSONError => e - raise NodeInfoFailure, e.message[0..255] + raise NodeInfoFailure, e.message[0..255].encode(Encoding.default_external, undef: :replace) rescue StandardError => e raise Failure, e.inspect end @@ -153,39 +149,29 @@ class ConnectionTester yield(@http) if block_given? end - def with_dns_resolver - dns = Resolv::DNS.new - yield(dns) if block_given? - ensure - dns.close - end - def http_uri?(uri) uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS) end - def uses_ssl? - @uses_ssl - end - # request root path, measure response time # measured time may be skewed, if there are redirects # # @return [Faraday::Response] def capture_response_time - start = Time.zone.now + start = Time.zone.now resp = yield if block_given? @result.rt = ((Time.zone.now - start) * 1000.0).to_i # milliseconds resp end def handle_http_response(response) - @uses_ssl = (response.env.url.scheme == "https") @result.status_code = Integer(response.status) if response.success? - @result.reachable = true - @result.ssl_status = @uses_ssl + raise HTTPFailure, "redirected to other hostname: #{response.env.url}" unless @uri.host == response.env.url.host + + @result.reachable = true + @result.ssl = (response.env.url.scheme == "https") else raise HTTPFailure, "unsuccessful response code: #{response.status}" end @@ -228,19 +214,16 @@ class ConnectionTester end Result = Struct.new( - :hostname, :ip, :reachable, :ssl_status, :status_code, :rt, :software_version, :error + :ip, :reachable, :ssl, :status_code, :rt, :software_version, :error ) do - # @!attribute hostname - # @return [String] hostname derived from the URL - # @!attribute ip # @return [String] resolved IP address from DNS query # @!attribute reachable # @return [Boolean] whether the host was reachable over the network - # @!attribute ssl_status - # @return [Boolean] indicating how the SSL verification went + # @!attribute ssl + # @return [Boolean] whether the host has working ssl # @!attribute status_code # @return [Integer] HTTP status code that was returned for the HEAD request diff --git a/lib/hydra_wrapper.rb b/lib/hydra_wrapper.rb index 7461d792a..d9d692113 100644 --- a/lib/hydra_wrapper.rb +++ b/lib/hydra_wrapper.rb @@ -86,10 +86,6 @@ class HydraWrapper # Save the reference to the pod to the database if not already present Pod.find_or_create_by(url: response.effective_url) - if redirecting_to_https? response - Person.url_batch_update people_for_receive_url, response.headers_hash['Location'] - end - unless response.success? logger.warn "event=http_multi_fail sender_id=#{@user.id} url=#{response.effective_url} " \ "return_code=#{response.return_code} response_code=#{response.response_code}" @@ -101,10 +97,4 @@ class HydraWrapper end end end - - # @return [Boolean] - def redirecting_to_https? response - response.code >= 300 && response.code < 400 && - response.headers_hash['Location'] == response.request.url.sub('http://', 'https://') - end end diff --git a/spec/controllers/admin/pods_controller_spec.rb b/spec/controllers/admin/pods_controller_spec.rb index 621836916..b5741b1ba 100644 --- a/spec/controllers/admin/pods_controller_spec.rb +++ b/spec/controllers/admin/pods_controller_spec.rb @@ -25,10 +25,11 @@ describe Admin::PodsController, type: :controller do end it "returns the json data" do - @pods = (0..2).map { FactoryGirl.create(:pod).reload } # normalize timestamps + 3.times { FactoryGirl.create(:pod) } + get :index, format: :json - expect(response.body).to eql(PodPresenter.as_collection(@pods).to_json) + expect(response.body).to eql(PodPresenter.as_collection(Pod.all).to_json) end end diff --git a/spec/factories.rb b/spec/factories.rb index a7a5a8dca..d1a5744b9 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -31,7 +31,7 @@ FactoryGirl.define do factory(:person, aliases: %i(author)) do sequence(:diaspora_handle) {|n| "bob-person-#{n}#{r_str}@example.net" } - url AppConfig.pod_uri.to_s + pod { Pod.find_or_create_by(url: "http://example.net") } serialized_public_key OpenSSL::PKey::RSA.generate(1024).public_key.export after(:build) do |person| unless person.profile.first_name.present? @@ -69,10 +69,11 @@ FactoryGirl.define do password_confirmation { |u| u.password } serialized_private_key OpenSSL::PKey::RSA.generate(1024).export after(:build) do |u| - u.person = FactoryGirl.build(:person, :profile => FactoryGirl.build(:profile), - :owner_id => u.id, - :serialized_public_key => u.encryption_key.public_key.export, - :diaspora_handle => "#{u.username}#{User.diaspora_id_host}") + u.person = FactoryGirl.build(:person, + profile: FactoryGirl.build(:profile), + pod: nil, + serialized_public_key: u.encryption_key.public_key.export, + diaspora_handle: "#{u.username}#{User.diaspora_id_host}") end after(:create) do |u| u.person.save diff --git a/spec/integration/federation/federation_helper.rb b/spec/integration/federation/federation_helper.rb index dd3609b71..4321e4f29 100644 --- a/spec/integration/federation/federation_helper.rb +++ b/spec/integration/federation/federation_helper.rb @@ -8,10 +8,13 @@ end def create_remote_user(pod) FactoryGirl.build(:user).tap do |user| - user.person = FactoryGirl.create(:person, - profile: FactoryGirl.build(:profile), - serialized_public_key: user.encryption_key.public_key.export, - diaspora_handle: "#{user.username}@#{pod}") + allow(user).to receive(:person).and_return( + FactoryGirl.create(:person, + profile: FactoryGirl.build(:profile), + serialized_public_key: user.encryption_key.public_key.export, + pod: Pod.find_or_create_by(url: "http://#{pod}"), + diaspora_handle: "#{user.username}@#{pod}") + ) allow(DiasporaFederation.callbacks).to receive(:trigger) .with(:fetch_private_key_by_diaspora_id, user.diaspora_handle) { user.encryption_key diff --git a/spec/lib/connection_tester_spec.rb b/spec/lib/connection_tester_spec.rb index 737003a80..f6cce045b 100644 --- a/spec/lib/connection_tester_spec.rb +++ b/spec/lib/connection_tester_spec.rb @@ -2,6 +2,10 @@ require "spec_helper" describe ConnectionTester do + let(:url) { "https://pod.example.com" } + let(:result) { ConnectionTester::Result.new } + let(:tester) { ConnectionTester.new(url, result) } + describe "::check" do it "takes a http url and returns a result object" do res = ConnectionTester.check("https://pod.example.com") @@ -29,102 +33,108 @@ describe ConnectionTester do end describe "#resolve" do - before do - @result = ConnectionTester::Result.new - @dns = instance_double("Resolv::DNS") - allow(@dns).to receive(:close).once - end - it "resolves the IP address" do - tester = ConnectionTester.new("https://pod.example.com", @result) - expect(tester).to receive(:with_dns_resolver).and_yield(@dns) - expect(@dns).to receive(:getaddress).and_return("192.168.1.2") + expect(IPSocket).to receive(:getaddress).with("pod.example.com").and_return("192.168.1.2") tester.resolve - expect(@result.ip).to eq("192.168.1.2") + expect(result.ip).to eq("192.168.1.2") + end + + it "raises DNSFailure if host is unknown" do + expect(IPSocket).to receive(:getaddress).with("pod.example.com").and_raise(SocketError.new("Error!")) + + expect { tester.resolve }.to raise_error(ConnectionTester::DNSFailure, "'pod.example.com' - Error!") end end describe "#request" do - before do - @url = "https://pod.example.com" - @stub = - @result = ConnectionTester::Result.new - @tester = ConnectionTester.new(@url, @result) - end + it "performs a successful GET request on '/' and '/.well-known/host-meta'" do + stub_request(:get, url).to_return(status: 200, body: "Hello World!") + stub_request(:get, "#{url}/.well-known/host-meta").to_return(status: 200, body: "host-meta") - it "performs a successful GET request on '/'" do - stub_request(:get, @url).to_return(status: 200, body: "Hello World!") - - @tester.request - expect(@result.rt).to be > -1 - expect(@result.reachable).to be_truthy - expect(@result.ssl_status).to be_truthy + tester.request + expect(result.rt).to be > -1 + expect(result.reachable).to be_truthy + expect(result.ssl).to be_truthy end it "receives a 'normal' 301 redirect" do - stub_request(:get, @url).to_return(status: 301, headers: {"Location" => "#{@url}/redirect"}) - stub_request(:get, "#{@url}/redirect").to_return(status: 200, body: "Hello World!") + stub_request(:get, url).to_return(status: 301, headers: {"Location" => "#{url}/redirect"}) + stub_request(:get, "#{url}/redirect").to_return(status: 200, body: "Hello World!") + stub_request(:get, "#{url}/.well-known/host-meta").to_return(status: 200, body: "host-meta") - @tester.request + tester.request + end + + it "updates ssl after https redirect" do + tester = ConnectionTester.new("http://pod.example.com/", result) + stub_request(:get, "http://pod.example.com/").to_return(status: 200, body: "Hello World!") + stub_request(:get, "http://pod.example.com/.well-known/host-meta") + .to_return(status: 301, headers: {"Location" => "#{url}/.well-known/host-meta"}) + stub_request(:get, "#{url}/.well-known/host-meta").to_return(status: 200, body: "host-meta") + + tester.request + expect(result.ssl).to be_truthy + end + + it "rejects other hostname after redirect redirect" do + stub_request(:get, url).to_return(status: 200, body: "Hello World!") + stub_request(:get, "#{url}/.well-known/host-meta") + .to_return(status: 301, headers: {"Location" => "https://example.com/.well-known/host-meta"}) + stub_request(:get, "https://example.com/.well-known/host-meta").to_return(status: 200, body: "host-meta") + + expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure) end it "receives too many 301 redirects" do - stub_request(:get, @url).to_return(status: 301, headers: {"Location" => "#{@url}/redirect"}) - stub_request(:get, "#{@url}/redirect").to_return(status: 301, headers: {"Location" => "#{@url}/redirect1"}) - stub_request(:get, "#{@url}/redirect1").to_return(status: 301, headers: {"Location" => "#{@url}/redirect2"}) - stub_request(:get, "#{@url}/redirect2").to_return(status: 301, headers: {"Location" => "#{@url}/redirect3"}) - stub_request(:get, "#{@url}/redirect3").to_return(status: 200, body: "Hello World!") + stub_request(:get, url).to_return(status: 301, headers: {"Location" => "#{url}/redirect"}) + stub_request(:get, "#{url}/redirect").to_return(status: 301, headers: {"Location" => "#{url}/redirect1"}) + stub_request(:get, "#{url}/redirect1").to_return(status: 301, headers: {"Location" => "#{url}/redirect2"}) + stub_request(:get, "#{url}/redirect2").to_return(status: 301, headers: {"Location" => "#{url}/redirect3"}) + stub_request(:get, "#{url}/redirect3").to_return(status: 200, body: "Hello World!") - expect { @tester.request }.to raise_error(ConnectionTester::HTTPFailure) + expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure) end it "receives a 404 not found" do - stub_request(:get, @url).to_return(status: 404, body: "Not Found!") - expect { @tester.request }.to raise_error(ConnectionTester::HTTPFailure) + stub_request(:get, url).to_return(status: 404, body: "Not Found!") + expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure) end it "cannot connect" do - stub_request(:get, @url).to_raise(Faraday::ConnectionFailed.new("Error!")) - expect { @tester.request }.to raise_error(ConnectionTester::NetFailure) + stub_request(:get, url).to_raise(Faraday::ConnectionFailed.new("Error!")) + expect { tester.request }.to raise_error(ConnectionTester::NetFailure) end it "encounters an invalid SSL setup" do - stub_request(:get, @url).to_raise(Faraday::SSLError.new("Error!")) - expect { @tester.request }.to raise_error(ConnectionTester::SSLFailure) + stub_request(:get, url).to_raise(Faraday::SSLError.new("Error!")) + expect { tester.request }.to raise_error(ConnectionTester::SSLFailure) end end describe "#nodeinfo" do - before do - @url = "https://diaspora.example.com" - @result = ConnectionTester::Result.new - @tester = ConnectionTester.new(@url, @result) - - @ni_wellknown = {links: [{rel: ConnectionTester::NODEINFO_SCHEMA, - href: "/nodeinfo"}]} - @ni_document = {software: {name: "diaspora", version: "a.b.c.d"}} - end - it "reads the version from the nodeinfo document" do - stub_request(:get, "#{@url}#{ConnectionTester::NODEINFO_FRAGMENT}") - .to_return(status: 200, body: JSON.generate(@ni_wellknown)) - stub_request(:get, "#{@url}/nodeinfo").to_return(status: 200, body: JSON.generate(@ni_document)) + ni_wellknown = {links: [{rel: ConnectionTester::NODEINFO_SCHEMA, href: "/nodeinfo"}]} + ni_document = {software: {name: "diaspora", version: "a.b.c.d"}} - @tester.nodeinfo - expect(@result.software_version).to eq("diaspora a.b.c.d") + stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}") + .to_return(status: 200, body: JSON.generate(ni_wellknown)) + stub_request(:get, "#{url}/nodeinfo").to_return(status: 200, body: JSON.generate(ni_document)) + + tester.nodeinfo + expect(result.software_version).to eq("diaspora a.b.c.d") end it "handles a missing nodeinfo document gracefully" do - stub_request(:get, "#{@url}#{ConnectionTester::NODEINFO_FRAGMENT}") + stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}") .to_return(status: 404, body: "Not Found") - expect { @tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure) + expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure) end it "handles a malformed document gracefully" do - stub_request(:get, "#{@url}#{ConnectionTester::NODEINFO_FRAGMENT}") + stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}") .to_return(status: 200, body: '{"json"::::"malformed"}') - expect { @tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure) + expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure) end end end diff --git a/spec/lib/diaspora/fetcher/public_spec.rb b/spec/lib/diaspora/fetcher/public_spec.rb index fd0a7a824..82eb8f2e7 100644 --- a/spec/lib/diaspora/fetcher/public_spec.rb +++ b/spec/lib/diaspora/fetcher/public_spec.rb @@ -13,9 +13,9 @@ describe Diaspora::Fetcher::Public do # the guid of the person is "7445f9a0a6c28ebb" @fixture = File.open(Rails.root.join('spec', 'fixtures', 'public_posts.json')).read @fetcher = Diaspora::Fetcher::Public.new - @person = FactoryGirl.create(:person, {:guid => "7445f9a0a6c28ebb", - :url => "https://remote-testpod.net", - :diaspora_handle => "testuser@remote-testpod.net"}) + @person = FactoryGirl.create(:person, guid: "7445f9a0a6c28ebb", + pod: Pod.find_or_create_by(url: "https://remote-testpod.net"), + diaspora_handle: "testuser@remote-testpod.net") stub_request(:get, /remote-testpod.net\/people\/.*\/stream/) .with(headers: { diff --git a/spec/lib/hydra_wrapper_spec.rb b/spec/lib/hydra_wrapper_spec.rb index 5a303052e..10eb94f54 100644 --- a/spec/lib/hydra_wrapper_spec.rb +++ b/spec/lib/hydra_wrapper_spec.rb @@ -74,37 +74,4 @@ describe HydraWrapper do end end - - describe '#redirecting_to_https?!' do - it 'does not execute unless response has a 3xx code' do - resp = double code: 200 - expect(@wrapper.send(:redirecting_to_https?, resp)).to be false - end - - it "returns true if just the protocol is different" do - host = "the-same.com/" - resp = double( - request: double(url: "http://#{host}"), - code: 302, - headers_hash: { - 'Location' => "https://#{host}" - } - ) - - expect(@wrapper.send(:redirecting_to_https?, resp)).to be true - end - - it "returns false if not just the protocol is different" do - host = "the-same.com/" - resp = double( - request: double(url: "http://#{host}"), - code: 302, - headers_hash: { - 'Location' => "https://not-the-same/" - } - ) - - expect(@wrapper.send(:redirecting_to_https?, resp)).to be false - end - end end diff --git a/spec/lib/postzord/receiver/public_spec.rb b/spec/lib/postzord/receiver/public_spec.rb index 090d67358..8519a1d71 100644 --- a/spec/lib/postzord/receiver/public_spec.rb +++ b/spec/lib/postzord/receiver/public_spec.rb @@ -19,6 +19,10 @@ describe Postzord::Receiver::Public do comment.save #bob signs his comment, and then sends it up 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.save bob.destroy comment.destroy expect{ diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index b06056903..54d9ec223 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -109,7 +109,7 @@ describe Person, :type => :model do describe "valid url" do context "https urls" do - let(:person) { FactoryGirl.build(:person, url: "https://example.com") } + let(:person) { FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com")) } it "should add trailing slash" do expect(person.url).to eq("https://example.com/") @@ -129,7 +129,9 @@ describe Person, :type => :model do end context "messed up urls" do - let(:person) { FactoryGirl.build(:person, url: "https://example.com/a/bit/messed/up") } + let(:person) { + FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com/a/bit/messed/up")) + } it "should return the correct url" do expect(person.url).to eq("https://example.com/") @@ -149,12 +151,12 @@ describe Person, :type => :model do end it "should allow ports in the url" do - person = FactoryGirl.build(:person, url: "https://example.com:3000/") + person = FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com:3000/")) expect(person.url).to eq("https://example.com:3000/") end it "should remove https port in the url" do - person = FactoryGirl.build(:person, url: "https://example.com:443/") + person = FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com:443/")) expect(person.url).to eq("https://example.com/") end end @@ -538,32 +540,6 @@ describe Person, :type => :model do end end - context 'updating urls' do - before do - @url = "http://new-url.com/" - end - - describe '.url_batch_update' do - it "calls #update_person_url given an array of users and a url" do - people = [double.as_null_object, double.as_null_object, double.as_null_object] - people.each do |person| - expect(person).to receive(:update_url).with(@url) - end - Person.url_batch_update(people, @url) - end - end - - describe '#update_url' do - it "updates a given person's url" do - expect { - alice.person.update_url(@url) - }.to change { - alice.person.reload.url - }.from(anything).to(@url) - end - end - end - describe '#lock_access!' do it 'sets the closed_account flag' do @person.lock_access! diff --git a/spec/models/pod_spec.rb b/spec/models/pod_spec.rb index a1e851d26..f612a6152 100644 --- a/spec/models/pod_spec.rb +++ b/spec/models/pod_spec.rb @@ -1,19 +1,45 @@ require "spec_helper" describe Pod, type: :model do - describe "::find_or_create_by" do + describe ".find_or_create_by" do it "takes a url, and makes one by host" do - pod = Pod.find_or_create_by(url: "https://joindiaspora.com/maxwell") - expect(pod.host).to eq("joindiaspora.com") + pod = Pod.find_or_create_by(url: "https://example.org/u/maxwell") + expect(pod.host).to eq("example.org") end - it "sets ssl boolean (side-effect)" do - pod = Pod.find_or_create_by(url: "https://joindiaspora.com/maxwell") + it "saves the port" do + pod = Pod.find_or_create_by(url: "https://example.org:3000/") + expect(pod.host).to eq("example.org") + expect(pod.port).to eq(3000) + end + + it "ignores default ports" do + pod = Pod.find_or_create_by(url: "https://example.org:443/") + expect(pod.host).to eq("example.org") + expect(pod.port).to be_nil + end + + it "sets ssl boolean" do + pod = Pod.find_or_create_by(url: "https://example.org/") + expect(pod.ssl).to be true + end + + it "updates ssl boolean if upgraded to https" do + pod = Pod.find_or_create_by(url: "http://joindiaspora.com/") + expect(pod.ssl).to be false + pod = Pod.find_or_create_by(url: "https://joindiaspora.com/") + 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/") + expect(pod.ssl).to be true + pod = Pod.find_or_create_by(url: "http://joindiaspora.com/") expect(pod.ssl).to be true end end - describe "::check_all!" do + describe ".check_all!" do before do @pods = (0..4).map do double("pod").tap do |pod| @@ -75,4 +101,11 @@ describe Pod, type: :model do end end end + + describe "#url_to" do + it "appends the path to the pod-url" do + pod = FactoryGirl.create(:pod) + expect(pod.url_to("/receive/public")).to eq("https://#{pod.host}/receive/public") + end + end end diff --git a/spec/models/user/querying_spec.rb b/spec/models/user/querying_spec.rb index 6adebe002..fd07454cd 100644 --- a/spec/models/user/querying_spec.rb +++ b/spec/models/user/querying_spec.rb @@ -200,7 +200,9 @@ describe User::Querying, :type => :model do connect_users(alice, @alices_aspect, remote_user, asp3) local_person = remote_user.person - local_person.owner_id = nil + local_person.diaspora_handle = "#{remote_user.username}@example.net" + local_person.owner = nil + local_person.pod = Pod.find_or_create_by(url: "http://example.net") local_person.save local_person.reload diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c9cede84e..e2ee22f3a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -111,21 +111,6 @@ describe User, :type => :model do end end - context 'callbacks' do - describe '#save_person!' do - it 'saves the corresponding user if it has changed' do - alice.person.url = "http://stuff.com" - expect_any_instance_of(Person).to receive(:save) - alice.save - end - - it 'does not save the corresponding user if it has not changed' do - expect_any_instance_of(Person).not_to receive(:save) - alice.save - end - end - end - describe 'hidden_shareables' do before do @sm = FactoryGirl.create(:status_message) @@ -1192,8 +1177,7 @@ describe User, :type => :model do invited_user.save(validate: false) closed_account = FactoryGirl.create(:user) - closed_account.person.closed_account = true - closed_account.save + closed_account.person.lock_access! end it "returns total_users excluding closed accounts & users without usernames" do diff --git a/spec/workers/http_multi_spec.rb b/spec/workers/http_multi_spec.rb index a4831614b..5e4202e47 100644 --- a/spec/workers/http_multi_spec.rb +++ b/spec/workers/http_multi_spec.rb @@ -108,21 +108,20 @@ describe Workers::HttpMulti do Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private" end - it 'updates http users who have moved to https' do + it "updates http users who have moved to https" do person = @people.first - person.update_url("http://remote.net/") response = Typhoeus::Response.new( - code: 301, - effective_url: 'https://foobar.com', + code: 301, + effective_url: "https://example.net", response_headers: "Location: #{person.receive_url.sub('http://', 'https://')}", - body: "", - time: 0.2 + body: "", + time: 0.2 ) Typhoeus.stub(person.receive_url).and_return response Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private" - expect(Person.find(person.id).url).to eq("https://remote.net/") + expect(Person.find(person.id).url).to eq("https://example.net/") end it 'only sends to users with valid RSA keys' do