Merge pull request #175 from azul/feature/view-for-valid-certs

Feature/view for valid certs

11 files changed, 176 insertions, 122 deletions

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/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/client_certificate.rb b/app/models/client_certificate.rb
index d5bb1e0..815801e 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 ||= lifespan.months.from_now.utc.at_midnight
+  end
+
   private
 
   def openssl_cert
@@ -99,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/app/models/identity.rb b/app/models/identity.rb
index eb67b1b..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,9 +125,9 @@ class Identity < CouchRest::Model::Base
   end
 
   def register_cert(cert)
-    today = DateTime.now.to_date.to_s
+    expiry = cert.expiry.to_date.to_s
     write_attribute 'cert_fingerprints',
-      cert_fingerprints.merge(cert.fingerprint => today)
+      cert_fingerprints.merge(cert.fingerprint => expiry)
   end
 
   # for LoginFormatValidation
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 df83c9f..883073f 100644
--- a/lib/extensions/couchrest.rb
+++ b/lib/extensions/couchrest.rb
@@ -15,13 +15,18 @@ 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')
             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/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
diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb
index 49b2075..cb0f6bd 100644
--- a/test/unit/identity_test.rb
+++ b/test/unit/identity_test.rb
@@ -7,135 +7,163 @@ 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 "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 "blank @identity does not crash on valid?" do
+    @id = Identity.new
+    assert !@id.valid?
   end
 
-  test "disabled identity requires no destination" do
-    id = Identity.new address: @user.email_address
-    assert id.valid?
+  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 "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 "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
   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
+    @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
     @alias_name ||= Faker::Internet.user_name
   end
@@ -147,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