diff options
author | Stan Hu <stanhu@gmail.com> | 2018-08-26 08:00:26 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-08-27 00:55:25 +0300 |
commit | 7d8bfba860db8c5894d73cb243f964d9cfe7e213 (patch) | |
tree | 9bb60c38a311dd50a38817b877df4506e305e5dc | |
parent | b1356f3622279a18b5359a0a9cf7639243830486 (diff) |
Fix HTTP to HTTPS redirection not working for default domains
If a default domain is in use, the group domain will be passed into the
IsHTTPSOnly() call, but this will always return false because the HTTPS-only
flag can only be determined by looking at the HTTP request host rather than
the URL.
For example, for the project https://gitlab.com/tanukitalks/tanukitalks.gitlab.io,
this is what happened previously:
1. For a project in the default domain (e.g. `gitlab.io`), GitLab Pages loads
`config.json` and sees the group is `tanukitalks` with the project name
`tanuitalks.gitlab.io`. It stores the HTTPS-only flag inside the project
config.
2. Note that for projects in the default domain, the `domainConfig` is
empty. This makes sense because there is no domain configuration specified
since the default domain is being used, and we need to redirect on a
project-by-project basis.
3. User requests https://tanukitalks.gitlab.io.
4. GitLab Pages looks up `tanukitalks.gitlab.io`, and it returns the domain
`tanukitalks` with an empty `domainConfig` and project `tanukitalks.gitlab.io`.
5. Since there is no `domainConfig`, `IsHTTPSOnly` attempts to resolve the
project from the URL.
6. However, since the URL is using the default domain, the path is `/`, which
doesn't resolve to any project.
In the new behavior, we check the hostname of the request to see if it matches
any project before trying to parse the URL.
Closes #162
-rw-r--r-- | internal/domain/domain.go | 12 | ||||
-rw-r--r-- | internal/domain/domain_test.go | 96 | ||||
-rw-r--r-- | internal/domain/map.go | 8 |
3 files changed, 111 insertions, 5 deletions
diff --git a/internal/domain/domain.go b/internal/domain/domain.go index f50dacd8..4f5c870e 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -100,16 +100,26 @@ func setContentType(w http.ResponseWriter, fullPath string) { // IsHTTPSOnly figures out if the request should be handled with HTTPS // only by looking at group and project level config. func (d *D) IsHTTPSOnly(r *http.Request) bool { + // Check custom domain config (e.g. http://example.com) if d.config != nil { return d.config.HTTPSOnly } + // Check default domain config (e.g. http://mydomain.gitlab.io) + groupProject := d.projects[strings.ToLower(r.Host)] + + if groupProject != nil { + return groupProject.HTTPSOnly + } + + // Check URLs with multiple projects for a group + // (e.g. http://group.gitlab.io/projectA and http://group.gitlab.io/projectB) split := strings.SplitN(r.URL.Path, "/", 3) if len(split) < 2 { return false } - project := d.projects[split[1]] + project := d.projects[strings.ToLower(split[1])] if project != nil { return project.HTTPSOnly diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go index 130501d6..39976bfe 100644 --- a/internal/domain/domain_test.go +++ b/internal/domain/domain_test.go @@ -69,6 +69,102 @@ func TestDomainServeHTTP(t *testing.T) { assert.HTTPError(t, testDomain.ServeHTTP, "GET", "/not-existing-file", nil) } +func TestIsHTTPSOnly(t *testing.T) { + tests := []struct { + name string + domain *D + url string + expected bool + }{ + { + name: "Custom domain with HTTPS-only enabled", + domain: &D{ + group: "group", + projectName: "project", + config: &domainConfig{HTTPSOnly: true}, + }, + url: "http://custom-domain", + expected: true, + }, + { + name: "Custom domain with HTTPS-only disabled", + domain: &D{ + group: "group", + projectName: "project", + config: &domainConfig{HTTPSOnly: false}, + }, + url: "http://custom-domain", + expected: false, + }, + { + name: "Default group domain with HTTPS-only enabled", + domain: &D{ + group: "group", + projectName: "project", + projects: projects{"test-domain": &project{HTTPSOnly: true}}, + }, + url: "http://test-domain", + expected: true, + }, + { + name: "Default group domain with HTTPS-only disabled", + domain: &D{ + group: "group", + projectName: "project", + projects: projects{"test-domain": &project{HTTPSOnly: false}}, + }, + url: "http://test-domain", + expected: false, + }, + { + name: "Case-insensitive default group domain with HTTPS-only enabled", + domain: &D{ + group: "group", + projectName: "project", + projects: projects{"test-domain": &project{HTTPSOnly: true}}, + }, + url: "http://Test-domain", + expected: true, + }, + { + name: "Other group domain with HTTPS-only enabled", + domain: &D{ + group: "group", + projectName: "project", + projects: projects{"project": &project{HTTPSOnly: true}}, + }, + url: "http://test-domain/project", + expected: true, + }, + { + name: "Other group domain with HTTPS-only disabled", + domain: &D{ + group: "group", + projectName: "project", + projects: projects{"project": &project{HTTPSOnly: false}}, + }, + url: "http://test-domain/project", + expected: false, + }, + { + name: "Unknown project", + domain: &D{ + group: "group", + projectName: "project", + }, + url: "http://test-domain/project", + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, test.url, nil) + assert.Equal(t, test.domain.IsHTTPSOnly(req), test.expected) + }) + } +} + func testHTTPGzip(t *testing.T, handler http.HandlerFunc, mode, url string, values url.Values, acceptEncoding string, str interface{}, ungzip bool) { w := httptest.NewRecorder() req, err := http.NewRequest(mode, url+"?"+values.Encode(), nil) diff --git a/internal/domain/map.go b/internal/domain/map.go index fb963fed..b7cb5c67 100644 --- a/internal/domain/map.go +++ b/internal/domain/map.go @@ -23,10 +23,10 @@ type domainsUpdater func(Map) func (dm Map) updateDomainMap(domainName string, domain *D) { if old, ok := dm[domainName]; ok { log.WithFields(log.Fields{ - "domain_name": domainName, - "new_group": domain.group, + "domain_name": domainName, + "new_group": domain.group, "new_project_name": domain.projectName, - "old_group": old.group, + "old_group": old.group, "old_project_name": old.projectName, }).Error("Duplicate domain") } @@ -57,7 +57,7 @@ func (dm Map) updateGroupDomain(rootDomain, group, projectName string, httpsOnly } } - groupDomain.projects[projectName] = &project{ + groupDomain.projects[strings.ToLower(projectName)] = &project{ HTTPSOnly: httpsOnly, } |