17217: Sign manifests in controller instead of RailsAPI.
authorTom Clegg <tom@curii.com>
Tue, 24 Aug 2021 20:07:34 +0000 (16:07 -0400)
committerTom Clegg <tom@curii.com>
Thu, 26 Aug 2021 04:05:56 +0000 (00:05 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

lib/controller/localdb/collection.go [new file with mode: 0644]
lib/controller/localdb/collection_test.go [new file with mode: 0644]
services/api/app/models/collection.rb

diff --git a/lib/controller/localdb/collection.go b/lib/controller/localdb/collection.go
new file mode 100644 (file)
index 0000000..5293105
--- /dev/null
@@ -0,0 +1,68 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package localdb
+
+import (
+       "context"
+       "time"
+
+       "git.arvados.org/arvados.git/sdk/go/arvados"
+       "git.arvados.org/arvados.git/sdk/go/auth"
+)
+
+// CollectionGet defers to railsProxy for everything except blob
+// signatures.
+func (conn *Conn) CollectionGet(ctx context.Context, opts arvados.GetOptions) (arvados.Collection, error) {
+       if len(opts.Select) > 0 {
+               // We need to know IsTrashed and TrashAt to implement
+               // signing properly, even if the caller doesn't want
+               // them.
+               opts.Select = append([]string{"is_trashed", "trash_at"}, opts.Select...)
+       }
+       resp, err := conn.railsProxy.CollectionGet(ctx, opts)
+       if err != nil {
+               return resp, err
+       }
+       conn.signCollection(ctx, &resp)
+       return resp, nil
+}
+
+// CollectionList defers to railsProxy for everything except blob
+// signatures.
+func (conn *Conn) CollectionList(ctx context.Context, opts arvados.ListOptions) (arvados.CollectionList, error) {
+       if len(opts.Select) > 0 {
+               // We need to know IsTrashed and TrashAt to implement
+               // signing properly, even if the caller doesn't want
+               // them.
+               opts.Select = append([]string{"is_trashed", "trash_at"}, opts.Select...)
+       }
+       resp, err := conn.railsProxy.CollectionList(ctx, opts)
+       if err != nil {
+               return resp, err
+       }
+       for i := range resp.Items {
+               conn.signCollection(ctx, &resp.Items[i])
+       }
+       return resp, nil
+}
+
+func (conn *Conn) signCollection(ctx context.Context, coll *arvados.Collection) {
+       if coll.IsTrashed || coll.ManifestText == "" {
+               return
+       }
+       var token string
+       if creds, ok := auth.FromContext(ctx); ok && len(creds.Tokens) > 0 {
+               token = creds.Tokens[0]
+       }
+       if token == "" {
+               return
+       }
+       ttl := conn.cluster.Collections.BlobSigningTTL.Duration()
+       exp := time.Now().Add(ttl)
+       if coll.TrashAt != nil && !coll.TrashAt.IsZero() && coll.TrashAt.Before(exp) {
+               exp = *coll.TrashAt
+       }
+       coll.ManifestText = arvados.SignManifest(coll.ManifestText, token, exp, ttl, []byte(conn.cluster.Collections.BlobSigningKey))
+}
diff --git a/lib/controller/localdb/collection_test.go b/lib/controller/localdb/collection_test.go
new file mode 100644 (file)
index 0000000..d75ca3c
--- /dev/null
@@ -0,0 +1,80 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package localdb
+
+import (
+       "context"
+
+       "git.arvados.org/arvados.git/lib/config"
+       "git.arvados.org/arvados.git/lib/controller/rpc"
+       "git.arvados.org/arvados.git/sdk/go/arvados"
+       "git.arvados.org/arvados.git/sdk/go/arvadostest"
+       "git.arvados.org/arvados.git/sdk/go/auth"
+       "git.arvados.org/arvados.git/sdk/go/ctxlog"
+       check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&CollectionSuite{})
+
+type CollectionSuite struct {
+       cluster  *arvados.Cluster
+       localdb  *Conn
+       railsSpy *arvadostest.Proxy
+}
+
+func (s *CollectionSuite) TearDownSuite(c *check.C) {
+       // Undo any changes/additions to the user database so they
+       // don't affect subsequent tests.
+       arvadostest.ResetEnv()
+       c.Check(arvados.NewClientFromEnv().RequestAndDecode(nil, "POST", "database/reset", nil, nil), check.IsNil)
+}
+
+func (s *CollectionSuite) SetUpTest(c *check.C) {
+       cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
+       c.Assert(err, check.IsNil)
+       s.cluster, err = cfg.GetCluster("")
+       c.Assert(err, check.IsNil)
+       s.localdb = NewConn(s.cluster)
+       s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
+       *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)
+}
+
+func (s *CollectionSuite) TearDownTest(c *check.C) {
+       s.railsSpy.Close()
+}
+
+func (s *CollectionSuite) TestSignatures(c *check.C) {
+       ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.ActiveTokenV2}})
+
+       resp, err := s.localdb.CollectionGet(ctx, arvados.GetOptions{UUID: arvadostest.FooCollection})
+       c.Check(err, check.IsNil)
+       c.Check(resp.ManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3\+A[0-9a-f]+@[0-9a-f]+ 0:.*`)
+
+       resp, err = s.localdb.CollectionGet(ctx, arvados.GetOptions{UUID: arvadostest.FooCollection, Select: []string{"manifest_text"}})
+       c.Check(err, check.IsNil)
+       c.Check(resp.ManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3\+A[0-9a-f]+@[0-9a-f]+ 0:.*`)
+
+       lresp, err := s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}}})
+       c.Check(err, check.IsNil)
+       if c.Check(lresp.Items, check.HasLen, 1) {
+               c.Check(lresp.Items[0].UUID, check.Equals, arvadostest.FooCollection)
+               c.Check(lresp.Items[0].ManifestText, check.Equals, "")
+               c.Check(lresp.Items[0].UnsignedManifestText, check.Equals, "")
+       }
+
+       lresp, err = s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}}, Select: []string{"manifest_text"}})
+       c.Check(err, check.IsNil)
+       if c.Check(lresp.Items, check.HasLen, 1) {
+               c.Check(lresp.Items[0].ManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3\+A[0-9a-f]+@[0-9a-f]+ 0:.*`)
+               c.Check(lresp.Items[0].UnsignedManifestText, check.Equals, "")
+       }
+
+       lresp, err = s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}}, Select: []string{"unsigned_manifest_text"}})
+       c.Check(err, check.IsNil)
+       if c.Check(lresp.Items, check.HasLen, 1) {
+               c.Check(lresp.Items[0].ManifestText, check.Equals, "")
+               c.Check(lresp.Items[0].UnsignedManifestText, check.Matches, `(?ms).* acbd[^ ]*\+3 0:.*`)
+       }
+}
index 1bbe8cc6614b2becdfeac1afef32cd8fcfb4beea..2560ac2c98dcf454e715a1d85f1576087dcf21cc 100644 (file)
@@ -420,17 +420,7 @@ class Collection < ArvadosModel
   end
 
   def self.sign_manifest manifest, token, exp=nil
-    if exp.nil?
-      exp = db_current_time.to_i + Rails.configuration.Collections.BlobSigningTTL.to_i
-    end
-    signing_opts = {
-      api_token: token,
-      expire: exp,
-    }
-    m = munge_manifest_locators(manifest) do |match|
-      Blob.sign_locator(match[0], signing_opts)
-    end
-    return m
+    return manifest
   end
 
   def self.munge_manifest_locators manifest