[ARVADOS] updated: e0e4f50570bf5337518e459decc4df8e78a172fe
git at public.curoverse.com
git at public.curoverse.com
Thu Jul 17 11:56:18 EDT 2014
Summary of changes:
.../app/assets/javascripts/select_modal.js | 2 +-
.../app/controllers/projects_controller.rb | 30 ++++++--------
apps/workbench/app/models/link.rb | 10 +++++
.../app/views/projects/_show_sharing.html.erb | 7 ++--
.../test/functional/projects_controller_test.rb | 39 +++++++++++++++++-
apps/workbench/test/integration/projects_test.rb | 8 +---
apps/workbench/test/unit/link_test.rb | 47 ++++++++++++++++++++--
services/api/app/models/link.rb | 3 +-
services/api/test/unit/link_test.rb | 24 +++++++++++
9 files changed, 135 insertions(+), 35 deletions(-)
via e0e4f50570bf5337518e459decc4df8e78a172fe (commit)
via f7270b6c2dc6aa113a4f1a2a131b0c9907799d85 (commit)
via 6302bc6d5da1a584f31fd8e9eb1079d866df1767 (commit)
via 960e2aac141eed0f8c660ab71e60b850a5549e0c (commit)
via 58a950d714d5cdf67b77ad8ba598a398935763f1 (commit)
via 3270dae2ff541e5fd8fba9a89b9cfe07127f2800 (commit)
from 5ff17b27bd039f3b219e18dfc2e1d67d8043e11d (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 e0e4f50570bf5337518e459decc4df8e78a172fe
Author: Brett Smith <brett at curoverse.com>
Date: Thu Jul 17 11:51:56 2014 -0400
2044: Workbench share_with method propagates failure messages.
diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb
index abd2a72..4492010 100644
--- a/apps/workbench/app/controllers/projects_controller.rb
+++ b/apps/workbench/app/controllers/projects_controller.rb
@@ -150,13 +150,13 @@ class ProjectsController < ApplicationController
@errors = ["No user/group UUIDs specified to share with."]
return render_error(status: 422)
end
- results = {"success" => [], "failure" => []}
+ results = {"success" => [], "failure" => {}}
params[:uuids].each do |shared_uuid|
begin
Link.create(tail_uuid: shared_uuid, link_class: "permission",
name: "can_read", head_uuid: @object.uuid)
rescue ArvadosApiClient::ApiError => error
- results["failure"] << shared_uuid
+ results["failure"][shared_uuid] = error.api_response.andand[:errors]
else
results["success"] << shared_uuid
end
commit f7270b6c2dc6aa113a4f1a2a131b0c9907799d85
Author: Brett Smith <brett at curoverse.com>
Date: Thu Jul 17 11:45:34 2014 -0400
2044: Bugfix Workbench project manager determination.
diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb
index e45e840..abd2a72 100644
--- a/apps/workbench/app/controllers/projects_controller.rb
+++ b/apps/workbench/app/controllers/projects_controller.rb
@@ -80,8 +80,6 @@ class ProjectsController < ApplicationController
@objects = @object.contents(limit: 50,
include_linked: true,
offset: params[:offset] || 0)
- @share_links = Link.filter([['head_uuid', '=', @object.uuid],
- ['link_class', '=', 'permission']])
@logs = Log.limit(10).filter([['object_uuid', '=', @object.uuid]])
@users = User.limit(10000).
select(["uuid", "is_active", "first_name", "last_name"]).
@@ -89,6 +87,16 @@ class ProjectsController < ApplicationController
@groups = Group.limit(10000).
select(["uuid", "name", "description"])
+ begin
+ @share_links = Link.permissions_for(@object)
+ @user_is_manager = true
+ rescue ArvadosApiClient::AccessForbiddenException,
+ ArvadosApiClient::NotFoundException
+ @share_links = Link.filter([['head_uuid', '=', @object.uuid],
+ ['link_class', '=', 'permission']])
+ @user_is_manager = false
+ end
+
@objects_and_names = []
@objects.each do |object|
if !(name_links = @objects.links_for(object, 'name')).empty?
@@ -126,20 +134,6 @@ class ProjectsController < ApplicationController
end
end
- helper_method :managed_by_user?
- def managed_by_user?(user=nil)
- user ||= current_user
- if user.nil?
- false
- else
- user_uuid = user.uuid
- user.is_admin or (user_uuid == @object.owner_uuid) or
- @share_links.any? { |link|
- (link.tail_uuid == user_uuid) and (link.name == "can_manage")
- }
- end
- end
-
def create
@new_resource_attrs = (params['project'] || {}).merge(group_class: 'project')
@new_resource_attrs[:name] ||= 'New project'
diff --git a/apps/workbench/app/views/projects/_show_sharing.html.erb b/apps/workbench/app/views/projects/_show_sharing.html.erb
index 0185d77..47dd2d6 100644
--- a/apps/workbench/app/views/projects/_show_sharing.html.erb
+++ b/apps/workbench/app/views/projects/_show_sharing.html.erb
@@ -13,7 +13,6 @@
perms_json << {value: link_name, text: link_desc}
end
perms_json = perms_json.to_json
- manager = managed_by_user?
owner_icon = fa_icon_class_for_uuid(@object.owner_uuid)
if owner_icon == "fa-users"
owner_icon = "fa-folder"
@@ -23,7 +22,7 @@
end
%>
-<% if manager %>
+<% if @user_is_manager %>
<div class="pull-right">
<% ["users", "groups"].each do |share_class| %>
@@ -52,7 +51,7 @@
<table id="project_sharing" class="topalign table" style="clear: both; margin-top: 1em;">
<tr>
<th>User/Group Name</th>
- <th<%= raw(' colspan="2"') if manager %>>Project Access</th>
+ <th<%= raw(' colspan="2"') if @user_is_manager %>>Project Access</th>
</tr>
<% @share_links.andand.each do |link|
@@ -69,7 +68,7 @@
<i class="fa fa-fw <%= fa_icon_class_for_uuid(link.tail_uuid) %>"></i>
<%= link_to_if_arvados_object(link.tail_uuid, link_text: link_name) %>
</td>
- <% if manager %>
+ <% if @user_is_manager %>
<td><%= link_to perm_name_desc_map[link.name], '#', {
"data-emptytext" => "Read",
"data-placement" => "bottom",
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index 39ee4b2..0637c13 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -31,4 +31,30 @@ class ProjectsControllerTest < ActionController::TestCase
session_for(:project_viewer))
assert_response 422
end
+
+ def user_can_manage(user_sym, group_key)
+ get(:show, {id: api_fixture("groups")[group_key]["uuid"]},
+ session_for(user_sym))
+ assigns(:user_is_manager)
+ end
+
+ test "admin can_manage aproject" do
+ assert user_can_manage(:admin, "aproject")
+ end
+
+ test "owner can_manage aproject" do
+ assert user_can_manage(:active, "aproject")
+ end
+
+ test "owner can_manage asubproject" do
+ assert user_can_manage(:active, "asubproject")
+ end
+
+ test "viewer can't manage aproject" do
+ refute user_can_manage(:project_viewer, "aproject")
+ end
+
+ test "viewer can't manage asubproject" do
+ refute user_can_manage(:project_viewer, "asubproject")
+ end
end
commit 6302bc6d5da1a584f31fd8e9eb1079d866df1767
Author: Brett Smith <brett at curoverse.com>
Date: Wed Jul 16 18:15:40 2014 -0400
2044: Ensure users can only give permission links to unreadable users.
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 4852461..6321145 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -108,7 +108,8 @@ class Link < ArvadosModel
# A user can give all other users permissions on folders.
def skip_uuid_read_permission_check
skipped_attrs = super
- if (ArvadosModel.resource_class_for_uuid(head_uuid) == Group) and
+ if link_class == "permission" and
+ (ArvadosModel.resource_class_for_uuid(head_uuid) == Group) and
(ArvadosModel.resource_class_for_uuid(tail_uuid) == User)
skipped_attrs << "tail_uuid"
end
diff --git a/services/api/test/unit/link_test.rb b/services/api/test/unit/link_test.rb
index 5bd8038..8423683 100644
--- a/services/api/test/unit/link_test.rb
+++ b/services/api/test/unit/link_test.rb
@@ -82,6 +82,30 @@ class LinkTest < ActiveSupport::TestCase
refute link.valid?
end
+ test "user can't add a Collection to a Project without permission" do
+ link = make_active_perm(link_class: "name",
+ name: "Permission denied test name",
+ tail_uuid: collections(:bar_file).uuid)
+ begin
+ refute link.valid?
+ rescue ArvadosModel::PermissionDeniedError
+ # That's good enough.
+ end
+ end
+
+ test "user can't add a User to a Project" do
+ # Users *can* give other users permissions to projects.
+ # This test helps ensure that that exception is specific to permissions.
+ link = make_active_perm(link_class: "name",
+ name: "Permission denied test name",
+ tail_uuid: users(:admin).uuid)
+ begin
+ refute link.valid?
+ rescue ArvadosModel::PermissionDeniedError => e
+ # That's good enough.
+ end
+ end
+
test "link granting project permissions to unreadable user is valid" do
link = make_active_perm(tail_uuid: users(:admin).uuid)
assert link.valid?
commit 960e2aac141eed0f8c660ab71e60b850a5549e0c
Author: Brett Smith <brett at curoverse.com>
Date: Wed Jul 16 17:44:50 2014 -0400
2044: Add share_with permission denied test.
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index a6018bc..39ee4b2 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -22,4 +22,13 @@ class ProjectsControllerTest < ActionController::TestCase
json_response = Oj.load(@response.body)
assert_equal(uuid_list, json_response["success"])
end
+
+ test "user with project read permission can't add permissions" do
+ post(:share_with, {
+ id: api_fixture("groups")["aproject"]["uuid"],
+ uuids: [api_fixture("users")["spectator"]["uuid"]],
+ format: "json"},
+ session_for(:project_viewer))
+ assert_response 422
+ end
end
commit 58a950d714d5cdf67b77ad8ba598a398935763f1
Author: Brett Smith <brett at curoverse.com>
Date: Tue Jul 15 17:31:11 2014 -0400
2044: Workbench project sharing code clean-ups.
diff --git a/apps/workbench/app/assets/javascripts/select_modal.js b/apps/workbench/app/assets/javascripts/select_modal.js
index fbdf15f..8220945 100644
--- a/apps/workbench/app/assets/javascripts/select_modal.js
+++ b/apps/workbench/app/assets/javascripts/select_modal.js
@@ -69,7 +69,7 @@ $(document).on('click', '.selectable', function() {
$(document).on('page-refresh', function(event, data, status, jqxhr, action_data) {
window.location.reload();
}).on('tab-refresh', function(event, data, status, jqxhr, action_data) {
- tab_name = $('.tab-pane.active')[0].id;
+ var tab_name = $('.tab-pane.active')[0].id;
tab_pane_valid_state[tab_name] = false;
ajaxRefreshTabPane(tab_name);
$('body > .modal-container .modal').modal('hide');
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index 86f525d..a6018bc 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -15,8 +15,8 @@ class ProjectsControllerTest < ActionController::TestCase
api_fixture("users")["future_project_user"]["uuid"]]
post(:share_with, {
id: api_fixture("groups")["asubproject"]["uuid"],
- format: "json",
- uuids: uuid_list},
+ uuids: uuid_list,
+ format: "json"},
session_for(:active))
assert_response :success
json_response = Oj.load(@response.body)
diff --git a/apps/workbench/test/integration/projects_test.rb b/apps/workbench/test/integration/projects_test.rb
index 846c57a..e3f79ff 100644
--- a/apps/workbench/test/integration/projects_test.rb
+++ b/apps/workbench/test/integration/projects_test.rb
@@ -92,11 +92,11 @@ class ProjectsTest < ActionDispatch::IntegrationTest
end
def add_share_and_check(share_type, name)
+ assert(page.has_no_text?(name), "project is already shared with #{name}")
start_share_count = count_shares
click_on("Share with #{share_type}")
find(".selectable", text: name).click
find(".modal-footer a,button", text: "Add").click
- # click_on "Sharing"; puts "FIXME: Manually re-clicking the Sharing tab shouldn't be necessary"
assert(page.has_link?(name),
"new share was not added to sharing table")
assert_equal(start_share_count + 1, count_shares,
@@ -125,7 +125,7 @@ class ProjectsTest < ActionDispatch::IntegrationTest
show_project_using("project_viewer")
click_on "Sharing"
assert(page.has_text?("Project Viewer"), "did not find self on sharing tab")
- assert(page.has_no_link?(/add a (user|group)/i),
+ assert(page.has_no_link?("Share with users"),
"read-only project user given option to add permissions")
assert_empty(all("#project_sharing a").
reject { |a| a[:href] =~ %r{/(users|groups)/[-0-9a-z]+$} },
@@ -138,8 +138,6 @@ class ProjectsTest < ActionDispatch::IntegrationTest
show_project_using("active")
click_on "Sharing"
- assert(page.has_no_text?(new_name),
- "project is already shared with user to add")
add_share_and_check("users", new_name)
modify_share_and_check(new_name)
end
@@ -149,8 +147,6 @@ class ProjectsTest < ActionDispatch::IntegrationTest
show_project_using("active")
click_on "Sharing"
- assert(page.has_no_text?(new_name),
- "project is already shared with group to add")
add_share_and_check("groups", new_name)
modify_share_and_check(new_name)
end
commit 3270dae2ff541e5fd8fba9a89b9cfe07127f2800
Author: Brett Smith <brett at curoverse.com>
Date: Thu Jul 17 11:39:24 2014 -0400
2044: Workbench can query the API /permissions method.
diff --git a/apps/workbench/app/models/link.rb b/apps/workbench/app/models/link.rb
index 868082b..271fa0f 100644
--- a/apps/workbench/app/models/link.rb
+++ b/apps/workbench/app/models/link.rb
@@ -8,4 +8,14 @@ class Link < ArvadosBase
def default_name
self.class.resource_class_for_uuid(head_uuid).default_name rescue super
end
+
+ def self.permissions_for(thing)
+ if thing.respond_to? :uuid
+ uuid = thing.uuid
+ else
+ uuid = thing
+ end
+ result = arvados_api_client.api("permissions", "/#{uuid}")
+ arvados_api_client.unpack_api_response(result)
+ end
end
diff --git a/apps/workbench/test/unit/link_test.rb b/apps/workbench/test/unit/link_test.rb
index 944bfce..7636335 100644
--- a/apps/workbench/test/unit/link_test.rb
+++ b/apps/workbench/test/unit/link_test.rb
@@ -1,7 +1,48 @@
require 'test_helper'
class LinkTest < ActiveSupport::TestCase
- # test "the truth" do
- # assert true
- # end
+ def uuid_for(fixture_name, object_name)
+ api_fixture(fixture_name)[object_name]["uuid"]
+ end
+
+ test "active user can get permissions for owned project object" do
+ use_token :active
+ project = Group.find(uuid_for("groups", "aproject"))
+ refute_empty(Link.permissions_for(project),
+ "no permissions found for managed project")
+ end
+
+ test "active user can get permissions for owned project by UUID" do
+ use_token :active
+ refute_empty(Link.permissions_for(uuid_for("groups", "aproject")),
+ "no permissions found for managed project")
+ end
+
+ test "admin can get permissions for project object" do
+ use_token :admin
+ project = Group.find(uuid_for("groups", "aproject"))
+ refute_empty(Link.permissions_for(project),
+ "no permissions found for managed project")
+ end
+
+ test "admin can get permissions for project by UUID" do
+ use_token :admin
+ refute_empty(Link.permissions_for(uuid_for("groups", "aproject")),
+ "no permissions found for managed project")
+ end
+
+ test "project viewer can't get permissions for readable project object" do
+ use_token :project_viewer
+ project = Group.find(uuid_for("groups", "aproject"))
+ assert_raises(ArvadosApiClient::AccessForbiddenException) do
+ Link.permissions_for(project)
+ end
+ end
+
+ test "project viewer can't get permissions for readable project by UUID" do
+ use_token :project_viewer
+ assert_raises(ArvadosApiClient::AccessForbiddenException) do
+ Link.permissions_for(uuid_for("groups", "aproject"))
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list