[ARVADOS] updated: 1.3.0-2853-g046863fce
Git user
git at public.arvados.org
Thu Aug 13 17:42:41 UTC 2020
Summary of changes:
doc/install/install-keep-web.html.textile.liquid | 1 +
doc/install/install-keepproxy.html.textile.liquid | 1 +
sdk/python/tests/fed-migrate/check.py | 61 +++++++++++++++++++++++
3 files changed, 63 insertions(+)
discards 7a28dbcb0b7a0c47392d24cfc9ed9925f1c03c07 (commit)
discards dac56d86ac0c62f4cb47de13755b0deee9df04d2 (commit)
discards 01143d3edb08e192fcc47d5eef1edbf80486f0d2 (commit)
via 046863fce3eefdd8f2b4588855b2335dcb0215e1 (commit)
via 5fde4c5e8c464ec55d002735003a564a7802c720 (commit)
via de89bbf6824a39990e1605e4bd041b5d1ed464ea (commit)
via 2ba6cc7a5e4bfd05cd51e8ab22be2a99a883349d (commit)
via ab92008f810e7cdbea981ce903670621c54c082e (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 (7a28dbcb0b7a0c47392d24cfc9ed9925f1c03c07)
\
N -- N -- N (046863fce3eefdd8f2b4588855b2335dcb0215e1)
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 046863fce3eefdd8f2b4588855b2335dcb0215e1
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Aug 12 17:32:06 2020 -0400
16683: Permit granting permissions to remote users
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 01a31adb9..eb2ea1731 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -749,6 +749,20 @@ class ArvadosModel < ApplicationRecord
%r/[a-z0-9]{5}-#{uuid_prefix}-[a-z0-9]{15}/
end
+ def check_readable_uuid attr, attr_value
+ return if attr_value.nil?
+ if (r = ArvadosModel::resource_class_for_uuid attr_value)
+ unless skip_uuid_read_permission_check.include? attr
+ r = r.readable_by(current_user)
+ end
+ if r.where(uuid: attr_value).count == 0
+ errors.add(attr, "'#{attr_value}' not found")
+ end
+ else
+ errors.add(attr, "'#{attr_value}' invalid uuid")
+ end
+ end
+
def ensure_valid_uuids
specials = [system_user_uuid]
@@ -757,16 +771,7 @@ class ArvadosModel < ApplicationRecord
next if skip_uuid_existence_check.include? attr
attr_value = send attr
next if specials.include? attr_value
- if attr_value
- if (r = ArvadosModel::resource_class_for_uuid attr_value)
- unless skip_uuid_read_permission_check.include? attr
- r = r.readable_by(current_user)
- end
- if r.where(uuid: attr_value).count == 0
- errors.add(attr, "'#{attr_value}' not found")
- end
- end
- end
+ check_readable_uuid attr, attr_value
end
end
end
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index e4ba7f3de..7f4433dd7 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -43,6 +43,27 @@ class Link < ArvadosModel
protected
+ def check_readable_uuid attr, attr_value
+ if attr == 'tail_uuid' &&
+ !attr_value.nil? &&
+ self.link_class == 'permission' &&
+ attr_value[0..4] != Rails.configuration.ClusterID &&
+ ArvadosModel::resource_class_for_uuid(attr_value) == User
+ # Permission link tail is a remote user (the user permissions
+ # are being granted to), so bypass the standard check that a
+ # referenced object uuid is readable by current user.
+ #
+ # We could do a call to the remote cluster to check if the user
+ # in tail_uuid exists. This would detect copy-and-paste errors,
+ # but add another way for the request to fail, and I don't think
+ # it would improve security. It doesn't seem to be worth the
+ # complexity tradeoff.
+ true
+ else
+ super
+ end
+ end
+
def permission_to_attach_to_objects
# Anonymous users cannot write links
return false if !current_user
@@ -76,6 +97,11 @@ class Link < ArvadosModel
head_obj = ArvadosModel.find_by_uuid(head_uuid)
+ if head_obj.nil?
+ errors.add(:head_uuid, "does not exist")
+ return false
+ end
+
# No permission links can be pointed to past collection versions
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")
commit 5fde4c5e8c464ec55d002735003a564a7802c720
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Aug 12 16:20:19 2020 -0400
16683: Sharing dialog query is compatible with federated user list
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/apps/workbench/app/views/application/_show_sharing.html.erb b/apps/workbench/app/views/application/_show_sharing.html.erb
index 7877e60d3..75773ab90 100644
--- a/apps/workbench/app/views/application/_show_sharing.html.erb
+++ b/apps/workbench/app/views/application/_show_sharing.html.erb
@@ -8,6 +8,8 @@ SPDX-License-Identifier: AGPL-3.0 %>
[User, Group].each do |type|
type
.filter([['uuid','in', at share_links.collect(&:tail_uuid)]])
+ .with_count("none")
+ .fetch_multiple_pages(false)
.each do |o|
uuid_map[o.uuid] = o
end
commit de89bbf6824a39990e1605e4bd041b5d1ed464ea
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Aug 12 16:10:32 2020 -0400
16683: Set and check ForwardedFor on federated list requests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/generated.go b/lib/controller/federation/generated.go
index 20edd90b9..8745f3b97 100755
--- a/lib/controller/federation/generated.go
+++ b/lib/controller/federation/generated.go
@@ -23,6 +23,7 @@ func (conn *Conn) generated_ContainerList(ctx context.Context, options arvados.L
var needSort atomic.Value
needSort.Store(false)
err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) {
+ options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
cl, err := backend.ContainerList(ctx, options)
if err != nil {
return nil, err
@@ -63,6 +64,7 @@ func (conn *Conn) generated_SpecimenList(ctx context.Context, options arvados.Li
var needSort atomic.Value
needSort.Store(false)
err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) {
+ options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
cl, err := backend.SpecimenList(ctx, options)
if err != nil {
return nil, err
@@ -103,6 +105,7 @@ func (conn *Conn) generated_UserList(ctx context.Context, options arvados.ListOp
var needSort atomic.Value
needSort.Store(false)
err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) {
+ options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
cl, err := backend.UserList(ctx, options)
if err != nil {
return nil, err
diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index 0a596eb9c..bc6d3e00a 100644
--- a/lib/controller/federation/list.go
+++ b/lib/controller/federation/list.go
@@ -27,6 +27,7 @@ func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.
var needSort atomic.Value
needSort.Store(false)
err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) {
+ options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
cl, err := backend.CollectionList(ctx, options)
if err != nil {
return nil, err
@@ -107,7 +108,7 @@ func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.
// backend.
func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions, fn func(context.Context, string, arvados.API, arvados.ListOptions) ([]string, error)) error {
- if opts.BypassFederation {
+ if opts.BypassFederation || opts.ForwardedFor != "" {
// Client requested no federation. Pass through.
_, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts)
return err
@@ -249,7 +250,7 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
done, err := fn(ctx, clusterID, backend, remoteOpts)
if err != nil {
- errs <- httpErrorf(http.StatusBadGateway, err.Error())
+ errs <- httpErrorf(http.StatusBadGateway, "%s", err.Error())
return
}
progress := false
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index c32f88864..4e7e74c8a 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -86,6 +86,7 @@ type ListOptions struct {
IncludeTrash bool `json:"include_trash"`
IncludeOldVersions bool `json:"include_old_versions"`
BypassFederation bool `json:"bypass_federation"`
+ ForwardedFor string `json:"forwarded_for,omitempty"`
}
type CreateOptions struct {
commit 2ba6cc7a5e4bfd05cd51e8ab22be2a99a883349d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Aug 13 12:44:29 2020 -0400
16683: Add checks related to sharing and remote users
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/python/tests/fed-migrate/check.py b/sdk/python/tests/fed-migrate/check.py
index c231cc073..e31ac0541 100644
--- a/sdk/python/tests/fed-migrate/check.py
+++ b/sdk/python/tests/fed-migrate/check.py
@@ -3,6 +3,7 @@
# SPDX-License-Identifier: Apache-2.0
import arvados
+import arvados.errors
import json
import sys
@@ -113,4 +114,64 @@ for i in (3, 5, 9):
users = apiC.users().list().execute()
check_A(users)
+
+####
+# bug 16683 tests
+
+# Check that this query returns empty, instead of returning a 500 or
+# 502 error.
+# Yes, we're asking for a group from the users endpoint. This is not a
+# mistake, this is something workbench does to populate the sharing
+# dialog.
+clusterID_B = apiB.configs().get().execute()["ClusterID"]
+i = apiB.users().list(filters=[["uuid", "in", ["%s-j7d0g-fffffffffffffff" % clusterID_B]]], count="none").execute()
+assert len(i["items"]) == 0
+
+# Check that we can create a project and give a remote user access to it
+
+tok3 = apiA.api_client_authorizations().create(body={"api_client_authorization": {"owner_uuid": by_username["case3"]}}).execute()
+tok4 = apiA.api_client_authorizations().create(body={"api_client_authorization": {"owner_uuid": by_username["case4"]}}).execute()
+
+v2_token3 = "v2/%s/%s" % (tok3["uuid"], tok3["api_token"])
+v2_token4 = "v2/%s/%s" % (tok4["uuid"], tok4["api_token"])
+
+apiB_3 = arvados.api(host=j["arvados_api_hosts"][1], token=v2_token3, insecure=True)
+apiB_4 = arvados.api(host=j["arvados_api_hosts"][1], token=v2_token4, insecure=True)
+
+assert apiB_3.users().current().execute()["uuid"] == by_username["case3"]
+assert apiB_4.users().current().execute()["uuid"] == by_username["case4"]
+
+newproject = apiB_3.groups().create(body={"group_class": "project",
+ "name":"fed test project"},
+ ensure_unique_name=True).execute()
+
+try:
+ # Expect to fail
+ apiB_4.groups().get(uuid=newproject["uuid"]).execute()
+except arvados.errors.ApiError as e:
+ if e.resp['status'] == '404':
+ pass
+ else:
+ raise
+
+l = apiB_3.links().create(body={"link_class": "permission",
+ "name":"can_read",
+ "tail_uuid": by_username["case4"],
+ "head_uuid": newproject["uuid"]}).execute()
+
+# Expect to succeed
+apiB_4.groups().get(uuid=newproject["uuid"]).execute()
+
+# remove permission
+apiB_3.links().delete(uuid=l["uuid"]).execute()
+
+try:
+ # Expect to fail again
+ apiB_4.groups().get(uuid=newproject["uuid"]).execute()
+except arvados.errors.ApiError as e:
+ if e.resp['status'] == '404':
+ pass
+ else:
+ raise
+
print("Passed checks")
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list