diff options
| author | Jaime Martinez <jmartinez@gitlab.com> | 2022-07-12 07:58:52 +0300 |
|---|---|---|
| committer | Jaime Martinez <jmartinez@gitlab.com> | 2022-07-12 07:58:52 +0300 |
| commit | e1921af5f1d207748107147b3ced441819f584c6 (patch) | |
| tree | 731f8298e41f87948af0a425c749c1d33b9e82da | |
| parent | a80c58682c3c313f52deecdebecfef2d1f7e3b21 (diff) | |
| parent | 42f01f5365b65058faa5d3e53681c06034dfe3dd (diff) | |
Merge branch 'security-pass-on-remote-addr-in-x-forwarded-for-backport-1-58' into '1-58-stable'
Include remote address through X-Forwarded-For in job artifact request
See merge request gitlab-org/security/gitlab-pages!40
| -rw-r--r-- | internal/artifact/artifact.go | 6 | ||||
| -rw-r--r-- | internal/artifact/artifact_test.go | 25 |
2 files changed, 26 insertions, 5 deletions
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index dab1fb91..e33a9ab9 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" + "gitlab.com/gitlab-org/gitlab-pages/internal/request" ) const ( @@ -88,6 +89,11 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u if token != "" { req.Header.Add("Authorization", "Bearer "+token) } + + // The GitLab API expects this value for Group IP restriction to work properly + // on requests coming through Pages. + req.Header.Set("X-Forwarded-For", request.GetRemoteAddrWithoutPort(r)) + resp, err := a.client.Do(req) if err != nil { if errors.Is(err, context.Canceled) { diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go index b8c2771b..472c0d7b 100644 --- a/internal/artifact/artifact_test.go +++ b/internal/artifact/artifact_test.go @@ -16,8 +16,6 @@ import ( func TestTryMakeRequest(t *testing.T) { content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>" contentType := "text/html; charset=utf-8" - testServer := makeArtifactServerStub(t, content, contentType) - defer testServer.Close() cases := []struct { Path string @@ -28,6 +26,8 @@ func TestTryMakeRequest(t *testing.T) { CacheControl string ContentType string Description string + RemoteAddr string + ForwardedIP string }{ { "/200.html", @@ -38,6 +38,8 @@ func TestTryMakeRequest(t *testing.T) { "max-age=3600", "text/html; charset=utf-8", "basic successful request", + "1.2.3.4:8000", + "1.2.3.4", }, { "/200.html", @@ -48,6 +50,8 @@ func TestTryMakeRequest(t *testing.T) { "", "text/html; charset=utf-8", "basic successful request", + "1.2.3.4", + "1.2.3.4", }, { "/max-caching.html", @@ -58,6 +62,8 @@ func TestTryMakeRequest(t *testing.T) { "max-age=3600", "text/html; charset=utf-8", "max caching request", + "1.2.3.4", + "1.2.3.4", }, { "/non-caching.html", @@ -68,17 +74,24 @@ func TestTryMakeRequest(t *testing.T) { "", "text/html; charset=utf-8", "no caching request", + "1.2.3.4", + "1.2.3.4", }, } for _, c := range cases { t.Run(c.Description, func(t *testing.T) { - result := httptest.NewRecorder() + testServer := makeArtifactServerStub(t, content, contentType, c.ForwardedIP) + defer testServer.Close() + reqURL, err := url.Parse("/-/subgroup/project/-/jobs/1/artifacts" + c.Path) require.NoError(t, err) r := &http.Request{URL: reqURL} + r.RemoteAddr = c.RemoteAddr art := artifact.New(testServer.URL, 1, "gitlab-example.io") + result := httptest.NewRecorder() + require.True(t, art.TryMakeRequest("group.gitlab-example.io", result, r, c.Token, func(resp *http.Response) bool { return false })) require.Equal(t, c.Status, result.Code) require.Equal(t, c.ContentType, result.Header().Get("Content-Type")) @@ -90,8 +103,10 @@ func TestTryMakeRequest(t *testing.T) { } // provide stub for testing different artifact responses -func makeArtifactServerStub(t *testing.T, content string, contentType string) *httptest.Server { +func makeArtifactServerStub(t *testing.T, content string, contentType string, expectedForwardedIP string) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, expectedForwardedIP, r.Header.Get("X-Forwarded-For")) + w.Header().Set("Content-Type", contentType) switch r.URL.RawPath { case "/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/200.html": @@ -280,7 +295,7 @@ func TestBuildURL(t *testing.T) { func TestContextCanceled(t *testing.T) { content := "<!DOCTYPE html><html><head><title>Title of the document</title></head><body></body></html>" contentType := "text/html; charset=utf-8" - testServer := makeArtifactServerStub(t, content, contentType) + testServer := makeArtifactServerStub(t, content, contentType, "") t.Cleanup(testServer.Close) result := httptest.NewRecorder() |