[ARVADOS] updated: 1.3.0-2679-g36e3f1ef5
Git user
git at public.arvados.org
Tue Jun 16 21:24:03 UTC 2020
Summary of changes:
apps/workbench/Gemfile.lock | 2 +-
.../app/views/layouts/application.html.erb | 4 +-
build/build-dev-docker-jobs-image.sh | 10 +-
build/run-build-docker-jobs-image.sh | 10 +-
build/run-library.sh | 17 +-
cmd/arvados-server/cmd.go | 2 +
doc/architecture/index.html.textile.liquid | 2 +-
doc/install/setup-login.html.textile.liquid | 14 +
.../topics/arvados-sync-groups.html.textile.liquid | 4 +-
lib/config/config.default.yml | 30 +-
lib/config/export.go | 5 +
lib/config/generated_config.go | 30 +-
lib/config/load.go | 18 +-
lib/controller/handler_test.go | 8 +-
lib/controller/localdb/login.go | 28 +-
.../localdb/{login_google.go => login_oidc.go} | 108 +++----
.../{login_google_test.go => login_oidc_test.go} | 132 +++++++--
lib/undelete/cmd.go | 316 +++++++++++++++++++++
lib/undelete/cmd_test.go | 117 ++++++++
sdk/cwl/setup.py | 2 +-
sdk/cwl/test_with_arvbox.sh | 21 +-
.../perms.go => arvados/blob_signature.go} | 19 +-
sdk/go/arvados/blob_signature_test.go | 88 ++++++
sdk/go/arvados/config.go | 6 +
sdk/go/arvados/container.go | 8 +
sdk/go/arvados/keep_service.go | 38 +++
sdk/go/keepclient/perms.go | 107 +------
sdk/go/keepclient/perms_test.go | 103 -------
sdk/python/setup.py | 1 +
services/api/Gemfile.lock | 2 +-
services/api/app/models/link.rb | 4 +-
services/api/app/models/user.rb | 13 +-
.../db/migrate/20200501150153_permission_table.rb | 65 +++--
services/api/db/structure.sql | 51 ++--
.../20200501150153_permission_table_constants.rb | 9 +-
services/api/lib/update_permissions.rb | 18 +-
services/api/test/unit/permission_test.rb | 119 ++++++++
services/keepstore/handler_test.go | 51 ++++
services/keepstore/handlers.go | 30 ++
services/keepstore/volume_test.go | 9 +-
tools/arvbox/lib/arvbox/docker/cluster-config.sh | 1 +
tools/arvbox/lib/arvbox/docker/common.sh | 8 +-
tools/sync-groups/sync-groups.go | 249 ++++++++++------
tools/sync-groups/sync-groups_test.go | 138 +++++++--
44 files changed, 1500 insertions(+), 517 deletions(-)
rename lib/controller/localdb/{login_google.go => login_oidc.go} (74%)
rename lib/controller/localdb/{login_google_test.go => login_oidc_test.go} (74%)
create mode 100644 lib/undelete/cmd.go
create mode 100644 lib/undelete/cmd_test.go
copy sdk/go/{keepclient/perms.go => arvados/blob_signature.go} (86%)
create mode 100644 sdk/go/arvados/blob_signature_test.go
delete mode 100644 sdk/go/keepclient/perms_test.go
discards 6fff1104ab84f8ed8683cd3a2d5b5d28832ca35e (commit)
discards 940e92544b0b52993315f3f7a92379941d826381 (commit)
discards 283027c14437ad5aa94a489a5a4ef29fdc666755 (commit)
discards a8683a6beb8c63d5db5442d5a3cc8c3c68a849e6 (commit)
discards 6508dc52a192117b9d2d4b20f8cd328844e85fa6 (commit)
discards fe054ed4c9019df6efcb93fdd0f55235048f6631 (commit)
discards 62cbe8a4373e5405ceb6fa4b3ef7acde2d85f9ea (commit)
discards 77dcd1d6de3782d2faa596dedac834045b925487 (commit)
discards 58201706fdf0cd305200d4bd5ff25922e281cdc2 (commit)
via 36e3f1ef5944fcb23eb9eda15457d15ce791e34b (commit)
via 8448f02e0bc329953f81268275e4157f643ec094 (commit)
via 9cebc0a070ef29777e2e797b74da66753bb22a25 (commit)
via eced1acec35d3b259b31bae8ea65856aca8add44 (commit)
via bc84a8ffbb1add0ebd8bc8bfb4a1721e4fea20a0 (commit)
via a95e63fb126abf0da69e0bb0591267babb501d3c (commit)
via 66002de50dfc4646a11df36d6e1d665bd507de58 (commit)
via 2aa0d4853c00b21ba5f1dd692b952fbb1b2ccee8 (commit)
via aa728d17c91d8d764fb3c6a4c10236c36baa0bbb (commit)
via 98ec1f0093bb097f9ccb78ac43a9858f20084ad6 (commit)
via b612ef0640ea45f03ad43ed4b124be1034d21071 (commit)
via 7105e7afc3436f95a09cc27e8a44e215a176dc38 (commit)
via f25b37a8e72716478e7cfae11ab5bf13b7051694 (commit)
via 2dc1a8cb43d8e8ff767675fbe3c76985e35ba140 (commit)
via 1cf9c94f99fd78347cd772e47c86d557eac54fbc (commit)
via 3e8a3101cb9191813f3c8ed557d6f189d3e42063 (commit)
via eadb94554a40517ebc367959e70bd41465a5ecdf (commit)
via 14a01f7be1267952af2287186f34c0312df6c773 (commit)
via 237a581b8872c4d95212bc5009815f046cebcf25 (commit)
via bcf8d387aaed911d955e1f26142caba785cd4e07 (commit)
via bf277d86fe874fa701c117365ed4c88060b8a984 (commit)
via 2be95ce7f069d9eb131b0d2d922a5e556f75810c (commit)
via f525d01de971b8b06dabc827bc0bbf46ee7ce9cc (commit)
via 58254d66b4a2c47a7c736c1e7c50203a8cf19805 (commit)
via 3d80ac6a1ba336ae75fd7afa499d6e0dfd05bff3 (commit)
via cff3ee7ddf7caf971bae2850b3a44b9d5142931f (commit)
via 1c3ce4051e76a877f41485f031c4219d2b732629 (commit)
via d9b06eb06c0ac38af922f102da0b8e405bb40f82 (commit)
via 3b4bb3d393adc3bd3ddfb4442a65087275a5c5c3 (commit)
via cd3f543b2ea20a7ac5851c118d5189df080207f2 (commit)
via d9b8396a05f3f4d187fbcadad6f63c019865e6fc (commit)
via 18de6db7e6f2336d69aad9dad7691ed123bec509 (commit)
via 0aec9ab099a57996f52f3c5d120ab0bafde6b2ab (commit)
via a8f51c3e348515e2f8f39ef38cad50760a598bcf (commit)
via 20470bcb23d77a7d45d649c530dd4b29245ba6e5 (commit)
via 0f97ce28deb04faf2d6b19c7312ef233f28665ad (commit)
via e830c0e8219cc6725892702f5215ff4b6d2e49ad (commit)
via 479b879e96bf60332b9a436e4ef4e5de517f028b (commit)
via ca3d1e44680d06398885e551ecf4bb601c0801b6 (commit)
via 15dbaf151a72d5cfc00b4ea4b4bcb64c7ed9ac14 (commit)
via 723a7500b82d9c85774a85f66fbeb489f233277c (commit)
via c3f40023778d234a76b8527b09e53e1ebb25301a (commit)
via 167e2537365ec68fe6be6a330a2eb698f177aa05 (commit)
via 05223e729d496ae80b8118dea3c03e1a1f6771a0 (commit)
via f4b5558a5ffca754f15a77446f43aed91ed44dae (commit)
via e9fed17eb1a7300d879a74a344dd52b00fb77d6d (commit)
via 33f8bec994716df827b07bbddb88e5944ff201c1 (commit)
via 29437a08213a7f295b95d9a14226ab41d98c5148 (commit)
via 46db403ca5c22e340fe6b586ba65045dc4aff409 (commit)
via 33c70b07361d9d08814a5c324282e7f39e2879f9 (commit)
via 9ab021c2c2720b563f012b99dfbf3e7034a3c245 (commit)
via 3cc81d1201ec9abfe818207c450df62232f5bce9 (commit)
via a7659334804bdbfe705f93309a55462e82320ef0 (commit)
via 5c2b2811fb3646bb42fb6a0154a79e3791afc73f (commit)
via 84a1f53eb78e189d37897627aed7c0213d1b13ee (commit)
via 95d08c91f6d902054eb9ed4f79cb6bda2c3e8342 (commit)
via 0ad80a1594e583be9821feb8369e8dc4f619ab65 (commit)
via ffd61b1958372df2b28821910ab8c08ac53ce327 (commit)
via 74afa26b1ab0349f5a07f0cd88a9ed0e7e5e9545 (commit)
via a0d5cb5096652c99f746bb9207bbf0f4220cb79d (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 (6fff1104ab84f8ed8683cd3a2d5b5d28832ca35e)
\
N -- N -- N (36e3f1ef5944fcb23eb9eda15457d15ce791e34b)
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 36e3f1ef5944fcb23eb9eda15457d15ce791e34b
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Jun 8 10:43:47 2020 -0400
16007: Migration works with large database
Changing individual permissions on groups that affect a lot of objects
is expensive. The migration now suppresses permission updates and
does a batch update at the end.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/db/migrate/20200602141328_fix_roles_projects.rb b/services/api/db/migrate/20200602141328_fix_roles_projects.rb
index f9b2bd6fa..e17ef6de5 100644
--- a/services/api/db/migrate/20200602141328_fix_roles_projects.rb
+++ b/services/api/db/migrate/20200602141328_fix_roles_projects.rb
@@ -5,9 +5,13 @@
require 'fix_roles_projects'
class FixRolesProjects < ActiveRecord::Migration[5.0]
- def change
+ def up
+ # defined in a function for easy testing.
+ fix_roles_projects
+ end
+
+ def down
# This migration is not reversible. However, the results are
# backwards compatible.
- fix_roles_projects
end
end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 469475ad9..83987d051 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -3196,6 +3196,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20190808145904'),
('20190809135453'),
('20190905151603'),
-('20200501150153');
+('20200501150153'),
+('20200602141328');
diff --git a/services/api/lib/fix_roles_projects.rb b/services/api/lib/fix_roles_projects.rb
index dafef61aa..448c50cee 100644
--- a/services/api/lib/fix_roles_projects.rb
+++ b/services/api/lib/fix_roles_projects.rb
@@ -2,23 +2,25 @@
#
# SPDX-License-Identifier: AGPL-3.0
+include CurrentApiClient
+
def fix_roles_projects
- # This migration is not reversible. However, the behavior it
- # enforces is backwards-compatible, and most of the time there
- # shouldn't be anything to do at all.
- act_as_system_user do
- ActiveRecord::Base.transaction do
- q = ActiveRecord::Base.connection.exec_query %{
+ batch_update_permissions do
+ # This migration is not reversible. However, the behavior it
+ # enforces is backwards-compatible, and most of the time there
+ # shouldn't be anything to do at all.
+ act_as_system_user do
+ ActiveRecord::Base.transaction do
+ q = ActiveRecord::Base.connection.exec_query %{
select uuid from groups limit 1
}
- # 1) any group not group_class != project becomes a 'role' (both empty and invalid groups)
- ActiveRecord::Base.connection.exec_query %{
+ # 1) any group not group_class != project becomes a 'role' (both empty and invalid groups)
+ ActiveRecord::Base.connection.exec_query %{
UPDATE groups set group_class='role' where group_class != 'project' or group_class is null
}
- Group.where(group_class: "role").each do |g|
- if g.owner_uuid != system_user_uuid
+ Group.where("group_class='role' and owner_uuid != '#{system_user_uuid}'").each do |g|
# 2) Ownership of a role becomes a can_manage link
Link.create!(link_class: 'permission',
name: 'can_manage',
@@ -28,35 +30,46 @@ UPDATE groups set group_class='role' where group_class != 'project' or group_cla
g.save_with_unique_name!
end
- # 3) If a role owns anything, give it to system user and it
- # becomes a can_manage link
ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass|
next if [ApiClientAuthorization,
AuthorizedKey,
- Log].include?(klass)
+ Log,
+ Group].include?(klass)
next if !klass.columns.collect(&:name).include?('owner_uuid')
- klass.where(owner_uuid: g.uuid).each do |owned|
+ # 3) If a role owns anything, give it to system user and it
+ # becomes a can_manage link
+ klass.joins("join groups on groups.uuid=#{klass.table_name}.owner_uuid and groups.group_class='role'").each do |owned|
Link.create!(link_class: 'permission',
name: 'can_manage',
- tail_uuid: g.uuid,
+ tail_uuid: owned.owner_uuid,
head_uuid: owned.uuid)
owned.owner_uuid = system_user_uuid
owned.save_with_unique_name!
end
end
- end
- # 4) Projects can't have outgoing permission links. Just delete them.
- q = ActiveRecord::Base.connection.exec_query %{
+ Group.joins("join groups as g2 on g2.uuid=groups.owner_uuid and g2.group_class='role'").each do |owned|
+ Link.create!(link_class: 'permission',
+ name: 'can_manage',
+ tail_uuid: owned.owner_uuid,
+ head_uuid: owned.uuid)
+ owned.owner_uuid = system_user_uuid
+ owned.save_with_unique_name!
+ end
+
+ # 4) Projects can't have outgoing permission links. Just
+ # print a warning and delete them.
+ q = ActiveRecord::Base.connection.exec_query %{
select links.uuid from links, groups where groups.uuid = links.tail_uuid and
links.link_class = 'permission' and groups.group_class = 'project'
}
- q.each do |lu|
- ln = Link.find_by_uuid(lu['uuid'])
- puts "Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed"
- Rails.logger.warn "Destroying invalid permission link from project #{ln.tail_uuid} to #{ln.head_uuid}"
- ln.destroy!
+ q.each do |lu|
+ ln = Link.find_by_uuid(lu['uuid'])
+ puts "WARNING: Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed"
+ Rails.logger.warn "Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed"
+ ln.destroy!
+ end
end
end
end
diff --git a/services/api/lib/update_permissions.rb b/services/api/lib/update_permissions.rb
index 4c2e72d95..7b1b900ca 100644
--- a/services/api/lib/update_permissions.rb
+++ b/services/api/lib/update_permissions.rb
@@ -8,6 +8,8 @@ REVOKE_PERM = 0
CAN_MANAGE_PERM = 3
def update_permissions perm_origin_uuid, starting_uuid, perm_level, edge_id=nil
+ return if Thread.current[:suppress_update_permissions]
+
#
# Update a subset of the permission table affected by adding or
# removing a particular permission relationship (ownership or a
@@ -140,7 +142,7 @@ end
def check_permissions_against_full_refresh
# No-op except when running tests
- return unless Rails.env == 'test' and !Thread.current[:no_check_permissions_against_full_refresh]
+ return unless Rails.env == 'test' and !Thread.current[:no_check_permissions_against_full_refresh] and !Thread.current[:suppress_update_permissions]
# For checking correctness of the incremental permission updates.
# Check contents of the current 'materialized_permission' table
@@ -191,6 +193,17 @@ def skip_check_permissions_against_full_refresh
end
end
+def batch_update_permissions
+ check_perm_was = Thread.current[:suppress_update_permissions]
+ Thread.current[:suppress_update_permissions] = true
+ begin
+ yield
+ ensure
+ Thread.current[:suppress_update_permissions] = check_perm_was
+ refresh_permissions
+ end
+end
+
# Used to account for permissions that a user gains by having
# can_manage on another user.
#
commit 8448f02e0bc329953f81268275e4157f643ec094
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 2 16:49:38 2020 -0400
16007: Fix FUSE test
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index e328e9a1b..ee0d786bb 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -9,6 +9,13 @@ public:
description: Public Project
group_class: project
+public_role:
+ uuid: zzzzz-j7d0g-jt30l961gq3t0oi
+ owner_uuid: zzzzz-tpzed-d9tiejq69daie8f
+ name: Public Role
+ description: Public Role
+ group_class: role
+
private:
uuid: zzzzz-j7d0g-rew6elm53kancon
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index 593d945cf..b2816ac16 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -128,7 +128,7 @@ class FuseMagicTest(MountTestBase):
super(FuseMagicTest, self).setUp(api=api)
self.test_project = run_test_server.fixture('groups')['aproject']['uuid']
- self.non_project_group = run_test_server.fixture('groups')['public']['uuid']
+ self.non_project_group = run_test_server.fixture('groups')['public_role']['uuid']
self.collection_in_test_project = run_test_server.fixture('collections')['foo_collection_in_aproject']['name']
cw = arvados.CollectionWriter()
commit 9cebc0a070ef29777e2e797b74da66753bb22a25
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 2 16:45:21 2020 -0400
16007: Add migration to fix invalid groups & permission links
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/db/migrate/20200602141328_fix_roles_projects.rb b/services/api/db/migrate/20200602141328_fix_roles_projects.rb
new file mode 100644
index 000000000..f9b2bd6fa
--- /dev/null
+++ b/services/api/db/migrate/20200602141328_fix_roles_projects.rb
@@ -0,0 +1,13 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'fix_roles_projects'
+
+class FixRolesProjects < ActiveRecord::Migration[5.0]
+ def change
+ # This migration is not reversible. However, the results are
+ # backwards compatible.
+ fix_roles_projects
+ end
+end
diff --git a/services/api/lib/fix_roles_projects.rb b/services/api/lib/fix_roles_projects.rb
new file mode 100644
index 000000000..dafef61aa
--- /dev/null
+++ b/services/api/lib/fix_roles_projects.rb
@@ -0,0 +1,63 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+def fix_roles_projects
+ # This migration is not reversible. However, the behavior it
+ # enforces is backwards-compatible, and most of the time there
+ # shouldn't be anything to do at all.
+ act_as_system_user do
+ ActiveRecord::Base.transaction do
+ q = ActiveRecord::Base.connection.exec_query %{
+select uuid from groups limit 1
+}
+
+ # 1) any group not group_class != project becomes a 'role' (both empty and invalid groups)
+ ActiveRecord::Base.connection.exec_query %{
+UPDATE groups set group_class='role' where group_class != 'project' or group_class is null
+ }
+
+ Group.where(group_class: "role").each do |g|
+ if g.owner_uuid != system_user_uuid
+ # 2) Ownership of a role becomes a can_manage link
+ Link.create!(link_class: 'permission',
+ name: 'can_manage',
+ tail_uuid: g.owner_uuid,
+ head_uuid: g.uuid)
+ g.owner_uuid = system_user_uuid
+ g.save_with_unique_name!
+ end
+
+ # 3) If a role owns anything, give it to system user and it
+ # becomes a can_manage link
+ ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass|
+ next if [ApiClientAuthorization,
+ AuthorizedKey,
+ Log].include?(klass)
+ next if !klass.columns.collect(&:name).include?('owner_uuid')
+
+ klass.where(owner_uuid: g.uuid).each do |owned|
+ Link.create!(link_class: 'permission',
+ name: 'can_manage',
+ tail_uuid: g.uuid,
+ head_uuid: owned.uuid)
+ owned.owner_uuid = system_user_uuid
+ owned.save_with_unique_name!
+ end
+ end
+ end
+
+ # 4) Projects can't have outgoing permission links. Just delete them.
+ q = ActiveRecord::Base.connection.exec_query %{
+select links.uuid from links, groups where groups.uuid = links.tail_uuid and
+ links.link_class = 'permission' and groups.group_class = 'project'
+}
+ q.each do |lu|
+ ln = Link.find_by_uuid(lu['uuid'])
+ puts "Projects cannot have outgoing permission links, '#{ln.name}' link from #{ln.tail_uuid} to #{ln.head_uuid} will be removed"
+ Rails.logger.warn "Destroying invalid permission link from project #{ln.tail_uuid} to #{ln.head_uuid}"
+ ln.destroy!
+ end
+ end
+ end
+end
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 4293b0466..b30d1edc2 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -198,20 +198,6 @@ foo_file_readable_by_active_redundant_permission_via_private_group:
head_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
properties: {}
-foo_file_readable_by_aproject:
- uuid: zzzzz-o0j2j-fp1d8395ldqw22p
- 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-000000000000000
- modified_at: 2014-01-24 20:42:26 -0800
- updated_at: 2014-01-24 20:42:26 -0800
- tail_uuid: zzzzz-j7d0g-v955i6s2oi1cbso
- link_class: permission
- name: can_read
- head_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
- properties: {}
-
bar_file_readable_by_active:
uuid: zzzzz-o0j2j-8hppiuduf8eqdng
owner_uuid: zzzzz-tpzed-000000000000000
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 631e0137c..3d1fda927 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -3,6 +3,7 @@
# SPDX-License-Identifier: AGPL-3.0
require 'test_helper'
+require 'fix_roles_projects'
class GroupTest < ActiveSupport::TestCase
@@ -293,4 +294,53 @@ class GroupTest < ActiveSupport::TestCase
end
end
end
+
+ def insert_group uuid, owner_uuid, name, group_class
+ q = ActiveRecord::Base.connection.exec_query %{
+insert into groups (uuid, owner_uuid, name, group_class, created_at, updated_at)
+ values ('#{uuid}', '#{owner_uuid}',
+ '#{name}', #{if group_class then "'"+group_class+"'" else 'NULL' end},
+ statement_timestamp(), statement_timestamp())
+}
+ uuid
+ end
+
+ test "migration to fix roles and projects" do
+ g1 = insert_group Group.generate_uuid, system_user_uuid, 'group with no class', nil
+ g2 = insert_group Group.generate_uuid, users(:active).uuid, 'role owned by a user', 'role'
+
+ g3 = insert_group Group.generate_uuid, system_user_uuid, 'role that owns a project', 'role'
+ g4 = insert_group Group.generate_uuid, g3, 'the project', 'project'
+
+ g5 = insert_group Group.generate_uuid, users(:active).uuid, 'a project with an outgoing permission link', 'project'
+
+ g6 = insert_group Group.generate_uuid, system_user_uuid, 'name collision', 'role'
+ g7 = insert_group Group.generate_uuid, users(:active).uuid, 'name collision', 'role'
+
+ refresh_permissions
+
+ act_as_system_user do
+ l1 = Link.create!(link_class: 'permission', name: 'can_manage', tail_uuid: g3, head_uuid: g4)
+ q = ActiveRecord::Base.connection.exec_query %{
+update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
+}
+ refresh_permissions
+ end
+
+ assert_equal nil, Group.find_by_uuid(g1).group_class
+ assert_equal users(:active).uuid, Group.find_by_uuid(g2).owner_uuid
+ assert_equal g3, Group.find_by_uuid(g4).owner_uuid
+ assert !Link.where(tail_uuid: users(:active).uuid, head_uuid: g2, link_class: "permission", name: "can_manage").any?
+ assert !Link.where(tail_uuid: g3, head_uuid: g4, link_class: "permission", name: "can_manage").any?
+ assert Link.where(link_class: 'permission', name: 'can_manage', tail_uuid: g5, head_uuid: g4).any?
+
+ fix_roles_projects
+
+ assert_equal 'role', Group.find_by_uuid(g1).group_class
+ assert_equal system_user_uuid, Group.find_by_uuid(g2).owner_uuid
+ assert_equal system_user_uuid, Group.find_by_uuid(g4).owner_uuid
+ assert Link.where(tail_uuid: users(:active).uuid, head_uuid: g2, link_class: "permission", name: "can_manage").any?
+ assert Link.where(tail_uuid: g3, head_uuid: g4, link_class: "permission", name: "can_manage").any?
+ assert !Link.where(link_class: 'permission', name: 'can_manage', tail_uuid: g5, head_uuid: g4).any?
+ end
end
commit eced1acec35d3b259b31bae8ea65856aca8add44
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 2 13:26:48 2020 -0400
16007: Reenable updating the permission level of a link
Add test to ensure permission table is synchronized when updating the
level of an existing permission link.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/apps/workbench/test/integration/projects_test.rb b/apps/workbench/test/integration/projects_test.rb
index 17ab5e466..7a5103007 100644
--- a/apps/workbench/test/integration/projects_test.rb
+++ b/apps/workbench/test/integration/projects_test.rb
@@ -132,7 +132,7 @@ class ProjectsTest < ActionDispatch::IntegrationTest
show_object_using('active', 'groups', 'aproject', 'A Project')
click_on "Sharing"
click_on "Share with groups"
- good_uuid = api_fixture("groups")["private"]["uuid"]
+ good_uuid = api_fixture("groups")["future_project_viewing_group"]["uuid"]
assert(page.has_selector?(".selectable[data-object-uuid=\"#{good_uuid}\"]"),
"'share with groups' listing missing owned user group")
bad_uuid = api_fixture("groups")["asubproject"]["uuid"]
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 0b7205612..60a94c7e2 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -13,7 +13,7 @@ class Link < ArvadosModel
validate :name_links_are_obsolete
validate :permission_to_attach_to_objects
- before_update :cannot_alter_permissions
+ before_update :restrict_alter_permissions
after_update :call_update_permissions
after_create :call_update_permissions
before_destroy :clear_permissions
@@ -50,6 +50,11 @@ class Link < ArvadosModel
# All users can write links that don't affect permissions
return true if self.link_class != 'permission'
+ if PERM_LEVEL[self.name].nil?
+ errors.add(:name, "is invalid permission, must be one of 'can_read', 'can_write', 'can_manage', 'can_login'")
+ return false
+ end
+
rsc_class = ArvadosModel::resource_class_for_uuid tail_uuid
if rsc_class == Group
tail_obj = Group.find_by_uuid(tail_uuid)
@@ -84,13 +89,13 @@ class Link < ArvadosModel
false
end
- def cannot_alter_permissions
+ def restrict_alter_permissions
return true if self.link_class != 'permission' && self.link_class_was != 'permission'
return true if current_user.andand.uuid == system_user.uuid
- if link_class_changed? || name_changed? || tail_uuid_changed? || head_uuid_changed?
- raise "Cannot alter a permission link"
+ if link_class_changed? || tail_uuid_changed? || head_uuid_changed?
+ raise "Can only alter permission link level"
end
end
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 03a8e82ee..07709b752 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -46,7 +46,7 @@ class PermissionTest < ActiveSupport::TestCase
end
test "readable_by" do
- set_user_from_auth :active_trustedclient
+ set_user_from_auth :admin
ob = Collection.create!
Link.create!(tail_uuid: users(:active).uuid,
@@ -57,7 +57,7 @@ class PermissionTest < ActiveSupport::TestCase
end
test "writable_by" do
- set_user_from_auth :active_trustedclient
+ set_user_from_auth :admin
ob = Collection.create!
Link.create!(tail_uuid: users(:active).uuid,
commit bc84a8ffbb1add0ebd8bc8bfb4a1721e4fea20a0
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 2 10:09:29 2020 -0400
16007: Update group-sync tool for new restrictions on roles
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 2f6643337..4293b0466 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -1111,3 +1111,17 @@ tagged_collection_readable_by_spectator:
name: can_read
head_uuid: zzzzz-4zz18-taggedcolletion
properties: {}
+
+active_manages_viewing_group:
+ uuid: zzzzz-o0j2j-activemanagesvi
+ 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-000000000000000
+ modified_at: 2014-01-24 20:42:26 -0800
+ updated_at: 2014-01-24 20:42:26 -0800
+ tail_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ link_class: permission
+ name: can_manage
+ head_uuid: zzzzz-j7d0g-futrprojviewgrp
+ properties: {}
diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index 9e2307b7a..4d03ba89e 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -226,8 +226,9 @@ func SetParentGroup(cfg *ConfigParams) error {
log.Println("Default parent group not found, creating...")
}
groupData := map[string]string{
- "name": cfg.ParentGroupName,
- "owner_uuid": cfg.SysUserUUID,
+ "name": cfg.ParentGroupName,
+ "owner_uuid": cfg.SysUserUUID,
+ "group_class": "role",
}
if err := CreateGroup(cfg, &parentGroup, groupData); err != nil {
return fmt.Errorf("error creating system user owned group named %q: %s", groupData["name"], err)
@@ -528,17 +529,21 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
params := arvados.ResourceListParams{
Filters: []arvados.Filter{{
- Attr: "owner_uuid",
+ Attr: "tail_uuid",
Operator: "=",
Operand: cfg.ParentGroupUUID,
}},
}
- results, err := GetAll(cfg.Client, "groups", params, &GroupList{})
+ results, err := GetAll(cfg.Client, "links", params, &LinkList{})
if err != nil {
return remoteGroups, groupNameToUUID, fmt.Errorf("error getting remote groups: %s", err)
}
for _, item := range results {
- group := item.(arvados.Group)
+ var group arvados.Group
+ err = GetGroup(cfg, &group, item.(arvados.Link).HeadUUID)
+ if err != nil {
+ return remoteGroups, groupNameToUUID, fmt.Errorf("error getting remote group: %s", err)
+ }
// Group -> User filter
g2uFilter := arvados.ResourceListParams{
Filters: []arvados.Filter{{
diff --git a/tools/sync-groups/sync-groups_test.go b/tools/sync-groups/sync-groups_test.go
index 9eec6b6d9..2da8c1cdd 100644
--- a/tools/sync-groups/sync-groups_test.go
+++ b/tools/sync-groups/sync-groups_test.go
@@ -170,7 +170,7 @@ func RemoteGroupExists(cfg *ConfigParams, groupName string) (uuid string, err er
}, {
Attr: "owner_uuid",
Operator: "=",
- Operand: cfg.ParentGroupUUID,
+ Operand: cfg.SysUserUUID,
}, {
Attr: "group_class",
Operator: "=",
commit a95e63fb126abf0da69e0bb0591267babb501d3c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Jun 1 18:03:56 2020 -0400
16007: Roles are owned by system user
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index e16433c81..21e57e143 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -25,6 +25,8 @@ class Group < ArvadosModel
before_update :before_ownership_change
after_update :after_ownership_change
+ after_create :add_role_manage_link
+
after_update :update_trash
before_destroy :clear_permissions_and_trash
@@ -114,4 +116,33 @@ delete from trashed_groups where group_uuid=$1
end
true
end
+
+ def ensure_owner_uuid_is_permitted
+ if group_class == "role"
+ @role_creator = nil
+ if new_record?
+ @role_creator = owner_uuid
+ self.owner_uuid = system_user_uuid
+ return true
+ end
+ if self.owner_uuid != system_user_uuid
+ raise "Owner uuid for role must be system user"
+ end
+ raise PermissionDeniedError unless current_user.can?(manage: uuid)
+ true
+ else
+ super
+ end
+ end
+
+ def add_role_manage_link
+ if group_class == "role" && @role_creator
+ act_as_system_user do
+ Link.create!(tail_uuid: @role_creator,
+ head_uuid: self.uuid,
+ link_class: "permission",
+ name: "can_manage")
+ end
+ end
+ end
end
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index a64970e60..e328e9a1b 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -18,14 +18,14 @@ private:
private_role:
uuid: zzzzz-j7d0g-pew6elm53kancon
- owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ owner_uuid: zzzzz-tpzed-000000000000000
name: Private Role
description: Private Role
group_class: role
private_and_can_read_foofile:
uuid: zzzzz-j7d0g-22xp1wpjul508rk
- owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ owner_uuid: zzzzz-tpzed-000000000000000
name: Private and Can Read Foofile
description: Another Private Group
group_class: role
@@ -95,7 +95,7 @@ asubproject:
future_project_viewing_group:
uuid: zzzzz-j7d0g-futrprojviewgrp
- owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ owner_uuid: zzzzz-tpzed-000000000000000
created_at: 2014-04-21 15:37:48 -0400
modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
commit 66002de50dfc4646a11df36d6e1d665bd507de58
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Jun 1 14:59:51 2020 -0400
16007: Only users and roles can be granted permission
Permission link tail_uuid must be a user or group_class role.
Also disallow modifying permission links.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 7270f7cdf..01a31adb9 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -585,7 +585,7 @@ class ArvadosModel < ApplicationRecord
# itself.
if !current_user.can?(write: self.uuid)
logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} without write permission"
- errors.add :uuid, "is not writable"
+ errors.add :uuid, " #{uuid} is not writable by #{current_user.uuid}"
raise PermissionDeniedError
end
end
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 21d89767c..0b7205612 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -12,8 +12,8 @@ class Link < ArvadosModel
attribute :properties, :jsonbHash, default: {}
validate :name_links_are_obsolete
- before_create :permission_to_attach_to_objects
- before_update :permission_to_attach_to_objects
+ validate :permission_to_attach_to_objects
+ before_update :cannot_alter_permissions
after_update :call_update_permissions
after_create :call_update_permissions
before_destroy :clear_permissions
@@ -50,13 +50,32 @@ class Link < ArvadosModel
# All users can write links that don't affect permissions
return true if self.link_class != 'permission'
+ rsc_class = ArvadosModel::resource_class_for_uuid tail_uuid
+ if rsc_class == Group
+ tail_obj = Group.find_by_uuid(tail_uuid)
+ if tail_obj.nil?
+ errors.add(:tail_uuid, "does not exist")
+ return false
+ end
+ if tail_obj.group_class != "role"
+ errors.add(:tail_uuid, "must be a role, was #{tail_obj.group_class}")
+ return false
+ end
+ elsif rsc_class != User
+ errors.add(:tail_uuid, "must be a user or role")
+ return false
+ end
+
# Administrators can grant permissions
return true if current_user.is_admin
head_obj = ArvadosModel.find_by_uuid(head_uuid)
# No permission links can be pointed to past collection versions
- return false if head_obj.is_a?(Collection) && head_obj.current_version_uuid != head_uuid
+ if head_obj.is_a?(Collection) && head_obj.current_version_uuid != head_uuid
+ errors.add(:head_uuid, "cannot point to a past version of a collection")
+ return false
+ end
# All users can grant permissions on objects they own or can manage
return true if current_user.can?(manage: head_obj)
@@ -65,6 +84,16 @@ class Link < ArvadosModel
false
end
+ def cannot_alter_permissions
+ return true if self.link_class != 'permission' && self.link_class_was != 'permission'
+
+ return true if current_user.andand.uuid == system_user.uuid
+
+ if link_class_changed? || name_changed? || tail_uuid_changed? || head_uuid_changed?
+ raise "Cannot alter a permission link"
+ end
+ end
+
PERM_LEVEL = {
'can_read' => 1,
'can_login' => 1,
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 89235d65b..a64970e60 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -16,6 +16,13 @@ private:
description: Private Project
group_class: project
+private_role:
+ uuid: zzzzz-j7d0g-pew6elm53kancon
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ name: Private Role
+ description: Private Role
+ group_class: role
+
private_and_can_read_foofile:
uuid: zzzzz-j7d0g-22xp1wpjul508rk
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index ff33fe65b..66a62543b 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -87,7 +87,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
tail_uuid: users(:spectator).uuid,
link_class: 'permission',
name: 'can_read',
- head_uuid: groups(:private).uuid,
+ head_uuid: groups(:private_role).uuid,
properties: {}
}
},
@@ -105,7 +105,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
params: {
:format => :json,
:link => {
- tail_uuid: groups(:private).uuid,
+ tail_uuid: groups(:private_role).uuid,
link_class: 'permission',
name: 'can_read',
head_uuid: collections(:foo_file).uuid,
@@ -149,7 +149,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
params: {
:format => :json,
:link => {
- tail_uuid: groups(:private).uuid,
+ tail_uuid: groups(:private_role).uuid,
link_class: 'permission',
name: 'can_read',
head_uuid: collections(:foo_file).uuid,
@@ -173,7 +173,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
tail_uuid: users(:spectator).uuid,
link_class: 'permission',
name: 'can_read',
- head_uuid: groups(:private).uuid,
+ head_uuid: groups(:private_role).uuid,
properties: {}
}
},
@@ -216,7 +216,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
tail_uuid: users(:spectator).uuid,
link_class: 'permission',
name: 'can_read',
- head_uuid: groups(:private).uuid,
+ head_uuid: groups(:private_role).uuid,
properties: {}
}
},
@@ -228,7 +228,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
params: {
:format => :json,
:link => {
- tail_uuid: groups(:private).uuid,
+ tail_uuid: groups(:private_role).uuid,
link_class: 'permission',
name: 'can_read',
head_uuid: groups(:empty_lonely_group).uuid,
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index e34c1a44f..631e0137c 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -205,7 +205,7 @@ class GroupTest < ActiveSupport::TestCase
test "trashed does not propagate across permission links" do
set_user_from_auth :admin
- g_foo = Group.create!(name: "foo", group_class: "project")
+ g_foo = Group.create!(name: "foo", group_class: "role")
u_bar = User.create!(first_name: "bar")
assert Group.readable_by(users(:admin)).where(uuid: g_foo.uuid).any?
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 4849b385b..03a8e82ee 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -423,8 +423,14 @@ class PermissionTest < ActiveSupport::TestCase
test "add user to group, then remove them" do
set_user_from_auth :admin
- grp = Group.create!(owner_uuid: system_user_uuid, group_class: "project")
- col = Collection.create!(owner_uuid: grp.uuid)
+ grp = Group.create!(owner_uuid: system_user_uuid, group_class: "role")
+ col = Collection.create!(owner_uuid: system_user_uuid)
+
+ l0 = Link.create!(tail_uuid: grp.uuid,
+ head_uuid: col.uuid,
+ link_class: 'permission',
+ name: 'can_read')
+
assert_empty Collection.readable_by(users(:active)).where(uuid: col.uuid)
assert_empty User.readable_by(users(:active)).where(uuid: users(:project_viewer).uuid)
commit 2aa0d4853c00b21ba5f1dd692b952fbb1b2ccee8
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri May 29 17:25:05 2020 -0400
16007: Make it so that only projects can own things
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 8afebfb79..7270f7cdf 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -574,6 +574,9 @@ class ArvadosModel < ApplicationRecord
logger.warn "User #{current_user.uuid} tried to set ownership of #{self.class.to_s} #{self.uuid} but does not have permission to write #{which} owner_uuid #{check_uuid}"
errors.add :owner_uuid, "cannot be set or changed without write permission on #{which} owner"
raise PermissionDeniedError
+ elsif rsc_class == Group && Group.find_by_uuid(owner_uuid).group_class != "project"
+ errors.add :owner_uuid, "must be a project"
+ raise PermissionDeniedError
end
end
else
diff --git a/services/api/app/models/database_seeds.rb b/services/api/app/models/database_seeds.rb
index 39f491503..a86a2c854 100644
--- a/services/api/app/models/database_seeds.rb
+++ b/services/api/app/models/database_seeds.rb
@@ -13,7 +13,6 @@ class DatabaseSeeds
anonymous_group
anonymous_group_read_permission
anonymous_user
- empty_collection
refresh_permissions
refresh_trashed
end
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index d6fc818cd..e16433c81 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -49,6 +49,9 @@ class Group < ArvadosModel
if group_class != 'project' && group_class != 'role'
errors.add :group_class, "value must be one of 'project' or 'role', was '#{group_class}'"
end
+ if group_class_changed? && !group_class_was.nil?
+ errors.add :group_class, "cannot be modified after record is created"
+ end
end
def update_trash
diff --git a/services/api/lib/current_api_client.rb b/services/api/lib/current_api_client.rb
index c7b48c0cd..f3ec0a03e 100644
--- a/services/api/lib/current_api_client.rb
+++ b/services/api/lib/current_api_client.rb
@@ -8,7 +8,6 @@ $all_users_group = nil
$anonymous_user = nil
$anonymous_group = nil
$anonymous_group_read_permission = nil
-$empty_collection = nil
module CurrentApiClient
def current_user
@@ -90,7 +89,8 @@ module CurrentApiClient
ActiveRecord::Base.transaction do
Group.where(uuid: system_group_uuid).
first_or_create!(name: "System group",
- description: "System group") do |g|
+ description: "System group",
+ group_class: "role") do |g|
g.save!
User.all.collect(&:uuid).each do |user_uuid|
Link.create!(link_class: 'permission',
@@ -188,22 +188,10 @@ module CurrentApiClient
end
end
- def empty_collection_uuid
+ def empty_collection_pdh
'd41d8cd98f00b204e9800998ecf8427e+0'
end
- def empty_collection
- $empty_collection = check_cache $empty_collection do
- act_as_system_user do
- ActiveRecord::Base.transaction do
- Collection.
- where(portable_data_hash: empty_collection_uuid).
- first_or_create!(manifest_text: '', owner_uuid: anonymous_group.uuid)
- end
- end
- end
- end
-
private
# If the given value is nil, or the cache has been cleared since it
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 68377eb5f..89235d65b 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -6,15 +6,15 @@ public:
uuid: zzzzz-j7d0g-it30l961gq3t0oi
owner_uuid: zzzzz-tpzed-d9tiejq69daie8f
name: Public
- description: Public Group
- group_class: role
+ description: Public Project
+ group_class: project
private:
uuid: zzzzz-j7d0g-rew6elm53kancon
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
name: Private
- description: Private Group
- group_class: role
+ description: Private Project
+ group_class: project
private_and_can_read_foofile:
uuid: zzzzz-j7d0g-22xp1wpjul508rk
@@ -248,17 +248,6 @@ fuse_owned_project:
description: Test project belonging to FUSE test user
group_class: project
-group_with_no_class:
- uuid: zzzzz-j7d0g-groupwithnoclas
- 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: group_with_no_class
- description: This group has no class at all. So rude!
-
# This wouldn't pass model validation, but it enables a workbench
# infinite-loop test. See #4389
project_owns_itself:
diff --git a/services/api/test/functional/arvados/v1/filters_test.rb b/services/api/test/functional/arvados/v1/filters_test.rb
index b30afd745..26270b1c3 100644
--- a/services/api/test/functional/arvados/v1/filters_test.rb
+++ b/services/api/test/functional/arvados/v1/filters_test.rb
@@ -6,16 +6,16 @@ require 'test_helper'
class Arvados::V1::FiltersTest < ActionController::TestCase
test '"not in" filter passes null values' do
- @controller = Arvados::V1::GroupsController.new
+ @controller = Arvados::V1::ContainerRequestsController.new
authorize_with :admin
get :index, params: {
- filters: [ ['group_class', 'not in', ['project']] ],
- controller: 'groups',
+ filters: [ ['container_uuid', 'not in', ['zzzzz-dz642-queuedcontainer', 'zzzzz-dz642-runningcontainr']] ],
+ controller: 'container_requests',
}
assert_response :success
found = assigns(:objects)
- assert_includes(found.collect(&:group_class), nil,
- "'group_class not in ['project']' filter should pass null")
+ assert_includes(found.collect(&:container_uuid), nil,
+ "'container_uuid not in [zzzzz-dz642-queuedcontainer, zzzzz-dz642-runningcontainr]' filter should pass null")
end
test 'error message for non-array element in filters array' do
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 2b5e8d5a9..ff89cd212 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -29,8 +29,9 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
end
assert_includes group_uuids, groups(:aproject).uuid
assert_includes group_uuids, groups(:asubproject).uuid
+ assert_includes group_uuids, groups(:private).uuid
assert_not_includes group_uuids, groups(:system_group).uuid
- assert_not_includes group_uuids, groups(:private).uuid
+ assert_not_includes group_uuids, groups(:private_and_can_read_foofile).uuid
end
test "get list of groups that are not projects" do
@@ -44,8 +45,6 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
end
assert_not_includes group_uuids, groups(:aproject).uuid
assert_not_includes group_uuids, groups(:asubproject).uuid
- assert_includes group_uuids, groups(:private).uuid
- assert_includes group_uuids, groups(:group_with_no_class).uuid
end
test "get list of groups with bogus group_class" do
@@ -746,20 +745,23 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal 0, json_response['included'].length
end
- test 'get shared, owned by non-project' do
+ test 'get shared, add permission link' do
authorize_with :user_bar_in_sharing_group
act_as_system_user do
- Group.find_by_uuid(groups(:project_owned_by_foo).uuid).update!(owner_uuid: groups(:group_for_sharing_tests).uuid)
+ Link.create!(tail_uuid: groups(:group_for_sharing_tests).uuid,
+ head_uuid: groups(:project_owned_by_foo).uuid,
+ link_class: 'permission',
+ name: 'can_manage')
end
get :shared, params: {:filters => [["group_class", "=", "project"]], :include => "owner_uuid"}
assert_equal 1, json_response['items'].length
- assert_equal json_response['items'][0]["uuid"], groups(:project_owned_by_foo).uuid
+ assert_equal groups(:project_owned_by_foo).uuid, json_response['items'][0]["uuid"]
assert_equal 1, json_response['included'].length
- assert_equal json_response['included'][0]["uuid"], groups(:group_for_sharing_tests).uuid
+ assert_equal users(:user_foo_in_sharing_group).uuid, json_response['included'][0]["uuid"]
end
### contents with exclude_home_project
@@ -810,20 +812,23 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_equal 0, json_response['included'].length
end
- test 'contents, exclude home, owned by non-project' do
+ test 'contents, exclude home, add permission link' do
authorize_with :user_bar_in_sharing_group
act_as_system_user do
- Group.find_by_uuid(groups(:project_owned_by_foo).uuid).update!(owner_uuid: groups(:group_for_sharing_tests).uuid)
+ Link.create!(tail_uuid: groups(:group_for_sharing_tests).uuid,
+ head_uuid: groups(:project_owned_by_foo).uuid,
+ link_class: 'permission',
+ name: 'can_manage')
end
get :contents, params: {:include => "owner_uuid", :exclude_home_project => true}
assert_equal 1, json_response['items'].length
- assert_equal json_response['items'][0]["uuid"], groups(:project_owned_by_foo).uuid
+ assert_equal groups(:project_owned_by_foo).uuid, json_response['items'][0]["uuid"]
assert_equal 1, json_response['included'].length
- assert_equal json_response['included'][0]["uuid"], groups(:group_for_sharing_tests).uuid
+ assert_equal users(:user_foo_in_sharing_group).uuid, json_response['included'][0]["uuid"]
end
test 'contents, exclude home, with parent specified' do
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index eec41aa08..ff33fe65b 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -6,7 +6,6 @@ require 'test_helper'
class PermissionsTest < ActionDispatch::IntegrationTest
include DbCurrentTime
- include CurrentApiClient # for empty_collection
fixtures :users, :groups, :api_client_authorizations, :collections
test "adding and removing direct can_read links" do
@@ -441,7 +440,7 @@ class PermissionsTest < ActionDispatch::IntegrationTest
test "active user can read the empty collection" do
# The active user should be able to read the empty collection.
- get("/arvados/v1/collections/#{empty_collection_uuid}",
+ get("/arvados/v1/collections/#{empty_collection_pdh}",
params: {:format => :json},
headers: auth(:active))
assert_response :success
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index f7a531fc0..e34c1a44f 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -60,6 +60,43 @@ class GroupTest < ActiveSupport::TestCase
assert g_foo.errors.messages[:owner_uuid].join(" ").match(/ownership cycle/)
end
+ test "cannot create a group that is not a 'role' or 'project'" do
+ set_user_from_auth :active_trustedclient
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ Group.create!(name: "foo")
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ Group.create!(name: "foo", group_class: "")
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ Group.create!(name: "foo", group_class: "bogus")
+ end
+ end
+
+ test "cannot change group_class on an already created group" do
+ set_user_from_auth :active_trustedclient
+ g = Group.create!(name: "foo", group_class: "role")
+ assert_raises(ActiveRecord::RecordInvalid) do
+ g.update_attributes!(group_class: "project")
+ end
+ end
+
+ test "role cannot own things" do
+ set_user_from_auth :active_trustedclient
+ role = Group.create!(name: "foo", group_class: "role")
+ assert_raises(ArvadosModel::PermissionDeniedError) do
+ Collection.create!(name: "bzzz123", owner_uuid: role.uuid)
+ end
+
+ c = Collection.create!(name: "bzzz124")
+ assert_raises(ArvadosModel::PermissionDeniedError) do
+ c.update_attributes!(owner_uuid: role.uuid)
+ end
+ end
+
test "trash group hides contents" do
set_user_from_auth :active_trustedclient
@@ -237,7 +274,8 @@ class GroupTest < ActiveSupport::TestCase
set_user_from_auth :active
["", "{SOLIDUS}"].each do |subst|
Rails.configuration.Collections.ForwardSlashNameSubstitution = subst
- g = Group.create group_class: "project"
+ proj = Group.create group_class: "project"
+ role = Group.create group_class: "role"
[[nil, true],
["", true],
[".", false],
@@ -248,11 +286,10 @@ class GroupTest < ActiveSupport::TestCase
["../..", subst != ""],
["/", subst != ""],
].each do |name, valid|
- g.name = name
- g.group_class = "role"
- assert_equal true, g.valid?
- g.group_class = "project"
- assert_equal valid, g.valid?, "#{name.inspect} should be #{valid ? "valid" : "invalid"}"
+ role.name = name
+ assert_equal true, role.valid?
+ proj.name = name
+ assert_equal valid, proj.valid?, "#{name.inspect} should be #{valid ? "valid" : "invalid"}"
end
end
end
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 8e2569ccc..4849b385b 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -423,7 +423,7 @@ class PermissionTest < ActiveSupport::TestCase
test "add user to group, then remove them" do
set_user_from_auth :admin
- grp = Group.create!(owner_uuid: system_user_uuid, group_class: "role")
+ grp = Group.create!(owner_uuid: system_user_uuid, group_class: "project")
col = Collection.create!(owner_uuid: grp.uuid)
assert_empty Collection.readable_by(users(:active)).where(uuid: col.uuid)
assert_empty User.readable_by(users(:active)).where(uuid: users(:project_viewer).uuid)
commit aa728d17c91d8d764fb3c6a4c10236c36baa0bbb
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri May 29 00:04:12 2020 -0400
16007: Validate group_class is set to 'project' or 'role'
Fix tests.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 36814a316..d6fc818cd 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -17,6 +17,7 @@ class Group < ArvadosModel
attribute :properties, :jsonbHash, default: {}
validate :ensure_filesystem_compatible_name
+ validate :check_group_class
before_create :assign_name
after_create :after_ownership_change
after_create :update_trash
@@ -44,6 +45,12 @@ class Group < ArvadosModel
super if group_class == 'project'
end
+ def check_group_class
+ if group_class != 'project' && group_class != 'role'
+ errors.add :group_class, "value must be one of 'project' or 'role', was '#{group_class}'"
+ end
+ end
+
def update_trash
if trash_at_changed? or owner_uuid_changed?
# The group was added or removed from the trash.
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 22c999ecd..68377eb5f 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -107,6 +107,7 @@ bad_group_has_ownership_cycle_a:
modified_at: 2014-05-03 18:50:08 -0400
updated_at: 2014-05-03 18:50:08 -0400
name: Owned by bad group b
+ group_class: project
bad_group_has_ownership_cycle_b:
uuid: zzzzz-j7d0g-0077nzts8c178lw
@@ -117,6 +118,7 @@ bad_group_has_ownership_cycle_b:
modified_at: 2014-05-03 18:50:08 -0400
updated_at: 2014-05-03 18:50:08 -0400
name: Owned by bad group a
+ group_class: project
anonymous_group:
uuid: zzzzz-j7d0g-anonymouspublic
diff --git a/services/api/test/functional/application_controller_test.rb b/services/api/test/functional/application_controller_test.rb
index 175a8f71e..2cfa05444 100644
--- a/services/api/test/functional/application_controller_test.rb
+++ b/services/api/test/functional/application_controller_test.rb
@@ -100,7 +100,7 @@ class ApplicationControllerTest < ActionController::TestCase
@controller = Arvados::V1::GroupsController.new
authorize_with :active
post :create, params: {
- group: {},
+ group: {group_class: "project"},
ensure_unique_name: boolparam
}
assert_response :success
@@ -113,7 +113,8 @@ class ApplicationControllerTest < ActionController::TestCase
post :create, params: {
group: {
name: groups(:aproject).name,
- owner_uuid: groups(:aproject).owner_uuid
+ owner_uuid: groups(:aproject).owner_uuid,
+ group_class: "project"
},
ensure_unique_name: boolparam
}
diff --git a/services/api/test/functional/arvados/v1/repositories_controller_test.rb b/services/api/test/functional/arvados/v1/repositories_controller_test.rb
index cfcd917d6..84bd846c9 100644
--- a/services/api/test/functional/arvados/v1/repositories_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/repositories_controller_test.rb
@@ -150,7 +150,7 @@ class Arvados::V1::RepositoriesControllerTest < ActionController::TestCase
test "get_all_permissions obeys group permissions" do
act_as_user system_user do
r = Repository.create!(name: 'admin/groupcanwrite', owner_uuid: users(:admin).uuid)
- g = Group.create!(group_class: 'group', name: 'repo-writers')
+ g = Group.create!(group_class: 'role', name: 'repo-writers')
u1 = users(:active)
u2 = users(:spectator)
Link.create!(tail_uuid: g.uuid, head_uuid: r.uuid, link_class: 'permission', name: 'can_manage')
@@ -158,7 +158,7 @@ class Arvados::V1::RepositoriesControllerTest < ActionController::TestCase
Link.create!(tail_uuid: u2.uuid, head_uuid: g.uuid, link_class: 'permission', name: 'can_read')
r = Repository.create!(name: 'admin/groupreadonly', owner_uuid: users(:admin).uuid)
- g = Group.create!(group_class: 'group', name: 'repo-readers')
+ g = Group.create!(group_class: 'role', name: 'repo-readers')
u1 = users(:active)
u2 = users(:spectator)
Link.create!(tail_uuid: g.uuid, head_uuid: r.uuid, link_class: 'permission', name: 'can_read')
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 817a1c9ef..0ce9f1137 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -660,7 +660,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
test "non-admin user gets only safe attributes from users#show" do
g = act_as_system_user do
- create :group
+ create :group, group_class: "role"
end
users = create_list :active_user, 2, join_groups: [g]
token = create :token, user: users[0]
@@ -672,7 +672,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
[2, 4].each do |limit|
test "non-admin user can limit index to #{limit}" do
g = act_as_system_user do
- create :group
+ create :group, group_class: "role"
end
users = create_list :active_user, 4, join_groups: [g]
token = create :token, user: users[0]
diff --git a/services/api/test/integration/groups_test.rb b/services/api/test/integration/groups_test.rb
index 445670a3d..702176127 100644
--- a/services/api/test/integration/groups_test.rb
+++ b/services/api/test/integration/groups_test.rb
@@ -206,7 +206,8 @@ class NonTransactionalGroupsTest < ActionDispatch::IntegrationTest
post "/arvados/v1/groups",
params: {
group: {
- name: name
+ name: name,
+ group_class: "project"
},
async: true
},
diff --git a/services/api/test/performance/permission_test.rb b/services/api/test/performance/permission_test.rb
index d0e6413b1..e5d62cf4c 100644
--- a/services/api/test/performance/permission_test.rb
+++ b/services/api/test/performance/permission_test.rb
@@ -24,7 +24,7 @@ class PermissionPerfTest < ActionDispatch::IntegrationTest
act_as_system_user do
puts("Time spent creating records:", Benchmark.measure do
ActiveRecord::Base.transaction do
- root = Group.create!(owner_uuid: users(:permission_perftest).uuid)
+ root = Group.create!(owner_uuid: users(:permission_perftest).uuid, group_class: "project")
n += 1
a = create_eight root.uuid
n += 8
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index d447c76c6..c1db8c8b5 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -97,7 +97,7 @@ class ArvadosModelTest < ActiveSupport::TestCase
while longstring.length < 2**16
longstring = longstring + longstring
end
- g = Group.create! name: 'Has a long description', description: longstring
+ g = Group.create! name: 'Has a long description', description: longstring, group_class: "project"
g = Group.find_by_uuid g.uuid
assert_equal g.description, longstring
end
@@ -248,7 +248,7 @@ class ArvadosModelTest < ActiveSupport::TestCase
test 'create and retrieve using created_at time' do
set_user_from_auth :active
- group = Group.create! name: 'test create and retrieve group'
+ group = Group.create! name: 'test create and retrieve group', group_class: "project"
assert group.valid?, "group is not valid"
results = Group.where(created_at: group.created_at)
@@ -258,7 +258,7 @@ class ArvadosModelTest < ActiveSupport::TestCase
test 'create and update twice and expect different update times' do
set_user_from_auth :active
- group = Group.create! name: 'test create and retrieve group'
+ group = Group.create! name: 'test create and retrieve group', group_class: "project"
assert group.valid?, "group is not valid"
# update 1
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 24d7333ab..f7a531fc0 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -31,8 +31,8 @@ class GroupTest < ActiveSupport::TestCase
test "cannot create a new ownership cycle" do
set_user_from_auth :active_trustedclient
- g_foo = Group.create!(name: "foo")
- g_bar = Group.create!(name: "bar")
+ g_foo = Group.create!(name: "foo", group_class: "project")
+ g_bar = Group.create!(name: "bar", group_class: "project")
g_foo.owner_uuid = g_bar.uuid
assert g_foo.save, lambda { g_foo.errors.messages }
@@ -45,7 +45,7 @@ class GroupTest < ActiveSupport::TestCase
test "cannot create a single-object ownership cycle" do
set_user_from_auth :active_trustedclient
- g_foo = Group.create!(name: "foo")
+ g_foo = Group.create!(name: "foo", group_class: "project")
assert g_foo.save
# Ensure I have permission to manage this group even when its owner changes
@@ -63,7 +63,7 @@ class GroupTest < ActiveSupport::TestCase
test "trash group hides contents" do
set_user_from_auth :active_trustedclient
- g_foo = Group.create!(name: "foo")
+ g_foo = Group.create!(name: "foo", group_class: "project")
col = Collection.create!(owner_uuid: g_foo.uuid)
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
@@ -77,9 +77,9 @@ class GroupTest < ActiveSupport::TestCase
test "trash group" do
set_user_from_auth :active_trustedclient
- g_foo = Group.create!(name: "foo")
- g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
- g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+ g_foo = Group.create!(name: "foo", group_class: "project")
+ g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid, group_class: "project")
+ g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid, group_class: "project")
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
@@ -98,9 +98,9 @@ class GroupTest < ActiveSupport::TestCase
test "trash subgroup" do
set_user_from_auth :active_trustedclient
- g_foo = Group.create!(name: "foo")
- g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
- g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+ g_foo = Group.create!(name: "foo", group_class: "project")
+ g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid, group_class: "project")
+ g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid, group_class: "project")
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
@@ -118,9 +118,9 @@ class GroupTest < ActiveSupport::TestCase
test "trash subsubgroup" do
set_user_from_auth :active_trustedclient
- g_foo = Group.create!(name: "foo")
- g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
- g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+ g_foo = Group.create!(name: "foo", group_class: "project")
+ g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid, group_class: "project")
+ g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid, group_class: "project")
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
@@ -168,7 +168,7 @@ class GroupTest < ActiveSupport::TestCase
test "trashed does not propagate across permission links" do
set_user_from_auth :admin
- g_foo = Group.create!(name: "foo")
+ g_foo = Group.create!(name: "foo", group_class: "project")
u_bar = User.create!(first_name: "bar")
assert Group.readable_by(users(:admin)).where(uuid: g_foo.uuid).any?
@@ -237,7 +237,7 @@ class GroupTest < ActiveSupport::TestCase
set_user_from_auth :active
["", "{SOLIDUS}"].each do |subst|
Rails.configuration.Collections.ForwardSlashNameSubstitution = subst
- g = Group.create
+ g = Group.create group_class: "project"
[[nil, true],
["", true],
[".", false],
diff --git a/services/api/test/unit/owner_test.rb b/services/api/test/unit/owner_test.rb
index ca02e2db5..e356f4d9f 100644
--- a/services/api/test/unit/owner_test.rb
+++ b/services/api/test/unit/owner_test.rb
@@ -21,7 +21,11 @@ class OwnerTest < ActiveSupport::TestCase
Group.all
[User, Group].each do |o_class|
test "create object with legit #{o_class} owner" do
- o = o_class.create!
+ if o_class == Group
+ o = o_class.create! group_class: "project"
+ else
+ o = o_class.create!
+ end
i = Specimen.create(owner_uuid: o.uuid)
assert i.valid?, "new item should pass validation"
assert i.uuid, "new item should have an ID"
@@ -44,9 +48,19 @@ class OwnerTest < ActiveSupport::TestCase
[User, Group].each do |new_o_class|
test "change owner from legit #{o_class} to legit #{new_o_class} owner" do
- o = o_class.create!
+ o = if o_class == Group
+ o_class.create! group_class: "project"
+ else
+ o_class.create!
+ end
i = Specimen.create!(owner_uuid: o.uuid)
- new_o = new_o_class.create!
+
+ new_o = if new_o_class == Group
+ new_o_class.create! group_class: "project"
+ else
+ new_o_class.create!
+ end
+
assert(Specimen.where(uuid: i.uuid).any?,
"new item should really be in DB")
assert(i.update_attributes(owner_uuid: new_o.uuid),
@@ -55,7 +69,11 @@ class OwnerTest < ActiveSupport::TestCase
end
test "delete #{o_class} that owns nothing" do
- o = o_class.create!
+ if o_class == Group
+ o = o_class.create! group_class: "project"
+ else
+ o = o_class.create!
+ end
assert(o_class.where(uuid: o.uuid).any?,
"new #{o_class} should really be in DB")
assert(o.destroy, "should delete #{o_class} that owns nothing")
@@ -65,7 +83,11 @@ class OwnerTest < ActiveSupport::TestCase
test "change uuid of #{o_class} that owns nothing" do
# (we're relying on our admin credentials here)
- o = o_class.create!
+ if o_class == Group
+ o = o_class.create! group_class: "project"
+ else
+ o = o_class.create!
+ end
assert(o_class.where(uuid: o.uuid).any?,
"new #{o_class} should really be in DB")
old_uuid = o.uuid
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index cb5ae7ba2..8e2569ccc 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -153,6 +153,7 @@ class PermissionTest < ActiveSupport::TestCase
set_user_from_auth :admin
owner_grp = Group.create!(owner_uuid: users(:active).uuid, group_class: "role")
+
sp_grp = Group.create!(group_class: "project")
Link.create!(link_class: 'permission',
@@ -227,7 +228,7 @@ class PermissionTest < ActiveSupport::TestCase
# anyone any additional permissions.)
g = nil
act_as_user manager do
- g = create :group, name: "NoBigSecret Lab"
+ g = create :group, name: "NoBigSecret Lab", group_class: "role"
assert_empty(User.readable_by(manager).where(uuid: minion.uuid),
"saw a user I shouldn't see")
assert_raises(ArvadosModel::PermissionDeniedError,
@@ -323,7 +324,7 @@ class PermissionTest < ActiveSupport::TestCase
"#{a.first_name} should not be able to see 'b' in the user list")
act_as_system_user do
- g = create :group
+ g = create :group, group_class: "role"
[a,b].each do |u|
create(:permission_link,
name: 'can_read', tail_uuid: u.uuid, head_uuid: g.uuid)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list