[ARVADOS] updated: f38d011a7289e2c1819dd7cbb76a738a24e9c825
git at public.curoverse.com
git at public.curoverse.com
Fri Jul 18 01:19:16 EDT 2014
Summary of changes:
.../views/application/_projects_tree_menu.html.erb | 2 +-
.../app/views/projects/_show_contents.html.erb | 8 +++---
.../app/controllers/user_sessions_controller.rb | 6 ++++-
.../api/app/models/api_client_authorization.rb | 2 +-
services/api/app/models/arvados_model.rb | 30 +++++++++-------------
services/api/test/fixtures/users.yml | 10 ++++++++
services/api/test/unit/permission_test.rb | 23 +++++++++++++++++
7 files changed, 55 insertions(+), 26 deletions(-)
discards 7b82554aa0de6f490170fba9117f252f777f0e2f (commit)
via f38d011a7289e2c1819dd7cbb76a738a24e9c825 (commit)
via 5f65177301647eca4b488b9d33fd62295a9bb081 (commit)
via 690bfc8b3d22f9b9bf80328b5758462f315cc1b3 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (7b82554aa0de6f490170fba9117f252f777f0e2f)
\
N -- N -- N (f38d011a7289e2c1819dd7cbb76a738a24e9c825)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
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 f38d011a7289e2c1819dd7cbb76a738a24e9c825
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Jul 18 01:06:42 2014 -0400
3214: Fix unclosed HTML tag.
diff --git a/apps/workbench/app/views/application/_projects_tree_menu.html.erb b/apps/workbench/app/views/application/_projects_tree_menu.html.erb
index 390ef77..3334912 100644
--- a/apps/workbench/app/views/application/_projects_tree_menu.html.erb
+++ b/apps/workbench/app/views/application/_projects_tree_menu.html.erb
@@ -16,7 +16,7 @@
<% end %>
</li>
<% end %>
- <li class="divider">
+ <li class="divider" />
<li role="presentation" class="dropdown-header">
Projects shared with me
</li>
commit 5f65177301647eca4b488b9d33fd62295a9bb081
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Jul 18 01:00:47 2014 -0400
3214: Ensure current_user has write permission on the new owner when setting owner_uuid.
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 0b80877..696ae02 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -42,7 +42,11 @@ class UserSessionsController < ApplicationController
:first_name => omniauth['info']['first_name'],
:last_name => omniauth['info']['last_name'],
:identity_url => omniauth['info']['identity_url'],
- :is_active => Rails.configuration.new_users_are_active)
+ :is_active => Rails.configuration.new_users_are_active,
+ :owner_uuid => system_user_uuid)
+ act_as_system_user do
+ user.save or raise Exception.new(user.errors.messages)
+ end
else
user.email = omniauth['info']['email']
user.first_name = omniauth['info']['first_name']
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 82dd0ec..5817ff6 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -30,7 +30,7 @@ class ApiClientAuthorization < ArvadosModel
self.user.andand.uuid
end
def owner_uuid_was
- self.user_id_changed? ? User.find(self.user_id_was).andand.uuid : self.user.andand.uuid
+ self.user_id_changed? ? User.where(id: self.user_id_was).first.andand.uuid : self.user.andand.uuid
end
def owner_uuid_changed?
self.user_id_changed?
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 1ea9332..469b0a3 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -206,29 +206,23 @@ class ArvadosModel < ActiveRecord::Base
def ensure_owner_uuid_is_permitted
raise PermissionDeniedError if !current_user
- if respond_to? :owner_uuid=
+ if new_record? and respond_to? :owner_uuid=
self.owner_uuid ||= current_user.uuid
end
- if self.owner_uuid_changed?
- if new_record?
- return true
- elsif current_user.uuid == self.owner_uuid or
- current_user.can? write: self.owner_uuid
- # current_user is, or has :write permission on, the new owner
- else
- logger.warn "User #{current_user.uuid} tried to change owner_uuid of #{self.class.to_s} #{self.uuid} to #{self.owner_uuid} but does not have permission to write to #{self.owner_uuid}"
+ if owner_uuid_changed? and owner_uuid_was
+ # Verify permission to write to existing owner
+ unless current_user.uuid == self.owner_uuid_was or
+ current_user.uuid == self.uuid or
+ current_user.can? write: self.owner_uuid_was
+ logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{uuid} but does not have permission to write existing owner_uuid #{owner_uuid_was}"
+ errors.add :owner_uuid, "cannot be changed without write permission on existing owner"
raise PermissionDeniedError
end
end
- if new_record?
- return true
- elsif current_user.uuid == self.owner_uuid_was or
- current_user.uuid == self.uuid or
- current_user.can? write: self.owner_uuid_was
- # current user is, or has :write permission on, the previous owner
- return true
- else
- logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} but does not have permission to write #{self.owner_uuid_was}"
+ # Verify permission to write to new owner
+ unless current_user == self or current_user.can? write: owner_uuid
+ logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{uuid} but does not have permission to write new owner_uuid #{owner_uuid}"
+ errors.add :owner_uuid, "cannot be changed without write permission on new owner"
raise PermissionDeniedError
end
end
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index f6d5b21..72a5aa3 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -1,6 +1,7 @@
# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/Fixtures.html
admin:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-d9tiejq69daie8f
email: admin at arvados.local
first_name: TestCase
@@ -11,6 +12,7 @@ admin:
prefs: {}
miniadmin:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-2bg9x0oeydcw5hm
email: miniadmin at arvados.local
first_name: TestCase
@@ -21,6 +23,7 @@ miniadmin:
prefs: {}
rominiadmin:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-4hvxm4n25emegis
email: rominiadmin at arvados.local
first_name: TestCase
@@ -31,6 +34,7 @@ rominiadmin:
prefs: {}
active:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-xurymjxw79nv3jz
email: active-user at arvados.local
first_name: Active
@@ -41,6 +45,7 @@ active:
prefs: {}
project_viewer:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-projectviewer1a
email: project-viewer at arvados.local
first_name: Project
@@ -51,6 +56,7 @@ project_viewer:
prefs: {}
spectator:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-l1s2piq4t4mps8r
email: spectator at arvados.local
first_name: Spect
@@ -61,6 +67,7 @@ spectator:
prefs: {}
inactive_uninvited:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-rf2ec3ryh4vb5ma
email: inactive-uninvited-user at arvados.local
first_name: Inactive and Uninvited
@@ -71,6 +78,7 @@ inactive_uninvited:
prefs: {}
inactive:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-x9kqpd79egh49c7
email: inactive-user at arvados.local
first_name: Inactive
@@ -81,6 +89,7 @@ inactive:
prefs: {}
inactive_but_signed_user_agreement:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-7sg468ezxwnodxs
email: inactive-user-signed-ua at arvados.local
first_name: Inactive But Agreeable
@@ -91,6 +100,7 @@ inactive_but_signed_user_agreement:
prefs: {}
anonymous:
+ owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-anonymouspublic
email: anonymouspublic
first_name: anonymouspublic
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 748c790..7a6e482 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -131,4 +131,27 @@ class PermissionTest < ActiveSupport::TestCase
perm_link.save
end
end
+
+ test "user cannot use owner_uuid without write permission on new owner" do
+ set_user_from_auth :rominiadmin
+
+ assert_raises ArvadosModel::PermissionDeniedError, "created with owner = unwritable user" do
+ Specimen.create!(owner_uuid: users(:active).uuid)
+ end
+
+ ob = Specimen.create!
+ assert_raises ArvadosModel::PermissionDeniedError, "changed owner to unwritable user" do
+ ob.update_attributes!(owner_uuid: users(:active).uuid)
+ end
+
+ assert_raises ArvadosModel::PermissionDeniedError, "created with owner = unwritable group" do
+ Specimen.create!(owner_uuid: groups(:aproject).uuid)
+ end
+
+ ob = Specimen.create!
+ assert_raises ArvadosModel::PermissionDeniedError, "changed owner to unwritable group" do
+ ob.update_attributes!(owner_uuid: groups(:aproject).uuid)
+ end
+
+ end
end
commit 690bfc8b3d22f9b9bf80328b5758462f315cc1b3
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jul 17 16:51:22 2014 -0400
3214: "New project" link in top nav dropdown create a new project at top level, instead of inside the current project.
diff --git a/apps/workbench/app/views/layouts/application.html.erb b/apps/workbench/app/views/layouts/application.html.erb
index 63de6c2..ec2c98b 100644
--- a/apps/workbench/app/views/layouts/application.html.erb
+++ b/apps/workbench/app/views/layouts/application.html.erb
@@ -175,7 +175,7 @@
link_to(project_path(pnode[:object].uuid), data: {object_uuid: pnode[:object].uuid, name: 'name'}, &block)
end,
:top_button => Proc.new do %>
- <% link_to projects_path('project[owner_uuid]' => current_project_uuid), method: 'post', class: 'btn btn-xs btn-default pull-right' do %>
+ <% link_to projects_path, method: 'post', class: 'btn btn-xs btn-default pull-right' do %>
<i class="fa fa-plus"></i> New project
<% end %>
<% end %>
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index b338aa8..a991ced 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -9,4 +9,23 @@ class ProjectsControllerTest < ActionController::TestCase
assert_template 'user_agreements/index',
"Inactive user was not presented with a user agreement at the front page"
end
+
+ [[:active, true],
+ [:project_viewer, false]].each do |which_user, should_show|
+ test "create subproject button #{'not ' unless should_show} shown to #{which_user}" do
+ readonly_project_uuid = api_fixture('groups')['aproject']['uuid']
+ get :show, {
+ id: readonly_project_uuid
+ }, session_for(which_user)
+ buttons = css_select('[data-method=post]').select do |el|
+ el.attributes['href'].match /project.*owner_uuid.*#{readonly_project_uuid}/
+ end
+ if should_show
+ assert_not_empty(buttons, "did not offer to create a subproject")
+ else
+ assert_empty(buttons.collect(&:to_s),
+ "offered to create a subproject in a non-writable project")
+ end
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list