Accept more paths as Git HTTP

With the introduction of group wikis we also need to support repository
paths on the toplevel.

We can also make the matching for info refs requests a bit stricter,
by checking for valid `service` parameters in the query string.

changelogs/unreleased/jv-git-http-routing.yml

+---
+title: Support Git HTTP on toplevel repositories
+merge_request: 670
+author:
+type: added

gitaly_test.go

@@ -169,6 +169,57 @@ func TestGetInfoRefsProxiedToGitalyInterruptedStream(t *testing.T) {
 	waitDone(t, done)
 }
 
+func TestGetInfoRefsRouting(t *testing.T) {
+	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
+	defer gitalyServer.GracefulStop()
+
+	apiResponse := gitOkBody(t)
+	apiResponse.GitalyServer.Address = "unix:" + socketPath
+	ts := testAuthServer(t, nil, nil, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method string
+		path   string
+		match  bool
+	}{
+		{"GET", "/toplevel.git/info/refs?service=git-receive-pack", true},
+		{"GET", "/toplevel.wiki.git/info/refs?service=git-upload-pack", true},
+		{"GET", "/toplevel/child/project.git/info/refs?service=git-receive-pack", true},
+		{"GET", "/toplevel/child/project.wiki.git/info/refs?service=git-upload-pack", true},
+		{"GET", "/toplevel/child/project/snippets/123.git/info/refs?service=git-receive-pack", true},
+		{"GET", "/snippets/123.git/info/refs?service=git-upload-pack", true},
+		{"GET", "/foo/bar.git/info/refs", false},
+		{"GET", "/foo/bar.git/info/refs?service=git-zzz-pack", false},
+		{"GET", "/.git/info/refs?service=git-upload-pack", false},
+		{"POST", "/toplevel.git/info/refs?service=git-receive-pack", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(tc.method, ws.URL+tc.path, nil)
+			require.NoError(t, err)
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			body := string(testhelper.ReadAll(t, resp.Body))
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Contains(t, body, "\x00", "expect response generated by test gitaly server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, body, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 func waitDone(t *testing.T, done chan struct{}) {
 	t.Helper()
 	select {
@@ -259,6 +310,65 @@ func TestPostReceivePackProxiedToGitalyInterrupted(t *testing.T) {
 	waitDone(t, done)
 }
 
+func TestPostReceivePackRouting(t *testing.T) {
+	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
+	defer gitalyServer.GracefulStop()
+
+	apiResponse := gitOkBody(t)
+	apiResponse.GitalyServer.Address = "unix:" + socketPath
+	ts := testAuthServer(t, nil, nil, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method      string
+		path        string
+		contentType string
+		match       bool
+	}{
+		{"POST", "/toplevel.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel.wiki.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel/child/project.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel/child/project.wiki.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel/child/project/snippets/123.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/snippets/123.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/foo/bar/git-receive-pack", "application/x-git-receive-pack-request", false},
+		{"POST", "/foo/bar.git/git-zzz-pack", "application/x-git-receive-pack-request", false},
+		{"POST", "/.git/git-receive-pack", "application/x-git-receive-pack-request", false},
+		{"POST", "/toplevel.git/git-receive-pack", "application/x-git-upload-pack-request", false},
+		{"GET", "/toplevel.git/git-receive-pack", "application/x-git-receive-pack-request", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(
+				tc.method,
+				ws.URL+tc.path,
+				bytes.NewReader(testhelper.GitalyReceivePackResponseMock),
+			)
+			require.NoError(t, err)
+
+			req.Header.Set("Content-Type", tc.contentType)
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			body := string(testhelper.ReadAll(t, resp.Body))
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Contains(t, body, "\x00", "expect response generated by test gitaly server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, body, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 // ReaderFunc is an adapter to turn a conforming function into an io.Reader.
 type ReaderFunc func(b []byte) (int, error)
 
@@ -376,6 +486,65 @@ func TestPostUploadPackProxiedToGitalyInterrupted(t *testing.T) {
 	waitDone(t, done)
 }
 
+func TestPostUploadPackRouting(t *testing.T) {
+	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
+	defer gitalyServer.GracefulStop()
+
+	apiResponse := gitOkBody(t)
+	apiResponse.GitalyServer.Address = "unix:" + socketPath
+	ts := testAuthServer(t, nil, nil, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method      string
+		path        string
+		contentType string
+		match       bool
+	}{
+		{"POST", "/toplevel.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel.wiki.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel/child/project.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel/child/project.wiki.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel/child/project/snippets/123.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/snippets/123.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/foo/bar/git-upload-pack", "application/x-git-upload-pack-request", false},
+		{"POST", "/foo/bar.git/git-zzz-pack", "application/x-git-upload-pack-request", false},
+		{"POST", "/.git/git-upload-pack", "application/x-git-upload-pack-request", false},
+		{"POST", "/toplevel.git/git-upload-pack", "application/x-git-receive-pack-request", false},
+		{"GET", "/toplevel.git/git-upload-pack", "application/x-git-upload-pack-request", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(
+				tc.method,
+				ws.URL+tc.path,
+				bytes.NewReader(testhelper.GitalyReceivePackResponseMock),
+			)
+			require.NoError(t, err)
+
+			req.Header.Set("Content-Type", tc.contentType)
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			body := string(testhelper.ReadAll(t, resp.Body))
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Contains(t, body, "\x00", "expect response generated by test gitaly server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, body, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 func TestGetDiffProxiedToGitalySuccessfully(t *testing.T) {
 	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
 	defer gitalyServer.GracefulStop()

internal/git/info-refs.go

@@ -20,6 +20,11 @@ func GetInfoRefsHandler(a *api.API) http.Handler {
 	return repoPreAuthorizeHandler(a, handleGetInfoRefs)
 }
 
+func IsSmartInfoRefs(r *http.Request) bool {
+	service := r.URL.Query().Get("service")
+	return r.Method == "GET" && (service == "git-upload-pack" || service == "git-receive-pack")
+}
+
 func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response) {
 	responseWriter := NewHttpResponseWriter(rw)
 	// Log 0 bytes in because we ignore the request body (and there usually is none anyway).

internal/git/info-refs_test.go

+package git
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsSmartInfoRefs(t *testing.T) {
+	testCases := []struct {
+		method string
+		url    string
+		match  bool
+	}{
+		{"GET", "?service=git-upload-pack", true},
+		{"GET", "?service=git-receive-pack", true},
+		{"GET", "", false},
+		{"GET", "?service=", false},
+		{"GET", "?service=foo", false},
+		{"POST", "?service=git-upload-pack", false},
+		{"POST", "?service=git-receive-pack", false},
+	}
+
+	for _, tc := range testCases {
+		url, err := url.Parse(tc.url)
+		require.NoError(t, err)
+
+		r := http.Request{Method: tc.method, URL: url}
+		require.Equal(t, tc.match, IsSmartInfoRefs(&r))
+	}
+}

internal/upstream/routes.go

@@ -53,13 +53,13 @@ type uploadPreparers struct {
 }
 
 const (
-	apiPattern           = `^/api/`
-	ciAPIPattern         = `^/ci/api/`
-	gitProjectPattern    = `^/([^/]+/){1,}[^/]+\.git/`
-	projectPattern       = `^/([^/]+/){1,}[^/]+/`
-	snippetUploadPattern = `^/uploads/personal_snippet`
-	userUploadPattern    = `^/uploads/user`
-	importPattern        = `^/import/`
+	apiPattern           = `\A/api/`
+	ciAPIPattern         = `\A/ci/api/`
+	gitRepositoryPattern = `\A/.+\.git/`
+	projectPattern       = `\A/([^/]+/){1,}[^/]+/`
+	snippetUploadPattern = `\A/uploads/personal_snippet`
+	userUploadPattern    = `\A/uploads/user`
+	importPattern        = `\A/import/`
 )
 
 func compileRegexp(regexpStr string) *regexp.Regexp {
@@ -222,10 +222,10 @@ func (u *upstream) configureRoutes() {
 
 	u.Routes = []routeEntry{
 		// Git Clone
-		u.route("GET", gitProjectPattern+`info/refs\z`, git.GetInfoRefsHandler(api)),
-		u.route("POST", gitProjectPattern+`git-upload-pack\z`, contentEncodingHandler(git.UploadPack(api)), withMatcher(isContentType("application/x-git-upload-pack-request"))),
-		u.route("POST", gitProjectPattern+`git-receive-pack\z`, contentEncodingHandler(git.ReceivePack(api)), withMatcher(isContentType("application/x-git-receive-pack-request"))),
-		u.route("PUT", gitProjectPattern+`gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`, lfs.PutStore(api, signingProxy, preparers.lfs), withMatcher(isContentType("application/octet-stream"))),
+		u.route("GET", gitRepositoryPattern+`info/refs\z`, git.GetInfoRefsHandler(api), withMatcher(git.IsSmartInfoRefs)),
+		u.route("POST", gitRepositoryPattern+`git-upload-pack\z`, contentEncodingHandler(git.UploadPack(api)), withMatcher(isContentType("application/x-git-upload-pack-request"))),
+		u.route("POST", gitRepositoryPattern+`git-receive-pack\z`, contentEncodingHandler(git.ReceivePack(api)), withMatcher(isContentType("application/x-git-receive-pack-request"))),
+		u.route("PUT", gitRepositoryPattern+`gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`, lfs.PutStore(api, signingProxy, preparers.lfs), withMatcher(isContentType("application/octet-stream"))),
 
 		// CI Artifacts
 		u.route("POST", apiPattern+`v4/jobs/[0-9]+/artifacts\z`, contentEncodingHandler(artifacts.UploadArtifacts(api, signingProxy, preparers.artifacts))),

main_test.go

@@ -694,6 +694,12 @@ func testAuthServer(t *testing.T, url *regexp.Regexp, params url.Values, code in
 	return testhelper.TestServerWithHandler(url, func(w http.ResponseWriter, r *http.Request) {
 		require.NotEmpty(t, r.Header.Get("X-Request-Id"))
 
+		// return a 204 No Content response if we don't receive the JWT header
+		if r.Header.Get(secret.RequestHeader) == "" {
+			w.WriteHeader(204)
+			return
+		}
+
 		w.Header().Set("Content-Type", api.ResponseContentType)
 
 		logEntry := log.WithFields(log.Fields{

upload_test.go

@@ -234,7 +234,7 @@ func TestLfsUpload(t *testing.T) {
 	reqBody := "test data"
 	rspBody := "test success"
 	oid := "916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9"
-	resource := fmt.Sprintf("/%s/gitlab-lfs/objects/%s/%d", testRepo, oid, len(reqBody))
+	resource := fmt.Sprintf("/gitlab-org/gitlab-test.git/gitlab-lfs/objects/%s/%d", oid, len(reqBody))
 
 	lfsApiResponse := fmt.Sprintf(
 		`{"TempPath":%q, "LfsOid":%q, "LfsSize": %d}`,
@@ -292,6 +292,74 @@ func TestLfsUpload(t *testing.T) {
 	require.Equal(t, rspBody, string(rspData))
 }
 
+func TestLfsUploadRouting(t *testing.T) {
+	reqBody := "test data"
+	rspBody := "test success"
+	oid := "916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9"
+
+	ts := testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get(secret.RequestHeader) == "" {
+			w.WriteHeader(204)
+		} else {
+			fmt.Fprint(w, rspBody)
+		}
+	})
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method      string
+		path        string
+		contentType string
+		match       bool
+	}{
+		{"PUT", "/toplevel.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel.wiki.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel/child/project.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel/child/project.wiki.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel/child/project/snippets/123.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/snippets/123.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/foo/bar/gitlab-lfs/objects", "application/octet-stream", false},
+		{"PUT", "/foo/bar.git/gitlab-lfs/objects/zzz", "application/octet-stream", false},
+		{"PUT", "/.git/gitlab-lfs/objects", "application/octet-stream", false},
+		{"PUT", "/toplevel.git/gitlab-lfs/objects", "application/zzz", false},
+		{"POST", "/toplevel.git/gitlab-lfs/objects", "application/octet-stream", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			resource := fmt.Sprintf(tc.path+"/%s/%d", oid, len(reqBody))
+
+			req, err := http.NewRequest(
+				tc.method,
+				ws.URL+resource,
+				strings.NewReader(reqBody),
+			)
+			require.NoError(t, err)
+
+			req.Header.Set("Content-Type", tc.contentType)
+			req.ContentLength = int64(len(reqBody))
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			rspData, err := ioutil.ReadAll(resp.Body)
+			require.NoError(t, err)
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Equal(t, rspBody, string(rspData), "expect response generated by test upstream server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, rspData, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 func packageUploadTestServer(t *testing.T, resource string, reqBody string, rspBody string) *httptest.Server {
 	return testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
 		require.Equal(t, r.Method, "PUT")