Merge branch 'auth-handler' into 'master'

Refactor middleware

- Use gitRequest to store all processed request data
- Drop some headers when creating upstream request

@jacobvosmaer @marin: I still need to write test for it.


See merge request !7

Makefile

 PREFIX=/usr/local
 VERSION=$(shell git describe)-$(shell date -u +%Y%m%d.%H%M%S)
 
-gitlab-workhorse: main.go upstream.go archive.go git-http.go helpers.go xsendfile.go
+gitlab-workhorse: main.go upstream.go archive.go git-http.go helpers.go xsendfile.go authorization.go
 	go build -ldflags "-X main.Version ${VERSION}" -o gitlab-workhorse
 
 install: gitlab-workhorse

archive.go

@@ -14,9 +14,25 @@ import (
 	"os/exec"
 	"path"
 	"time"
+	"path/filepath"
+	"errors"
 )
 
-func handleGetArchive(w http.ResponseWriter, r *gitRequest, format string) {
+func handleGetArchive(w http.ResponseWriter, r *gitRequest) {
+	var format string
+	switch filepath.Base(r.URL.Path) {
+	case "archive.zip":
+		format = "zip"
+	case "archive.tar":
+		format = "tar"
+	case "archive", "archive.tar.gz":
+		format = "tar.gz"
+	case "archive.tar.bz2":
+		format = "tar.bz2"
+	default:
+		fail500(w, "handleGetArchive", errors.New("invalid archive format"))
+	}
+
 	archiveFilename := path.Base(r.ArchivePath)
 
 	if cachedArchive, err := os.Open(r.ArchivePath); err == nil {

authorization.go

+package main
+
+import (
+	"encoding/json"
+	"errors"
+	"io"
+	"net/http"
+	"strings"
+)
+
+func preAuthorizeHandler(handleFunc serviceHandleFunc, suffix string) serviceHandleFunc {
+	return func(w http.ResponseWriter, r *gitRequest) {
+		authReq, err := r.u.newUpstreamRequest(r.Request, nil, suffix)
+		if err != nil {
+			fail500(w, "newUpstreamRequest", err)
+			return
+		}
+
+		authResponse, err := r.u.httpClient.Do(authReq)
+		if err != nil {
+			fail500(w, "doAuthRequest", err)
+			return
+		}
+		defer authResponse.Body.Close()
+
+		if authResponse.StatusCode != 200 {
+			// The Git request is not allowed by the backend. Maybe the
+			// client needs to send HTTP Basic credentials.  Forward the
+			// response from the auth backend to our client. This includes
+			// the 'WWW-Authenticate' header that acts as a hint that
+			// Basic auth credentials are needed.
+			for k, v := range authResponse.Header {
+				// Accomodate broken clients that do case-sensitive header lookup
+				if k == "Www-Authenticate" {
+					w.Header()["WWW-Authenticate"] = v
+				} else {
+					w.Header()[k] = v
+				}
+			}
+			w.WriteHeader(authResponse.StatusCode)
+			io.Copy(w, authResponse.Body)
+			return
+		}
+
+		// The auth backend validated the client request and told us additional
+		// request metadata. We must extract this information from the auth
+		// response body.
+		if err := json.NewDecoder(authResponse.Body).Decode(&r.authorizationResponse); err != nil {
+			fail500(w, "decode authorization response", err)
+			return
+		}
+		// Don't hog a TCP connection in CLOSE_WAIT, we can already close it now
+		authResponse.Body.Close()
+
+		// Negotiate authentication (Kerberos) may need to return a WWW-Authenticate
+		// header to the client even in case of success as per RFC4559.
+		for k, v := range authResponse.Header {
+			// Case-insensitive comparison as per RFC7230
+			if strings.EqualFold(k, "WWW-Authenticate") {
+				w.Header()[k] = v
+			}
+		}
+
+		handleFunc(w, r)
+	}
+}
+
+func repoPreAuthorizeHandler(handleFunc serviceHandleFunc) serviceHandleFunc {
+	return preAuthorizeHandler(func(w http.ResponseWriter, r *gitRequest) {
+		if r.RepoPath == "" {
+			fail500(w, "repoPreAuthorizeHandler", errors.New("missing authorization response"))
+			return
+		}
+
+		if !looksLikeRepo(r.RepoPath) {
+			http.Error(w, "Not Found", 404)
+			return
+		}
+
+		handleFunc(w, r)
+	}, "")
+}

git-http.go

@@ -10,9 +10,10 @@ import (
 	"io"
 	"net/http"
 	"strings"
+	"path/filepath"
 )
 
-func handleGetInfoRefs(w http.ResponseWriter, r *gitRequest, _ string) {
+func handleGetInfoRefs(w http.ResponseWriter, r *gitRequest) {
 	rpc := r.URL.Query().Get("service")
 	if !(rpc == "git-upload-pack" || rpc == "git-receive-pack") {
 		// The 'dumb' Git HTTP protocol is not supported
@@ -56,10 +57,18 @@ func handleGetInfoRefs(w http.ResponseWriter, r *gitRequest, _ string) {
 	}
 }
 
-func handlePostRPC(w http.ResponseWriter, r *gitRequest, rpc string) {
+func handlePostRPC(w http.ResponseWriter, r *gitRequest) {
 	var body io.ReadCloser
 	var err error
 
+	// Get Git action from URL
+	action := filepath.Base(r.URL.Path)
+	if !(action == "git-upload-pack" || action == "git-receive-pack") {
+		// The 'dumb' Git HTTP protocol is not supported
+		fail500(w, "handlePostRPC", err)
+		return
+	}
+
 	// The client request body may have been gzipped.
 	if r.Header.Get("Content-Encoding") == "gzip" {
 		body, err = gzip.NewReader(r.Body)
@@ -73,7 +82,7 @@ func handlePostRPC(w http.ResponseWriter, r *gitRequest, rpc string) {
 	defer body.Close()
 
 	// Prepare our Git subprocess
-	cmd := gitCommand(r.GL_ID, "git", subCommand(rpc), "--stateless-rpc", r.RepoPath)
+	cmd := gitCommand(r.GL_ID, "git", subCommand(action), "--stateless-rpc", r.RepoPath)
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
 		fail500(w, "handlePostRPC", err)
@@ -108,7 +117,7 @@ func handlePostRPC(w http.ResponseWriter, r *gitRequest, rpc string) {
 	body.Close()
 
 	// Start writing the response
-	w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-result", rpc))
+	w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-result", action))
 	w.Header().Add("Cache-Control", "no-cache")
 	w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just return
 

main_test.go

@@ -336,7 +336,8 @@ func testAuthServer(code int, body string) *httptest.Server {
 }
 
 func startServerOrFail(t *testing.T, ts *httptest.Server) *exec.Cmd {
-	cmd := exec.Command("go", "run", "main.go", "upstream.go", "archive.go", "git-http.go", "helpers.go", "xsendfile.go", fmt.Sprintf("-authBackend=%s", ts.URL), fmt.Sprintf("-listenAddr=%s", servAddr))
+	cmd := exec.Command("go", "run", "main.go", "upstream.go", "archive.go", "git-http.go", "helpers.go", "xsendfile.go",
+		"authorization.go", fmt.Sprintf("-authBackend=%s", ts.URL), fmt.Sprintf("-listenAddr=%s", servAddr))
 	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr

upstream.go

@@ -7,33 +7,28 @@ In this file we handle request routing and interaction with the authBackend.
 package main
 
 import (
-	"encoding/json"
 	"io"
 	"log"
 	"net/http"
 	"os"
 	"path"
 	"regexp"
-	"strings"
 )
 
+type serviceHandleFunc func(w http.ResponseWriter, r *gitRequest)
+
 type upstream struct {
 	httpClient  *http.Client
 	authBackend string
 }
 
 type gitService struct {
-	method         string
-	regex          *regexp.Regexp
-	middlewareFunc func(u *upstream, w http.ResponseWriter, r *http.Request, handleFunc func(w http.ResponseWriter, r *gitRequest, rpc string), rpc string)
-	handleFunc     func(w http.ResponseWriter, r *gitRequest, rpc string)
-	rpc            string
+	method     string
+	regex      *regexp.Regexp
+	handleFunc serviceHandleFunc
 }
 
-// A gitReqest is an *http.Request decorated with attributes returned by the
-// GitLab Rails application.
-type gitRequest struct {
-	*http.Request
+type authorizationResponse struct {
 	// GL_ID is an environment variable used by gitlab-shell hooks during 'git
 	// push' and 'git pull'
 	GL_ID string
@@ -51,17 +46,25 @@ type gitRequest struct {
 	CommitId string
 }
 
+// A gitReqest is an *http.Request decorated with attributes returned by the
+// GitLab Rails application.
+type gitRequest struct {
+	*http.Request
+	authorizationResponse
+	u   *upstream
+}
+
 // Routing table
 var gitServices = [...]gitService{
-	gitService{"GET", regexp.MustCompile(`/info/refs\z`), repoPreAuth, handleGetInfoRefs, ""},
-	gitService{"POST", regexp.MustCompile(`/git-upload-pack\z`), repoPreAuth, handlePostRPC, "git-upload-pack"},
-	gitService{"POST", regexp.MustCompile(`/git-receive-pack\z`), repoPreAuth, handlePostRPC, "git-receive-pack"},
-	gitService{"GET", regexp.MustCompile(`/repository/archive\z`), repoPreAuth, handleGetArchive, "tar.gz"},
-	gitService{"GET", regexp.MustCompile(`/repository/archive.zip\z`), repoPreAuth, handleGetArchive, "zip"},
-	gitService{"GET", regexp.MustCompile(`/repository/archive.tar\z`), repoPreAuth, handleGetArchive, "tar"},
-	gitService{"GET", regexp.MustCompile(`/repository/archive.tar.gz\z`), repoPreAuth, handleGetArchive, "tar.gz"},
-	gitService{"GET", regexp.MustCompile(`/repository/archive.tar.bz2\z`), repoPreAuth, handleGetArchive, "tar.bz2"},
-	gitService{"GET", regexp.MustCompile(`/uploads/`), xSendFile, nil, ""},
+	gitService{"GET", regexp.MustCompile(`/info/refs\z`), repoPreAuthorizeHandler(handleGetInfoRefs)},
+	gitService{"POST", regexp.MustCompile(`/git-upload-pack\z`), repoPreAuthorizeHandler(handlePostRPC)},
+	gitService{"POST", regexp.MustCompile(`/git-receive-pack\z`), repoPreAuthorizeHandler(handlePostRPC)},
+	gitService{"GET", regexp.MustCompile(`/repository/archive\z`), repoPreAuthorizeHandler(handleGetArchive)},
+	gitService{"GET", regexp.MustCompile(`/repository/archive.zip\z`), repoPreAuthorizeHandler(handleGetArchive)},
+	gitService{"GET", regexp.MustCompile(`/repository/archive.tar\z`), repoPreAuthorizeHandler(handleGetArchive)},
+	gitService{"GET", regexp.MustCompile(`/repository/archive.tar.gz\z`), repoPreAuthorizeHandler(handleGetArchive)},
+	gitService{"GET", regexp.MustCompile(`/repository/archive.tar.bz2\z`), repoPreAuthorizeHandler(handleGetArchive)},
+	gitService{"GET", regexp.MustCompile(`/uploads/`), handleSendFile},
 }
 
 func newUpstream(authBackend string, authTransport http.RoundTripper) *upstream {
@@ -88,68 +91,12 @@ func (u *upstream) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	g.middlewareFunc(u, w, r, g.handleFunc, g.rpc)
-}
-
-func repoPreAuth(u *upstream, w http.ResponseWriter, r *http.Request, handleFunc func(w http.ResponseWriter, r *gitRequest, rpc string), rpc string) {
-	authReq, err := u.newUpstreamRequest(r)
-	if err != nil {
-		fail500(w, "newUpstreamRequest", err)
-		return
+	request := gitRequest{
+		Request: r,
+		u:       u,
 	}
 
-	authResponse, err := u.httpClient.Do(authReq)
-	if err != nil {
-		fail500(w, "doAuthRequest", err)
-		return
-	}
-	defer authResponse.Body.Close()
-
-	if authResponse.StatusCode != 200 {
-		// The Git request is not allowed by the backend. Maybe the
-		// client needs to send HTTP Basic credentials.  Forward the
-		// response from the auth backend to our client. This includes
-		// the 'WWW-Authenticate' header that acts as a hint that
-		// Basic auth credentials are needed.
-		for k, v := range authResponse.Header {
-			// Accomodate broken clients that do case-sensitive header lookup
-			if k == "Www-Authenticate" {
-				w.Header()["WWW-Authenticate"] = v
-			} else {
-				w.Header()[k] = v
-			}
-		}
-		w.WriteHeader(authResponse.StatusCode)
-		io.Copy(w, authResponse.Body)
-		return
-	}
-
-	// The auth backend validated the client request and told us additional
-	// request metadata. We must extract this information from the auth
-	// response body.
-	gitReq := &gitRequest{Request: r}
-	if err := json.NewDecoder(authResponse.Body).Decode(gitReq); err != nil {
-		fail500(w, "decode JSON GL_ID", err)
-		return
-	}
-	// Don't hog a TCP connection in CLOSE_WAIT, we can already close it now
-	authResponse.Body.Close()
-
-	// Negotiate authentication (Kerberos) may need to return a WWW-Authenticate
-	// header to the client even in case of success as per RFC4559.
-	for k, v := range authResponse.Header {
-		// Case-insensitive comparison as per RFC7230
-		if strings.EqualFold(k, "WWW-Authenticate") {
-			w.Header()[k] = v
-		}
-	}
-
-	if !looksLikeRepo(gitReq.RepoPath) {
-		http.Error(w, "Not Found", 404)
-		return
-	}
-
-	handleFunc(w, gitReq, rpc)
+	g.handleFunc(w, &request)
 }
 
 func looksLikeRepo(p string) bool {
@@ -162,9 +109,9 @@ func looksLikeRepo(p string) bool {
 	return true
 }
 
-func (u *upstream) newUpstreamRequest(r *http.Request) (*http.Request, error) {
-	url := u.authBackend + r.URL.RequestURI()
-	authReq, err := http.NewRequest(r.Method, url, nil)
+func (u *upstream) newUpstreamRequest(r *http.Request, body io.Reader, suffix string) (*http.Request, error) {
+	url := u.authBackend + r.URL.RequestURI() + suffix
+	authReq, err := http.NewRequest(r.Method, url, body)
 	if err != nil {
 		return nil, err
 	}
@@ -173,6 +120,16 @@ func (u *upstream) newUpstreamRequest(r *http.Request) (*http.Request, error) {
 	for k, v := range r.Header {
 		authReq.Header[k] = v
 	}
+
+	// Clean some headers when issuing a new request without body
+	if body == nil {
+		authReq.Header.Del("Content-Type")
+		authReq.Header.Del("Content-Encoding")
+		authReq.Header.Del("Content-Length")
+		authReq.Header.Del("Accept-Encoding")
+		authReq.Header.Del("Transfer-Encoding")
+	}
+
 	// Also forward the Host header, which is excluded from the Header map by the http libary.
 	// This allows the Host header received by the backend to be consistent with other
 	// requests not going through gitlab-workhorse.

xsendfile.go

@@ -13,15 +13,16 @@ import (
 	"os"
 )
 
-func xSendFile(u *upstream, w http.ResponseWriter, r *http.Request, _ func(http.ResponseWriter, *gitRequest, string), _ string) {
-	upRequest, err := u.newUpstreamRequest(r)
+func handleSendFile(w http.ResponseWriter, r *gitRequest) {
+	upRequest, err := r.u.newUpstreamRequest(r.Request, r.Body, "")
 	if err != nil {
 		fail500(w, "newUpstreamRequest", err)
 		return
 	}
 
 	upRequest.Header.Set("X-Sendfile-Type", "X-Sendfile")
-	upResponse, err := u.httpClient.Do(upRequest)
+	upResponse, err := r.u.httpClient.Do(upRequest)
+	r.Body.Close()
 	if err != nil {
 		fail500(w, "do upstream request", err)
 		return
@@ -63,5 +64,5 @@ func xSendFile(u *upstream, w http.ResponseWriter, r *http.Request, _ func(http.
 		fail500(w, "xSendFile get mtime", err)
 		return
 	}
-	http.ServeContent(w, r, "", fi.ModTime(), content)
+	http.ServeContent(w, r.Request, "", fi.ModTime(), content)
 }