From 3224eb6418fcc0fcb0fa5bd3203c0c0c25b4704b Mon Sep 17 00:00:00 2001 From: Albert Salim Date: Fri, 1 Jul 2022 16:11:35 +0800 Subject: Include remote address through X-Forwarded-For in job artifact request Changelog: security --- internal/artifact/artifact.go | 5 +++++ 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 := "Title of the document" 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 := "Title of the document" contentType := "text/html; charset=utf-8" - testServer := makeArtifactServerStub(t, content, contentType) + testServer := makeArtifactServerStub(t, content, contentType, "") t.Cleanup(testServer.Close) result := httptest.NewRecorder() -- cgit v1.2.3 From 223cab62bf3368215542db11e6703060090dfc14 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Tue, 19 Jul 2022 16:51:31 +0530 Subject: Chore: bump version to 1.61.1 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index bb120e87..30e298c7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.59.0 +1.61.1 -- cgit v1.2.3 From c9fa72a7ba17f67d3ed0d4455d0dc893c039d250 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Tue, 19 Jul 2022 11:24:37 +0000 Subject: Docs: add changelog for version 1.61.1 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 066619c4..367c0309 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.61.1 (2022-07-19) + +### Security (1 change) + +- [Include remote address through X-Forwarded-For in job artifact request](gitlab-org/security/gitlab-pages@3224eb6418fcc0fcb0fa5bd3203c0c0c25b4704b) ([merge request](gitlab-org/security/gitlab-pages!38)) + ## 1.59.0 (2022-06-13) ### Added (2 changes) -- cgit v1.2.3