[ARVADOS] created: 5c11190dda23801d0f7d177bf2c0a0ac5d899898
Git user
git at public.curoverse.com
Wed Jun 8 10:01:57 EDT 2016
at 5c11190dda23801d0f7d177bf2c0a0ac5d899898 (commit)
commit 5c11190dda23801d0f7d177bf2c0a0ac5d899898
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Jun 8 09:52:30 2016 -0400
9278: Ensure locator signatures expire no later than expires_at.
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index df969e3..4a61292 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -39,7 +39,10 @@ class Collection < ArvadosModel
# expose signed_manifest_text as manifest_text in the
# API response, and never let clients select the
# manifest_text column.
- 'manifest_text' => ['manifest_text'],
+ #
+ # We need expires_at to determine the correct
+ # timestamp in signed_manifest_text.
+ 'manifest_text' => ['manifest_text', 'expires_at'],
)
end
@@ -213,14 +216,19 @@ class Collection < ArvadosModel
def signed_manifest_text
if has_attribute? :manifest_text
token = current_api_client_authorization.andand.api_token
- @signed_manifest_text = self.class.sign_manifest manifest_text, token
+ exp = [db_current_time.to_i + Rails.configuration.blob_signature_ttl,
+ expires_at].compact.map(&:to_i).min
+ @signed_manifest_text = self.class.sign_manifest manifest_text, token, exp
end
end
- def self.sign_manifest manifest, token
+ def self.sign_manifest manifest, token, exp=nil
+ if exp.nil?
+ exp = db_current_time.to_i + Rails.configuration.blob_signature_ttl
+ end
signing_opts = {
api_token: token,
- expire: db_current_time.to_i + Rails.configuration.blob_signature_ttl,
+ expire: exp,
}
m = munge_manifest_locators(manifest) do |match|
Blob.sign_locator(match[0], signing_opts)
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 536d9ca..9156892 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -307,6 +307,29 @@ class CollectionTest < ActiveSupport::TestCase
end
end
+ test 'signature expiry does not exceed expires_at' do
+ act_as_user users(:active) do
+ t0 = db_current_time
+ c = Collection.create!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:x\n", name: 'foo')
+ c.update_attributes! expires_at: (t0 + 1.hours)
+ c.reload
+ sig_exp = /\+A[0-9a-f]{40}\@([0-9]+)/.match(c.signed_manifest_text)[1].to_i
+ assert_operator sig_exp.to_i, :<=, (t0 + 1.hours).to_i
+ end
+ end
+
+ test 'far-future expiry date cannot be used to circumvent configured permission ttl' do
+ act_as_user users(:active) do
+ c = Collection.create!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:x\n",
+ name: 'foo',
+ expires_at: db_current_time + 1.years)
+ sig_exp = /\+A[0-9a-f]{40}\@([0-9]+)/.match(c.signed_manifest_text)[1].to_i
+ expect_max_sig_exp = db_current_time.to_i + Rails.configuration.blob_signature_ttl
+ assert_operator c.expires_at.to_i, :>, expect_max_sig_exp
+ assert_operator sig_exp.to_i, :<=, expect_max_sig_exp
+ end
+ end
+
test "create collection with properties" do
act_as_system_user do
c = Collection.create(manifest_text: ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo\n",
commit 1a0738a8169097ea90933108222359c8746a2042
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Jun 7 13:59:29 2016 -0400
9278: Expose expires_at in API response.
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 459eacc..df969e3 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -29,6 +29,7 @@ class Collection < ArvadosModel
t.add :replication_desired
t.add :replication_confirmed
t.add :replication_confirmed_at
+ t.add :expires_at
end
def self.attributes_required_columns
commit 67d893f6ed7c527f88d8f92ec9909a9e8d978624
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Jun 7 13:59:19 2016 -0400
9278: Set expires_at=now if a client sets it to a time in the past.
The definition of "now" in the default collection scope changes from
current_timestamp (time the current transaction started) to
statement_timestamp() (time the current statement started) so a test
case can expire a collection and then confirm that it is not in the
default scope, all within a single test transaction.
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index f1e7b4f..459eacc 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -15,9 +15,10 @@ class Collection < ArvadosModel
before_validation :strip_signatures_and_update_replication_confirmed
validate :ensure_pdh_matches_manifest_text
before_save :set_file_names
+ before_save :expires_at_not_in_past
# Query only undeleted collections by default.
- default_scope where("expires_at IS NULL or expires_at > CURRENT_TIMESTAMP")
+ default_scope where("expires_at IS NULL or expires_at > statement_timestamp()")
api_accessible :user, extend: :common do |t|
t.add :name
@@ -381,4 +382,14 @@ class Collection < ArvadosModel
end
super
end
+
+ # If expires_at is being changed to a time in the past, change it to
+ # now. This allows clients to say "expires {client-current-time}"
+ # without failing due to clock skew, while avoiding odd log entries
+ # like "expiry date changed to {1 year ago}".
+ def expires_at_not_in_past
+ if expires_at_changed? and expires_at
+ self.expires_at = [db_current_time, expires_at].max
+ end
+ end
end
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index c81d543..536d9ca 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -1,6 +1,8 @@
require 'test_helper'
class CollectionTest < ActiveSupport::TestCase
+ include DbCurrentTime
+
def create_collection name, enc=nil
txt = ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:#{name}.txt\n"
txt.force_encoding(enc) if enc
@@ -340,4 +342,14 @@ class CollectionTest < ActiveSupport::TestCase
coll_uuids = coll_list.map(&:uuid)
assert_includes(coll_uuids, collections(:docker_image).uuid)
end
+
+ test 'expires_at cannot be set too far in the past' do
+ act_as_user users(:active) do
+ t0 = db_current_time
+ c = Collection.create!(manifest_text: '', name: 'foo')
+ c.update_attributes! expires_at: (t0 - 2.weeks)
+ c.reload
+ assert_operator c.expires_at, :>, t0
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list