[ARVADOS] created: 2.1.0-108-g6a613ba16
Git user
git at public.arvados.org
Wed Nov 18 14:56:29 UTC 2020
at 6a613ba162b66beab17bcdf6192034d6ed335ad4 (commit)
commit 6a613ba162b66beab17bcdf6192034d6ed335ad4
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Nov 17 15:42:28 2020 -0500
17106: Improve handling of bare tokens issued by remote clusters.
When caching, use the remote cluster's original token UUID.
When returning the current api_client_authorization record, include
the secret supplied by the caller, even when only storing the HMAC in
the database.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 74a4c1efa..1c1c669de 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -130,6 +130,7 @@ class ApiClientAuthorization < ArvadosModel
token_uuid = ''
secret = token
+ stored_secret = nil # ...if different from secret
optional = nil
case token[0..2]
@@ -206,8 +207,7 @@ class ApiClientAuthorization < ArvadosModel
# below. If so, we'll stuff the database with hmac instead of
# the real OIDC token.
upstream_cluster_id = Rails.configuration.Login.LoginCluster
- token_uuid = upstream_cluster_id + generate_uuid[5..27]
- secret = hmac
+ stored_secret = hmac
else
return nil
end
@@ -246,6 +246,23 @@ class ApiClientAuthorization < ArvadosModel
remote_user_prefix = remote_user['uuid'][0..4]
+ if token_uuid == ''
+ # Use the same UUID as the remote when caching the token.
+ begin
+ remote_token = SafeJSON.load(
+ clnt.get_content('https://' + host + '/arvados/v1/api_client_authorizations/current',
+ {'remote' => Rails.configuration.ClusterID},
+ {'Authorization' => 'Bearer ' + token}))
+ token_uuid = remote_token['uuid']
+ if !token_uuid.match(HasUuid::UUID_REGEX) || token_uuid[0..4] != upstream_cluster_id
+ raise "remote cluster #{upstream_cluster_id} returned invalid token uuid #{token_uuid.inspect}"
+ end
+ rescue => e
+ Rails.logger.warn "error getting remote token details for #{token.inspect}: #{e}"
+ return nil
+ end
+ end
+
# Clusters can only authenticate for their own users.
if remote_user_prefix != upstream_cluster_id
Rails.logger.warn "remote authentication rejected: claimed remote user #{remote_user_prefix} but token was issued by #{upstream_cluster_id}"
@@ -328,11 +345,13 @@ class ApiClientAuthorization < ArvadosModel
auth.user = user
auth.api_client_id = 0
end
+ stored_secret = stored_secret || secret
auth.update_attributes!(user: user,
- api_token: secret,
+ api_token: stored_secret,
api_client_id: 0,
expires_at: Time.now + Rails.configuration.Login.RemoteTokenRefresh)
- Rails.logger.debug "cached remote token #{token_uuid} with secret #{secret} in local db"
+ Rails.logger.debug "cached remote token #{token_uuid} with secret #{stored_secret} in local db"
+ auth.api_token = secret
return auth
end
commit 40a52eaf419c4a7c2f0e32cc87aaba29e0098439
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Nov 17 15:42:11 2020 -0500
17106: Clean up test.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 5c1181b70..6ac8c2e33 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -294,71 +294,63 @@ func (s *IntegrationSuite) TestS3WithFederatedToken(c *check.C) {
conn1 := s.conn("z1111")
rootctx1, _, _ := s.rootClients("z1111")
- userctx1, ac1, kc1, _ := s.userClients(rootctx1, c, conn1, "z1111", true)
+ userctx1, ac1, _, _ := s.userClients(rootctx1, c, conn1, "z1111", true)
conn3 := s.conn("z3333")
- _, ac3, kc3 := s.clientsWithToken("z3333", ac1.AuthToken)
-
- // Create a collection on z1111
- var coll arvados.Collection
- fs1, err := coll.FileSystem(ac1, kc1)
- c.Assert(err, check.IsNil)
- f, err := fs1.OpenFile("test.txt", os.O_CREATE|os.O_RDWR, 0777)
- c.Assert(err, check.IsNil)
- _, err = io.WriteString(f, testText)
- c.Assert(err, check.IsNil)
- err = f.Close()
- c.Assert(err, check.IsNil)
- mtxt, err := fs1.MarshalManifest(".")
- c.Assert(err, check.IsNil)
- coll1, err := conn1.CollectionCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
- "manifest_text": mtxt,
- }})
- c.Assert(err, check.IsNil)
- // Create same collection on z3333
- fs3, err := coll.FileSystem(ac3, kc3)
- c.Assert(err, check.IsNil)
- f, err = fs3.OpenFile("test.txt", os.O_CREATE|os.O_RDWR, 0777)
- c.Assert(err, check.IsNil)
- _, err = io.WriteString(f, testText)
- c.Assert(err, check.IsNil)
- err = f.Close()
- c.Assert(err, check.IsNil)
- mtxt, err = fs3.MarshalManifest(".")
- c.Assert(err, check.IsNil)
- coll3, err := conn3.CollectionCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
- "manifest_text": mtxt,
- }})
- c.Assert(err, check.IsNil)
+ createColl := func(clusterID string) arvados.Collection {
+ _, ac, kc := s.clientsWithToken(clusterID, ac1.AuthToken)
+ var coll arvados.Collection
+ fs, err := coll.FileSystem(ac, kc)
+ c.Assert(err, check.IsNil)
+ f, err := fs.OpenFile("test.txt", os.O_CREATE|os.O_RDWR, 0777)
+ c.Assert(err, check.IsNil)
+ _, err = io.WriteString(f, testText)
+ c.Assert(err, check.IsNil)
+ err = f.Close()
+ c.Assert(err, check.IsNil)
+ mtxt, err := fs.MarshalManifest(".")
+ c.Assert(err, check.IsNil)
+ coll, err = s.conn(clusterID).CollectionCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
+ "manifest_text": mtxt,
+ }})
+ c.Assert(err, check.IsNil)
+ return coll
+ }
for _, trial := range []struct {
- label string
- conn *rpc.Conn
- coll arvados.Collection
+ clusterID string // create the collection on this cluster (then use z3333 to access it)
+ token string
}{
- {"z1111", conn1, coll1},
- {"z3333", conn3, coll3},
+ // Try the hardest test first: z3333 hasn't seen
+ // z1111's token yet, and we're just passing the
+ // opaque secret part, so z3333 has to guess that it
+ // belongs to z1111.
+ {"z1111", strings.Split(ac1.AuthToken, "/")[2]},
+ {"z3333", strings.Split(ac1.AuthToken, "/")[2]},
+ {"z1111", strings.Replace(ac1.AuthToken, "/", "_", -1)},
+ {"z3333", strings.Replace(ac1.AuthToken, "/", "_", -1)},
} {
- c.Logf("================ %s", trial.label)
- cfgjson, err := trial.conn.ConfigGet(userctx1)
+ c.Logf("================ %v", trial)
+ coll := createColl(trial.clusterID)
+
+ cfgjson, err := conn3.ConfigGet(userctx1)
c.Assert(err, check.IsNil)
var cluster arvados.Cluster
err = json.Unmarshal(cfgjson, &cluster)
c.Assert(err, check.IsNil)
c.Logf("TokenV2 is %s", ac1.AuthToken)
- mungedtoken := strings.Replace(ac1.AuthToken, "/", "_", -1)
host := cluster.Services.WebDAV.ExternalURL.Host
s3args := []string{
"--ssl", "--no-check-certificate",
"--host=" + host, "--host-bucket=" + host,
- "--access_key=" + mungedtoken, "--secret_key=" + mungedtoken,
+ "--access_key=" + trial.token, "--secret_key=" + trial.token,
}
- buf, err := exec.Command("s3cmd", append(s3args, "ls", "s3://"+trial.coll.UUID)...).CombinedOutput()
+ buf, err := exec.Command("s3cmd", append(s3args, "ls", "s3://"+coll.UUID)...).CombinedOutput()
c.Check(err, check.IsNil)
- c.Check(string(buf), check.Matches, `.* `+fmt.Sprintf("%d", len(testText))+` +s3://`+trial.coll.UUID+`/test.txt\n`)
+ c.Check(string(buf), check.Matches, `.* `+fmt.Sprintf("%d", len(testText))+` +s3://`+coll.UUID+`/test.txt\n`)
- buf, err = exec.Command("s3cmd", append(s3args, "get", "s3://"+trial.coll.UUID+"/test.txt", c.MkDir()+"/tmpfile")...).CombinedOutput()
+ buf, err = exec.Command("s3cmd", append(s3args, "get", "s3://"+coll.UUID+"/test.txt", c.MkDir()+"/tmpfile")...).CombinedOutput()
// Command fails because we don't return Etag header.
// c.Check(err, check.IsNil)
flen := strconv.Itoa(len(testText))
commit 395fa167d4be136cd34e9712d98f55d70872de45
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Nov 17 09:39:50 2020 -0500
17106: Skip s3cmd test if s3cmd not installed.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 1f28c877a..5c1181b70 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -285,6 +285,11 @@ func (s *IntegrationSuite) TestGetCollectionByPDH(c *check.C) {
}
func (s *IntegrationSuite) TestS3WithFederatedToken(c *check.C) {
+ if _, err := exec.LookPath("s3cmd"); err != nil {
+ c.Skip("s3cmd not in PATH")
+ return
+ }
+
testText := "IntegrationSuite.TestS3WithFederatedToken"
conn1 := s.conn("z1111")
commit 41a24992268299cf05abbf723558b7b7f0d0b296
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Nov 16 21:25:12 2020 -0500
17106: Test S3 with modified v2 token issued by LoginCluster.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 3da01ca68..1f28c877a 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -8,6 +8,7 @@ import (
"bytes"
"context"
"encoding/json"
+ "fmt"
"io"
"io/ioutil"
"math"
@@ -15,7 +16,10 @@ import (
"net/http"
"net/url"
"os"
+ "os/exec"
"path/filepath"
+ "strconv"
+ "strings"
"git.arvados.org/arvados.git/lib/boot"
"git.arvados.org/arvados.git/lib/config"
@@ -280,6 +284,83 @@ func (s *IntegrationSuite) TestGetCollectionByPDH(c *check.C) {
c.Check(coll.PortableDataHash, check.Equals, pdh)
}
+func (s *IntegrationSuite) TestS3WithFederatedToken(c *check.C) {
+ testText := "IntegrationSuite.TestS3WithFederatedToken"
+
+ conn1 := s.conn("z1111")
+ rootctx1, _, _ := s.rootClients("z1111")
+ userctx1, ac1, kc1, _ := s.userClients(rootctx1, c, conn1, "z1111", true)
+ conn3 := s.conn("z3333")
+ _, ac3, kc3 := s.clientsWithToken("z3333", ac1.AuthToken)
+
+ // Create a collection on z1111
+ var coll arvados.Collection
+ fs1, err := coll.FileSystem(ac1, kc1)
+ c.Assert(err, check.IsNil)
+ f, err := fs1.OpenFile("test.txt", os.O_CREATE|os.O_RDWR, 0777)
+ c.Assert(err, check.IsNil)
+ _, err = io.WriteString(f, testText)
+ c.Assert(err, check.IsNil)
+ err = f.Close()
+ c.Assert(err, check.IsNil)
+ mtxt, err := fs1.MarshalManifest(".")
+ c.Assert(err, check.IsNil)
+ coll1, err := conn1.CollectionCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
+ "manifest_text": mtxt,
+ }})
+ c.Assert(err, check.IsNil)
+
+ // Create same collection on z3333
+ fs3, err := coll.FileSystem(ac3, kc3)
+ c.Assert(err, check.IsNil)
+ f, err = fs3.OpenFile("test.txt", os.O_CREATE|os.O_RDWR, 0777)
+ c.Assert(err, check.IsNil)
+ _, err = io.WriteString(f, testText)
+ c.Assert(err, check.IsNil)
+ err = f.Close()
+ c.Assert(err, check.IsNil)
+ mtxt, err = fs3.MarshalManifest(".")
+ c.Assert(err, check.IsNil)
+ coll3, err := conn3.CollectionCreate(userctx1, arvados.CreateOptions{Attrs: map[string]interface{}{
+ "manifest_text": mtxt,
+ }})
+ c.Assert(err, check.IsNil)
+
+ for _, trial := range []struct {
+ label string
+ conn *rpc.Conn
+ coll arvados.Collection
+ }{
+ {"z1111", conn1, coll1},
+ {"z3333", conn3, coll3},
+ } {
+ c.Logf("================ %s", trial.label)
+ cfgjson, err := trial.conn.ConfigGet(userctx1)
+ c.Assert(err, check.IsNil)
+ var cluster arvados.Cluster
+ err = json.Unmarshal(cfgjson, &cluster)
+ c.Assert(err, check.IsNil)
+
+ c.Logf("TokenV2 is %s", ac1.AuthToken)
+ mungedtoken := strings.Replace(ac1.AuthToken, "/", "_", -1)
+ host := cluster.Services.WebDAV.ExternalURL.Host
+ s3args := []string{
+ "--ssl", "--no-check-certificate",
+ "--host=" + host, "--host-bucket=" + host,
+ "--access_key=" + mungedtoken, "--secret_key=" + mungedtoken,
+ }
+ buf, err := exec.Command("s3cmd", append(s3args, "ls", "s3://"+trial.coll.UUID)...).CombinedOutput()
+ c.Check(err, check.IsNil)
+ c.Check(string(buf), check.Matches, `.* `+fmt.Sprintf("%d", len(testText))+` +s3://`+trial.coll.UUID+`/test.txt\n`)
+
+ buf, err = exec.Command("s3cmd", append(s3args, "get", "s3://"+trial.coll.UUID+"/test.txt", c.MkDir()+"/tmpfile")...).CombinedOutput()
+ // Command fails because we don't return Etag header.
+ // c.Check(err, check.IsNil)
+ flen := strconv.Itoa(len(testText))
+ c.Check(string(buf), check.Matches, `(?ms).*`+flen+` of `+flen+`.*`)
+ }
+}
+
func (s *IntegrationSuite) TestGetCollectionAsAnonymous(c *check.C) {
conn1 := s.conn("z1111")
conn3 := s.conn("z3333")
commit 0024bcc58d6401c50b73c67b653535ccc4799db4
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Nov 16 20:37:05 2020 -0500
17106: Accept v2 token with / replaced by _ as s3 access/secret key.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 49fb2456f..ef3a16404 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -152,7 +152,14 @@ func (h *handler) checks3signature(r *http.Request) (string, error) {
} else {
// Access key and secret key are both an entire
// Arvados token or OIDC access token.
- ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+key)
+ mungedKey := key
+ if strings.HasPrefix(key, "v2_") {
+ // Entire Arvados token, with "/" replaced by
+ // "_" to avoid colliding with the
+ // Authorization header format.
+ mungedKey = strings.Replace(key, "_", "/", -1)
+ }
+ ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+mungedKey)
err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/current", nil, nil)
secret = key
}
@@ -170,7 +177,7 @@ func (h *handler) checks3signature(r *http.Request) (string, error) {
} else if expect != signature {
return "", fmt.Errorf("signature does not match (scope %q signedHeaders %q stringToSign %q)", scope, signedHeaders, stringToSign)
}
- return secret, nil
+ return aca.TokenV2(), nil
}
// serveS3 handles r and returns true if r is a request from an S3
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list