Unify how we handle custom and group domain in serving

6 files changed, 154 insertions, 205 deletions

diff --git a/internal/domain/config.go b/internal/domain/config.go
deleted file mode 100644
index 040b2279..00000000
--- a/internal/domain/config.go
+++ /dev/null
@@ -1,11 +0,0 @@
-package domain
-
-// ProjectConfig holds a custom project domain configuration
-type ProjectConfig struct {
-	DomainName    string
-	Certificate   string
-	Key           string
-	HTTPSOnly     bool
-	ProjectID     uint64
-	AccessControl bool
-}
diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 361c3b87..1dd5be51 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -5,31 +5,23 @@ import (
 	"errors"
 	"net/http"
 	"sync"
-	"time"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
 )
 
-// GroupConfig represents a per-request config for a group domain
-type GroupConfig interface {
-	IsHTTPSOnly(*http.Request) bool
-	HasAccessControl(*http.Request) bool
-	IsNamespaceProject(*http.Request) bool
-	ProjectID(*http.Request) uint64
-	ProjectExists(*http.Request) bool
-	ProjectWithSubpath(*http.Request) (string, string, error)
-}
-
 // Domain is a domain that gitlab-pages can serve.
 type Domain struct {
-	Group   string
-	Project string
+	Name            string
+	Location        string
+	CertificateCert string
+	CertificateKey  string
+	Customized      bool // TODO we should get rid of this
 
-	GroupConfig   GroupConfig // handles group domain config
-	ProjectConfig *ProjectConfig
+	Resolver Resolver
 
-	serving serving.Serving
+	lookupPaths map[string]*Project
+	serving     serving.Serving
 
 	certificate      *tls.Certificate
 	certificateError error
@@ -38,15 +30,7 @@ type Domain struct {
 
 // String implements Stringer.
 func (d *Domain) String() string {
-	if d.Group != "" && d.Project != "" {
-		return d.Group + "/" + d.Project
-	}
-
-	if d.Group != "" {
-		return d.Group
-	}
-
-	return d.Project
+	return d.Name
 }
 
 func (d *Domain) isCustomDomain() bool {
@@ -54,7 +38,7 @@ func (d *Domain) isCustomDomain() bool {
 		panic("project config and group config should not be nil at the same time")
 	}
 
-	return d.ProjectConfig != nil && d.GroupConfig == nil
+	return d.Customized
 }
 
 func (d *Domain) isUnconfigured() bool {
@@ -62,22 +46,52 @@ func (d *Domain) isUnconfigured() bool {
 		return true
 	}
 
-	return d.ProjectConfig == nil && d.GroupConfig == nil
+	return d.Resolver == nil
+}
+
+func (d *Domain) resolve(r *http.Request) (*Project, string) {
+	// TODO use lookupPaths first
+
+	project, subpath, _ := d.Resolver.Resolve(r)
+	// current implementation does not return errors in any case
+
+	if project == nil {
+		return nil, ""
+	}
+
+	return project, subpath
+}
+
+func (d *Domain) getProject(r *http.Request) *Project {
+	project, _ := d.resolve(r)
+
+	return project
 }
 
 // Serving returns domain serving driver
 func (d *Domain) Serving() serving.Serving {
 	if d.serving == nil {
-		if d.isCustomDomain() {
-			d.serving = serving.NewProjectDiskServing(d.Project, d.Group)
-		} else {
-			d.serving = serving.NewGroupDiskServing(d.Group, d.GroupConfig)
-		}
+		d.serving = serving.NewDiskServing(d.Name, d.Location)
 	}
 
 	return d.serving
 }
 
+func (d *Domain) toHandler(w http.ResponseWriter, r *http.Request) *handler {
+	project, subpath := d.resolve(r)
+
+	return &handler{
+		writer:  w,
+		request: r,
+		project: project,
+		subpath: subpath,
+	}
+}
+
+func (d *Domain) hasProject(r *http.Request) bool {
+	return d.getProject(r) != nil
+}
+
 // IsHTTPSOnly figures out if the request should be handled with HTTPS
 // only by looking at group and project level config.
 func (d *Domain) IsHTTPSOnly(r *http.Request) bool {
@@ -85,13 +99,11 @@ func (d *Domain) IsHTTPSOnly(r *http.Request) bool {
 		return false
 	}
 
-	// Check custom domain config (e.g. http://example.com)
-	if d.isCustomDomain() {
-		return d.ProjectConfig.HTTPSOnly
+	if project := d.getProject(r); project != nil {
+		return project.IsHTTPSOnly
 	}
 
-	// Check projects served under the group domain, including the default one
-	return d.GroupConfig.IsHTTPSOnly(r)
+	return false
 }
 
 // IsAccessControlEnabled figures out if the request is to a project that has access control enabled
@@ -100,22 +112,20 @@ func (d *Domain) IsAccessControlEnabled(r *http.Request) bool {
 		return false
 	}
 
-	// Check custom domain config (e.g. http://example.com)
-	if d.isCustomDomain() {
-		return d.ProjectConfig.AccessControl
+	if project := d.getProject(r); project != nil {
+		return project.HasAccessControl
 	}
 
-	// Check projects served under the group domain, including the default one
-	return d.GroupConfig.HasAccessControl(r)
+	return false
 }
 
 // HasAcmeChallenge checks domain directory contains particular acme challenge
-func (d *Domain) HasAcmeChallenge(token string) bool {
-	if d.isUnconfigured() || !d.isCustomDomain() {
+func (d *Domain) HasAcmeChallenge(r *http.Request, token string) bool {
+	if d.isUnconfigured() || !d.isCustomDomain() || !d.hasProject(r) {
 		return false
 	}
 
-	return d.Serving().HasAcmeChallenge(token)
+	return d.Serving().HasAcmeChallenge(d.toHandler(nil, r), token) // TODO
 }
 
 // IsNamespaceProject figures out if the request is to a namespace project
@@ -126,12 +136,15 @@ func (d *Domain) IsNamespaceProject(r *http.Request) bool {
 
 	// If request is to a custom domain, we do not handle it as a namespace project
 	// as there can't be multiple projects under the same custom domain
-	if d.isCustomDomain() {
+	if d.isCustomDomain() { // TODO do we need a separate path for this
 		return false
 	}
 
-	// Check projects served under the group domain, including the default one
-	return d.GroupConfig.IsNamespaceProject(r)
+	if project := d.getProject(r); project != nil {
+		return project.IsNamespaceProject
+	}
+
+	return false
 }
 
 // GetID figures out what is the ID of the project user tries to access
@@ -140,11 +153,11 @@ func (d *Domain) GetID(r *http.Request) uint64 {
 		return 0
 	}
 
-	if d.isCustomDomain() {
-		return d.ProjectConfig.ProjectID
+	if project := d.getProject(r); project != nil {
+		return project.ID
 	}
 
-	return d.GroupConfig.ProjectID(r)
+	return 0
 }
 
 // HasProject figures out if the project exists that the user tries to access
@@ -153,15 +166,16 @@ func (d *Domain) HasProject(r *http.Request) bool {
 		return false
 	}
 
-	if d.isCustomDomain() {
+	if project := d.getProject(r); project != nil {
 		return true
 	}
 
-	return d.GroupConfig.ProjectExists(r)
+	return false
 }
 
 // EnsureCertificate parses the PEM-encoded certificate for the domain
 func (d *Domain) EnsureCertificate() (*tls.Certificate, error) {
+	// TODO check len certificates instead of custom domain!
 	if d.isUnconfigured() || !d.isCustomDomain() {
 		return nil, errors.New("tls certificates can be loaded only for pages with configuration")
 	}
@@ -169,8 +183,8 @@ func (d *Domain) EnsureCertificate() (*tls.Certificate, error) {
 	d.certificateOnce.Do(func() {
 		var cert tls.Certificate
 		cert, d.certificateError = tls.X509KeyPair(
-			[]byte(d.ProjectConfig.Certificate),
-			[]byte(d.ProjectConfig.Key),
+			[]byte(d.CertificateCert),
+			[]byte(d.CertificateKey),
 		)
 		if d.certificateError == nil {
 			d.certificate = &cert
@@ -182,26 +196,20 @@ func (d *Domain) EnsureCertificate() (*tls.Certificate, error) {
 
 // ServeFileHTTP returns true if something was served, false if not.
 func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
-	if d.isUnconfigured() {
+	if d.isUnconfigured() || !d.hasProject(r) {
 		httperrors.Serve404(w)
 		return true
 	}
 
-	if !d.IsAccessControlEnabled(r) {
-		// Set caching headers
-		w.Header().Set("Cache-Control", "max-age=600")
-		w.Header().Set("Expires", time.Now().Add(10*time.Minute).Format(time.RFC1123))
-	}
-
-	return d.Serving().ServeFileHTTP(w, r)
+	return d.Serving().ServeFileHTTP(d.toHandler(w, r))
 }
 
 // ServeNotFoundHTTP serves the not found pages from the projects.
 func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
-	if d.isUnconfigured() {
+	if d.isUnconfigured() || !d.hasProject(r) {
 		httperrors.Serve404(w)
 		return
 	}
 
-	d.Serving().ServeNotFoundHTTP(w, r)
+	d.Serving().ServeNotFoundHTTP(d.toHandler(w, r))
 }
diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index 2be86e20..8b369075 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -11,10 +11,19 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	"gitlab.com/gitlab-org/gitlab-pages/internal/fixture"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
 )
 
+type stubbedResolver struct {
+	project *Project
+	subpath string
+	err     error
+}
+
+func (resolver *stubbedResolver) Resolve(*http.Request) (*Project, string, error) {
+	return resolver.project, resolver.subpath, resolver.err
+}
+
 func serveFileOrNotFound(domain *Domain) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if !domain.ServeFileHTTP(w, r) {
@@ -23,27 +32,6 @@ func serveFileOrNotFound(domain *Domain) http.HandlerFunc {
 	}
 }
 
-func TestDomainServeHTTP(t *testing.T) {
-	cleanup := setUpTests(t)
-	defer cleanup()
-
-	testDomain := &Domain{
-		Project:       "project2",
-		Group:         "group",
-		ProjectConfig: &ProjectConfig{DomainName: "test.domain.com"},
-	}
-
-	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/", nil, "project2-main")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/index.html", nil, "project2-main")
-	require.HTTPRedirect(t, serveFileOrNotFound(testDomain), "GET", "/subdir", nil)
-	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir", nil,
-		`<a href="/subdir/">Found</a>`)
-	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir/", nil, "project2-subdir")
-	require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir/index.html", nil, "project2-subdir")
-	require.HTTPError(t, serveFileOrNotFound(testDomain), "GET", "//about.gitlab.com/%2e%2e", nil)
-	require.HTTPError(t, serveFileOrNotFound(testDomain), "GET", "/not-existing-file", nil)
-}
-
 func TestIsHTTPSOnly(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -54,9 +42,8 @@ func TestIsHTTPSOnly(t *testing.T) {
 		{
 			name: "Custom domain with HTTPS-only enabled",
 			domain: &Domain{
-				Project:       "project",
-				Group:         "group",
-				ProjectConfig: &ProjectConfig{HTTPSOnly: true},
+				Location: "group/project",
+				Resolver: &stubbedResolver{project: &Project{IsHTTPSOnly: true}},
 			},
 			url:      "http://custom-domain",
 			expected: true,
@@ -64,9 +51,8 @@ func TestIsHTTPSOnly(t *testing.T) {
 		{
 			name: "Custom domain with HTTPS-only disabled",
 			domain: &Domain{
-				Project:       "project",
-				Group:         "group",
-				ProjectConfig: &ProjectConfig{HTTPSOnly: false},
+				Location: "group/project",
+				Resolver: &stubbedResolver{project: &Project{IsHTTPSOnly: false}},
 			},
 			url:      "http://custom-domain",
 			expected: false,
@@ -74,8 +60,7 @@ func TestIsHTTPSOnly(t *testing.T) {
 		{
 			name: "Unknown project",
 			domain: &Domain{
-				Project: "project",
-				Group:   "group",
+				Location: "group/project",
 			},
 			url:      "http://test-domain/project",
 			expected: false,
@@ -90,69 +75,6 @@ func TestIsHTTPSOnly(t *testing.T) {
 	}
 }
 
-func TestHasAcmeChallenge(t *testing.T) {
-	cleanup := setUpTests(t)
-	defer cleanup()
-
-	tests := []struct {
-		name     string
-		domain   *Domain
-		token    string
-		expected bool
-	}{
-		{
-			name: "Project containing acme challenge",
-			domain: &Domain{
-				Group:         "group.acme",
-				Project:       "with.acme.challenge",
-				ProjectConfig: &ProjectConfig{HTTPSOnly: true},
-			},
-			token:    "existingtoken",
-			expected: true,
-		},
-		{
-			name: "Project containing acme challenge",
-			domain: &Domain{
-				Group:         "group.acme",
-				Project:       "with.acme.challenge",
-				ProjectConfig: &ProjectConfig{HTTPSOnly: true},
-			},
-			token:    "foldertoken",
-			expected: true,
-		},
-		{
-			name: "Project containing another token",
-			domain: &Domain{
-				Group:         "group.acme",
-				Project:       "with.acme.challenge",
-				ProjectConfig: &ProjectConfig{HTTPSOnly: true},
-			},
-			token:    "notexistingtoken",
-			expected: false,
-		},
-		{
-			name:     "nil domain",
-			domain:   nil,
-			token:    "existingtoken",
-			expected: false,
-		},
-		{
-			name: "Domain without config",
-			domain: &Domain{
-				Group:   "group.acme",
-				Project: "with.acme.challenge",
-			},
-			token:    "existingtoken",
-			expected: false,
-		},
-	}
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			require.Equal(t, test.expected, test.domain.HasAcmeChallenge(test.token))
-		})
-	}
-}
-
 func testHTTPGzip(t *testing.T, handler http.HandlerFunc, mode, url string, values url.Values, acceptEncoding string, str interface{}, contentType string, ungzip bool) {
 	w := httptest.NewRecorder()
 	req, err := http.NewRequest(mode, url+"?"+values.Encode(), nil)
@@ -180,26 +102,12 @@ func testHTTPGzip(t *testing.T, handler http.HandlerFunc, mode, url string, valu
 	require.Equal(t, contentType, w.Header().Get("Content-Type"))
 }
 
-func TestDomain404ServeHTTP(t *testing.T) {
-	cleanup := setUpTests(t)
-	defer cleanup()
-
-	testDomain := &Domain{
-		Group:         "group.404",
-		Project:       "domain.404",
-		ProjectConfig: &ProjectConfig{DomainName: "domain.404.com"},
-	}
-
-	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.404.test.io/not-existing-file", nil, "Custom 404 group page")
-	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.404.test.io/", nil, "Custom 404 group page")
-}
-
 func TestPredefined404ServeHTTP(t *testing.T) {
 	cleanup := setUpTests(t)
 	defer cleanup()
 
 	testDomain := &Domain{
-		Group: "group",
+		Location: "group",
 	}
 
 	testhelpers.AssertHTTP404(t, serveFileOrNotFound(testDomain), "GET", "http://group.test.io/not-existing-file", nil, "The page you're looking for could not be found")
@@ -207,8 +115,7 @@ func TestPredefined404ServeHTTP(t *testing.T) {
 
 func TestGroupCertificate(t *testing.T) {
 	testGroup := &Domain{
-		Project: "",
-		Group:   "group",
+		Location: "group",
 	}
 
 	tls, err := testGroup.EnsureCertificate()
@@ -218,9 +125,8 @@ func TestGroupCertificate(t *testing.T) {
 
 func TestDomainNoCertificate(t *testing.T) {
 	testDomain := &Domain{
-		Group:         "group",
-		Project:       "project2",
-		ProjectConfig: &ProjectConfig{DomainName: "test.domain.com"},
+		Name:     "test.domain.com",
+		Location: "group/project2",
 	}
 
 	tls, err := testDomain.EnsureCertificate()
@@ -232,22 +138,6 @@ func TestDomainNoCertificate(t *testing.T) {
 	require.Equal(t, err, err2)
 }
 
-func TestDomainCertificate(t *testing.T) {
-	testDomain := &Domain{
-		Group:   "group",
-		Project: "project2",
-		ProjectConfig: &ProjectConfig{
-			DomainName:  "test.domain.com",
-			Certificate: fixture.Certificate,
-			Key:         fixture.Key,
-		},
-	}
-
-	tls, err := testDomain.EnsureCertificate()
-	require.NotNil(t, tls)
-	require.NoError(t, err)
-}
-
 var chdirSet = false
 
 func setUpTests(t require.TestingT) func() {
diff --git a/internal/domain/handler.go b/internal/domain/handler.go
new file mode 100644
index 00000000..55697700
--- /dev/null
+++ b/internal/domain/handler.go
@@ -0,0 +1,42 @@
+package domain
+
+import "net/http"
+
+type handler struct {
+	writer  http.ResponseWriter
+	request *http.Request
+	project *Project
+	subpath string
+}
+
+func (h *handler) Writer() http.ResponseWriter {
+	return h.writer
+}
+
+func (h *handler) Request() *http.Request {
+	return h.request
+}
+
+func (h *handler) LookupPath() string {
+	return h.project.LookupPath
+}
+
+func (h *handler) Subpath() string {
+	return h.subpath
+}
+
+func (h *handler) IsNamespaceProject() bool {
+	return h.project.IsNamespaceProject
+}
+
+func (h *handler) IsHTTPSOnly() bool {
+	return h.project.IsHTTPSOnly
+}
+
+func (h *handler) HasAccessControl() bool {
+	return h.project.HasAccessControl
+}
+
+func (h *handler) ProjectID() uint64 {
+	return h.project.ID
+}
diff --git a/internal/domain/project.go b/internal/domain/project.go
new file mode 100644
index 00000000..9ea7306f
--- /dev/null
+++ b/internal/domain/project.go
@@ -0,0 +1,10 @@
+package domain
+
+// Project holds a domain / project configuration
+type Project struct {
+	LookupPath         string
+	IsNamespaceProject bool
+	IsHTTPSOnly        bool
+	HasAccessControl   bool
+	ID                 uint64
+}
diff --git a/internal/domain/resolver.go b/internal/domain/resolver.go
new file mode 100644
index 00000000..5bde31ec
--- /dev/null
+++ b/internal/domain/resolver.go
@@ -0,0 +1,10 @@
+package domain
+
+import "net/http"
+
+// Resolver represents an interface responsible for resolving a project
+// per-request
+type Resolver interface {
+	// Resolve returns a project with a file path and an error if it occured
+	Resolve(*http.Request) (*Project, string, error)
+}