From 726c035b7b7fac636982ac6e4c28e199d4c8cc8a Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 11 Oct 2017 15:46:48 +0200 Subject: style: more rubocop fixes --- bin/nickserver | 3 ++- test/functional/bin_test.rb | 2 +- test/functional/sample_test.rb | 4 ++-- test/integration/dispatcher_test.rb | 17 +++++++++----- test/integration/nickserver_test.rb | 44 ++++++++++++++++++++----------------- test/remote/hkp_source_test.rb | 31 ++++++++++++++------------ test/remote/nicknym_source_test.rb | 2 +- test/remote/wkd_source_test.rb | 4 ++-- 8 files changed, 60 insertions(+), 47 deletions(-) diff --git a/bin/nickserver b/bin/nickserver index 4706d8b..0890def 100755 --- a/bin/nickserver +++ b/bin/nickserver @@ -5,7 +5,8 @@ # def load_local_gem(dir_path = '../..') - base_directory = File.expand_path(dir_path, File.symlink?(__FILE__) ? File.readlink(__FILE__) : __FILE__) + actual_file = File.symlink?(__FILE__) ? File.readlink(__FILE__) : __FILE__ + base_directory = File.expand_path(dir_path, actual_file) unless $LOAD_PATH.include? "#{base_directory}/lib" if File.exist?("#{base_directory}/Gemfile.lock") ENV['BUNDLE_GEMFILE'] ||= "#{base_directory}/Gemfile" diff --git a/test/functional/bin_test.rb b/test/functional/bin_test.rb index 3a4e6c7..4d9f904 100644 --- a/test/functional/bin_test.rb +++ b/test/functional/bin_test.rb @@ -33,7 +33,7 @@ class BinTest < Minitest::Test def assert_command_runs(command) out = run_command command - assert ($CHILD_STATUS.exitstatus == 0), + assert $CHILD_STATUS.exitstatus.zero?, "failed to run 'nickserver #{command}':\n #{out}" end diff --git a/test/functional/sample_test.rb b/test/functional/sample_test.rb index 412555e..1bfe8b5 100644 --- a/test/functional/sample_test.rb +++ b/test/functional/sample_test.rb @@ -32,7 +32,7 @@ class SampleTest < FunctionalTest # platform/#8674 handle nonexisting domains def test_nicknym_handles_missing_domain - assert_lookup_status 404, 'postmaster@now-dont-you-dare-register-this-domain.coop' + assert_lookup_status 404, 'postmaster@dont-you-dare-register-this.coop' end def test_no_file_descriptors_leak @@ -63,7 +63,7 @@ class SampleTest < FunctionalTest def run_command(command) `#{command} 2>&1`.tap do |out| - assert ($CHILD_STATUS.exitstatus == 0), + assert $CHILD_STATUS.exitstatus.zero?, "failed to run '#{command}':\n #{out}" end end diff --git a/test/integration/dispatcher_test.rb b/test/integration/dispatcher_test.rb index 58aa972..1973e84 100644 --- a/test/integration/dispatcher_test.rb +++ b/test/integration/dispatcher_test.rb @@ -29,8 +29,9 @@ class Nickserver::DispatcherTest < Minitest::Test end def test_fingerprint_is_not_hex - handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX'] - assert_response error('Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX') + fingerprint = 'X36E738D69173C13Z709E44F2F455E2824D18DDX' + handle fingerprint: [fingerprint] + assert_response error("Fingerprint invalid: #{fingerprint}") end def test_missing_domain @@ -41,28 +42,32 @@ class Nickserver::DispatcherTest < Minitest::Test end def test_email_via_hkp - handle address: ['valid@email.tld'], headers: { 'Host' => 'http://nickserver.me' } + handle address: ['valid@email.tld'], + headers: { 'Host' => 'http://nickserver.me' } stub_nicknym_not_available hkp_source.expect :query, success, [Nickserver::EmailAddress] assert_response success end def test_email_via_hkp_nicknym_unreachable - handle address: ['valid@email.tld'], headers: { 'Host' => 'http://nickserver.me' } + handle address: ['valid@email.tld'], + headers: { 'Host' => 'http://nickserver.me' } stub_nicknym_raises hkp_source.expect :query, success, [Nickserver::EmailAddress] assert_response success end def test_email_not_found_hkp_nicknym_unreachable - handle address: ['valid@email.tld'], headers: { 'Host' => 'http://nickserver.me' } + handle address: ['valid@email.tld'], + headers: { 'Host' => 'http://nickserver.me' } stub_nicknym_raises hkp_source.expect :query, nil, [Nickserver::EmailAddress] assert_response http_connection_error end def test_email_via_nicknym - handle address: ['valid@email.tld'], headers: { 'Host' => 'http://nickserver.me' } + handle address: ['valid@email.tld'], + headers: { 'Host' => 'http://nickserver.me' } nicknym_source.expect :available_for?, true, [String] nicknym_source.expect :query, success, [Nickserver::EmailAddress] assert_response success diff --git a/test/integration/nickserver_test.rb b/test/integration/nickserver_test.rb index fb9b952..832a68c 100644 --- a/test/integration/nickserver_test.rb +++ b/test/integration/nickserver_test.rb @@ -19,13 +19,14 @@ require 'json' # (2) We actually start the Reelserver on 127.0.0.1 and talk to it via http. # In order to run the Reelserver properly this is a celluloid test. # -# (3) the "Host" header for requests to nickserver must be set (or Config.domain set) +# (3) the "Host" header for requests to nickserver must be set +# (or Config.domain set) # class NickserverTest < CelluloidTest include HttpStubHelper - def test_GET_key_by_email_address_served_via_SKS + def test_key_by_email_address_from_sks uid = 'cloudadmin@leap.se' key_id = 'E818C478D3141282F7590D29D041EB11B1647490' stub_nicknym_available_response 'leap.se', status: 404 @@ -34,51 +35,43 @@ class NickserverTest < CelluloidTest start do params = { query: { 'address' => uid } } - get(params) do |response| - assert_equal file_content(:leap_public_key), JSON.parse(response.to_s)['openpgp'] - end + assert_responds_to params, key: :leap_public_key end end - def test_GET_key_by_fingerprint_served_via_SKS + def test_key_by_fingerprint_from_sks fingerprint = 'E818C478D3141282F7590D29D041EB11B1647490' stub_sks_get_reponse(fingerprint, body: file_content(:leap_public_key)) start do params = { query: { 'fingerprint' => fingerprint } } - get(params) do |response| - assert_equal file_content(:leap_public_key), JSON.parse(response.to_s)['openpgp'] - end + assert_responds_to params, key: :leap_public_key end end - def test_GET_served_via_couch_not_found + def test_couch_user_not_found domain = 'example.org' uid = 'bananas@' + domain stub_couch_response(uid, status: 404) do start do params = { query: { 'address' => uid }, head: { 'Host' => domain } } - get(params) do |response| - assert_equal 404, response.code - end + assert_responds_to params, code: 404 end end end - def test_GET_served_via_couch_empty_results + def test_couch_empty_results domain = 'example.org' uid = 'stompy@' + domain stub_couch_response(uid, body: file_content(:empty_couchdb_result)) do start do params = { query: { 'address' => uid }, head: { host: domain } } - get(params) do |response| - assert_equal 404, response.code - end + assert_responds_to params, code: 404 end end end - def test_GET_served_via_couch_success + def test_couch_success_response domain = 'example.org' uid = 'blue@' + domain stub_couch_response(uid, body: file_content(:blue_couchdb_result)) do @@ -91,7 +84,7 @@ class NickserverTest < CelluloidTest end end - def test_GET_empty + def test_empty_get start do get({}) do |response| assert_equal "404 Not Found\n", response.to_s @@ -113,6 +106,16 @@ class NickserverTest < CelluloidTest server.terminate if server && server.alive? end + def assert_responds_to(params, key: nil, code: nil) + get(params) do |response| + assert_equal code, response.code if code + if key + assert_equal file_content(key), + JSON.parse(response.to_s)['openpgp'] + end + end + end + # # http GET requests to nickserver # @@ -130,7 +133,8 @@ class NickserverTest < CelluloidTest # # http request to nickserver # - # this works because http requests to 127.0.0.1 are not stubbed, but requests to other domains are. + # this works because http requests to 127.0.0.1 are not stubbed, but + # requests to other domains are. # def request(method, options = {}) response = HTTP diff --git a/test/remote/hkp_source_test.rb b/test/remote/hkp_source_test.rb index 8232dce..469941b 100644 --- a/test/remote/hkp_source_test.rb +++ b/test/remote/hkp_source_test.rb @@ -7,32 +7,27 @@ class RemoteHkpSourceTest < CelluloidTest include HttpAdapterHelper def test_key_info - uid = 'elijah@riseup.net' - assert_key_info_for_uid uid do |keys| - assert_equal 1, keys.size - assert keys.first.keyid =~ /00440025$/ - end + assert_key_found 'elijah@riseup.net', /00440025$/ end def test_tls_validation - hkp_url = 'https://keys.mayfirst.org/pks/lookup' - ca_file = file_path('mayfirst-ca.pem') - config.stub(:hkp_url, hkp_url) do - # config.stub(:hkp_ca_file, file_path('autistici-ca.pem')) do config.stub(:hkp_ca_file, ca_file) do assert File.exist?(Nickserver::Config.hkp_ca_file) - uid = 'elijah@riseup.net' - assert_key_info_for_uid uid do |keys| - assert_equal 1, keys.size - assert keys.first.keyid =~ /00440025$/ - end + assert_key_found 'elijah@riseup.net', /00440025$/ end end end protected + def assert_key_found(uid, fingerprint_regexp) + assert_key_info_for_uid uid do |keys| + assert_equal 1, keys.size + assert keys.first.keyid =~ fingerprint_regexp + end + end + def assert_key_info_for_uid(uid) status, keys = source.search uid assert_equal 200, status @@ -44,4 +39,12 @@ class RemoteHkpSourceTest < CelluloidTest def source Nickserver::Hkp::Source.new adapter end + + def hkp_url + 'https://keys.mayfirst.org/pks/lookup' + end + + def ca_file + file_path('mayfirst-ca.pem') + end end diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb index 6fff1f6..ef74653 100644 --- a/test/remote/nicknym_source_test.rb +++ b/test/remote/nicknym_source_test.rb @@ -63,6 +63,6 @@ class RemoteNicknymSourceTest < CelluloidTest end def email_without_key - Nickserver::EmailAddress.new('pleaseneverusethisemailweuseittotest@mail.bitmask.net') + Nickserver::EmailAddress.new('neverusethisweuseittotest@mail.bitmask.net') end end diff --git a/test/remote/wkd_source_test.rb b/test/remote/wkd_source_test.rb index 1ed7ea5..46a6bbb 100644 --- a/test/remote/wkd_source_test.rb +++ b/test/remote/wkd_source_test.rb @@ -27,8 +27,8 @@ class RemoteWkdSourceTest < CelluloidTest def assert_pgp_key_in(response) json = JSON.parse response.content - assert_equal email_with_key.to_s, json["address"] - refute_empty json["openpgp"] + assert_equal email_with_key.to_s, json['address'] + refute_empty json['openpgp'] assert_equal file_content('dewey.pgp.asc'), json['openpgp'] end -- cgit v1.2.3 From 61ebd2908da912ee269dbeb71b5bddc2b50efbb2 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 11 Oct 2017 15:57:39 +0200 Subject: docs: update README.md * We are using Celluloid instead of EventMachine now * The json responses are currently not signed * we actually pick a key rather than sending all available * There are only two install options listed. We were claiming there are three. --- README.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 50460c4..01eaa07 100644 --- a/README.md +++ b/README.md @@ -17,17 +17,18 @@ About nickserver: * Written in Ruby 2.1, licensed GPLv3 * Lightweight and scalable (high concurrency, reasonable latency) -* Uses asynchronous network IO for both server and client connections (via EventMachine) +* Uses asynchronous network IO for both server and client connections + (via Celluloid IO) API ================================== -You query the nickserver via HTTP. The API is very minimal: +You send requests to nickserver via HTTP. The API is very minimal: curl -X POST -d address=alice@domain.org https://nicknym.domain.org:6425 -The response consists of a signed JSON document with fields for the available -public keys corresponding to the address. +The response consists of a JSON document with fields for the selected +public key corresponding to the address. For more details, see https://leap.se/nicknym @@ -40,8 +41,6 @@ Requirements Installation ================================== -You have three fine options for installing nickserver: - Install prerequisites $ sudo apt-get install ruby-dev libssl-dev -- cgit v1.2.3 From ec996134a1f23ee36aff9d3ad2c800af71623207 Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 3 Nov 2017 14:33:51 +0100 Subject: fix: no expiration date means not outdated We were using Time.at(expirationdate) even if it was nil which led to using the Time.at(0). Instead an unset expirationdate is meant to not expire the key at all. Our tests did not catch this because the assertions were in blocks that did not get run at all. (at least in the HKP integration test). --- lib/nickserver/hkp/key_info.rb | 2 +- test/integration/hkp_test.rb | 34 ++++++++++++---------------------- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/lib/nickserver/hkp/key_info.rb b/lib/nickserver/hkp/key_info.rb index e1a9500..c1b1ad3 100644 --- a/lib/nickserver/hkp/key_info.rb +++ b/lib/nickserver/hkp/key_info.rb @@ -49,7 +49,7 @@ module Nickserver::Hkp def expirationdate expires = properties[4] - Time.at(expires.to_i) + expires && Time.at(expires.to_i) end def flags diff --git a/test/integration/hkp_test.rb b/test/integration/hkp_test.rb index c12588c..f6675e9 100644 --- a/test/integration/hkp_test.rb +++ b/test/integration/hkp_test.rb @@ -33,7 +33,7 @@ class HkpTest < Minitest::Test stubbing_http do uid = 'leaping_lemur@leap.se' stub_sks_vindex_reponse(uid, status: 404) - assert_response_status_for_uid uid, 404 + assert_nil response_for_uid(uid) end end @@ -41,7 +41,7 @@ class HkpTest < Minitest::Test stubbing_http do uid = 'leaping_lemur@leap.se' stub_sks_vindex_reponse(uid, status: 200) - assert_response_status_for_uid uid, 404 + assert_nil response_for_uid(uid) end end @@ -51,10 +51,9 @@ class HkpTest < Minitest::Test stubbing_http do stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result)) stub_sks_get_reponse(key_id, body: file_content(:leap_public_key)) - assert_response_for_uid(uid) do |response| - content = JSON.parse response.content - assert_equal file_content(:leap_public_key), content['openpgp'] - end + response = response_for_uid(uid) + content = JSON.parse response.content + assert_equal file_content(:leap_public_key), content['openpgp'] end end @@ -65,7 +64,7 @@ class HkpTest < Minitest::Test stubbing_http do stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result)) stub_sks_get_reponse(key_id, status: 404) - assert_response_status_for_uid uid, 404 + assert_equal 404, response_for_uid(uid).status end end @@ -74,29 +73,20 @@ class HkpTest < Minitest::Test stubbing_http do stub_sks_vindex_reponse(uid, body: file_content(:short_key_vindex_result)) - assert_response_status_for_uid uid, 500 + assert_equal 500, response_for_uid(uid).status end end protected - def assert_response_status_for_uid(uid, status) - assert_response_for_uid(uid) do |response| - assert_equal status, response.status - end - end - - def assert_response_for_uid(uid) - Nickserver::Hkp::Source.new(adapter).query uid do |response| - yield response - end + def response_for_uid(uid) + Nickserver::Hkp::Source.new(adapter).query uid end def assert_key_info_for_uid(uid) - Nickserver::Hkp::Source.new(adapter).search uid do |status, keys| - assert_equal 200, status - yield keys - end + status, keys = Nickserver::Hkp::Source.new(adapter).search uid + assert_equal 200, status + yield keys end def fetch_key_info(body_source, uid, &block) -- cgit v1.2.3 From 85a567286cf61a3a8193c339dd0967116d79299c Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 4 Nov 2017 08:30:08 +0100 Subject: refactor: turn Hkp::Response into KeyResponse We now also use it from wkd and it seems like a generally useful kind of response. --- lib/nickserver/hkp/response.rb | 16 ---------------- lib/nickserver/hkp/source.rb | 4 ++-- lib/nickserver/key_response.rb | 16 ++++++++++++++++ lib/nickserver/wkd/source.rb | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) delete mode 100644 lib/nickserver/hkp/response.rb create mode 100644 lib/nickserver/key_response.rb diff --git a/lib/nickserver/hkp/response.rb b/lib/nickserver/hkp/response.rb deleted file mode 100644 index 2cc69d3..0000000 --- a/lib/nickserver/hkp/response.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Nickserver::Hkp - class Response - attr_reader :status, :content - - def initialize(uid, key) - @content = format_response(address: uid, openpgp: key) - @status = 200 - end - - protected - - def format_response(map) - map.to_json - end - end -end diff --git a/lib/nickserver/hkp/source.rb b/lib/nickserver/hkp/source.rb index d7c86a3..fe3c4a5 100644 --- a/lib/nickserver/hkp/source.rb +++ b/lib/nickserver/hkp/source.rb @@ -1,6 +1,6 @@ require 'nickserver/source' require 'nickserver/response' -require 'nickserver/hkp/response' +require 'nickserver/key_response' require 'nickserver/hkp/client' require 'nickserver/hkp/parse_key_info' require 'nickserver/hkp/key_info' @@ -30,7 +30,7 @@ module Nickserver::Hkp def get_key_by_fingerprint(fingerprint, nick = nil) status, response = client.get_key_by_fingerprint fingerprint if status == 200 - Response.new nick, response + Nickserver::KeyResponse.new nick, response else Nickserver::Response.new status, 'HKP Request failed' end diff --git a/lib/nickserver/key_response.rb b/lib/nickserver/key_response.rb new file mode 100644 index 0000000..438dfc3 --- /dev/null +++ b/lib/nickserver/key_response.rb @@ -0,0 +1,16 @@ +module Nickserver + class KeyResponse + attr_reader :status, :content + + def initialize(uid, key) + @content = format_response(address: uid, openpgp: key) + @status = 200 + end + + protected + + def format_response(map) + map.to_json + end + end +end diff --git a/lib/nickserver/wkd/source.rb b/lib/nickserver/wkd/source.rb index 43f0b2e..bf30c94 100644 --- a/lib/nickserver/wkd/source.rb +++ b/lib/nickserver/wkd/source.rb @@ -1,7 +1,7 @@ require 'nickserver/source' require 'nickserver/response' require 'nickserver/wkd/url' -require 'nickserver/hkp/response' +require 'nickserver/key_response' module Nickserver::Wkd # Query the web key directory for a given email address @@ -10,7 +10,7 @@ module Nickserver::Wkd url = Url.new(email) status, blob = adapter.get url if status == 200 - Nickserver::Hkp::Response.new(email.to_s, armor_key(blob)) + Nickserver::KeyResponse.new(email.to_s, armor_key(blob)) end end -- cgit v1.2.3