From 07b09df3feb019238f514f92b116bfe6d60b8a5e Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 24 Jan 2022 20:50:13 +0100 Subject: [PATCH] internal: add more outpost tests, add support for X-Original-URL Signed-off-by: Jens Langhammer --- Makefile | 3 + .../proxyv2/application/mode_common.go | 36 ++-- .../proxyv2/application/mode_forward.go | 22 ++- .../proxyv2/application/mode_forward_test.go | 155 ++++++++++++++++-- .../outpost/proxyv2/application/mode_proxy.go | 2 +- 5 files changed, 184 insertions(+), 34 deletions(-) diff --git a/Makefile b/Makefile index ef1794f65..16583cab4 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,9 @@ test-e2e-provider: test-e2e-rest: coverage run manage.py test tests/e2e/test_flows* tests/e2e/test_source* +test-go: + go test -timeout 0 -v -race ./... + test: coverage run manage.py test authentik coverage html diff --git a/internal/outpost/proxyv2/application/mode_common.go b/internal/outpost/proxyv2/application/mode_common.go index a60af16e9..605ce1b60 100644 --- a/internal/outpost/proxyv2/application/mode_common.go +++ b/internal/outpost/proxyv2/application/mode_common.go @@ -2,6 +2,7 @@ package application import ( "encoding/base64" + "errors" "fmt" "net/http" "net/url" @@ -57,6 +58,7 @@ func (a *Application) addHeaders(headers http.Header, c *Claims) { } } +// getTraefikForwardUrl See https://doc.traefik.io/traefik/middlewares/forwardauth/ func (a *Application) getTraefikForwardUrl(r *http.Request) *url.URL { u, err := url.Parse(fmt.Sprintf( "%s://%s%s", @@ -72,26 +74,32 @@ func (a *Application) getTraefikForwardUrl(r *http.Request) *url.URL { return u } -func (a *Application) IsAllowlisted(r *http.Request) bool { - url := r.URL - // In Forward auth mode, we can't directly match against the requested URL - // Since that would be /akprox/auth/... - if a.Mode() == api.PROXYMODE_FORWARD_SINGLE || a.Mode() == api.PROXYMODE_FORWARD_DOMAIN { - // For traefik, we can get the Upstream URL from headers - // For nginx we can attempt to as well, but it's not guaranteed to work. - if strings.HasPrefix(r.URL.Path, "/akprox/auth") { - url = a.getTraefikForwardUrl(r) - } +// getNginxForwardUrl See https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/template/nginx.tmpl#L1044 +func (a *Application) getNginxForwardUrl(r *http.Request) *url.URL { + h := r.Header.Get("X-Original-URI") + if len(h) < 1 { + a.log.WithError(errors.New("blank URL")).Warning("blank URL") + return r.URL } - for _, u := range a.UnauthenticatedRegex { + u, err := url.Parse(h) + if err != nil { + a.log.WithError(err).Warning("failed to parse URL from nginx") + return r.URL + } + a.log.WithField("url", u.String()).Trace("nginx forwarded url") + return u +} + +func (a *Application) IsAllowlisted(u *url.URL) bool { + for _, ur := range a.UnauthenticatedRegex { var testString string if a.Mode() == api.PROXYMODE_PROXY || a.Mode() == api.PROXYMODE_FORWARD_SINGLE { - testString = url.Path + testString = u.Path } else { - testString = url.String() + testString = u.String() } a.log.WithField("regex", u.String()).WithField("url", testString).Trace("Matching URL against allow list") - if u.MatchString(testString) { + if ur.MatchString(testString) { return true } } diff --git a/internal/outpost/proxyv2/application/mode_forward.go b/internal/outpost/proxyv2/application/mode_forward.go index 4c7887b89..8c323ac87 100644 --- a/internal/outpost/proxyv2/application/mode_forward.go +++ b/internal/outpost/proxyv2/application/mode_forward.go @@ -25,13 +25,14 @@ func (a *Application) configureForward() error { } func (a *Application) forwardHandleTraefik(rw http.ResponseWriter, r *http.Request) { + fwd := a.getTraefikForwardUrl(r) claims, err := a.getClaims(r) if claims != nil && err == nil { a.addHeaders(rw.Header(), claims) rw.Header().Set("User-Agent", r.Header.Get("User-Agent")) a.log.WithField("headers", rw.Header()).Trace("headers written to forward_auth") return - } else if claims == nil && a.IsAllowlisted(r) { + } else if claims == nil && a.IsAllowlisted(fwd) { a.log.Trace("path can be accessed without authentication") return } @@ -51,9 +52,8 @@ func (a *Application) forwardHandleTraefik(rw http.ResponseWriter, r *http.Reque // set the redirect flag to the current URL we have, since we redirect // to a (possibly) different domain, but we want to be redirected back // to the application - // see https://doc.traefik.io/traefik/middlewares/forwardauth/ // X-Forwarded-Uri is only the path, so we need to build the entire URL - s.Values[constants.SessionRedirect] = a.getTraefikForwardUrl(r).String() + s.Values[constants.SessionRedirect] = fwd.String() err = s.Save(r, rw) if err != nil { a.log.WithError(err).Warning("failed to save session before redirect") @@ -69,6 +69,7 @@ func (a *Application) forwardHandleTraefik(rw http.ResponseWriter, r *http.Reque } func (a *Application) forwardHandleNginx(rw http.ResponseWriter, r *http.Request) { + fwd := a.getNginxForwardUrl(r) claims, err := a.getClaims(r) if claims != nil && err == nil { a.addHeaders(rw.Header(), claims) @@ -76,13 +77,20 @@ func (a *Application) forwardHandleNginx(rw http.ResponseWriter, r *http.Request rw.WriteHeader(200) a.log.WithField("headers", rw.Header()).Trace("headers written to forward_auth") return - } else if claims == nil && a.IsAllowlisted(r) { + } else if claims == nil && a.IsAllowlisted(fwd) { a.log.Trace("path can be accessed without authentication") return } - fwu := a.getTraefikForwardUrl(r) - if fwu.String() != r.URL.String() { - if strings.HasPrefix(fwu.Path, "/akprox") { + + s, _ := a.sessions.Get(r, constants.SeesionName) + s.Values[constants.SessionRedirect] = fwd.String() + err = s.Save(r, rw) + if err != nil { + a.log.WithError(err).Warning("failed to save session before redirect") + } + + if fwd.String() != r.URL.String() { + if strings.HasPrefix(fwd.Path, "/akprox") { a.log.WithField("url", r.URL.String()).Trace("path begins with /akprox, allowing access") return } diff --git a/internal/outpost/proxyv2/application/mode_forward_test.go b/internal/outpost/proxyv2/application/mode_forward_test.go index d2b1a0cd6..08055eef8 100644 --- a/internal/outpost/proxyv2/application/mode_forward_test.go +++ b/internal/outpost/proxyv2/application/mode_forward_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/quasoft/memstore" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "goauthentik.io/api" "goauthentik.io/internal/outpost/ak" @@ -15,13 +16,16 @@ import ( func newTestApplication() *Application { a, _ := NewApplication( api.ProxyOutpostConfig{ - Name: *api.PtrString(ak.TestSecret()), - ClientId: api.PtrString(ak.TestSecret()), - ClientSecret: api.PtrString(ak.TestSecret()), - CookieSecret: api.PtrString(ak.TestSecret()), - CookieDomain: api.PtrString(""), - Mode: api.PROXYMODE_FORWARD_SINGLE.Ptr(), - SkipPathRegex: api.PtrString(""), + Name: ak.TestSecret(), + ClientId: api.PtrString(ak.TestSecret()), + ClientSecret: api.PtrString(ak.TestSecret()), + CookieSecret: api.PtrString(ak.TestSecret()), + CookieDomain: api.PtrString(""), + Mode: api.PROXYMODE_FORWARD_SINGLE.Ptr(), + SkipPathRegex: api.PtrString("/skip.*"), + BasicAuthEnabled: api.PtrBool(true), + BasicAuthUserAttribute: api.PtrString("username"), + BasicAuthPasswordAttribute: api.PtrString("password"), }, http.DefaultClient, nil, @@ -47,13 +51,27 @@ func TestForwardHandleTraefik_Blank(t *testing.T) { rr := httptest.NewRecorder() a.forwardHandleTraefik(rr, req) - assert.Equal(t, rr.Code, http.StatusTemporaryRedirect) + assert.Equal(t, http.StatusTemporaryRedirect, rr.Code) loc, _ := rr.Result().Location() - assert.Equal(t, loc.String(), "/akprox/start") + assert.Equal(t, "/akprox/start", loc.String()) s, _ := a.sessions.Get(req, constants.SeesionName) // Since we're not setting the traefik specific headers, expect it to redirect to the auth URL - assert.Equal(t, s.Values[constants.SessionRedirect], "/akprox/auth/traefik") + assert.Equal(t, "/akprox/auth/traefik", s.Values[constants.SessionRedirect]) +} + +func TestForwardHandleTraefik_Skip(t *testing.T) { + logrus.SetLevel(logrus.TraceLevel) + a := newTestApplication() + req, _ := http.NewRequest("GET", "/akprox/auth/traefik", nil) + req.Header.Set("X-Forwarded-Proto", "http") + req.Header.Set("X-Forwarded-Host", "test.goauthentik.io") + req.Header.Set("X-Forwarded-Uri", "/skip") + + rr := httptest.NewRecorder() + a.forwardHandleTraefik(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) } func TestForwardHandleTraefik_Headers(t *testing.T) { @@ -71,7 +89,51 @@ func TestForwardHandleTraefik_Headers(t *testing.T) { assert.Equal(t, loc.String(), "http://test.goauthentik.io/akprox/start") s, _ := a.sessions.Get(req, constants.SeesionName) - assert.Equal(t, s.Values[constants.SessionRedirect], "http://test.goauthentik.io/") + assert.Equal(t, "http://test.goauthentik.io/", s.Values[constants.SessionRedirect]) +} + +func TestForwardHandleTraefik_Claims(t *testing.T) { + a := newTestApplication() + req, _ := http.NewRequest("GET", "/akprox/auth/traefik", nil) + + rr := httptest.NewRecorder() + a.forwardHandleTraefik(rr, req) + + s, _ := a.sessions.Get(req, constants.SeesionName) + s.Values[constants.SessionClaims] = Claims{ + Sub: "foo", + Proxy: ProxyClaims{ + UserAttributes: map[string]interface{}{ + "username": "foo", + "password": "bar", + "additionalHeaders": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + } + err := a.sessions.Save(req, rr, s) + if err != nil { + panic(err) + } + + rr = httptest.NewRecorder() + a.forwardHandleTraefik(rr, req) + + h := rr.Result().Header + + assert.Equal(t, []string{"Basic Zm9vOmJhcg=="}, h["Authorization"]) + assert.Equal(t, []string{"bar"}, h["Foo"]) + assert.Equal(t, []string{""}, h["User-Agent"]) + assert.Equal(t, []string{""}, h["X-Authentik-Email"]) + assert.Equal(t, []string{""}, h["X-Authentik-Groups"]) + assert.Equal(t, []string{""}, h["X-Authentik-Jwt"]) + assert.Equal(t, []string{""}, h["X-Authentik-Meta-App"]) + assert.Equal(t, []string{""}, h["X-Authentik-Meta-Jwks"]) + assert.Equal(t, []string{""}, h["X-Authentik-Meta-Outpost"]) + assert.Equal(t, []string{""}, h["X-Authentik-Name"]) + assert.Equal(t, []string{"foo"}, h["X-Authentik-Uid"]) + assert.Equal(t, []string{""}, h["X-Authentik-Username"]) } func TestForwardHandleNginx_Blank(t *testing.T) { @@ -81,5 +143,74 @@ func TestForwardHandleNginx_Blank(t *testing.T) { rr := httptest.NewRecorder() a.forwardHandleNginx(rr, req) - assert.Equal(t, rr.Code, http.StatusUnauthorized) + assert.Equal(t, http.StatusUnauthorized, rr.Code) +} + +func TestForwardHandleNginx_Skip(t *testing.T) { + a := newTestApplication() + req, _ := http.NewRequest("GET", "/akprox/auth/nginx", nil) + req.Header.Set("X-Original-URI", "http://test.goauthentik.io/skip") + + rr := httptest.NewRecorder() + a.forwardHandleNginx(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) +} + +func TestForwardHandleNginx_Headers(t *testing.T) { + a := newTestApplication() + req, _ := http.NewRequest("GET", "/akprox/auth/nginx", nil) + req.Header.Set("X-Original-URI", "http://test.goauthentik.io") + + rr := httptest.NewRecorder() + a.forwardHandleNginx(rr, req) + + assert.Equal(t, rr.Code, http.StatusUnauthorized) + + s, _ := a.sessions.Get(req, constants.SeesionName) + assert.Equal(t, "http://test.goauthentik.io", s.Values[constants.SessionRedirect]) +} + +func TestForwardHandleNginx_Claims(t *testing.T) { + a := newTestApplication() + req, _ := http.NewRequest("GET", "/akprox/auth/nginx", nil) + + rr := httptest.NewRecorder() + a.forwardHandleNginx(rr, req) + + s, _ := a.sessions.Get(req, constants.SeesionName) + s.Values[constants.SessionClaims] = Claims{ + Sub: "foo", + Proxy: ProxyClaims{ + UserAttributes: map[string]interface{}{ + "username": "foo", + "password": "bar", + "additionalHeaders": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + } + err := a.sessions.Save(req, rr, s) + if err != nil { + panic(err) + } + + rr = httptest.NewRecorder() + a.forwardHandleTraefik(rr, req) + + h := rr.Result().Header + + assert.Equal(t, []string{"Basic Zm9vOmJhcg=="}, h["Authorization"]) + assert.Equal(t, []string{"bar"}, h["Foo"]) + assert.Equal(t, []string{""}, h["User-Agent"]) + assert.Equal(t, []string{""}, h["X-Authentik-Email"]) + assert.Equal(t, []string{""}, h["X-Authentik-Groups"]) + assert.Equal(t, []string{""}, h["X-Authentik-Jwt"]) + assert.Equal(t, []string{""}, h["X-Authentik-Meta-App"]) + assert.Equal(t, []string{""}, h["X-Authentik-Meta-Jwks"]) + assert.Equal(t, []string{""}, h["X-Authentik-Meta-Outpost"]) + assert.Equal(t, []string{""}, h["X-Authentik-Name"]) + assert.Equal(t, []string{"foo"}, h["X-Authentik-Uid"]) + assert.Equal(t, []string{""}, h["X-Authentik-Username"]) } diff --git a/internal/outpost/proxyv2/application/mode_proxy.go b/internal/outpost/proxyv2/application/mode_proxy.go index 7cdb0c6bd..5e74f78ec 100644 --- a/internal/outpost/proxyv2/application/mode_proxy.go +++ b/internal/outpost/proxyv2/application/mode_proxy.go @@ -35,7 +35,7 @@ func (a *Application) configureProxy() error { rp.ModifyResponse = a.proxyModifyResponse a.mux.PathPrefix("/").HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { claims, err := a.getClaims(r) - if claims == nil && a.IsAllowlisted(r) { + if claims == nil && a.IsAllowlisted(r.URL) { a.log.Trace("path can be accessed without authentication") } else if claims == nil && err != nil { a.redirectToStart(rw, r)