19146: Add can_write/can_manage to users#list, fix select=can_*.
authorTom Clegg <tom@curii.com>
Tue, 7 Jun 2022 15:09:57 +0000 (11:09 -0400)
committerTom Clegg <tom@curii.com>
Tue, 7 Jun 2022 15:09:57 +0000 (11:09 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

lib/controller/localdb/group_test.go
services/api/app/controllers/arvados/v1/users_controller.rb
services/api/app/models/group.rb
services/api/app/models/user.rb
services/api/test/functional/arvados/v1/users_controller_test.rb

index 1fde64d119a15892f560062895157238aa3a62e3..78150c95527dc0f66ed6187bd64e93c7aef428c8 100644 (file)
@@ -157,6 +157,17 @@ func (s *GroupSuite) TestCanWriteCanManageResponses(c *check.C) {
        c.Check(subproject.CanWrite, check.Equals, true)
        c.Check(subproject.CanManage, check.Equals, true)
 
+       projlist, err := s.localdb.GroupList(ctxUser1, arvados.ListOptions{
+               Limit:   -1,
+               Filters: []arvados.Filter{{"uuid", "in", []string{project.UUID, subproject.UUID}}},
+       })
+       c.Assert(err, check.IsNil)
+       c.Assert(projlist.Items, check.HasLen, 2)
+       for _, p := range projlist.Items {
+               c.Check(p.CanWrite, check.Equals, true)
+               c.Check(p.CanManage, check.Equals, true)
+       }
+
        // Give 2nd user permission to read
        permlink, err := s.localdb.LinkCreate(ctxAdmin, arvados.CreateOptions{
                Attrs: map[string]interface{}{
@@ -228,4 +239,30 @@ func (s *GroupSuite) TestCanWriteCanManageResponses(c *check.C) {
        c.Assert(err, check.IsNil)
        c.Check(subproject2.CanWrite, check.Equals, false)
        c.Check(subproject2.CanManage, check.Equals, true)
+
+       u, err := s.localdb.UserGet(ctxUser1, arvados.GetOptions{
+               UUID: arvadostest.ActiveUserUUID,
+       })
+       c.Assert(err, check.IsNil)
+       c.Check(u.CanWrite, check.Equals, true)
+       c.Check(u.CanManage, check.Equals, true)
+
+       for _, selectParam := range [][]string{
+               nil,
+               {"can_write", "can_manage"},
+       } {
+               c.Logf("selectParam: %+v", selectParam)
+               ulist, err := s.localdb.UserList(ctxUser1, arvados.ListOptions{
+                       Limit:   -1,
+                       Filters: []arvados.Filter{{"uuid", "=", arvadostest.ActiveUserUUID}},
+                       Select:  selectParam,
+               })
+               c.Assert(err, check.IsNil)
+               c.Assert(ulist.Items, check.HasLen, 1)
+               c.Logf("%+v", ulist.Items)
+               for _, u := range ulist.Items {
+                       c.Check(u.CanWrite, check.Equals, true)
+                       c.Check(u.CanManage, check.Equals, true)
+               }
+       }
 }
index 54db521768592742dd9c4406ff987f0d7c7ee496..507cb4ac339fe5fd63fbbf7fb3013411fc44b5e9 100644 (file)
@@ -274,7 +274,7 @@ class Arvados::V1::UsersController < ApplicationController
     return super if @read_users.any?(&:is_admin)
     if params[:uuid] != current_user.andand.uuid
       # Non-admin index/show returns very basic information about readable users.
-      safe_attrs = ["uuid", "is_active", "email", "first_name", "last_name", "username"]
+      safe_attrs = ["uuid", "is_active", "email", "first_name", "last_name", "username", "can_write", "can_manage"]
       if @select
         @select = @select & safe_attrs
       else
index e18ee5ef38748fa66c71fc1132af6d6e8cb6199f..0c36a048dcfd9f31679abf642c085e14e97dc1e4 100644 (file)
@@ -48,6 +48,15 @@ class Group < ArvadosModel
     t.add :can_manage
   end
 
+  protected
+
+  def self.attributes_required_columns
+    super.merge(
+                'can_write' => ['owner_uuid', 'uuid'],
+                'can_manage' => ['owner_uuid', 'uuid'],
+                )
+  end
+
   def ensure_filesystem_compatible_name
     # project and filter groups need filesystem-compatible names, but others
     # don't.
index 52b96f9c512699c8cc7f7f71b6bdd31804fd5902..444946613545dd300d23bce717c1d9d72f4066af 100644 (file)
@@ -584,6 +584,13 @@ SELECT target_uuid, perm_level
 
   protected
 
+  def self.attributes_required_columns
+    super.merge(
+                'can_write' => ['owner_uuid', 'uuid'],
+                'can_manage' => ['owner_uuid', 'uuid'],
+                )
+  end
+
   def change_all_uuid_refs(old_uuid:, new_uuid:)
     ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass|
       klass.columns.each do |col|
index ae7b21dec83e556f8321ee7290e5680824be5881..6a7b00a00573f9413a61856d66fd6e6eb3900c65 100644 (file)
@@ -674,6 +674,12 @@ The Arvados team.
     get(:index)
     check_non_admin_index
     check_readable_users_index [:spectator], [:inactive, :active]
+    json_response["items"].each do |u|
+      if u["uuid"] == users(:spectator).uuid
+        assert_equal true, u["can_write"]
+        assert_equal true, u["can_manage"]
+      end
+    end
   end
 
   test "non-admin user gets only safe attributes from users#show" do
@@ -1079,7 +1085,7 @@ The Arvados team.
   end
 
   NON_ADMIN_USER_DATA = ["uuid", "kind", "is_active", "email", "first_name",
-                         "last_name", "username"].sort
+                         "last_name", "username", "can_write", "can_manage"].sort
 
   def check_non_admin_index
     assert_response :success