providers/proxy: rework redirect mechanism (#8594)

* providers/proxy: rework redirect mechanism Signed-off-by: Jens Langhammer <jens@goauthentik.io> * add session id, don't tie to state in session Signed-off-by: Jens Langhammer <jens@goauthentik.io> * handle state failing to parse Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix Signed-off-by: Jens Langhammer <jens@goauthentik.io> * save session after creating state Signed-off-by: Jens Langhammer <jens@goauthentik.io> * remove debug Signed-off-by: Jens Langhammer <jens@goauthentik.io> * include task expiry in status Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix redirect URL detection Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix tests Signed-off-by: Jens Langhammer <jens@goauthentik.io> --------- Signed-off-by: Jens Langhammer <jens@goauthentik.io>
goauthentik · May 6, 2024 · c45bb8e · c45bb8e
1 parent 3e4fea8
commit c45bb8e
Show file tree

Hide file tree

Showing 16 changed files with 262 additions and 180 deletions.
diff --git a/authentik/events/api/tasks.py b/authentik/events/api/tasks.py
@@ -60,6 +60,8 @@ class Meta:
             "duration",
             "status",
             "messages",
+            "expires",
+            "expiring",
         ]
 
 

diff --git a/go.mod b/go.mod
@@ -10,7 +10,7 @@ require (
 	github.com/go-ldap/ldap/v3 v3.4.8
 	github.com/go-openapi/runtime v0.28.0
 	github.com/go-openapi/strfmt v0.23.0
-	github.com/golang-jwt/jwt v3.2.2+incompatible
+	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/handlers v1.5.2
 	github.com/gorilla/mux v1.8.1

diff --git a/go.sum b/go.sum
@@ -111,8 +111,8 @@ github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+Gr
 github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ=
 github.com/go-openapi/validate v0.24.0 h1:LdfDKwNbpB6Vn40xhTdNZAnfLECL81w+VX3BumrGD58=
 github.com/go-openapi/validate v0.24.0/go.mod h1:iyeX1sEufmv3nPbBdX3ieNviWnOZaJ1+zquzJEf2BAQ=
-github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY=
-github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
+github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
+github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=

diff --git a/internal/outpost/proxyv2/application/application.go b/internal/outpost/proxyv2/application/application.go
@@ -192,7 +192,9 @@ func NewApplication(p api.ProxyOutpostConfig, c *http.Client, server Server) (*A
 		})
 	})
 
-	mux.HandleFunc("/outpost.goauthentik.io/start", a.handleAuthStart)
+	mux.HandleFunc("/outpost.goauthentik.io/start", func(w http.ResponseWriter, r *http.Request) {
+		a.handleAuthStart(w, r, "")
+	})
 	mux.HandleFunc("/outpost.goauthentik.io/callback", a.handleAuthCallback)
 	mux.HandleFunc("/outpost.goauthentik.io/sign_out", a.handleSignOut)
 	switch *p.Mode {

diff --git a/internal/outpost/proxyv2/application/mode_forward.go b/internal/outpost/proxyv2/application/mode_forward.go
@@ -59,19 +59,11 @@ func (a *Application) forwardHandleTraefik(rw http.ResponseWriter, r *http.Reque
 		a.log.Trace("path can be accessed without authentication")
 		return
 	}
-	a.handleAuthStart(rw, r)
 	// 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
 	// X-Forwarded-Uri is only the path, so we need to build the entire URL
-	s, _ := a.sessions.Get(r, a.SessionName())
-	if _, redirectSet := s.Values[constants.SessionRedirect]; !redirectSet {
-		s.Values[constants.SessionRedirect] = fwd.String()
-		err = s.Save(r, rw)
-		if err != nil {
-			a.log.WithError(err).Warning("failed to save session")
-		}
-	}
+	a.handleAuthStart(rw, r, fwd.String())
 }
 
 func (a *Application) forwardHandleCaddy(rw http.ResponseWriter, r *http.Request) {
@@ -110,19 +102,11 @@ func (a *Application) forwardHandleCaddy(rw http.ResponseWriter, r *http.Request
 		a.log.Trace("path can be accessed without authentication")
 		return
 	}
-	a.handleAuthStart(rw, r)
 	// 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
 	// X-Forwarded-Uri is only the path, so we need to build the entire URL
-	s, _ := a.sessions.Get(r, a.SessionName())
-	if _, redirectSet := s.Values[constants.SessionRedirect]; !redirectSet {
-		s.Values[constants.SessionRedirect] = fwd.String()
-		err = s.Save(r, rw)
-		if err != nil {
-			a.log.WithError(err).Warning("failed to save session")
-		}
-	}
+	a.handleAuthStart(rw, r, fwd.String())
 }
 
 func (a *Application) forwardHandleNginx(rw http.ResponseWriter, r *http.Request) {
@@ -185,17 +169,9 @@ func (a *Application) forwardHandleEnvoy(rw http.ResponseWriter, r *http.Request
 		a.log.Trace("path can be accessed without authentication")
 		return
 	}
-	a.handleAuthStart(rw, r)
 	// 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
 	// X-Forwarded-Uri is only the path, so we need to build the entire URL
-	s, _ := a.sessions.Get(r, a.SessionName())
-	if _, redirectSet := s.Values[constants.SessionRedirect]; !redirectSet {
-		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")
-		}
-	}
+	a.handleAuthStart(rw, r, fwd.String())
 }
diff --git a/internal/outpost/proxyv2/application/mode_forward_caddy_test.go b/internal/outpost/proxyv2/application/mode_forward_caddy_test.go
@@ -47,16 +47,14 @@ func TestForwardHandleCaddy_Single_Headers(t *testing.T) {
 	a.forwardHandleCaddy(rr, req)
 
 	assert.Equal(t, http.StatusFound, rr.Code)
-	loc, _ := rr.Result().Location()
-	s, _ := a.sessions.Get(req, a.SessionName())
+	loc, st := a.assertState(t, req, rr)
 	shouldUrl := url.Values{
 		"client_id":     []string{*a.proxyConfig.ClientId},
 		"redirect_uri":  []string{"https://ext.t.goauthentik.io/outpost.goauthentik.io/callback?X-authentik-auth-callback=true"},
 		"response_type": []string{"code"},
-		"state":         []string{s.Values[constants.SessionOAuthState].(string)},
 	}
 	assert.Equal(t, fmt.Sprintf("http://fake-auth.t.goauthentik.io/auth?%s", shouldUrl.Encode()), loc.String())
-	assert.Equal(t, "http://test.goauthentik.io/app", s.Values[constants.SessionRedirect])
+	assert.Equal(t, "http://test.goauthentik.io/app", st.Redirect)
 }
 
 func TestForwardHandleCaddy_Single_Claims(t *testing.T) {
@@ -134,14 +132,12 @@ func TestForwardHandleCaddy_Domain_Header(t *testing.T) {
 	a.forwardHandleCaddy(rr, req)
 
 	assert.Equal(t, http.StatusFound, rr.Code)
-	loc, _ := rr.Result().Location()
-	s, _ := a.sessions.Get(req, a.SessionName())
+	loc, st := a.assertState(t, req, rr)
 	shouldUrl := url.Values{
 		"client_id":     []string{*a.proxyConfig.ClientId},
 		"redirect_uri":  []string{"https://ext.t.goauthentik.io/outpost.goauthentik.io/callback?X-authentik-auth-callback=true"},
 		"response_type": []string{"code"},
-		"state":         []string{s.Values[constants.SessionOAuthState].(string)},
 	}
 	assert.Equal(t, fmt.Sprintf("http://fake-auth.t.goauthentik.io/auth?%s", shouldUrl.Encode()), loc.String())
-	assert.Equal(t, "http://test.goauthentik.io/app", s.Values[constants.SessionRedirect])
+	assert.Equal(t, "http://test.goauthentik.io/app", st.Redirect)
 }
diff --git a/internal/outpost/proxyv2/application/mode_forward_envoy_test.go b/internal/outpost/proxyv2/application/mode_forward_envoy_test.go
@@ -32,16 +32,14 @@ func TestForwardHandleEnvoy_Single_Headers(t *testing.T) {
 	a.forwardHandleEnvoy(rr, req)
 
 	assert.Equal(t, http.StatusFound, rr.Code)
-	loc, _ := rr.Result().Location()
-	s, _ := a.sessions.Get(req, a.SessionName())
+	loc, st := a.assertState(t, req, rr)
 	shouldUrl := url.Values{
 		"client_id":     []string{*a.proxyConfig.ClientId},
 		"redirect_uri":  []string{"https://ext.t.goauthentik.io/outpost.goauthentik.io/callback?X-authentik-auth-callback=true"},
 		"response_type": []string{"code"},
-		"state":         []string{s.Values[constants.SessionOAuthState].(string)},
 	}
 	assert.Equal(t, fmt.Sprintf("http://fake-auth.t.goauthentik.io/auth?%s", shouldUrl.Encode()), loc.String())
-	assert.Equal(t, "http://ext.t.goauthentik.io/app", s.Values[constants.SessionRedirect])
+	assert.Equal(t, "http://ext.t.goauthentik.io/app", st.Redirect)
 }
 
 func TestForwardHandleEnvoy_Single_Claims(t *testing.T) {
@@ -102,15 +100,13 @@ func TestForwardHandleEnvoy_Domain_Header(t *testing.T) {
 	a.forwardHandleEnvoy(rr, req)
 
 	assert.Equal(t, http.StatusFound, rr.Code)
-	loc, _ := rr.Result().Location()
-	s, _ := a.sessions.Get(req, a.SessionName())
+	loc, st := a.assertState(t, req, rr)
 
 	shouldUrl := url.Values{
 		"client_id":     []string{*a.proxyConfig.ClientId},
 		"redirect_uri":  []string{"https://ext.t.goauthentik.io/outpost.goauthentik.io/callback?X-authentik-auth-callback=true"},
 		"response_type": []string{"code"},
-		"state":         []string{s.Values[constants.SessionOAuthState].(string)},
 	}
 	assert.Equal(t, fmt.Sprintf("http://fake-auth.t.goauthentik.io/auth?%s", shouldUrl.Encode()), loc.String())
-	assert.Equal(t, "http://test.goauthentik.io/app", s.Values[constants.SessionRedirect])
+	assert.Equal(t, "http://test.goauthentik.io/app", st.Redirect)
 }
diff --git a/internal/outpost/proxyv2/application/mode_forward_traefik_test.go b/internal/outpost/proxyv2/application/mode_forward_traefik_test.go
@@ -47,16 +47,14 @@ func TestForwardHandleTraefik_Single_Headers(t *testing.T) {
 	a.forwardHandleTraefik(rr, req)
 
 	assert.Equal(t, http.StatusFound, rr.Code)
-	loc, _ := rr.Result().Location()
-	s, _ := a.sessions.Get(req, a.SessionName())
+	loc, st := a.assertState(t, req, rr)
 	shouldUrl := url.Values{
 		"client_id":     []string{*a.proxyConfig.ClientId},
 		"redirect_uri":  []string{"https://ext.t.goauthentik.io/outpost.goauthentik.io/callback?X-authentik-auth-callback=true"},
 		"response_type": []string{"code"},
-		"state":         []string{s.Values[constants.SessionOAuthState].(string)},
 	}
 	assert.Equal(t, fmt.Sprintf("http://fake-auth.t.goauthentik.io/auth?%s", shouldUrl.Encode()), loc.String())
-	assert.Equal(t, "http://test.goauthentik.io/app", s.Values[constants.SessionRedirect])
+	assert.Equal(t, "http://test.goauthentik.io/app", st.Redirect)
 }
 
 func TestForwardHandleTraefik_Single_Claims(t *testing.T) {
@@ -134,14 +132,12 @@ func TestForwardHandleTraefik_Domain_Header(t *testing.T) {
 	a.forwardHandleTraefik(rr, req)
 
 	assert.Equal(t, http.StatusFound, rr.Code)
-	loc, _ := rr.Result().Location()
-	s, _ := a.sessions.Get(req, a.SessionName())
+	loc, st := a.assertState(t, req, rr)
 	shouldUrl := url.Values{
 		"client_id":     []string{*a.proxyConfig.ClientId},
 		"redirect_uri":  []string{"https://ext.t.goauthentik.io/outpost.goauthentik.io/callback?X-authentik-auth-callback=true"},
 		"response_type": []string{"code"},
-		"state":         []string{s.Values[constants.SessionOAuthState].(string)},
 	}
 	assert.Equal(t, fmt.Sprintf("http://fake-auth.t.goauthentik.io/auth?%s", shouldUrl.Encode()), loc.String())
-	assert.Equal(t, "http://test.goauthentik.io/app", s.Values[constants.SessionRedirect])
+	assert.Equal(t, "http://test.goauthentik.io/app", st.Redirect)
 }
diff --git a/internal/outpost/proxyv2/application/oauth.go b/internal/outpost/proxyv2/application/oauth.go
@@ -1,13 +1,10 @@
 package application
 
 import (
-	"encoding/base64"
 	"net/http"
 	"net/url"
 	"strings"
-	"time"
 
-	"github.com/gorilla/securecookie"
 	"goauthentik.io/api/v3"
 	"goauthentik.io/internal/outpost/proxyv2/constants"
 )
@@ -48,69 +45,59 @@ func (a *Application) checkRedirectParam(r *http.Request) (string, bool) {
 	return u.String(), true
 }
 
-func (a *Application) handleAuthStart(rw http.ResponseWriter, r *http.Request) {
-	newState := base64.RawURLEncoding.EncodeToString(securecookie.GenerateRandomKey(32))
-	s, _ := a.sessions.Get(r, a.SessionName())
-	// Check if we already have a state in the session,
-	// and if we do we don't do anything here
-	currentState, ok := s.Values[constants.SessionOAuthState].(string)
-	if ok {
-		claims, err := a.checkAuth(rw, r)
-		if err != nil && claims != nil {
-			a.log.Trace("auth start request with existing authenticated session")
-			a.redirect(rw, r)
-			return
-		}
-		a.log.Trace("session already has state, sending redirect to current state")
-		http.Redirect(rw, r, a.oauthConfig.AuthCodeURL(currentState), http.StatusFound)
+func (a *Application) handleAuthStart(rw http.ResponseWriter, r *http.Request, fwd string) {
+	state, err := a.createState(r, fwd)
+	if err != nil {
+		a.log.WithError(err).Warning("failed to create state")
 		return
 	}
-	rd, ok := a.checkRedirectParam(r)
-	if ok {
-		s.Values[constants.SessionRedirect] = rd
-		a.log.WithField("rd", rd).Trace("Setting redirect")
-	}
-	s.Values[constants.SessionOAuthState] = newState
-	err := s.Save(r, rw)
+	s, _ := a.sessions.Get(r, a.SessionName())
+	err = s.Save(r, rw)
 	if err != nil {
 		a.log.WithError(err).Warning("failed to save session")
 	}
-	http.Redirect(rw, r, a.oauthConfig.AuthCodeURL(newState), http.StatusFound)
+	http.Redirect(rw, r, a.oauthConfig.AuthCodeURL(state), http.StatusFound)
 }
 
-func (a *Application) handleAuthCallback(rw http.ResponseWriter, r *http.Request) {
+func (a *Application) redirectToStart(rw http.ResponseWriter, r *http.Request) {
 	s, err := a.sessions.Get(r, a.SessionName())
 	if err != nil {
-		a.log.WithError(err).Trace("failed to get session")
+		a.log.WithError(err).Warning("failed to decode session")
 	}
-	state, ok := s.Values[constants.SessionOAuthState]
-	if !ok {
-		a.log.Warning("No state saved in session")
-		a.redirect(rw, r)
-		return
+	if r.Header.Get(constants.HeaderAuthorization) != "" && *a.proxyConfig.InterceptHeaderAuth {
+		rw.WriteHeader(401)
+		er := a.errorTemplates.Execute(rw, ErrorPageData{
+			Title:       "Unauthenticated",
+			Message:     "Due to 'Receive header authentication' being set, no redirect is performed.",
+			ProxyPrefix: "/outpost.goauthentik.io",
+		})
+		if er != nil {
+			http.Error(rw, "Internal Server Error", http.StatusInternalServerError)
+		}
 	}
-	claims, err := a.redeemCallback(state.(string), r.URL, r.Context())
-	if err != nil {
-		a.log.WithError(err).Warning("failed to redeem code")
-		rw.WriteHeader(400)
-		// To prevent the user from just refreshing and cause more errors, delete
-		// the state from the session
-		delete(s.Values, constants.SessionOAuthState)
-		err := s.Save(r, rw)
+
+	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 = a.proxyConfig.ExternalHost
+		}
+	}
+	if _, redirectSet := s.Values[constants.SessionRedirect]; !redirectSet {
+		s.Values[constants.SessionRedirect] = redirectUrl
+		err = s.Save(r, rw)
 		if err != nil {
-			a.log.WithError(err).Warning("failed to save session")
-			rw.WriteHeader(400)
-			return
+			a.log.WithError(err).Warning("failed to save session before redirect")
 		}
-		return
 	}
-	s.Options.MaxAge = int(time.Until(time.Unix(int64(claims.Exp), 0)).Seconds())
-	s.Values[constants.SessionClaims] = &claims
-	err = s.Save(r, rw)
-	if err != nil {
-		a.log.WithError(err).Warning("failed to save session")
-		rw.WriteHeader(400)
-		return
+
+	urlArgs := url.Values{
+		redirectParam: []string{redirectUrl},
 	}
-	a.redirect(rw, r)
+	authUrl := urlJoin(a.proxyConfig.ExternalHost, "/outpost.goauthentik.io/start")
+	http.Redirect(rw, r, authUrl+"?"+urlArgs.Encode(), http.StatusFound)
 }
diff --git a/internal/outpost/proxyv2/application/oauth_callback.go b/internal/outpost/proxyv2/application/oauth_callback.go
@@ -3,22 +3,43 @@ package application
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"net/url"
+	"time"
 
-	log "github.com/sirupsen/logrus"
+	"goauthentik.io/internal/outpost/proxyv2/constants"
 	"golang.org/x/oauth2"
 )
 
-func (a *Application) redeemCallback(savedState string, u *url.URL, c context.Context) (*Claims, error) {
-	state := u.Query().Get("state")
-	a.log.WithFields(log.Fields{
-		"states":   savedState,
-		"expected": state,
-	}).Trace("tracing states")
-	if savedState != state {
-		return nil, fmt.Errorf("invalid state")
+func (a *Application) handleAuthCallback(rw http.ResponseWriter, r *http.Request) {
+	state := a.stateFromRequest(r)
+	if state == nil {
+		a.log.Warning("invalid state")
+		a.redirect(rw, r)
+		return
 	}
+	claims, err := a.redeemCallback(r.URL, r.Context())
+	if err != nil {
+		a.log.WithError(err).Warning("failed to redeem code")
+		a.redirect(rw, r)
+		return
+	}
+	s, err := a.sessions.Get(r, a.SessionName())
+	if err != nil {
+		a.log.WithError(err).Trace("failed to get session")
+	}
+	s.Options.MaxAge = int(time.Until(time.Unix(int64(claims.Exp), 0)).Seconds())
+	s.Values[constants.SessionClaims] = &claims
+	err = s.Save(r, rw)
+	if err != nil {
+		a.log.WithError(err).Warning("failed to save session")
+		rw.WriteHeader(400)
+		return
+	}
+	a.redirect(rw, r)
+}
 
+func (a *Application) redeemCallback(u *url.URL, c context.Context) (*Claims, error) {
 	code := u.Query().Get("code")
 	if code == "" {
 		return nil, fmt.Errorf("blank code")