From: Tom Clegg <tom@curii.com>
Date: Tue, 7 Jun 2022 15:09:57 +0000 (-0400)
Subject: 19146: Add can_write/can_manage to users#list, fix select=can_*.
X-Git-Tag: 2.5.0~138^2~1
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/9551b59d3aab67f77240b90bbb550faec6b2a7d9

19146: Add can_write/can_manage to users#list, fix select=can_*.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---

diff --git a/lib/controller/localdb/group_test.go b/lib/controller/localdb/group_test.go
index 1fde64d119..78150c9552 100644
--- a/lib/controller/localdb/group_test.go
+++ b/lib/controller/localdb/group_test.go
@@ -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)
+		}
+	}
 }
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 54db521768..507cb4ac33 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -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
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index e18ee5ef38..0c36a048dc 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -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.
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 52b96f9c51..4449466135 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -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|
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index ae7b21dec8..6a7b00a005 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -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