[ARVADOS] updated: 442e5c6553061ef1bbb8d9194bb5b8d9cdb68545
git at public.curoverse.com
git at public.curoverse.com
Thu Apr 10 15:35:53 EDT 2014
Summary of changes:
apps/workbench/Gemfile | 3 +-
apps/workbench/Gemfile.lock | 4 +
apps/workbench/README.textile | 14 ++
.../app/assets/javascripts/application.js | 1 +
apps/workbench/app/controllers/users_controller.rb | 92 +++++++++-
apps/workbench/app/models/user.rb | 4 +
.../workbench/app/views/application/index.html.erb | 19 ++-
.../app/views/layouts/application.html.erb | 1 +
.../app/views/users/_setup_popup.html.erb | 69 +++++++
.../workbench/app/views/users/_show_admin.html.erb | 10 +
apps/workbench/app/views/users/setup.js.erb | 2 +
apps/workbench/app/views/users/setup_popup.js.erb | 44 +++++
apps/workbench/config/application.default.yml | 1 +
apps/workbench/config/routes.rb | 3 +
apps/workbench/test/integration/users_test.rb | 193 ++++++++++++++++++++
sdk/cli/bin/crunch-job | 1 +
.../app/controllers/arvados/v1/users_controller.rb | 3 +-
.../api/app/models/api_client_authorization.rb | 7 +
services/api/app/models/arvados_model.rb | 44 +++--
services/api/app/models/log.rb | 45 ++---
services/api/app/models/user.rb | 95 +++++++----
services/api/config/database.yml.sample | 4 +-
services/api/test/fixtures/jobs.yml | 2 +
.../functional/arvados/v1/logs_controller_test.rb | 11 +
.../functional/arvados/v1/users_controller_test.rb | 26 ++-
services/api/test/unit/log_test.rb | 69 +++++++-
services/api/test/unit/user_test.rb | 10 +-
27 files changed, 682 insertions(+), 95 deletions(-)
create mode 100644 apps/workbench/app/views/users/_setup_popup.html.erb
create mode 100644 apps/workbench/app/views/users/setup.js.erb
create mode 100644 apps/workbench/app/views/users/setup_popup.js.erb
create mode 100644 apps/workbench/test/integration/users_test.rb
via 442e5c6553061ef1bbb8d9194bb5b8d9cdb68545 (commit)
via 2386475f3bf86824e320ad121838955278ed3083 (commit)
via 44df50add91dc3117291e3a6a908fa442b883106 (commit)
via bb3b70b2435a0886b4a2ace9070e24903b04c8a9 (commit)
via 91f2216807f5b7fb521d61f1a110484a6e5aa2e7 (commit)
via e37caee5e3cfc165c6505ea5f3b55a4b8b07fe5e (commit)
via e35ed29187d83ebd4cbc493b9251119013825ac1 (commit)
via 71b0d0fb51e3c54a7959f51fd4dbf523fbaf57db (commit)
via 6ea2e62b70a015226c4f3361ab3591509100a820 (commit)
via e4b0ff638bb41ce55ab3770c4f2b7f744d653aac (commit)
via 50ee9817061629f9ffe2568f937149f6e877df04 (commit)
via 7488683807205fdfebd3c52e6ba50a0879ef1da7 (commit)
via 351718c4524ede8442f1cc078d61ced8839440c5 (commit)
via 0c632a2429105322f793809f1ab5bb158050ed56 (commit)
via da1c3891473b02f891eaa0af3d9d799ec6b6ed54 (commit)
via 6783336d968c8e47ad63e929086aa704299bb403 (commit)
via 3dd13d1a0643fdcc9e4f391b74b2496ca2ebbc7c (commit)
via dcbdaf47dd1486f58893413c07a9cb5c6d180923 (commit)
via c7b2768cd590b633fe4154dedec2d8ad387a9d9b (commit)
via a0d41ce990cf0a6fcd516a6a10f662a85258238c (commit)
via 19d60097652447fe7c71ec78f5c8d52a7002b3b8 (commit)
via 4c179135909d37a6cd9722af909785393d9e117d (commit)
via 64d449da29bbbe6bce2b54a2ed67eb4cb44243c9 (commit)
via c7f17227456c27d71bca83895c84bc83fb3b4ec5 (commit)
via 2908852ef9d52b12eb715474c5f31e35f7c44b18 (commit)
via 11a9f64426c1fc50529e694fead97f81e6eb4457 (commit)
via e9339eb1983e33798270f61f57d90cde4b656bd8 (commit)
via 8ea149362539b3d50e14a7fbd8831c7cbf347446 (commit)
via 847b4a5d6e179dde49295a7118962e764d63e544 (commit)
via 1d6d51202a936a28eee2384ceaa7c725813a2b03 (commit)
via d0dc31b9feb56c026f7c7ae0d46e63434b46742f (commit)
via a6724f72c5a93edf2b8a456783a474024743f1ff (commit)
via cbdb5dc18ccbd8cd2cdc4ebeaace82aa33b36f70 (commit)
via 1418580063cbe02ab3376adb27928c5325ba10b3 (commit)
via a3212e71057712af58a7d2b849d69915b4fd79b1 (commit)
via 59b0a3a8e5209ef85967a93c500167b7cf882757 (commit)
via 9349484f1e607064cf96fa7f7212979d30e58448 (commit)
via 5a7b7aa6a6a11b5e0a397d01a6fe527169ed5f15 (commit)
via 9f87668ecf0768f83b95253d8888481b1d8ff95e (commit)
via 4882cd9ebd6311c4f84d8efff21561b1a229e244 (commit)
via a992c11d76c013d42470a5142714d68e918bccfa (commit)
via 1697fcdd746eb5a30127275e533c3d799aa9f681 (commit)
via 921ec0ddab4c949659d54490204acdd431986450 (commit)
via c6cb3423e9ca2b421bda3bb97c1448dcde19df97 (commit)
via 3ca0861ae65a9c61f0db226bbe9f70cfc0a4ec7b (commit)
via fe0d9b8e3a46c610deb95b9a8b501e8c2aad9f54 (commit)
via cb19d007add188f2c83a082c419ee0b523fb664b (commit)
via a15ff495c8c576378f55f58fe22ce9f3c49121e8 (commit)
from 54978ac2108a5b1913f641d480143356659a413a (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 442e5c6553061ef1bbb8d9194bb5b8d9cdb68545
Author: Brett Smith <brett at curoverse.com>
Date: Thu Apr 10 15:11:09 2014 -0400
api: Don't log common changes to API tokens.
The last_used_* attributes are updated very often. Logging those
changes would probably make the logs less useful.
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index fca57dc..38a90d5 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -20,6 +20,9 @@ class ApiClientAuthorization < ArvadosModel
t.add :scopes
end
+ UNLOGGED_ATTRIBUTES = ['last_used_at', 'last_used_by_ip_address',
+ 'updated_at']
+
def assign_random_api_token
self.api_token ||= rand(2**256).to_s(36)
end
@@ -71,4 +74,8 @@ class ApiClientAuthorization < ArvadosModel
not self.user_id_changed? and
not self.owner_uuid_changed?)
end
+
+ def log_update
+ super unless (changed - UNLOGGED_ATTRIBUTES).empty?
+ end
end
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 0939c28..39b45a3 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -184,4 +184,19 @@ class LogTest < ActiveSupport::TestCase
end
end
end
+
+ test "don't log changes only to ApiClientAuthorization.last_used_*" do
+ set_user_from_auth :admin_trustedclient
+ auth = api_client_authorizations(:spectator)
+ start_log_count = get_logs_about(auth).size
+ auth.last_used_at = Time.now
+ auth.last_used_by_ip_address = '::1'
+ auth.save!
+ assert_equal(start_log_count, get_logs_about(auth).size,
+ "log count changed after 'using' ApiClientAuthorization")
+ auth.created_by_ip_address = '::1'
+ auth.save!
+ assert_equal(start_log_count + 1, get_logs_about(auth).size,
+ "no log after changed stable ApiClientAuthorization attribute")
+ end
end
commit 2386475f3bf86824e320ad121838955278ed3083
Author: Brett Smith <brett at curoverse.com>
Date: Thu Apr 10 14:46:45 2014 -0400
api: Raise errors from saving logs.
With this change, I realized it'd be cleaner to write the
implementation as after_* callbacks, so this includes that. But the
fundamental change is to use log.save! instead of plain log.save.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 731eecf..a87e2d5 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -14,9 +14,9 @@ class ArvadosModel < ActiveRecord::Base
before_destroy :ensure_permission_to_destroy
before_create :update_modified_by_fields
before_update :maybe_update_modified_by_fields
- around_create { |&block| make_log_around(:create, nil, self, &block) }
- around_update { |&block| make_log_around(:update, self, self, &block) }
- around_destroy { |&block| make_log_around(:destroy, self, nil, &block) }
+ after_create :log_create
+ after_update :log_update
+ after_destroy :log_destroy
validate :ensure_serialized_attribute_type
validate :normalize_collection_uuids
@@ -219,19 +219,31 @@ class ArvadosModel < ActiveRecord::Base
@old_attributes = attributes
end
- def make_log_around(event_type, old_thing, new_thing)
- if self.is_a? Log
- yield
- else
- log = Log.start_from(old_thing, event_type.to_s)
- if not old_thing.nil?
- log.properties['old_etag'] = @old_etag
- log.properties['old_attributes'] = @old_attributes
- end
- yield
- log.update_to new_thing
- log_start_state
- log.save
+ def log_change(event_type)
+ log = Log.new(event_type: event_type).fill_object(self)
+ yield log
+ log.save!
+ log_start_state
+ end
+
+ def log_create
+ log_change('create') do |log|
+ log.fill_properties('old', nil, nil)
+ log.update_to self
+ end
+ end
+
+ def log_update
+ log_change('update') do |log|
+ log.fill_properties('old', @old_etag, @old_attributes)
+ log.update_to self
+ end
+ end
+
+ def log_destroy
+ log_change('destroy') do |log|
+ log.fill_properties('old', @old_etag, @old_attributes)
+ log.update_to nil
end
end
end
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 223d125..e9fe352 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -16,22 +16,21 @@ class Log < ArvadosModel
t.add :properties
end
- def self.start_from(thing, event_type)
- self.new do |log|
- log.event_type = event_type
- log.properties = {
- 'old_etag' => nil,
- 'old_attributes' => nil,
- }
- log.seed_basics_from thing
- end
+ def fill_object(thing)
+ self.object_kind ||= thing.kind
+ self.object_uuid ||= thing.uuid
+ self.summary ||= "#{self.event_type} of #{thing.uuid}"
+ self
+ end
+
+ def fill_properties(age, etag_prop, attrs_prop)
+ self.properties.merge!({"#{age}_etag" => etag_prop,
+ "#{age}_attributes" => attrs_prop})
end
def update_to(thing)
- self.seed_basics_from thing
- self.properties["new_etag"] = thing.andand.etag
- self.properties["new_attributes"] = thing.andand.attributes
- case self.event_type
+ fill_properties('new', thing.andand.etag, thing.andand.attributes)
+ case event_type
when "create"
self.event_at = thing.created_at
when "update"
@@ -39,14 +38,7 @@ class Log < ArvadosModel
when "destroy"
self.event_at = Time.now
end
- end
-
- def seed_basics_from(thing)
- if not thing.nil?
- self.object_kind ||= thing.kind
- self.object_uuid ||= thing.uuid
- self.summary ||= "#{self.event_type} of #{thing.uuid}"
- end
+ self
end
protected
@@ -64,4 +56,8 @@ class Log < ArvadosModel
def set_default_event_at
self.event_at ||= Time.now
end
+
+ def log_change(event_type)
+ # Don't log changes to logs.
+ end
end
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 7a27767..0939c28 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -165,4 +165,23 @@ class LogTest < ActiveSupport::TestCase
assert_nothing_raised { log.save! }
assert_nothing_raised { log.destroy }
end
+
+ test "failure saving log causes failure saving object" do
+ Log.class_eval do
+ alias_method :_orig_validations, :perform_validations
+ def perform_validations(options)
+ false
+ end
+ end
+ begin
+ set_user_from_auth :active_trustedclient
+ user = users(:active)
+ user.first_name = 'Test'
+ assert_raise(ActiveRecord::RecordInvalid) { user.save! }
+ ensure
+ Log.class_eval do
+ alias_method :perform_validations, :_orig_validations
+ end
+ end
+ end
end
commit 44df50add91dc3117291e3a6a908fa442b883106
Author: Brett Smith <brett at curoverse.com>
Date: Thu Apr 10 11:43:55 2014 -0400
api: Have users own the logs they generate.
Setting the owner to the system user was intended to provide the
security policy we wanted. Now that we've opted instead to give logs
overrides for the permission_to_ACTION methods, users should own their
own logs so they can read them.
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 17d6ec4..223d125 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -4,7 +4,6 @@ class Log < ArvadosModel
include CommonApiTemplate
serialize :properties, Hash
before_validation :set_default_event_at
- before_save { self.owner_uuid = self.system_user_uuid }
attr_accessor :object
api_accessible :user, extend: :common do |t|
diff --git a/services/api/test/functional/arvados/v1/logs_controller_test.rb b/services/api/test/functional/arvados/v1/logs_controller_test.rb
index 3e8508a..9c41099 100644
--- a/services/api/test/functional/arvados/v1/logs_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/logs_controller_test.rb
@@ -1,4 +1,15 @@
require 'test_helper'
class Arvados::V1::LogsControllerTest < ActionController::TestCase
+ test "non-admins can read their own logs" do
+ authorize_with :active
+ post :create, log: {summary: "test log"}
+ assert_response :success
+ uuid = JSON.parse(@response.body)['uuid']
+ assert_not_nil uuid
+ get :show, {id: uuid}
+ assert_response(:success, "failed to load created log")
+ assert_equal("test log", assigns(:object).summary,
+ "loaded wrong log after creation")
+ end
end
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 8879dfa..7a27767 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -32,8 +32,8 @@ class LogTest < ActiveSupport::TestCase
@log_count += 1
log = logs.last
props = log.properties
- assert_equal(system_user_uuid, log.owner_uuid,
- "log is not owned by system user")
+ assert_equal(current_user.andand.uuid, log.owner_uuid,
+ "log is not owned by current user")
assert_equal(current_user.andand.uuid, log.modified_by_user_uuid,
"log is not 'modified by' current user")
assert_equal(current_api_client.andand.uuid, log.modified_by_client_uuid,
commit bb3b70b2435a0886b4a2ace9070e24903b04c8a9
Author: Brett Smith <brett at curoverse.com>
Date: Thu Apr 10 10:41:49 2014 -0400
api: Let admin users delete logs.
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 08038db..17d6ec4 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -57,12 +57,10 @@ class Log < ArvadosModel
end
def permission_to_update
- false
+ current_user.andand.is_admin
end
- def permission_to_destroy
- false
- end
+ alias_method :permission_to_delete, :permission_to_update
def set_default_event_at
self.event_at ||= Time.now
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 0d55e43..8879dfa 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -147,4 +147,22 @@ class LogTest < ActiveSupport::TestCase
log.save!
assert_equal(0, get_logs_about(log).size, "made a Log about a Log")
end
+
+ test "non-admins can't modify or delete logs" do
+ set_user_from_auth :active_trustedclient
+ log = Log.new(summary: "immutable log test")
+ assert_nothing_raised { log.save! }
+ log.summary = "log mutation test should fail"
+ assert_raise(ArvadosModel::PermissionDeniedError) { log.save! }
+ assert_raise(ArvadosModel::PermissionDeniedError) { log.destroy }
+ end
+
+ test "admins can modify and delete logs" do
+ set_user_from_auth :admin_trustedclient
+ log = Log.new(summary: "admin log mutation test")
+ assert_nothing_raised { log.save! }
+ log.summary = "admin mutated log test"
+ assert_nothing_raised { log.save! }
+ assert_nothing_raised { log.destroy }
+ end
end
commit 91f2216807f5b7fb521d61f1a110484a6e5aa2e7
Author: Brett Smith <brett at curoverse.com>
Date: Thu Apr 10 10:15:53 2014 -0400
api: Test that we don't make logs about logs.
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 1554e0a..0d55e43 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -22,8 +22,12 @@ class LogTest < ActiveSupport::TestCase
end
end
+ def get_logs_about(thing)
+ Log.where(object_uuid: thing.uuid).order("created_at ASC").all
+ end
+
def assert_logged(thing, event_type)
- logs = Log.where(object_uuid: thing.uuid).order("created_at ASC").all
+ logs = get_logs_about(thing)
assert_equal(@log_count, logs.size, "log count mismatch")
@log_count += 1
log = logs.last
@@ -136,4 +140,11 @@ class LogTest < ActiveSupport::TestCase
"group final name mismatch")
end
end
+
+ test "making a log doesn't get logged" do
+ set_user_from_auth :active_trustedclient
+ log = Log.new
+ log.save!
+ assert_equal(0, get_logs_about(log).size, "made a Log about a Log")
+ end
end
commit e37caee5e3cfc165c6505ea5f3b55a4b8b07fe5e
Merge: 54978ac e35ed29
Author: Brett Smith <brett at curoverse.com>
Date: Thu Apr 10 10:05:28 2014 -0400
Merge branch 'master' into 2375-log-table
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list