[ARVADOS] created: 1.1.1-262-ge112475
Git user
git at public.curoverse.com
Thu Dec 14 22:14:12 EST 2017
at e112475459e4cab53cac0121606bc895efeb5ae5 (commit)
commit e112475459e4cab53cac0121606bc895efeb5ae5
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Thu Dec 14 22:13:17 2017 -0500
12822: Allow reading from keepproxy with collection sharing tokens.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index eab7a27..3a611a3 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -37,6 +37,9 @@ const (
FooRepoName = "active/foo"
Repository2UUID = "zzzzz-s0uqq-382brsig8rp3667"
Repository2Name = "active/foo2"
+
+ FooCollectionSharingTokenUUID = "zzzzz-gj3su-gf02tdm4g1z3e3u"
+ FooCollectionSharingToken = "iknqgmunrhgsyfok8uzjlwun9iscwm3xacmzmg65fa1j1lpdss"
)
// PathologicalManifest : A valid manifest designed to test
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 23e63f2..8b283db 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -316,3 +316,14 @@ permission_perftest:
user: permission_perftest
api_token: 3kg6k6lzmp9kjabonentustoecn5bahbt2fod9zru30k1jqdmi
expires_at: 2038-01-01 00:00:00
+
+foo_collection_sharing_token:
+ uuid: zzzzz-gj3su-gf02tdm4g1z3e3u
+ api_client: untrusted
+ user: active
+ api_token: iknqgmunrhgsyfok8uzjlwun9iscwm3xacmzmg65fa1j1lpdss
+ expires_at: 2038-01-01 00:00:00
+ scopes:
+ - GET /arvados/v1/collections/zzzzz-4zz18-znfnqtbbv4spc3w
+ - GET /arvados/v1/collections/zzzzz-4zz18-znfnqtbbv4spc3w/
+ - GET /arvados/v1/keep_services/accessible
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 145b39d..0c0c08f 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -232,31 +232,43 @@ func GetRemoteAddress(req *http.Request) string {
}
func CheckAuthorizationHeader(kc *keepclient.KeepClient, cache *ApiTokenCache, req *http.Request) (pass bool, tok string) {
- var auth string
- if auth = req.Header.Get("Authorization"); auth == "" {
+ parts := strings.SplitN(req.Header.Get("Authorization"), " ", 2)
+ if len(parts) < 2 || !(parts[0] == "OAuth2" || parts[0] == "Bearer") || len(parts[1]) == 0 {
return false, ""
}
+ tok = parts[1]
- _, err := fmt.Sscanf(auth, "OAuth2 %s", &tok)
- if err != nil {
- // Scanning error
- return false, ""
+ // Tokens are validated differently depending on what kind of
+ // operation is being performed. For example, tokens in
+ // collection-sharing links permit GET requests, but not
+ // PUT requests.
+ var op string
+ if req.Method == "GET" || req.Method == "HEAD" {
+ op = "read"
+ } else {
+ op = "write"
}
- if cache.RecallToken(tok) {
+ if cache.RecallToken(op + ":" + tok) {
// Valid in the cache, short circuit
return true, tok
}
+ var err error
arv := *kc.Arvados
arv.ApiToken = tok
- if err := arv.Call("HEAD", "users", "", "current", nil, nil); err != nil {
+ if op == "read" {
+ err = arv.Call("HEAD", "keep_services", "", "accessible", nil, nil)
+ } else {
+ err = arv.Call("HEAD", "users", "", "current", nil, nil)
+ }
+ if err != nil {
log.Printf("%s: CheckAuthorizationHeader error: %v", GetRemoteAddress(req), err)
return false, ""
}
// Success! Update cache
- cache.RememberToken(tok)
+ cache.RememberToken(op + ":" + tok)
return true, tok
}
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index a7b608b..bb0e9bb 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -323,41 +323,26 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
kc := runProxy(c, nil, true)
defer closeListener()
- hash := fmt.Sprintf("%x", md5.Sum([]byte("bar")))
+ hash := fmt.Sprintf("%x+3", md5.Sum([]byte("bar")))
- {
- _, _, err := kc.Ask(hash)
- errNotFound, _ := err.(keepclient.ErrNotFound)
- c.Check(errNotFound, NotNil)
- c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
- c.Log("Ask 1")
- }
+ _, _, err := kc.Ask(hash)
+ c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
- {
- hash2, rep, err := kc.PutB([]byte("bar"))
- c.Check(hash2, Equals, "")
- c.Check(rep, Equals, 0)
- c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
- c.Log("PutB")
- }
+ hash2, rep, err := kc.PutB([]byte("bar"))
+ c.Check(hash2, Equals, "")
+ c.Check(rep, Equals, 0)
+ c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
- {
- blocklen, _, err := kc.Ask(hash)
- errNotFound, _ := err.(keepclient.ErrNotFound)
- c.Check(errNotFound, NotNil)
- c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
- c.Check(blocklen, Equals, int64(0))
- c.Log("Ask 2")
- }
+ blocklen, _, err := kc.Ask(hash)
+ c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
+ c.Check(err, ErrorMatches, ".*not found.*")
+ c.Check(blocklen, Equals, int64(0))
+
+ _, blocklen, _, err = kc.Get(hash)
+ c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
+ c.Check(err, ErrorMatches, ".*not found.*")
+ c.Check(blocklen, Equals, int64(0))
- {
- _, blocklen, _, err := kc.Get(hash)
- errNotFound, _ := err.(keepclient.ErrNotFound)
- c.Check(errNotFound, NotNil)
- c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
- c.Check(blocklen, Equals, int64(0))
- c.Log("Get")
- }
}
func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
@@ -544,35 +529,53 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
c.Assert((err != nil), Equals, true)
}
+func (s *ServerRequiredSuite) TestCollectionSharingToken(c *C) {
+ kc := runProxy(c, nil, false)
+ defer closeListener()
+ hash, _, err := kc.PutB([]byte("shareddata"))
+ c.Check(err, IsNil)
+ kc.Arvados.ApiToken = arvadostest.FooCollectionSharingToken
+ rdr, _, _, err := kc.Get(hash)
+ c.Assert(err, IsNil)
+ data, err := ioutil.ReadAll(rdr)
+ c.Check(err, IsNil)
+ c.Check(data, DeepEquals, []byte("shareddata"))
+}
+
func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
kc := runProxy(c, nil, false)
defer closeListener()
// Put a test block
hash, rep, err := kc.PutB([]byte("foo"))
- c.Check(err, Equals, nil)
+ c.Check(err, IsNil)
c.Check(rep, Equals, 2)
- for _, token := range []string{
+ for _, badToken := range []string{
"nosuchtoken",
"2ym314ysp27sk7h943q6vtc378srb06se3pq6ghurylyf3pdmx", // expired
} {
- // Change token to given bad token
- kc.Arvados.ApiToken = token
+ kc.Arvados.ApiToken = badToken
+
+ // Ask and Get will fail only if the upstream
+ // keepstore server checks for valid signatures.
+ // Without knowing the blob signing key, there is no
+ // way for keepproxy to know whether a given token is
+ // permitted to read a block. So these tests fail:
+ if false {
+ _, _, err = kc.Ask(hash)
+ c.Assert(err, FitsTypeOf, &keepclient.ErrNotFound{})
+ c.Check(err.(*keepclient.ErrNotFound).Temporary(), Equals, false)
+ c.Check(err, ErrorMatches, ".*HTTP 403.*")
+
+ _, _, _, err = kc.Get(hash)
+ c.Assert(err, FitsTypeOf, &keepclient.ErrNotFound{})
+ c.Check(err.(*keepclient.ErrNotFound).Temporary(), Equals, false)
+ c.Check(err, ErrorMatches, ".*HTTP 403 \"Missing or invalid Authorization header\".*")
+ }
- // Ask should result in error
- _, _, err = kc.Ask(hash)
- c.Check(err, NotNil)
- errNotFound, _ := err.(keepclient.ErrNotFound)
- c.Check(errNotFound.Temporary(), Equals, false)
- c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
-
- // Get should result in error
- _, _, _, err = kc.Get(hash)
- c.Check(err, NotNil)
- errNotFound, _ = err.(keepclient.ErrNotFound)
- c.Check(errNotFound.Temporary(), Equals, false)
- c.Assert(strings.Contains(err.Error(), "HTTP 403 \"Missing or invalid Authorization header\""), Equals, true)
+ _, _, err = kc.PutB([]byte("foo"))
+ c.Check(err, ErrorMatches, ".*403.*Missing or invalid Authorization header")
}
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list