[ARVADOS] updated: 2fccbc1d172fe4bd680651261adfdca8f1ba2a63

git at public.curoverse.com git at public.curoverse.com
Mon Jul 21 12:41:10 EDT 2014


Summary of changes:
 .../app/assets/javascripts/select_modal.js         |  5 ++
 .../app/controllers/projects_controller.rb         | 44 +++++++++-
 apps/workbench/app/helpers/application_helper.rb   | 61 +++++++-------
 apps/workbench/app/models/arvados_base.rb          |  4 +
 apps/workbench/app/models/arvados_resource_list.rb |  7 ++
 apps/workbench/app/models/link.rb                  | 10 +++
 .../app/views/application/_choose.html.erb         |  8 +-
 .../_choose_rows.html.erb                          |  5 +-
 .../app/views/projects/_show_permissions.html.erb  | 44 ----------
 .../app/views/projects/_show_sharing.html.erb      | 95 ++++++++++++++++++++++
 .../_choose_rows.html.erb                          |  7 +-
 apps/workbench/config/routes.rb                    |  6 +-
 .../test/functional/projects_controller_test.rb    | 54 ++++++++++++
 apps/workbench/test/integration/projects_test.rb   | 73 +++++++++++++++--
 apps/workbench/test/unit/group_test.rb             |  9 ++
 apps/workbench/test/unit/link_test.rb              | 47 ++++++++++-
 apps/workbench/test/unit/user_test.rb              |  8 ++
 .../app/controllers/arvados/v1/links_controller.rb | 14 +++-
 services/api/app/models/link.rb                    | 11 +++
 services/api/test/fixtures/groups.yml              | 11 +++
 services/api/test/fixtures/links.yml               | 17 ++++
 services/api/test/fixtures/users.yml               | 11 +++
 .../functional/arvados/v1/links_controller_test.rb | 67 +++++++++++++++
 services/api/test/unit/link_test.rb                | 42 ++++++++++
 24 files changed, 564 insertions(+), 96 deletions(-)
 copy apps/workbench/app/views/{pipeline_templates => groups}/_choose_rows.html.erb (57%)
 delete mode 100644 apps/workbench/app/views/projects/_show_permissions.html.erb
 create mode 100644 apps/workbench/app/views/projects/_show_sharing.html.erb
 copy apps/workbench/app/views/{pipeline_templates => users}/_choose_rows.html.erb (50%)

       via  2fccbc1d172fe4bd680651261adfdca8f1ba2a63 (commit)
       via  46a7d27450216b1e022d23396931aa1b46c58c73 (commit)
       via  189016bba4c9c0ef2f66663dd8ff38843c8b25e1 (commit)
       via  13f5b1cf8db5a63b825c3dc81520c69500376adb (commit)
       via  67e443d5d1e4a6b6dfe671b3f4310258d386e4f2 (commit)
       via  54050447a56ab3e019746f12eb36b026dd5d2fb0 (commit)
       via  83f2087d9950975591ce5fe9c7f7c9e5db6749d9 (commit)
       via  1b785fd50c259eb42cd2c8c4ba2e440d7bfe0032 (commit)
      from  8d3335359e3785be35818b0c4519cc499f3f2fa8 (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 2fccbc1d172fe4bd680651261adfdca8f1ba2a63
Merge: 8d33353 46a7d27
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Jul 21 12:42:03 2014 -0400

    Merge branch '2044-workbench-project-sharing'
    
    Closes #2044, #2749, #3238.


commit 46a7d27450216b1e022d23396931aa1b46c58c73
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Jul 15 16:16:38 2014 -0400

    2044: Add Workbench interface to manage project sharing.
    
    This empowers administrators to add, modify, and remove permission
    links on projects.

diff --git a/apps/workbench/app/assets/javascripts/select_modal.js b/apps/workbench/app/assets/javascripts/select_modal.js
index 0a58213..8220945 100644
--- a/apps/workbench/app/assets/javascripts/select_modal.js
+++ b/apps/workbench/app/assets/javascripts/select_modal.js
@@ -68,6 +68,11 @@ $(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) {
+    var tab_name = $('.tab-pane.active')[0].id;
+    tab_pane_valid_state[tab_name] = false;
+    ajaxRefreshTabPane(tab_name);
+    $('body > .modal-container .modal').modal('hide');
 }).on('redirect-to-created-object', function(event, data, status, jqxhr, action_data) {
     window.location.href = data.href.replace(/^[^\/]*\/\/[^\/]*/, '');
 });
diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb
index be92ed3..31cb5ef 100644
--- a/apps/workbench/app/controllers/projects_controller.rb
+++ b/apps/workbench/app/controllers/projects_controller.rb
@@ -8,7 +8,11 @@ class ProjectsController < ApplicationController
   end
 
   def show_pane_list
-    %w(Contents Permissions Advanced)
+    if @user_is_manager
+      %w(Contents Sharing Advanced)
+    else
+      %w(Contents Advanced)
+    end
   end
 
   def remove_item
@@ -80,9 +84,21 @@ 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"]).
+      filter([['is_active', '=', 'true']])
+    @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 = []
+      @user_is_manager = false
+    end
 
     @objects_and_names = []
     @objects.each do |object|
diff --git a/apps/workbench/app/views/application/_choose.html.erb b/apps/workbench/app/views/application/_choose.html.erb
index a3ca815..dd4ed6f 100644
--- a/apps/workbench/app/views/application/_choose.html.erb
+++ b/apps/workbench/app/views/application/_choose.html.erb
@@ -36,16 +36,20 @@
           </div>
         </nav>
 
+        <% preview_pane = (params[:preview_pane] != "false")
+           pane_col_class = preview_pane ? "col-sm-6" : "" %>
         <div class="row" style="height: 20em">
-          <div class="col-sm-6 container-fluid arv-filterable-list selectable-container"
+          <div class="<%= pane_col_class %> container-fluid arv-filterable-list selectable-container"
                style="height: 100%; overflow-y: scroll"
                data-infinite-scroller="#choose-scroll"
                id="choose-scroll"
                data-infinite-content-href="<%= @next_page_href %>">
             <%= render partial: 'choose_rows', locals: {multiple: multiple} %>
           </div>
-          <div class="col-sm-6 modal-dialog-preview-pane" style="height: 100%; overflow-y: scroll">
+          <% if preview_pane %>
+          <div class="<%= pane_col_class %> modal-dialog-preview-pane" style="height: 100%; overflow-y: scroll">
           </div>
+          <% end %>
         </div>
 
         <div class="modal-footer">
diff --git a/apps/workbench/app/views/groups/_choose_rows.html.erb b/apps/workbench/app/views/groups/_choose_rows.html.erb
new file mode 100644
index 0000000..772ef19
--- /dev/null
+++ b/apps/workbench/app/views/groups/_choose_rows.html.erb
@@ -0,0 +1,9 @@
+<% icon_class = fa_icon_class_for_class(Group) %>
+<% @objects.each do |object| %>
+  <div class="row filterable selectable <%= 'multiple' if multiple %>" data-object-uuid="<%= object.uuid %>">
+    <div class="col-sm-12" style="overflow-x:hidden">
+      <i class="fa fa-fw <%= icon_class %>"></i>
+      <%= object.name %>
+    </div>
+  </div>
+<% end %>
diff --git a/apps/workbench/app/views/projects/_show_permissions.html.erb b/apps/workbench/app/views/projects/_show_permissions.html.erb
deleted file mode 100644
index 0817062..0000000
--- a/apps/workbench/app/views/projects/_show_permissions.html.erb
+++ /dev/null
@@ -1,44 +0,0 @@
-<div>
-  <div class="row">
-    <div class="col-md-6 col-md-push-6">
-      <div class="panel panel-default">
-        <div class="panel-heading">
-          Additional permissions
-        </div>
-        <div class="panel-body">
-          <p>
-            <% if not @share_links.any? %>
-              <span class="deemphasize">(No additional permissions)</span>
-            <% else %>
-              Also shared with:
-              <% @share_links.andand.each do |link| %>
-                <br /><%= link_to_if_arvados_object link.tail_uuid, friendly_name: true %>
-              <% end %>
-            <% end %>
-          </p>
-        </div>
-      </div>
-    </div>
-    <div class="col-md-6 col-md-pull-6">
-      <% if @object.owner %>
-        <div class="panel panel-default">
-          <div class="panel-heading">
-            Inherited permissions
-          </div>
-          <div class="panel-body">
-            <p>Permissions for this project are inherited by the owner or parent project:
-            </p>
-            <p style="margin-left: 4em">
-              <% if User == resource_class_for_uuid(@object.owner_uuid) %>
-                <i class="fa fa-fw fa-user"></i>
-              <% else %>
-                <i class="fa fa-fw fa-folder"></i>
-              <% end %>
-              <%= link_to_if_arvados_object @object.owner_uuid, friendly_name: true %>
-            </p>
-          </div>
-        </div>
-      <% end %>
-    </div>
-  </div>
-</div>
diff --git a/apps/workbench/app/views/projects/_show_sharing.html.erb b/apps/workbench/app/views/projects/_show_sharing.html.erb
new file mode 100644
index 0000000..a5482ef
--- /dev/null
+++ b/apps/workbench/app/views/projects/_show_sharing.html.erb
@@ -0,0 +1,95 @@
+<%
+   uuid_map = {}
+   [@users, @groups].each do |obj_list|
+     obj_list.each { |o| uuid_map[o.uuid] = o }
+   end
+   perm_name_desc_map = {}
+   perm_desc_name_map = {}
+   perms_json = []
+   ['Read', 'Write', 'Manage'].each do |link_desc|
+     link_name = "can_#{link_desc.downcase}"
+     perm_name_desc_map[link_name] = link_desc
+     perm_desc_name_map[link_desc] = link_name
+     perms_json << {value: link_name, text: link_desc}
+   end
+   perms_json = perms_json.to_json
+   owner_icon = fa_icon_class_for_uuid(@object.owner_uuid)
+   if owner_icon == "fa-users"
+     owner_icon = "fa-folder"
+     owner_type = "parent project"
+   else
+     owner_type = "owning user"
+   end
+%>
+
+<div class="pull-right">
+  <% ["users", "groups"].each do |share_class| %>
+
+  <%= link_to(send("choose_#{share_class}_path",
+      title: "Share with #{share_class}",
+      preview_pane: false,
+      multiple: true,
+      limit: 10000,
+      action_method: 'post',
+      action_href: share_with_project_path,
+      action_name: 'Add',
+      action_data: {selection_param: 'uuids[]', success: 'tab-refresh'}.to_json),
+      class: "btn btn-primary btn-sm", remote: true, method: 'get') do %>
+  <i class="fa fa-fw fa-plus"></i> Share with <%= share_class %>…
+  <% end %>
+
+  <% end %>
+</div>
+
+<p>Permissions for this project are inherited from the <%= owner_type %>
+  <i class="fa fa-fw <%= owner_icon %>"></i>
+  <%= link_to_if_arvados_object @object.owner_uuid, friendly_name: true %>.
+</p>
+
+<table id="project_sharing" class="topalign table" style="clear: both; margin-top: 1em;">
+  <tr>
+    <th>User/Group Name</th>
+    <th colspan="2">Project Access</th>
+  </tr>
+
+  <% @share_links.andand.each do |link|
+       shared_with = uuid_map[link.tail_uuid]
+       if shared_with.nil?
+         link_name = link.tail_uuid
+       elsif shared_with.respond_to?(:full_name)
+         link_name = shared_with.full_name
+       else
+         link_name = shared_with.name
+       end %>
+  <tr data-object-uuid="<%= link.uuid %>">
+    <td>
+      <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>
+    <td><%= link_to perm_name_desc_map[link.name], '#', {
+      "data-emptytext" => "Read",
+      "data-placement" => "bottom",
+      "data-type" => "select",
+      "data-url" => url_for(action: "update", id: link.uuid, controller: "links", merge: true),
+      "data-title" => "Set #{link_name}'s access level",
+      "data-name" => "[name]",
+      "data-pk" => {id: link.tail_uuid, key: "link"}.to_json,
+      "data-value" => link.name,
+      "data-clear" => false,
+      "data-source" => perms_json,
+      "data-tpl" => "<select id=\"share_change_level\"></select>",
+      "class" => "editable form-control",
+      } %>
+    </td>
+    <td>
+      <%= link_to(
+          {action: 'destroy', id: link.uuid, controller: "links"},
+          {title: 'Revoke', class: 'btn btn-default btn-nodecorate', method: :delete,
+           data: {confirm: "Revoke #{link_name}'s access to this project?",
+                  remote: true}}) do %>
+      <i class="fa fa-fw fa-trash-o"></i>
+      <% end %>
+    </td>
+  </tr>
+  <% end %>
+</table>
diff --git a/apps/workbench/app/views/users/_choose_rows.html.erb b/apps/workbench/app/views/users/_choose_rows.html.erb
new file mode 100644
index 0000000..a893e5e
--- /dev/null
+++ b/apps/workbench/app/views/users/_choose_rows.html.erb
@@ -0,0 +1,9 @@
+<% icon_class = fa_icon_class_for_class(User) %>
+<% @objects.each do |object| %>
+  <div class="row filterable selectable <%= 'multiple' if multiple %>" data-object-uuid="<%= object.uuid %>">
+    <div class="col-sm-12" style="overflow-x:hidden">
+      <i class="fa fa-fw <%= icon_class %>"></i>
+      <%= object.full_name %>
+    </div>
+  </div>
+<% end %>
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index 9701f41..55e2fda 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -24,6 +24,7 @@ ArvadosWorkbench::Application.routes.draw do
   match '/logout' => 'sessions#destroy', via: [:get, :post]
   get '/logged_out' => 'sessions#index'
   resources :users do
+    get 'choose', :on => :collection
     get 'home', :on => :member
     get 'welcome', :on => :collection
     get 'activity', :on => :collection
@@ -35,7 +36,9 @@ ArvadosWorkbench::Application.routes.draw do
   resources :logs
   resources :factory_jobs
   resources :uploaded_datasets
-  resources :groups
+  resources :groups do
+    get 'choose', on: :collection
+  end
   resources :specimens
   resources :pipeline_templates do
     get 'choose', on: :collection
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index 3c6f0f9..ff56928 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -34,8 +34,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)
@@ -50,4 +50,36 @@ 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))
+    is_manager = assigns(:user_is_manager)
+    assert_not_nil(is_manager, "user_is_manager flag not set")
+    if not is_manager
+      assert_empty(assigns(:share_links),
+                   "non-manager has share links set")
+    end
+    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
diff --git a/apps/workbench/test/integration/projects_test.rb b/apps/workbench/test/integration/projects_test.rb
index 5758fc7..d1bf403 100644
--- a/apps/workbench/test/integration/projects_test.rb
+++ b/apps/workbench/test/integration/projects_test.rb
@@ -75,13 +75,74 @@ class ProjectsTest < ActionDispatch::IntegrationTest
     find('.modal-footer a,button', text: 'Move').click
     wait_for_ajax
 
-    # Wait for the page to refresh and show the new parent in Permissions panel
-    click_link 'Permissions'
-    find('.panel', text: 'Project 1234')
-
-    assert(find('.panel', text: 'Permissions for this project are inherited by the owner or parent project').
-           all('*', text: 'Project 1234').any?,
+    # Wait for the page to refresh and show the new parent in Sharing panel
+    click_link 'Sharing'
+    assert(page.has_link?("Project 1234"),
            "Project 5678 should now be inside project 1234")
   end
 
+  def show_project_using(auth_key, proj_key='aproject')
+    project_uuid = api_fixture('groups')[proj_key]['uuid']
+    visit(page_with_token(auth_key, "/projects/#{project_uuid}"))
+    assert(page.has_text?("A Project"), "not on expected project page")
+  end
+
+  def count_shares
+    page.all("#project_sharing tr").size
+  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
+    assert(page.has_link?(name),
+           "new share was not added to sharing table")
+    assert_equal(start_share_count + 1, count_shares,
+                 "new share did not add row to sharing table")
+  end
+
+  def modify_share_and_check(name)
+    start_share_count = count_shares
+    link_row = all("#project_sharing tr").select { |row| row.has_text?(name) }
+    assert_equal(1, link_row.size, "row with new permission not found")
+    within(link_row.first) do
+      click_on("Read")
+      select("Write", from: "share_change_level")
+      click_on("editable-submit")
+      assert(has_link?("Write"),
+             "failed to change access level on new share")
+      click_on "Revoke"
+    end
+    assert(page.has_no_text?(name),
+           "new share row still exists after being revoked")
+    assert_equal(start_share_count - 1, count_shares,
+                 "revoking share did not remove row from sharing table")
+  end
+
+  test "project viewer can't see project sharing tab" do
+    show_project_using("project_viewer")
+    assert(page.has_no_link?("Sharing"),
+           "read-only project user sees sharing tab")
+  end
+
+  test "project owner can manage sharing for another user" do
+    add_user = api_fixture('users')['future_project_user']
+    new_name = ["first_name", "last_name"].map { |k| add_user[k] }.join(" ")
+
+    show_project_using("active")
+    click_on "Sharing"
+    add_share_and_check("users", new_name)
+    modify_share_and_check(new_name)
+  end
+
+  test "project owner can manage sharing for another group" do
+    new_name = api_fixture('groups')['future_project_viewing_group']['name']
+
+    show_project_using("active")
+    click_on "Sharing"
+    add_share_and_check("groups", new_name)
+    modify_share_and_check(new_name)
+  end
 end

commit 189016bba4c9c0ef2f66663dd8ff38843c8b25e1
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Jul 21 12:18:24 2014 -0400

    2044: Workbench route to share a project with others.
    
    This route is intended to support AJAX requests from the interface to
    share a project with one or more users/groups.

diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb
index 91d6e8a..be92ed3 100644
--- a/apps/workbench/app/controllers/projects_controller.rb
+++ b/apps/workbench/app/controllers/projects_controller.rb
@@ -131,4 +131,26 @@ class ProjectsController < ApplicationController
     @updates = params['project']
     super
   end
+
+  def share_with
+    if not params[:uuids].andand.any?
+      @errors = ["No user/group UUIDs specified to share with."]
+      return render_error(status: 422)
+    end
+    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] = error.api_response.andand[:errors]
+      else
+        results["success"] << shared_uuid
+      end
+    end
+    status = (results["failure"].empty?) ? 200 : 422
+    respond_to do |f|
+      f.json { render(json: results, status: status) }
+    end
+  end
 end
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index a4f69b3..9701f41 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -61,6 +61,7 @@ ArvadosWorkbench::Application.routes.draw do
     match 'remove/:item_uuid', on: :member, via: :delete, action: :remove_item
     match 'remove_items', on: :member, via: :delete, action: :remove_items
     get 'choose', on: :collection
+    post 'share_with', on: :member
   end
 
   post 'actions' => 'actions#post'
diff --git a/apps/workbench/test/functional/projects_controller_test.rb b/apps/workbench/test/functional/projects_controller_test.rb
index a991ced..3c6f0f9 100644
--- a/apps/workbench/test/functional/projects_controller_test.rb
+++ b/apps/workbench/test/functional/projects_controller_test.rb
@@ -28,4 +28,26 @@ class ProjectsControllerTest < ActionController::TestCase
       end
     end
   end
+
+  test "sharing a project with a user and group" do
+    uuid_list = [api_fixture("groups")["future_project_viewing_group"]["uuid"],
+                 api_fixture("users")["future_project_user"]["uuid"]]
+    post(:share_with, {
+           id: api_fixture("groups")["asubproject"]["uuid"],
+           format: "json",
+           uuids: uuid_list},
+         session_for(:active))
+    assert_response :success
+    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
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index cd6157b..62b9958 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -68,6 +68,17 @@ asubproject:
   description: "Test project belonging to active user's first test project"
   group_class: folder
 
+future_project_viewing_group:
+  uuid: zzzzz-j7d0g-futrprojviewgrp
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-04-21 15:37:48 -0400
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2014-04-21 15:37:48 -0400
+  updated_at: 2014-04-21 15:37:48 -0400
+  name: Future Project Viewing Group
+  description: "Group used to test granting Group Project viewing"
+
 bad_group_has_ownership_cycle_a:
   uuid: zzzzz-j7d0g-cx2al9cqkmsf1hs
   owner_uuid: zzzzz-j7d0g-0077nzts8c178lw
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 72a5aa3..acb67b4 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -55,6 +55,17 @@ project_viewer:
   is_admin: false
   prefs: {}
 
+future_project_user:
+  # Workbench tests give this user permission on aproject.
+  uuid: zzzzz-tpzed-futureprojview2
+  email: future-project-user at arvados.local
+  first_name: Future Project
+  last_name: User
+  identity_url: https://future-project-user.openid.local
+  is_active: true
+  is_admin: false
+  prefs: {}
+
 spectator:
   owner_uuid: zzzzz-tpzed-000000000000000
   uuid: zzzzz-tpzed-l1s2piq4t4mps8r

commit 13f5b1cf8db5a63b825c3dc81520c69500376adb
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

commit 67e443d5d1e4a6b6dfe671b3f4310258d386e4f2
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Jul 15 15:35:12 2014 -0400

    2044: Add more helpers to get FA icons for objects.

diff --git a/apps/workbench/app/helpers/application_helper.rb b/apps/workbench/app/helpers/application_helper.rb
index d1bac0c..c3856c2 100644
--- a/apps/workbench/app/helpers/application_helper.rb
+++ b/apps/workbench/app/helpers/application_helper.rb
@@ -393,38 +393,39 @@ module ApplicationHelper
     end
   end
 
-  def fa_icon_class_for_object object
-    case object.class.to_s.to_sym
-    when :User
-      'fa-user'
-    when :Group
+  RESOURCE_CLASS_ICONS = {
+    "Collection" => "fa-archive",
+    "Group" => "fa-users",
+    "Human" => "fa-male",  # FIXME: Use a more inclusive icon.
+    "Job" => "fa-gears",
+    "KeepDisk" => "fa-hdd-o",
+    "KeepService" => "fa-exchange",
+    "Link" => "fa-arrows-h",
+    "Node" => "fa-cloud",
+    "PipelineInstance" => "fa-gears",
+    "PipelineTemplate" => "fa-gears",
+    "Repository" => "fa-code-fork",
+    "Specimen" => "fa-flask",
+    "Trait" => "fa-clipboard",
+    "User" => "fa-user",
+    "VirtualMachine" => "fa-terminal",
+  }
+  DEFAULT_ICON_CLASS = "fa-cube"
+
+  def fa_icon_class_for_class(resource_class, default=DEFAULT_ICON_CLASS)
+    RESOURCE_CLASS_ICONS.fetch(resource_class.to_s, default)
+  end
+
+  def fa_icon_class_for_uuid(uuid, default=DEFAULT_ICON_CLASS)
+    fa_icon_class_for_class(resource_class_for_uuid(uuid), default)
+  end
+
+  def fa_icon_class_for_object(object, default=DEFAULT_ICON_CLASS)
+    case class_name = object.class.to_s
+    when "Group"
       object.group_class ? 'fa-folder' : 'fa-users'
-    when :Job, :PipelineInstance, :PipelineTemplate
-      'fa-gears'
-    when :Collection
-      'fa-archive'
-    when :Specimen
-      'fa-flask'
-    when :Trait
-      'fa-clipboard'
-    when :Human
-      'fa-male'
-    when :VirtualMachine
-      'fa-terminal'
-    when :Repository
-      'fa-code-fork'
-    when :Link
-      'fa-arrows-h'
-    when :User
-      'fa-user'
-    when :Node
-      'fa-cloud'
-    when :KeepService
-      'fa-exchange'
-    when :KeepDisk
-      'fa-hdd-o'
     else
-      'fa-cube'
+      RESOURCE_CLASS_ICONS.fetch(class_name, default)
     end
   end
 end

commit 54050447a56ab3e019746f12eb36b026dd5d2fb0
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jul 3 14:10:31 2014 -0400

    2044: Workbench supports API select parameter.
    
    This can help ensure symmetry between admin and non-admin requests for
    the user index.

diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb
index 6d427fd..1dac43f 100644
--- a/apps/workbench/app/models/arvados_base.rb
+++ b/apps/workbench/app/models/arvados_base.rb
@@ -131,6 +131,10 @@ class ArvadosBase < ActiveRecord::Base
     ArvadosResourceList.new(self).limit(*args)
   end
 
+  def self.select(*args)
+    ArvadosResourceList.new(self).select(*args)
+  end
+
   def self.eager(*args)
     ArvadosResourceList.new(self).eager(*args)
   end
diff --git a/apps/workbench/app/models/arvados_resource_list.rb b/apps/workbench/app/models/arvados_resource_list.rb
index dedd18c..3164c79 100644
--- a/apps/workbench/app/models/arvados_resource_list.rb
+++ b/apps/workbench/app/models/arvados_resource_list.rb
@@ -26,6 +26,12 @@ class ArvadosResourceList
     self
   end
 
+  def select(columns)
+    @select ||= []
+    @select += columns
+    self
+  end
+
   def filter _filters
     @filters ||= []
     @filters += _filters
@@ -64,6 +70,7 @@ class ArvadosResourceList
     api_params[:eager] = '1' if @eager
     api_params[:limit] = @limit if @limit
     api_params[:offset] = @offset if @offset
+    api_params[:select] = @select if @select
     api_params[:order] = @orderby_spec if @orderby_spec
     api_params[:filters] = @filters if @filters
     res = arvados_api_client.api @resource_class, '', api_params
diff --git a/apps/workbench/test/unit/group_test.rb b/apps/workbench/test/unit/group_test.rb
index 435463a..c555c4a 100644
--- a/apps/workbench/test/unit/group_test.rb
+++ b/apps/workbench/test/unit/group_test.rb
@@ -25,4 +25,13 @@ class GroupTest < ActiveSupport::TestCase
     assert_equal(expect_name, oi.name_for(expect_uuid),
                  "Expected name_for '#{expect_uuid}' to be '#{expect_name}'")
   end
+
+  test "can select specific group columns" do
+    use_token :admin
+    Group.select(["uuid", "name"]).limit(5).each do |user|
+      assert_not_nil user.uuid
+      assert_not_nil user.name
+      assert_nil user.owner_uuid
+    end
+  end
 end
diff --git a/apps/workbench/test/unit/user_test.rb b/apps/workbench/test/unit/user_test.rb
index 20cde7e..89e95df 100644
--- a/apps/workbench/test/unit/user_test.rb
+++ b/apps/workbench/test/unit/user_test.rb
@@ -1,4 +1,12 @@
 require 'test_helper'
 
 class UserTest < ActiveSupport::TestCase
+  test "can select specific user columns" do
+    use_token :admin
+    User.select(["uuid", "is_active"]).limit(5).each do |user|
+      assert_not_nil user.uuid
+      assert_not_nil user.is_active
+      assert_nil user.first_name
+    end
+  end
 end

commit 83f2087d9950975591ce5fe9c7f7c9e5db6749d9
Author: Brett Smith <brett at curoverse.com>
Date:   Fri Jul 18 16:13:11 2014 -0400

    2044: API server lets group managers see group permission links.
    
    This implementation hooks into find_object_by_uuid because that was
    the simplest thing that could work and unblock larger development on
    this story.  Longer-term, we would like to solve this problem more
    generally.  See the comments added to the links controller test for
    more explanation about that.

diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb
index f76af60..798217d 100644
--- a/services/api/app/controllers/arvados/v1/links_controller.rb
+++ b/services/api/app/controllers/arvados/v1/links_controller.rb
@@ -34,16 +34,26 @@ class Arvados::V1::LinksController < ApplicationController
 
   protected
 
-  # Override find_object_by_uuid: the get_permissions method may be
-  # called on a uuid belonging to any class.
   def find_object_by_uuid
     if action_name == 'get_permissions'
+      # get_permissions accepts a UUID for any kind of object.
       @object = ArvadosModel::resource_class_for_uuid(params[:uuid])
         .readable_by(*@read_users)
         .where(uuid: params[:uuid])
         .first
     else
       super
+      if @object.nil?
+        # Normally group permission links are not readable_by users.
+        # Make an exception for users with permission to manage the group.
+        # FIXME: Solve this more generally - see the controller tests.
+        link = Link.find_by_uuid(params[:uuid])
+        if (not link.nil?) and
+            (link.link_class == "permission") and
+            (@read_users.any? { |u| u.can?(manage: link.head_uuid) })
+          @object = link
+        end
+      end
     end
   end
 
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 313db7f..1227618 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -318,6 +318,23 @@ test_timestamps:
   name: test
   properties: {}
 
+admin_can_write_aproject:
+  # Yes, this permission is effectively redundant.
+  # We use it to test that other project admins can see
+  # all the project's sharing.
+  uuid: zzzzz-o0j2j-adminmgsproject
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2014-01-24 20:42:26 -0800
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2014-01-24 20:42:26 -0800
+  updated_at: 2014-01-24 20:42:26 -0800
+  tail_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  link_class: permission
+  name: can_write
+  head_uuid: zzzzz-j7d0g-v955i6s2oi1cbso
+  properties: {}
+
 project_viewer_can_read_project:
   uuid: zzzzz-o0j2j-projviewerreadp
   owner_uuid: zzzzz-tpzed-000000000000000
diff --git a/services/api/test/functional/arvados/v1/links_controller_test.rb b/services/api/test/functional/arvados/v1/links_controller_test.rb
index d5b4266..b131947 100644
--- a/services/api/test/functional/arvados/v1/links_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/links_controller_test.rb
@@ -283,4 +283,71 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
     }
     assert_response 422
   end
+
+  test "project owner can show a project permission" do
+    uuid = links(:project_viewer_can_read_project).uuid
+    authorize_with :active
+    get :show, id: uuid
+    assert_response :success
+    assert_equal(uuid, assigns(:object).andand.uuid)
+  end
+
+  test "admin can show a project permission" do
+    uuid = links(:project_viewer_can_read_project).uuid
+    authorize_with :admin
+    get :show, id: uuid
+    assert_response :success
+    assert_equal(uuid, assigns(:object).andand.uuid)
+  end
+
+  test "project viewer can't show others' project permissions" do
+    authorize_with :project_viewer
+    get :show, id: links(:admin_can_write_aproject).uuid
+    assert_response 404
+  end
+
+  test "requesting a nonexistent link returns 404" do
+    authorize_with :active
+    get :show, id: 'zzzzz-zzzzz-zzzzzzzzzzzzzzz'
+    assert_response 404
+  end
+
+  test "project owner can index project permissions" do
+    skip "Test tickles known bug"
+    # readable_by only lets users see permission links that relate to them
+    # directly.  It does not allow users to see permission links for groups
+    # they manage.
+    # We'd like to fix this general issue, but we haven't settled on a general
+    # way to do it that doesn't involve making readable_by ridiculously hairy.
+    # This test demonstrates the desired behavior once we're ready to tackle
+    # it.  In the meantime, clients should use /permissions to get this
+    # information.
+    authorize_with :active
+    get :index, filters: [['link_class', '=', 'permission'],
+                          ['head_uuid', '=', groups(:aproject).uuid]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map(&:uuid),
+                    links(:project_viewer_can_read_project).uuid)
+  end
+
+  test "admin can index project permissions" do
+    authorize_with :admin
+    get :index, filters: [['link_class', '=', 'permission'],
+                          ['head_uuid', '=', groups(:aproject).uuid]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map(&:uuid),
+                    links(:project_viewer_can_read_project).uuid)
+  end
+
+  test "project viewer can't index others' project permissions" do
+    authorize_with :project_viewer
+    get :index, filters: [['link_class', '=', 'permission'],
+                          ['head_uuid', '=', groups(:aproject).uuid],
+                          ['tail_uuid', '!=', users(:project_viewer).uuid]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_empty assigns(:objects)
+  end
 end

commit 1b785fd50c259eb42cd2c8c4ba2e440d7bfe0032
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Jul 14 17:07:39 2014 -0400

    2044: All users can give all other users folder permissions.
    
    This supports a Workbench sharing button, the same way that the
    changes to users#index does.

diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index bb069ee..6321145 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -104,4 +104,15 @@ class Link < ArvadosModel
       super
     end
   end
+
+  # A user can give all other users permissions on folders.
+  def skip_uuid_read_permission_check
+    skipped_attrs = super
+    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
+    skipped_attrs
+  end
 end
diff --git a/services/api/test/unit/link_test.rb b/services/api/test/unit/link_test.rb
index e403265..640b26c 100644
--- a/services/api/test/unit/link_test.rb
+++ b/services/api/test/unit/link_test.rb
@@ -61,4 +61,46 @@ class LinkTest < ActiveSupport::TestCase
       ob.destroy
     end
   end
+
+  def new_active_link_valid?(link_attrs)
+    set_user_from_auth :active
+    begin
+      Link.
+        create({link_class: "permission",
+                 name: "can_read",
+                 head_uuid: groups(:aproject).uuid,
+               }.merge(link_attrs)).
+        valid?
+    rescue ArvadosModel::PermissionDeniedError
+      false
+    end
+  end
+
+  test "link granting permission to nonexistent user is invalid" do
+    refute new_active_link_valid?(tail_uuid:
+                                  users(:active).uuid.sub(/-\w+$/, "-#{'z' * 15}"))
+  end
+
+  test "link granting non-project permission to unreadable user is invalid" do
+    refute new_active_link_valid?(tail_uuid: users(:admin).uuid,
+                                  head_uuid: collections(:bar_file).uuid)
+  end
+
+  test "user can't add a Collection to a Project without permission" do
+    refute new_active_link_valid?(link_class: "name",
+                                  name: "Permission denied test name",
+                                  tail_uuid: collections(:bar_file).uuid)
+  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.
+    refute new_active_link_valid?(link_class: "name",
+                                  name: "Permission denied test name",
+                                  tail_uuid: users(:admin).uuid)
+  end
+
+  test "link granting project permissions to unreadable user is valid" do
+    assert new_active_link_valid?(tail_uuid: users(:admin).uuid)
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list