From 1c2b452406f6f7c91f2cf31fc5c9a421588e8ce8 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 25 Jan 2022 10:57:53 +0100 Subject: [PATCH] outposts/proxy: fix potential empty redirect, add tests Signed-off-by: Jens Langhammer #2141 --- .github/workflows/ci-outpost.yml | 9 --- .../application/mode_forward_nginx_test.go | 2 +- .../outpost/proxyv2/application/mode_proxy.go | 1 + internal/outpost/proxyv2/application/utils.go | 11 +-- .../outpost/proxyv2/application/utils_test.go | 81 +++++++++++++++++++ 5 files changed, 85 insertions(+), 19 deletions(-) create mode 100644 internal/outpost/proxyv2/application/utils_test.go diff --git a/.github/workflows/ci-outpost.yml b/.github/workflows/ci-outpost.yml index f3cfa24d4..3b4b86d3c 100644 --- a/.github/workflows/ci-outpost.yml +++ b/.github/workflows/ci-outpost.yml @@ -45,15 +45,6 @@ jobs: - name: Go unittests run: | go test -timeout 0 -v -race -coverprofile=coverage.out -covermode=atomic -cover ./... | go-junit-report > junit.xml - - uses: testspace-com/setup-testspace@v1 - with: - domain: ${{github.repository_owner}} - - name: run testspace - if: ${{ always() }} - run: | - gocov convert coverage.out | gocov-xml > coverage.xml - testspace [outpost]junit.xml --link=codecov - testspace [outpost]coverage.xml --link=codecov ci-outpost-mark: needs: - lint-golint diff --git a/internal/outpost/proxyv2/application/mode_forward_nginx_test.go b/internal/outpost/proxyv2/application/mode_forward_nginx_test.go index d68d5fbbd..b63b4fb9b 100644 --- a/internal/outpost/proxyv2/application/mode_forward_nginx_test.go +++ b/internal/outpost/proxyv2/application/mode_forward_nginx_test.go @@ -71,7 +71,7 @@ func TestForwardHandleNginx_Single_Claims(t *testing.T) { } rr = httptest.NewRecorder() - a.forwardHandleTraefik(rr, req) + a.forwardHandleNginx(rr, req) h := rr.Result().Header diff --git a/internal/outpost/proxyv2/application/mode_proxy.go b/internal/outpost/proxyv2/application/mode_proxy.go index 5e74f78ec..0271b0201 100644 --- a/internal/outpost/proxyv2/application/mode_proxy.go +++ b/internal/outpost/proxyv2/application/mode_proxy.go @@ -79,5 +79,6 @@ func (a *Application) proxyModifyRequest(u *url.URL) func(req *http.Request) { } func (a *Application) proxyModifyResponse(res *http.Response) error { + res.Header.Set("X-Powered-By", "authentik_proxy2") return nil } diff --git a/internal/outpost/proxyv2/application/utils.go b/internal/outpost/proxyv2/application/utils.go index fe7c88a7b..eacbad6e0 100644 --- a/internal/outpost/proxyv2/application/utils.go +++ b/internal/outpost/proxyv2/application/utils.go @@ -26,21 +26,14 @@ func (a *Application) redirectToStart(rw http.ResponseWriter, r *http.Request) { if err == nil { a.log.WithError(err).Warning("failed to decode session") } - redirectUrl := r.URL.String() - // simple way to copy the URL - u, _ := url.Parse(redirectUrl) - // In proxy and forward_single mode we only have one URL that we route on - // if we somehow got here without that URL, make sure we're at least redirected back to it - if a.Mode() == api.PROXYMODE_PROXY || a.Mode() == api.PROXYMODE_FORWARD_SINGLE { - u.Host = a.proxyConfig.ExternalHost - } + redirectUrl := urlJoin(a.proxyConfig.ExternalHost, r.URL.Path) if a.Mode() == api.PROXYMODE_FORWARD_DOMAIN { dom := strings.TrimPrefix(*a.proxyConfig.CookieDomain, ".") // In forward_domain we only check that the current URL's host // ends with the cookie domain (remove the leading period if set) if !strings.HasSuffix(r.URL.Hostname(), dom) { a.log.WithField("url", r.URL.String()).WithField("cd", dom).Warning("Invalid redirect found") - redirectUrl = "" + redirectUrl = a.proxyConfig.ExternalHost } } s.Values[constants.SessionRedirect] = redirectUrl diff --git a/internal/outpost/proxyv2/application/utils_test.go b/internal/outpost/proxyv2/application/utils_test.go new file mode 100644 index 000000000..3d84f7608 --- /dev/null +++ b/internal/outpost/proxyv2/application/utils_test.go @@ -0,0 +1,81 @@ +package application + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "goauthentik.io/api" + "goauthentik.io/internal/outpost/proxyv2/constants" +) + +func TestRedirectToStart_Proxy(t *testing.T) { + a := newTestApplication() + a.proxyConfig.Mode = api.PROXYMODE_PROXY.Ptr() + a.proxyConfig.ExternalHost = "https://test.goauthentik.io" + req, _ := http.NewRequest("GET", "/foo/bar/baz", nil) + + rr := httptest.NewRecorder() + a.redirectToStart(rr, req) + + assert.Equal(t, http.StatusFound, rr.Code) + loc, _ := rr.Result().Location() + assert.Equal(t, "https://test.goauthentik.io/akprox/start", loc.String()) + + s, _ := a.sessions.Get(req, constants.SeesionName) + assert.Equal(t, "https://test.goauthentik.io/foo/bar/baz", s.Values[constants.SessionRedirect]) +} + +func TestRedirectToStart_Forward(t *testing.T) { + a := newTestApplication() + a.proxyConfig.Mode = api.PROXYMODE_FORWARD_SINGLE.Ptr() + a.proxyConfig.ExternalHost = "https://test.goauthentik.io" + req, _ := http.NewRequest("GET", "/foo/bar/baz", nil) + + rr := httptest.NewRecorder() + a.redirectToStart(rr, req) + + assert.Equal(t, http.StatusFound, rr.Code) + loc, _ := rr.Result().Location() + assert.Equal(t, "https://test.goauthentik.io/akprox/start", loc.String()) + + s, _ := a.sessions.Get(req, constants.SeesionName) + assert.Equal(t, "https://test.goauthentik.io/foo/bar/baz", s.Values[constants.SessionRedirect]) +} + +func TestRedirectToStart_Forward_Domain_Invalid(t *testing.T) { + a := newTestApplication() + a.proxyConfig.CookieDomain = api.PtrString("foo") + a.proxyConfig.Mode = api.PROXYMODE_FORWARD_DOMAIN.Ptr() + a.proxyConfig.ExternalHost = "https://test.goauthentik.io" + req, _ := http.NewRequest("GET", "/foo/bar/baz", nil) + + rr := httptest.NewRecorder() + a.redirectToStart(rr, req) + + assert.Equal(t, http.StatusFound, rr.Code) + loc, _ := rr.Result().Location() + assert.Equal(t, "https://test.goauthentik.io/akprox/start", loc.String()) + + s, _ := a.sessions.Get(req, constants.SeesionName) + assert.Equal(t, "https://test.goauthentik.io", s.Values[constants.SessionRedirect]) +} + +func TestRedirectToStart_Forward_Domain(t *testing.T) { + a := newTestApplication() + a.proxyConfig.CookieDomain = api.PtrString("goauthentik.io") + a.proxyConfig.Mode = api.PROXYMODE_FORWARD_DOMAIN.Ptr() + a.proxyConfig.ExternalHost = "https://test.goauthentik.io" + req, _ := http.NewRequest("GET", "/foo/bar/baz", nil) + + rr := httptest.NewRecorder() + a.redirectToStart(rr, req) + + assert.Equal(t, http.StatusFound, rr.Code) + loc, _ := rr.Result().Location() + assert.Equal(t, "https://test.goauthentik.io/akprox/start", loc.String()) + + s, _ := a.sessions.Get(req, constants.SeesionName) + assert.Equal(t, "https://test.goauthentik.io", s.Values[constants.SessionRedirect]) +}