1885: GET and HEAD through the proxy work correctly now. Added invalid API token...

diff --git a/sdk/go/src/arvados.org/keepclient/keepclient_test.go b/sdk/go/src/arvados.org/keepclient/keepclient_test.go
index c67e09f5369f6f7f9f4644c432ed6c3b8ff0a180..600d7393c519190ca849a4ad33fd5594cdec55c7 100644 (file)
--- a/sdk/go/src/arvados.org/keepclient/keepclient_test.go
+++ b/sdk/go/src/arvados.org/keepclient/keepclient_test.go
@@ -585,11 +585,19 @@ func (s *ServerRequiredSuite) TestPutGetHead(c *C) {
        kc, err := MakeKeepClient()
        c.Assert(err, Equals, nil)
 
-       hash, replicas, err := kc.PutB([]byte("foo"))
-       c.Check(hash, Equals, fmt.Sprintf("%x", md5.Sum([]byte("foo"))))
-       c.Check(replicas, Equals, 2)
-       c.Check(err, Equals, nil)
+       hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
 
+       {
+               n, _, err := kc.Ask(hash)
+               c.Check(err, Equals, BlockNotFound)
+               c.Check(n, Equals, int64(0))
+       }
+       {
+               hash2, replicas, err := kc.PutB([]byte("foo"))
+               c.Check(hash2, Equals, hash)
+               c.Check(replicas, Equals, 2)
+               c.Check(err, Equals, nil)
+       }
        {
                r, n, url2, err := kc.Get(hash)
                c.Check(err, Equals, nil)
@@ -600,7 +608,6 @@ func (s *ServerRequiredSuite) TestPutGetHead(c *C) {
                c.Check(err2, Equals, nil)
                c.Check(content, DeepEquals, []byte("foo"))
        }
-
        {
                n, url2, err := kc.Ask(hash)
                c.Check(err, Equals, nil)

diff --git a/services/keep/src/arvados.org/keepproxy/keepproxy.go b/services/keep/src/arvados.org/keepproxy/keepproxy.go
index 8b8ff18a31160cd29baee163d6b7b6d8d14ab7f7..ed33ac9bbd62a79a59e293f9170af9a2b1b3cf2a 100644 (file)
--- a/services/keep/src/arvados.org/keepproxy/keepproxy.go
+++ b/services/keep/src/arvados.org/keepproxy/keepproxy.go
@@ -25,7 +25,6 @@ func main() {
                listen           string
                no_get           bool
                no_put           bool
-               no_head          bool
                default_replicas int
                pidfile          string
        )
@@ -41,7 +40,7 @@ func main() {
        flag.BoolVar(
                &no_get,
                "no-get",
-               true,
+               false,
                "If set, disable GET operations")
 
        flag.BoolVar(
@@ -50,12 +49,6 @@ func main() {
                false,
                "If set, disable PUT operations")
 
-       flag.BoolVar(
-               &no_head,
-               "no-head",
-               true,
-               "If set, disable HEAD operations")
-
        flag.IntVar(
                &default_replicas,
                "default-replicas",
@@ -70,8 +63,8 @@ func main() {
 
        flag.Parse()
 
-       /*if no_get == false || no_head == false {
-               log.Print("Must specify -no-get and -no-head")
+       /*if no_get == false {
+               log.Print("Must specify -no-get")
                return
        }*/
 
@@ -100,7 +93,7 @@ func main() {
        }
 
        // Start listening for requests.
-       http.Serve(listener, MakeRESTRouter(!no_get, !no_put, !no_head, kc))
+       http.Serve(listener, MakeRESTRouter(!no_get, !no_put, kc))
 }
 
 type ApiTokenCache struct {
@@ -204,7 +197,6 @@ type PutBlockHandler struct {
 func MakeRESTRouter(
        enable_get bool,
        enable_put bool,
-       enable_head bool,
        kc keepclient.KeepClient) *mux.Router {
 
        t := &ApiTokenCache{tokens: make(map[string]int64), expireTime: 300}
@@ -217,23 +209,23 @@ func MakeRESTRouter(
        ph := rest.Handle(`/{hash:[0-9a-f]{32}}`, PutBlockHandler{kc, t})
 
        if enable_get {
-               gh.Methods("GET")
-               ghsig.Methods("GET")
+               gh.Methods("GET", "HEAD")
+               ghsig.Methods("GET", "HEAD")
        }
 
        if enable_put {
                ph.Methods("PUT")
        }
 
-       if enable_head {
-               gh.Methods("HEAD")
-               ghsig.Methods("HEAD")
-       }
-
        return rest
 }
 
 func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
+
+       if !CheckAuthorizationHeader(this.KeepClient, this.ApiTokenCache, req) {
+               http.Error(resp, "Missing or invalid Authorization header", http.StatusForbidden)
+       }
+
        hash := mux.Vars(req)["hash"]
        signature := mux.Vars(req)["signature"]
        timestamp := mux.Vars(req)["timestamp"]
@@ -264,6 +256,9 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 }
 
 func (this PutBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
+
+       log.Print("PutBlockHandler start")
+
        if !CheckAuthorizationHeader(this.KeepClient, this.ApiTokenCache, req) {
                http.Error(resp, "Missing or invalid Authorization header", http.StatusForbidden)
        }

diff --git a/services/keep/src/arvados.org/keepproxy/keepproxy_test.go b/services/keep/src/arvados.org/keepproxy/keepproxy_test.go
index 98712819a690d49bbddc45edebba9d373aa11ade..af1377b249c6fca6e811f258f00aad5a965927a1 100644 (file)
--- a/services/keep/src/arvados.org/keepproxy/keepproxy_test.go
+++ b/services/keep/src/arvados.org/keepproxy/keepproxy_test.go
@@ -47,9 +47,15 @@ func (s *ServerRequiredSuite) SetUpSuite(c *C) {
        os.Setenv("ARVADOS_API_HOST_INSECURE", "true")
 
        SetupProxyService()
+
+       os.Args = []string{"keepproxy", "-listen=:29950"}
+       go main()
+       time.Sleep(100 * time.Millisecond)
 }
 
 func (s *ServerRequiredSuite) TearDownSuite(c *C) {
+       listener.Close()
+
        cwd, _ := os.Getwd()
        defer os.Chdir(cwd)
 
@@ -110,21 +116,13 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 
        log.Print("keepclient created")
 
-       os.Args = []string{"keepproxy", "-listen=:29950"}
-       go main()
-
-       time.Sleep(100 * time.Millisecond)
-
-       log.Print("keepproxy main started")
-
        hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
 
-       // Uncomment this when actual keep server supports HEAD
-       /*{
+       {
                _, _, err := kc.Ask(hash)
                c.Check(err, Equals, keepclient.BlockNotFound)
                log.Print("Ask 1")
-       }*/
+       }
 
        {
                hash2, rep, err := kc.PutB([]byte("foo"))
@@ -134,25 +132,69 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
                log.Print("PutB")
        }
 
-       // Uncomment this when actual keep server supports HEAD
-       /*{
+       {
                blocklen, _, err := kc.Ask(hash)
+               c.Assert(err, Equals, nil)
                c.Check(blocklen, Equals, int64(3))
-               c.Check(err, Equals, nil)
                log.Print("Ask 2")
-       }*/
+       }
 
        {
                reader, blocklen, _, err := kc.Get(hash)
+               c.Assert(err, Equals, nil)
                all, err := ioutil.ReadAll(reader)
                c.Check(all, DeepEquals, []byte("foo"))
                c.Check(blocklen, Equals, int64(3))
-               c.Check(err, Equals, nil)
                log.Print("Get")
        }
 
-       // Close internal listener socket.
-       listener.Close()
-
        log.Print("TestPutAndGet done")
 }
+
+func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
+       log.Print("TestPutAndGet start")
+
+       os.Setenv("ARVADOS_EXTERNAL_CLIENT", "true")
+       kc, err := keepclient.MakeKeepClient()
+       kc.ApiToken = "123xyz"
+       c.Check(kc.External, Equals, true)
+       c.Check(kc.Using_proxy, Equals, true)
+       c.Check(len(kc.Service_roots), Equals, 1)
+       c.Check(kc.Service_roots[0], Equals, "http://localhost:29950")
+       c.Check(err, Equals, nil)
+       os.Setenv("ARVADOS_EXTERNAL_CLIENT", "")
+
+       log.Print("keepclient created")
+
+       hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
+
+       {
+               _, _, err := kc.Ask(hash)
+               c.Check(err, Equals, keepclient.BlockNotFound)
+               log.Print("Ask 1")
+       }
+
+       {
+               hash2, rep, err := kc.PutB([]byte("foo"))
+               c.Check(hash2, Equals, hash)
+               c.Check(rep, Equals, 0)
+               c.Check(err, Equals, keepclient.InsufficientReplicasError)
+               log.Print("PutB")
+       }
+
+       {
+               blocklen, _, err := kc.Ask(hash)
+               c.Assert(err, Equals, keepclient.BlockNotFound)
+               c.Check(blocklen, Equals, int64(0))
+               log.Print("Ask 2")
+       }
+
+       {
+               _, blocklen, _, err := kc.Get(hash)
+               c.Assert(err, Equals, keepclient.BlockNotFound)
+               c.Check(blocklen, Equals, int64(0))
+               log.Print("Get")
+       }
+
+       log.Print("TestPutAndGetForbidden done")
+}