[ARVADOS] updated: 427da80092302506bbf5f97b30e9e2e927e06596
Git user
git at public.curoverse.com
Thu Jun 9 10:22:53 EDT 2016
Summary of changes:
services/api/app/models/collection.rb | 30 +++++++++++++++++++++-----
services/api/test/unit/collection_test.rb | 35 +++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 5 deletions(-)
via 427da80092302506bbf5f97b30e9e2e927e06596 (commit)
via bd872ed1fbda80e4e20c2b1e916d210f670afe4e (commit)
via 62b3629a76473ae3846860362923b2e180c42e08 (commit)
via d7d46dfe9b8be649e7bfdd3f65a0f2313b7597d3 (commit)
from 5604e411dec23b6ebaa9f52b4994ed7c30182f92 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 427da80092302506bbf5f97b30e9e2e927e06596
Merge: 5604e41 bd872ed
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jun 9 10:21:29 2016 -0400
Merge branch '9278-expiring-collections'
refs #9278
commit bd872ed1fbda80e4e20c2b1e916d210f670afe4e
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 62b3629a76473ae3846860362923b2e180c42e08
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 d7d46dfe9b8be649e7bfdd3f65a0f2313b7597d3
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