From: Tom Clegg <tom@tomclegg.ca>
Date: Tue, 22 Sep 2020 18:59:14 +0000 (-0400)
Subject: 16809: Accept S3 reqs with same token as AccessKey and SecretKey.
X-Git-Tag: 2.1.0~13^2~4
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/580d77ef4d6b244971bc26c649e017e912ca8737

16809: Accept S3 reqs with same token as AccessKey and SecretKey.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>
---

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 629f3c1ab1..d555cb5b70 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -138,19 +138,32 @@ func (h *handler) checks3signature(r *http.Request) (string, error) {
 		Insecure: h.Config.cluster.TLS.Insecure,
 	}).WithRequestID(r.Header.Get("X-Request-Id"))
 	var aca arvados.APIClientAuthorization
-	ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+h.Config.cluster.SystemRootToken)
-	err := client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/"+key, nil, nil)
+	var secret string
+	var err error
+	if len(key) == 27 && key[5:12] == "-gj3su-" {
+		// Access key is the UUID of an Arvados token, secret
+		// key is the secret part.
+		ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+h.Config.cluster.SystemRootToken)
+		err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/"+key, nil, nil)
+		secret = aca.APIToken
+	} else {
+		// Access key and secret key are both an entire
+		// Arvados token or OIDC access token.
+		ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+key)
+		err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/current", nil, nil)
+		secret = key
+	}
 	if err != nil {
-		ctxlog.FromContext(ctx).WithError(err).WithField("UUID", key).Info("token lookup failed")
+		ctxlog.FromContext(r.Context()).WithError(err).WithField("UUID", key).Info("token lookup failed")
 		return "", errors.New("invalid access key")
 	}
-	expect, err := s3signature(s3SignAlgorithm, aca.APIToken, scope, signedHeaders, r)
+	expect, err := s3signature(s3SignAlgorithm, secret, scope, signedHeaders, r)
 	if err != nil {
 		return "", err
 	} else if expect != signature {
 		return "", errors.New("signature does not match")
 	}
-	return aca.TokenV2(), nil
+	return secret, nil
 }
 
 // serveS3 handles r and returns true if r is a request from an S3
diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index 9a3989f611..5acc18e494 100644
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -105,6 +105,40 @@ func (stage s3stage) teardown(c *check.C) {
 	}
 }
 
+func (s *IntegrationSuite) TestS3Signatures(c *check.C) {
+	stage := s.s3setup(c)
+	defer stage.teardown(c)
+
+	bucket := stage.collbucket
+	for _, trial := range []struct {
+		success   bool
+		signature int
+		accesskey string
+		secretkey string
+	}{
+		{true, aws.V2Signature, arvadostest.ActiveToken, "none"},
+		{false, aws.V2Signature, "none", "none"},
+		{false, aws.V2Signature, "none", arvadostest.ActiveToken},
+
+		{true, aws.V4Signature, arvadostest.ActiveTokenUUID, arvadostest.ActiveToken},
+		{true, aws.V4Signature, arvadostest.ActiveToken, arvadostest.ActiveToken},
+		{false, aws.V4Signature, arvadostest.ActiveToken, ""},
+		{false, aws.V4Signature, arvadostest.ActiveToken, "none"},
+		{false, aws.V4Signature, "none", arvadostest.ActiveToken},
+		{false, aws.V4Signature, "none", "none"},
+	} {
+		c.Logf("%#v", trial)
+		bucket.S3.Auth = *(aws.NewAuth(trial.accesskey, trial.secretkey, "", time.Now().Add(time.Hour)))
+		bucket.S3.Signature = trial.signature
+		_, err := bucket.GetReader("emptyfile")
+		if trial.success {
+			c.Check(err, check.IsNil)
+		} else {
+			c.Check(err, check.NotNil)
+		}
+	}
+}
+
 func (s *IntegrationSuite) TestS3HeadBucket(c *check.C) {
 	stage := s.s3setup(c)
 	defer stage.teardown(c)