diff options
| author | Albert Salim <asalim@gitlab.com> | 2022-07-01 11:11:35 +0300 |
|---|---|---|
| committer | Albert Salim <asalim@gitlab.com> | 2022-07-07 08:42:54 +0300 |
| commit | 3224eb6418fcc0fcb0fa5bd3203c0c0c25b4704b (patch) | |
| tree | f49e1f967775e42e5d83fcf52a2337e31800b400 /internal | |
| parent | 30f67e89a01173207bd1e40d151c700c18ccddc7 (diff) | |
Include remote address through X-Forwarded-For in job artifact request
Changelog: security
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/artifact/artifact.go | 5 | ||||
| -rw-r--r-- | internal/artifact/artifact_test.go | 27 |
2 files changed, 27 insertions, 5 deletions
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 87ce1450..69b74322 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -91,6 +91,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 c015e44f..6da0e2d9 100644 --- a/internal/artifact/artifact_test.go +++ b/internal/artifact/artifact_test.go @@ -15,8 +15,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 @@ -27,6 +25,8 @@ func TestTryMakeRequest(t *testing.T) { CacheControl string ContentType string Description string + RemoteAddr string + ForwardedIP string }{ { "/200.html", @@ -37,6 +37,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", @@ -47,6 +49,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", @@ -57,6 +61,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", @@ -67,17 +73,26 @@ 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() + url := "https://group.gitlab-example.io/-/subgroup/project/-/jobs/1/artifacts" + c.Path r, err := http.NewRequest(http.MethodGet, url, nil) require.NoError(t, err) + + r.RemoteAddr = c.RemoteAddr + art := artifact.New(testServer.URL, 1, "gitlab-example.io") + result := httptest.NewRecorder() + require.True(t, art.TryMakeRequest(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")) @@ -89,8 +104,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": @@ -279,7 +296,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() |