[ARVADOS] created: 2.1.0-740-gd48cff711

Git user git at public.arvados.org
Thu Apr 29 19:09:17 UTC 2021


        at  d48cff711af8255f3b2b69506b54a283c1aab776 (commit)


commit d48cff711af8255f3b2b69506b54a283c1aab776
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Thu Apr 29 15:07:44 2021 -0400

    17598: Handle comparison URLs with :80 or :443
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 94b59ebd4..8d59a8a27 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -6,6 +6,7 @@ package main
 
 import (
 	"encoding/json"
+	"fmt"
 	"html"
 	"html/template"
 	"io"
@@ -184,6 +185,14 @@ var (
 	}
 )
 
+func StripDefaultPort(host string) string {
+	// Will consider port 80 and port 443 to be the same vhost.  I think that's fine.
+	if strings.HasSuffix(host, ":80") || strings.HasSuffix(host, ":443") {
+		return host[0:strings.Index(host, ":")]
+	}
+	return host
+}
+
 // ServeHTTP implements http.Handler.
 func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	h.setupOnce.Do(h.setup)
@@ -240,14 +249,19 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	var attachment bool
 	var useSiteFS bool
 	credentialsOK := h.Config.cluster.Collections.TrustAllContent
+	reasonNotAcceptingCredentials := ""
 
-	if r.Host != "" && r.Host == h.Config.cluster.Services.WebDAVDownload.ExternalURL.Host {
+	if r.Host != "" && StripDefaultPort(r.Host) == StripDefaultPort(h.Config.cluster.Services.WebDAVDownload.ExternalURL.Host) {
 		credentialsOK = true
 		attachment = true
 	} else if r.FormValue("disposition") == "attachment" {
 		attachment = true
 	}
 
+	if !credentialsOK {
+		reasonNotAcceptingCredentials = fmt.Sprintf("Collections.TrustAllContent is false and provided virtual host '%s' did not match either Services.WebDAV or Services.WebDAVDownload", r.Host)
+	}
+
 	if collectionID = parseCollectionIDFromDNSName(r.Host); collectionID != "" {
 		// http://ID.collections.example/PATH...
 		credentialsOK = true
@@ -278,6 +292,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 			// data. Tokens provided with the request are
 			// ignored.
 			credentialsOK = false
+			reasonNotAcceptingCredentials = "the '/collections/UUID/PATH' form only works for public data"
 		}
 	}
 
@@ -346,7 +361,19 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	}
 
 	if tokens == nil {
-		tokens = append(reqTokens, h.Config.cluster.Users.AnonymousUserToken)
+		tokens = reqTokens
+		if h.Config.cluster.Users.AnonymousUserToken != "" {
+			tokens = append(tokens, h.Config.cluster.Users.AnonymousUserToken)
+		}
+	}
+
+	if tokens == nil {
+		if !credentialsOK {
+			http.Error(w, fmt.Sprintf("Authorization tokens were not accepted because %v, and no anonymous user token is configured.", reasonNotAcceptingCredentials), http.StatusUnauthorized)
+		} else {
+			http.Error(w, fmt.Sprintf("No authorization token in request, and no anonymous user token is configured."), http.StatusUnauthorized)
+		}
+		return
 	}
 
 	if len(targetPath) > 0 && targetPath[0] == "_" {
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 9252bd82d..4560adc68 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -193,11 +193,18 @@ func (s *IntegrationSuite) TestVhost404(c *check.C) {
 // the token is invalid.
 type authorizer func(*http.Request, string) int
 
-func (s *IntegrationSuite) TestVhostViaAuthzHeader(c *check.C) {
-	s.doVhostRequests(c, authzViaAuthzHeader)
+func (s *IntegrationSuite) TestVhostViaAuthzHeaderOAuth2(c *check.C) {
+	s.doVhostRequests(c, authzViaAuthzHeaderOAuth2)
 }
-func authzViaAuthzHeader(r *http.Request, tok string) int {
-	r.Header.Add("Authorization", "OAuth2 "+tok)
+func authzViaAuthzHeaderOAuth2(r *http.Request, tok string) int {
+	r.Header.Add("Authorization", "Bearer "+tok)
+	return http.StatusUnauthorized
+}
+func (s *IntegrationSuite) TestVhostViaAuthzHeaderBearer(c *check.C) {
+	s.doVhostRequests(c, authzViaAuthzHeaderBearer)
+}
+func authzViaAuthzHeaderBearer(r *http.Request, tok string) int {
+	r.Header.Add("Authorization", "Bearer "+tok)
 	return http.StatusUnauthorized
 }
 
@@ -319,6 +326,28 @@ func (s *IntegrationSuite) doVhostRequestsWithHostPath(c *check.C, authz authori
 	}
 }
 
+func (s *IntegrationSuite) TestVhostPortMatch(c *check.C) {
+	for _, port := range []string{"80", "443", "8000"} {
+		s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = fmt.Sprintf("download.example.com:%v", port)
+		u := mustParseURL(fmt.Sprintf("http://download.example.com/by_id/%v/foo", arvadostest.FooCollection))
+		req := &http.Request{
+			Method:     "GET",
+			Host:       u.Host,
+			URL:        u,
+			RequestURI: u.RequestURI(),
+			Header:     http.Header{"Authorization": []string{"Bearer " + arvadostest.ActiveToken}},
+		}
+		req, resp := s.doReq(req)
+		code, _ := resp.Code, resp.Body.String()
+
+		if port == "8000" {
+			c.Check(code, check.Equals, 401)
+		} else {
+			c.Check(code, check.Equals, 200)
+		}
+	}
+}
+
 func (s *IntegrationSuite) doReq(req *http.Request) (*http.Request, *httptest.ResponseRecorder) {
 	resp := httptest.NewRecorder()
 	s.testServer.Handler.ServeHTTP(resp, req)
@@ -743,7 +772,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 		{
 			// URLs of this form ignore authHeader, and
 			// FooAndBarFilesInDirUUID isn't public, so
-			// this returns 404.
+			// this returns 401.
 			uri:    "download.example.com/collections/" + arvadostest.FooAndBarFilesInDirUUID + "/",
 			header: authHeader,
 			expect: nil,
@@ -886,7 +915,11 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 			c.Check(req.URL.Path, check.Equals, trial.redirect, comment)
 		}
 		if trial.expect == nil {
-			c.Check(resp.Code, check.Equals, http.StatusNotFound, comment)
+			if s.testServer.Config.cluster.Users.AnonymousUserToken == "" {
+				c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment)
+			} else {
+				c.Check(resp.Code, check.Equals, http.StatusNotFound, comment)
+			}
 		} else {
 			c.Check(resp.Code, check.Equals, http.StatusOK, comment)
 			for _, e := range trial.expect {
@@ -907,7 +940,11 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 		resp = httptest.NewRecorder()
 		s.testServer.Handler.ServeHTTP(resp, req)
 		if trial.expect == nil {
-			c.Check(resp.Code, check.Equals, http.StatusNotFound, comment)
+			if s.testServer.Config.cluster.Users.AnonymousUserToken == "" {
+				c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment)
+			} else {
+				c.Check(resp.Code, check.Equals, http.StatusNotFound, comment)
+			}
 		} else {
 			c.Check(resp.Code, check.Equals, http.StatusOK, comment)
 		}
@@ -923,7 +960,11 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 		resp = httptest.NewRecorder()
 		s.testServer.Handler.ServeHTTP(resp, req)
 		if trial.expect == nil {
-			c.Check(resp.Code, check.Equals, http.StatusNotFound, comment)
+			if s.testServer.Config.cluster.Users.AnonymousUserToken == "" {
+				c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment)
+			} else {
+				c.Check(resp.Code, check.Equals, http.StatusNotFound, comment)
+			}
 		} else {
 			c.Check(resp.Code, check.Equals, http.StatusMultiStatus, comment)
 			for _, e := range trial.expect {

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list