From 0e9c41a286b49b5ce52abcf0e014668d0167bbae Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 7 Jul 2014 10:05:37 +0200 Subject: store expiry with cert fingerprints We used to store the creation date but this way it's easier to query for non expired certs --- app/models/client_certificate.rb | 6 +++++- app/models/identity.rb | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/client_certificate.rb b/app/models/client_certificate.rb index d5bb1e0..6b57985 100644 --- a/app/models/client_certificate.rb +++ b/app/models/client_certificate.rb @@ -25,7 +25,7 @@ class ClientCertificate # set expiration cert.not_before = last_month - cert.not_after = months_from_yesterday(APP_CONFIG[:client_cert_lifespan]) + cert.not_after = expiry # generate key cert.serial_number.number = cert_serial_number @@ -47,6 +47,10 @@ class ClientCertificate OpenSSL::Digest::SHA1.hexdigest(openssl_cert.to_der).scan(/../).join(':') end + def expiry + @expiry ||= months_from_yesterday(APP_CONFIG[:client_cert_lifespan]) + end + private def openssl_cert diff --git a/app/models/identity.rb b/app/models/identity.rb index eb67b1b..1d69437 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -146,9 +146,9 @@ class Identity < CouchRest::Model::Base end def register_cert(cert) - today = DateTime.now.to_date.to_s + expiry = cert.expiry.to_data.to_s write_attribute 'cert_fingerprints', - cert_fingerprints.merge(cert.fingerprint => today) + cert_fingerprints.merge(cert.fingerprint => expiry) end # for LoginFormatValidation -- cgit v1.2.3 From cc1666d9832415058bf0b22bb5912e432261af4f Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 7 Jul 2014 10:12:53 +0200 Subject: Identity view cert_fingerprints_by_expiry Also move complex identity views into js designs. Includes test. Here's how you would query it from outside rails: ``` $ curl 'localhost:5984/identities/_design/Identity/_view/cert_fingerprints_by_expiry?startkey="2014-07-05"' {"total_rows":4,"offset":1,"rows":[ {"id":"6c9091d4f13eaeaa6062c9d0528fd34d","key":"2014-07-05","value":"fingerprint"}, {"id":"6f3aa93828b4f6978d551f2623b9d103","key":"2014-07-05","value":"fingerprint"}, {"id":"b6cafacfa65042679691cd5065fb19e3","key":"2014-07-07","value":"fp"} ]} ``` Note that the expiry will be used as the key. So you should use the current data (or yesterday) as the startkey to get all fingerprints that have not expired yet. The fingerprint itself is in the value. No need to include docs. --- .../identity/cert_fingerprints_by_expiry.js | 12 ++++++++++ app/designs/identity/disabled.js | 8 +++++++ app/designs/identity/pgp_key_by_email.js | 8 +++++++ app/models/identity.rb | 27 +++------------------- lib/extensions/couchrest.rb | 12 +++++++--- test/unit/identity_test.rb | 16 +++++++++++++ 6 files changed, 56 insertions(+), 27 deletions(-) create mode 100644 app/designs/identity/cert_fingerprints_by_expiry.js create mode 100644 app/designs/identity/disabled.js create mode 100644 app/designs/identity/pgp_key_by_email.js diff --git a/app/designs/identity/cert_fingerprints_by_expiry.js b/app/designs/identity/cert_fingerprints_by_expiry.js new file mode 100644 index 0000000..995219b --- /dev/null +++ b/app/designs/identity/cert_fingerprints_by_expiry.js @@ -0,0 +1,12 @@ +function(doc) { + if (doc.type != 'Identity') { + return; + } + if (typeof doc.cert_fingerprints === "object") { + for (fp in doc.cert_fingerprints) { + if (doc.cert_fingerprints.hasOwnProperty(fp)) { + emit(doc.cert_fingerprints[fp], fp); + } + } + } +} diff --git a/app/designs/identity/disabled.js b/app/designs/identity/disabled.js new file mode 100644 index 0000000..5509575 --- /dev/null +++ b/app/designs/identity/disabled.js @@ -0,0 +1,8 @@ +function(doc) { + if (doc.type != 'Identity') { + return; + } + if (typeof doc.user_id === "undefined") { + emit(doc._id, 1); + } +} diff --git a/app/designs/identity/pgp_key_by_email.js b/app/designs/identity/pgp_key_by_email.js new file mode 100644 index 0000000..f783908 --- /dev/null +++ b/app/designs/identity/pgp_key_by_email.js @@ -0,0 +1,8 @@ +function(doc) { + if (doc.type != 'Identity') { + return; + } + if (typeof doc.keys === "object") { + emit(doc.address, doc.keys["pgp"]); + } +} diff --git a/app/models/identity.rb b/app/models/identity.rb index 1d69437..9dc9c7a 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -18,32 +18,11 @@ class Identity < CouchRest::Model::Base validate :destination_email design do + own_path = Pathname.new(File.dirname(__FILE__)) + load_views(own_path.join('..', 'designs', 'identity'), nil) view :by_user_id view :by_address_and_destination view :by_address - view :pgp_key_by_email, - map: <<-EOJS - function(doc) { - if (doc.type != 'Identity') { - return; - } - if (typeof doc.keys === "object") { - emit(doc.address, doc.keys["pgp"]); - } - } - EOJS - view :disabled, - map: <<-EOJS - function(doc) { - if (doc.type != 'Identity') { - return; - } - if (typeof doc.user_id === "undefined") { - emit(doc._id, 1); - } - } - EOJS - end def self.address_starts_with(query) @@ -146,7 +125,7 @@ class Identity < CouchRest::Model::Base end def register_cert(cert) - expiry = cert.expiry.to_data.to_s + expiry = cert.expiry.to_date.to_s write_attribute 'cert_fingerprints', cert_fingerprints.merge(cert.fingerprint => expiry) end diff --git a/lib/extensions/couchrest.rb b/lib/extensions/couchrest.rb index df83c9f..290cd32 100644 --- a/lib/extensions/couchrest.rb +++ b/lib/extensions/couchrest.rb @@ -15,13 +15,19 @@ module CouchRest end class DesignMapper - def load_views(dir) + DEFAULT_REDUCE = <<-EOJS + function(key, values, rereduce) { + return sum(values); + } + EOJS + def load_views(dir, reduce=DEFAULT_REDUCE) Dir.glob("#{dir}/*.js") do |js| name = File.basename(js, '.js') file = File.open(js, 'r') + reduce = view name.to_sym, - :map => file.read, - :reduce => "function(key, values, rereduce) { return sum(values); }" + map: file.read, + reduce: reduce end end end diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb index 49b2075..933b0ff 100644 --- a/test/unit/identity_test.rb +++ b/test/unit/identity_test.rb @@ -136,6 +136,22 @@ class IdentityTest < ActiveSupport::TestCase assert_equal 0, Identity.disabled.count end + test "store cert fingerprint" do + begin + id = Identity.for(@user) + cert = stub expiry: Time.now, fingerprint: "fp" + id.register_cert cert + id.save + entry = {cert.fingerprint => cert.expiry.to_date.to_s} + assert_equal entry, id.cert_fingerprints + row = Identity.cert_fingerprints_by_expiry.descending.rows.first + assert_equal row['key'], cert.expiry.to_date.to_s + assert_equal row['value'], cert.fingerprint + ensure + id.reload.destroy if id && id.persisted? + end + end + def alias_name @alias_name ||= Faker::Internet.user_name end -- cgit v1.2.3 From bdd5060ccc13951524c171e2d3b81eeddec1625d Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 9 Jul 2014 22:53:05 +0200 Subject: fix tests and simplify time calculations --- app/models/client_certificate.rb | 30 ++++++++---------------- test/functional/v1/smtp_certs_controller_test.rb | 3 ++- test/integration/api/smtp_cert_test.rb | 6 +++-- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/app/models/client_certificate.rb b/app/models/client_certificate.rb index 6b57985..815801e 100644 --- a/app/models/client_certificate.rb +++ b/app/models/client_certificate.rb @@ -48,7 +48,7 @@ class ClientCertificate end def expiry - @expiry ||= months_from_yesterday(APP_CONFIG[:client_cert_lifespan]) + @expiry ||= lifespan.months.from_now.utc.at_midnight end private @@ -103,28 +103,18 @@ class ClientCertificate } end - ## - ## TIME HELPERS - ## - ## note: we use 'yesterday' instead of 'today', because times are in UTC, and some people on the planet - ## are behind UTC. - ## - - def yesterday - t = Time.now - 24*60*60 - Time.utc t.year, t.month, t.day - end + # + # TIME HELPERS + # + # We normalize timestamps at utc and midnight + # to reduce the fingerprinting possibilities. + # def last_month - t = Time.now - 24*60*60*30 - Time.utc t.year, t.month, t.day + 1.month.ago.utc.at_midnight end - def months_from_yesterday(num) - t = yesterday - date = Date.new t.year, t.month, t.day - date = date >> num # >> is months in the future operator - Time.utc date.year, date.month, date.day + def lifespan + APP_CONFIG[:client_cert_lifespan] end - end diff --git a/test/functional/v1/smtp_certs_controller_test.rb b/test/functional/v1/smtp_certs_controller_test.rb index 9281ae6..3427e2d 100644 --- a/test/functional/v1/smtp_certs_controller_test.rb +++ b/test/functional/v1/smtp_certs_controller_test.rb @@ -27,7 +27,8 @@ class V1::SmtpCertsControllerTest < ActionController::TestCase protected def expect_cert(prefix) - cert = stub :to_s => "#{prefix.downcase} cert" + cert = stub to_s: "#{prefix.downcase} cert", + expiry: 1.month.from_now.utc.at_midnight ClientCertificate.expects(:new). with(:prefix => prefix). returns(cert) diff --git a/test/integration/api/smtp_cert_test.rb b/test/integration/api/smtp_cert_test.rb index f72362d..7697e44 100644 --- a/test/integration/api/smtp_cert_test.rb +++ b/test/integration/api/smtp_cert_test.rb @@ -33,8 +33,10 @@ class SmtpCertTest < ApiIntegrationTest assert_text_response cert = OpenSSL::X509::Certificate.new(get_response.body) fingerprint = OpenSSL::Digest::SHA1.hexdigest(cert.to_der).scan(/../).join(':') - today = DateTime.now.to_date.to_s - assert_equal({fingerprint => today}, @user.reload.identity.cert_fingerprints) + expiry = APP_CONFIG[:client_cert_lifespan].months.from_now.utc.midnight + expiry_string = expiry.to_date.to_s + fingerprints = {fingerprint => expiry_string} + assert_equal fingerprints, @user.reload.identity.cert_fingerprints end test "fetching smtp certs requires email account" do -- cgit v1.2.3 From 0666c06fd7e66c783345a9810ace319a1cd9321f Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 10 Jul 2014 10:56:17 +0200 Subject: minor: fix typo in load_views It removed most of the reduce functions... really not what we wanted --- engines/support/test/functional/tickets_list_test.rb | 2 +- lib/extensions/couchrest.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/engines/support/test/functional/tickets_list_test.rb b/engines/support/test/functional/tickets_list_test.rb index 4c4cdef..ab76f5f 100644 --- a/engines/support/test/functional/tickets_list_test.rb +++ b/engines/support/test/functional/tickets_list_test.rb @@ -108,7 +108,7 @@ class TicketsListTest < ActionController::TestCase assert_equal [closed_ticket], assigns(:all_tickets).all end - test "list all tickets inludes closed + open" do + test "list all tickets" do login open_ticket = FactoryGirl.create :ticket_with_comment, created_by: @current_user.id diff --git a/lib/extensions/couchrest.rb b/lib/extensions/couchrest.rb index 290cd32..883073f 100644 --- a/lib/extensions/couchrest.rb +++ b/lib/extensions/couchrest.rb @@ -24,7 +24,6 @@ module CouchRest Dir.glob("#{dir}/*.js") do |js| name = File.basename(js, '.js') file = File.open(js, 'r') - reduce = view name.to_sym, map: file.read, reduce: reduce -- cgit v1.2.3 From 3bbfdf29bbc40432c21e34fe84060cf9ae70273f Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 12 Jul 2014 09:55:26 +0200 Subject: allow querying for the expiry of a particular fingerprint --- app/designs/identity/cert_expiry_by_fingerprint.js | 12 ++ test/unit/identity_test.rb | 183 +++++++++++---------- 2 files changed, 112 insertions(+), 83 deletions(-) create mode 100644 app/designs/identity/cert_expiry_by_fingerprint.js diff --git a/app/designs/identity/cert_expiry_by_fingerprint.js b/app/designs/identity/cert_expiry_by_fingerprint.js new file mode 100644 index 0000000..0636da5 --- /dev/null +++ b/app/designs/identity/cert_expiry_by_fingerprint.js @@ -0,0 +1,12 @@ +function(doc) { + if (doc.type != 'Identity') { + return; + } + if (typeof doc.cert_fingerprints === "object") { + for (fp in doc.cert_fingerprints) { + if (doc.cert_fingerprints.hasOwnProperty(fp)) { + emit(fp, doc.cert_fingerprints[fp]); + } + } + } +} diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb index 933b0ff..cb0f6bd 100644 --- a/test/unit/identity_test.rb +++ b/test/unit/identity_test.rb @@ -7,149 +7,161 @@ class IdentityTest < ActiveSupport::TestCase @user = find_record :user end - test "blank identity does not crash on valid?" do - id = Identity.new - assert !id.valid? + teardown do + if @id && @id.persisted? + id = Identity.find(@id.id) + id.destroy if id.present? + end + end + + test "blank @identity does not crash on valid?" do + @id = Identity.new + assert !@id.valid? end - test "enabled identity requires destination" do - id = Identity.new user: @user, address: @user.email_address - assert !id.valid? - assert_equal ["can't be blank"], id.errors[:destination] + test "enabled @identity requires destination" do + @id = Identity.new user: @user, address: @user.email_address + assert !@id.valid? + assert_equal ["can't be blank"], @id.errors[:destination] end - test "disabled identity requires no destination" do - id = Identity.new address: @user.email_address - assert id.valid? + test "disabled @identity requires no destination" do + @id = Identity.new address: @user.email_address + assert @id.valid? end - test "initial identity for a user" do - id = Identity.for(@user) - assert_equal @user.email_address, id.address - assert_equal @user.email_address, id.destination - assert_equal @user, id.user + test "initial @identity for a user" do + @id = Identity.for(@user) + assert_equal @user.email_address, @id.address + assert_equal @user.email_address, @id.destination + assert_equal @user, @id.user end test "add alias" do - id = Identity.for @user, address: alias_name - assert_equal LocalEmail.new(alias_name), id.address - assert_equal @user.email_address, id.destination - assert_equal @user, id.user + @id = Identity.for @user, address: alias_name + assert_equal LocalEmail.new(alias_name), @id.address + assert_equal @user.email_address, @id.destination + assert_equal @user, @id.user end test "add forward" do - id = Identity.for @user, destination: forward_address - assert_equal @user.email_address, id.address - assert_equal Email.new(forward_address), id.destination - assert_equal @user, id.user + @id = Identity.for @user, destination: forward_address + assert_equal @user.email_address, @id.address + assert_equal Email.new(forward_address), @id.destination + assert_equal @user, @id.user end test "forward alias" do - id = Identity.for @user, address: alias_name, destination: forward_address - assert_equal LocalEmail.new(alias_name), id.address - assert_equal Email.new(forward_address), id.destination - assert_equal @user, id.user + @id = Identity.for @user, address: alias_name, destination: forward_address + assert_equal LocalEmail.new(alias_name), @id.address + assert_equal Email.new(forward_address), @id.destination + assert_equal @user, @id.user end test "prevents duplicates" do - id = Identity.create_for @user, address: alias_name, destination: forward_address + @id = Identity.create_for @user, address: alias_name, destination: forward_address dup = Identity.build_for @user, address: alias_name, destination: forward_address assert !dup.valid? assert_equal ["has already been taken"], dup.errors[:destination] - id.destroy end test "validates availability" do other_user = find_record :user - id = Identity.create_for @user, address: alias_name, destination: forward_address + @id = Identity.create_for @user, address: alias_name, destination: forward_address taken = Identity.build_for other_user, address: alias_name assert !taken.valid? assert_equal ["has already been taken"], taken.errors[:address] - id.destroy end test "setting and getting pgp key" do - id = Identity.for(@user) - id.set_key(:pgp, pgp_key_string) - assert_equal pgp_key_string, id.keys[:pgp] + @id = Identity.for(@user) + @id.set_key(:pgp, pgp_key_string) + assert_equal pgp_key_string, @id.keys[:pgp] end test "querying pgp key via couch" do - id = Identity.for(@user) - id.set_key(:pgp, pgp_key_string) - id.save - view = Identity.pgp_key_by_email.key(id.address) + @id = Identity.for(@user) + @id.set_key(:pgp, pgp_key_string) + @id.save + view = Identity.pgp_key_by_email.key(@id.address) assert_equal 1, view.rows.count assert result = view.rows.first - assert_equal id.address, result["key"] - assert_equal id.keys[:pgp], result["value"] - id.destroy + assert_equal @id.address, result["key"] + assert_equal @id.keys[:pgp], result["value"] end - test "fail to add non-local email address as identity address" do - id = Identity.for @user, address: forward_address - assert !id.valid? - assert_match /needs to end in/, id.errors[:address].first + test "fail to add non-local email address as @identity address" do + @id = Identity.for @user, address: forward_address + assert !@id.valid? + assert_match /needs to end in/, @id.errors[:address].first end test "alias must meet same conditions as login" do - id = Identity.create_for @user, address: alias_name.capitalize - assert !id.valid? + @id = Identity.create_for @user, address: alias_name.capitalize + assert !@id.valid? #hacky way to do this, but okay for now: - assert id.errors.messages.flatten(2).include? "Must begin with a lowercase letter" - assert id.errors.messages.flatten(2).include? "Only lowercase letters, digits, . - and _ allowed." + assert @id.errors.messages.flatten(2).include? "Must begin with a lowercase letter" + assert @id.errors.messages.flatten(2).include? "Only lowercase letters, digits, . - and _ allowed." end test "destination must be valid email address" do - id = Identity.create_for @user, address: @user.email_address, destination: 'ASKJDLFJD' - assert !id.valid? - assert id.errors.messages[:destination].include? "needs to be a valid email address" + @id = Identity.create_for @user, address: @user.email_address, destination: 'ASKJDLFJD' + assert !@id.valid? + assert @id.errors.messages[:destination].include? "needs to be a valid email address" end - test "disabled identity" do - id = Identity.for(@user) - id.disable - assert_equal @user.email_address, id.address - assert_equal nil, id.destination - assert_equal nil, id.user - assert !id.enabled? - assert id.valid? + test "disabled @identity" do + @id = Identity.for(@user) + @id.disable + assert_equal @user.email_address, @id.address + assert_equal nil, @id.destination + assert_equal nil, @id.user + assert !@id.enabled? + assert @id.valid? end - test "disabled identity blocks handle" do - id = Identity.for(@user) - id.disable - id.save + test "disabled @identity blocks handle" do + @id = Identity.for(@user) + @id.disable + @id.save other_user = find_record :user - taken = Identity.build_for other_user, address: id.address + taken = Identity.build_for other_user, address: @id.address assert !taken.valid? assert_equal ["has already been taken"], taken.errors[:address] - Identity.destroy_all_disabled end test "destroy all disabled identities" do - id = Identity.for(@user) - id.disable - id.save - assert Identity.count > 0 + @id = Identity.for(@user) + @id.disable + @id.save + assert Identity.disabled.count > 0 Identity.destroy_all_disabled assert_equal 0, Identity.disabled.count end test "store cert fingerprint" do - begin - id = Identity.for(@user) - cert = stub expiry: Time.now, fingerprint: "fp" - id.register_cert cert - id.save - entry = {cert.fingerprint => cert.expiry.to_date.to_s} - assert_equal entry, id.cert_fingerprints - row = Identity.cert_fingerprints_by_expiry.descending.rows.first - assert_equal row['key'], cert.expiry.to_date.to_s - assert_equal row['value'], cert.fingerprint - ensure - id.reload.destroy if id && id.persisted? - end + @id = Identity.for(@user) + @id.register_cert cert_stub + entry = {cert_stub.fingerprint => cert_stub.expiry.to_date.to_s} + assert_equal entry, @id.cert_fingerprints + end + + test "query cert fingerprints by expiry" do + @id = Identity.for(@user) + @id.register_cert cert_stub + @id.save + row = Identity.cert_fingerprints_by_expiry.descending.rows.first + assert_equal row['key'], cert_stub.expiry.to_date.to_s + assert_equal row['value'], cert_stub.fingerprint + end + + test "query cert expiry for a cert fingerprint" do + @id = Identity.for(@user) + @id.register_cert cert_stub + @id.save + row = Identity.cert_expiry_by_fingerprint.key(cert_stub.fingerprint).rows.first + assert_equal row['key'], cert_stub.fingerprint + assert_equal row['value'], cert_stub.expiry.to_date.to_s end def alias_name @@ -163,4 +175,9 @@ class IdentityTest < ActiveSupport::TestCase def pgp_key_string @pgp_key ||= "DUMMY PGP KEY ... "+SecureRandom.base64(4096) end + + def cert_stub + @cert_stub ||= stub expiry: 1.month.from_now, + fingerprint: SecureRandom.hex + end end -- cgit v1.2.3