From c45bb8e985eb80866e2a7f1c2a605e99542e0a67 Mon Sep 17 00:00:00 2001 From: Jens L Date: Mon, 6 May 2024 03:07:08 +0200 Subject: [PATCH] providers/proxy: rework redirect mechanism (#8594) * providers/proxy: rework redirect mechanism Signed-off-by: Jens Langhammer * add session id, don't tie to state in session Signed-off-by: Jens Langhammer * handle state failing to parse Signed-off-by: Jens Langhammer * fix Signed-off-by: Jens Langhammer * save session after creating state Signed-off-by: Jens Langhammer * remove debug Signed-off-by: Jens Langhammer * include task expiry in status Signed-off-by: Jens Langhammer * fix redirect URL detection Signed-off-by: Jens Langhammer * fix tests Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/events/api/tasks.py | 2 + go.mod | 2 +- go.sum | 4 +- .../proxyv2/application/application.go | 4 +- .../proxyv2/application/mode_forward.go | 30 +---- .../application/mode_forward_caddy_test.go | 12 +- .../application/mode_forward_envoy_test.go | 12 +- .../application/mode_forward_traefik_test.go | 12 +- internal/outpost/proxyv2/application/oauth.go | 113 ++++++++---------- .../proxyv2/application/oauth_callback.go | 39 ++++-- .../proxyv2/application/oauth_state.go | 95 +++++++++++++++ internal/outpost/proxyv2/application/test.go | 27 ++++- internal/outpost/proxyv2/application/utils.go | 66 ++-------- internal/outpost/proxyv2/hs256/hs256.go | 17 ++- schema.yml | 6 + .../admin/system-tasks/SystemTaskListPage.ts | 21 ++++ 16 files changed, 272 insertions(+), 190 deletions(-) create mode 100644 internal/outpost/proxyv2/application/oauth_state.go diff --git a/authentik/events/api/tasks.py b/authentik/events/api/tasks.py index 09cec8457e..529464ad5f 100644 --- a/authentik/events/api/tasks.py +++ b/authentik/events/api/tasks.py @@ -60,6 +60,8 @@ class SystemTaskSerializer(ModelSerializer): "duration", "status", "messages", + "expires", + "expiring", ] diff --git a/go.mod b/go.mod index 66d33856ef..ecd1b6107a 100644 --- 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 index 461f69e6a5..f4176ff391 100644 --- 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 index 3f3fcaa3b6..0a308280da 100644 --- 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 index 47b5dd47b1..4a2c987d9e 100644 --- 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 index c2994f7dac..6e0e860e0e 100644 --- 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 index a8b2abfa00..dbb8b5412f 100644 --- 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 index 227dc1cb52..a6b8963a83 100644 --- 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 index 9f9c18d8f2..f2dc93319d 100644 --- 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)) +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 + } 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) - 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) - if err != nil { - a.log.WithError(err).Warning("failed to save session") - } - http.Redirect(rw, r, a.oauthConfig.AuthCodeURL(newState), http.StatusFound) -} - -func (a *Application) handleAuthCallback(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") - } - state, ok := s.Values[constants.SessionOAuthState] - if !ok { - a.log.Warning("No state saved in session") - a.redirect(rw, r) - return - } - 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) - if err != nil { - a.log.WithError(err).Warning("failed to save session") - rw.WriteHeader(400) - return - } - 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 } - a.redirect(rw, r) + http.Redirect(rw, r, a.oauthConfig.AuthCodeURL(state), http.StatusFound) +} + +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).Warning("failed to decode session") + } + 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) + } + } + + 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 before redirect") + } + } + + urlArgs := url.Values{ + redirectParam: []string{redirectUrl}, + } + 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 index 13f28e67a3..ac64626378 100644 --- 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") diff --git a/internal/outpost/proxyv2/application/oauth_state.go b/internal/outpost/proxyv2/application/oauth_state.go new file mode 100644 index 0000000000..798e658396 --- /dev/null +++ b/internal/outpost/proxyv2/application/oauth_state.go @@ -0,0 +1,95 @@ +package application + +import ( + "encoding/base32" + "encoding/base64" + "fmt" + "net/http" + + "github.com/golang-jwt/jwt/v5" + "github.com/gorilla/securecookie" + "github.com/mitchellh/mapstructure" +) + +type OAuthState struct { + Issuer string `json:"iss" mapstructure:"iss"` + SessionID string `json:"sid" mapstructure:"sid"` + State string `json:"state" mapstructure:"state"` + Redirect string `json:"redirect" mapstructure:"redirect"` +} + +func (oas *OAuthState) GetExpirationTime() (*jwt.NumericDate, error) { return nil, nil } +func (oas *OAuthState) GetIssuedAt() (*jwt.NumericDate, error) { return nil, nil } +func (oas *OAuthState) GetNotBefore() (*jwt.NumericDate, error) { return nil, nil } +func (oas *OAuthState) GetIssuer() (string, error) { return oas.Issuer, nil } +func (oas *OAuthState) GetSubject() (string, error) { return oas.State, nil } +func (oas *OAuthState) GetAudience() (jwt.ClaimStrings, error) { return nil, nil } + +var base32RawStdEncoding = base32.StdEncoding.WithPadding(base32.NoPadding) + +func (a *Application) createState(r *http.Request, fwd string) (string, error) { + s, _ := a.sessions.Get(r, a.SessionName()) + if s.ID == "" { + // Ensure session has an ID + s.ID = base32RawStdEncoding.EncodeToString(securecookie.GenerateRandomKey(32)) + } + st := &OAuthState{ + Issuer: fmt.Sprintf("goauthentik.io/outpost/%s", a.proxyConfig.GetClientId()), + State: base64.RawURLEncoding.EncodeToString(securecookie.GenerateRandomKey(32)), + SessionID: s.ID, + Redirect: fwd, + } + if fwd == "" { + // This should only really be hit for nginx forward_auth + // as for that the auth start redirect URL is generated by the + // reverse proxy, and as such we won't have a request we just + // denied to reference for final URL + rd, ok := a.checkRedirectParam(r) + if ok { + a.log.WithField("rd", rd).Trace("Setting redirect") + st.Redirect = rd + } + } + token := jwt.NewWithClaims(jwt.SigningMethodHS256, st) + tokenString, err := token.SignedString([]byte(a.proxyConfig.GetCookieSecret())) + if err != nil { + return "", err + } + return tokenString, nil +} + +func (a *Application) stateFromRequest(r *http.Request) *OAuthState { + stateJwt := r.URL.Query().Get("state") + token, err := jwt.Parse(stateJwt, func(token *jwt.Token) (interface{}, error) { + // Don't forget to validate the alg is what you expect: + if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + return []byte(a.proxyConfig.GetCookieSecret()), nil + }) + if err != nil { + a.log.WithError(err).Warning("failed to parse state jwt") + return nil + } + iss, err := token.Claims.GetIssuer() + if err != nil { + a.log.WithError(err).Warning("state jwt without issuer") + return nil + } + if iss != fmt.Sprintf("goauthentik.io/outpost/%s", a.proxyConfig.GetClientId()) { + a.log.WithField("issuer", iss).Warning("invalid state jwt issuer") + return nil + } + claims := &OAuthState{} + err = mapstructure.Decode(token.Claims, &claims) + if err != nil { + a.log.WithError(err).Warning("failed to mapdecode") + return nil + } + s, _ := a.sessions.Get(r, a.SessionName()) + if claims.SessionID != s.ID { + a.log.WithField("is", claims.SessionID).WithField("should", s.ID).Warning("mismatched session ID") + return nil + } + return claims +} diff --git a/internal/outpost/proxyv2/application/test.go b/internal/outpost/proxyv2/application/test.go index 827bf79703..f7970776c4 100644 --- a/internal/outpost/proxyv2/application/test.go +++ b/internal/outpost/proxyv2/application/test.go @@ -2,6 +2,9 @@ package application import ( "net/http" + "net/http/httptest" + "net/url" + "testing" "goauthentik.io/api/v3" "goauthentik.io/internal/outpost/ak" @@ -45,11 +48,11 @@ func newTestApplication() *Application { Name: ak.TestSecret(), ClientId: api.PtrString(ak.TestSecret()), ClientSecret: api.PtrString(ak.TestSecret()), + CookieDomain: api.PtrString(""), CookieSecret: api.PtrString(ak.TestSecret()), ExternalHost: "https://ext.t.goauthentik.io", InternalHost: api.PtrString("http://backend"), InternalHostSslValidation: api.PtrBool(true), - CookieDomain: api.PtrString(""), Mode: api.PROXYMODE_FORWARD_SINGLE.Ptr(), SkipPathRegex: api.PtrString("/skip.*"), BasicAuthEnabled: api.PtrBool(true), @@ -67,3 +70,25 @@ func newTestApplication() *Application { ts.apps = append(ts.apps, a) return a } + +func (a *Application) assertState(t *testing.T, req *http.Request, response *httptest.ResponseRecorder) (*url.URL, *OAuthState) { + loc, _ := response.Result().Location() + q := loc.Query() + state := q.Get("state") + a.log.WithField("actual", state).Warning("actual state") + // modify request to set state so we can parse it + nr := req.Clone(req.Context()) + nrq := nr.URL.Query() + nrq.Set("state", state) + nr.URL.RawQuery = nrq.Encode() + // parse state + parsed := a.stateFromRequest(nr) + if parsed == nil { + panic("Could not parse state") + } + + // Remove state from URL + q.Del("state") + loc.RawQuery = q.Encode() + return loc, parsed +} diff --git a/internal/outpost/proxyv2/application/utils.go b/internal/outpost/proxyv2/application/utils.go index f8a2d6575f..248d7ddf09 100644 --- a/internal/outpost/proxyv2/application/utils.go +++ b/internal/outpost/proxyv2/application/utils.go @@ -4,10 +4,6 @@ import ( "net/http" "net/url" "strconv" - "strings" - - "goauthentik.io/api/v3" - "goauthentik.io/internal/outpost/proxyv2/constants" ) func urlJoin(originalUrl string, newPath string) string { @@ -18,62 +14,18 @@ func urlJoin(originalUrl string, newPath string) string { return u } -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).Warning("failed to decode session") - } - 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) - } - } - - 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 before redirect") - } - } - - urlArgs := url.Values{ - redirectParam: []string{redirectUrl}, - } - authUrl := urlJoin(a.proxyConfig.ExternalHost, "/outpost.goauthentik.io/start") - http.Redirect(rw, r, authUrl+"?"+urlArgs.Encode(), http.StatusFound) -} - func (a *Application) redirect(rw http.ResponseWriter, r *http.Request) { - redirect := a.proxyConfig.ExternalHost - s, _ := a.sessions.Get(r, a.SessionName()) - redirectR, ok := s.Values[constants.SessionRedirect] - if ok { - redirect = redirectR.(string) + fallbackRedirect := a.proxyConfig.ExternalHost + state := a.stateFromRequest(r) + if state == nil { + rw.WriteHeader(http.StatusBadRequest) + return } - rd, ok := a.checkRedirectParam(r) - if ok { - redirect = rd + if state.Redirect == "" { + state.Redirect = fallbackRedirect } - a.log.WithField("redirect", redirect).Trace("final redirect") - http.Redirect(rw, r, redirect, http.StatusFound) + a.log.WithField("redirect", state.Redirect).Trace("final redirect") + http.Redirect(rw, r, state.Redirect, http.StatusFound) } // toString Generic to string function, currently supports actual strings and integers diff --git a/internal/outpost/proxyv2/hs256/hs256.go b/internal/outpost/proxyv2/hs256/hs256.go index 3df8c73e82..29ace0995d 100644 --- a/internal/outpost/proxyv2/hs256/hs256.go +++ b/internal/outpost/proxyv2/hs256/hs256.go @@ -3,9 +3,10 @@ package hs256 import ( "context" "encoding/base64" + "fmt" "strings" - "github.com/golang-jwt/jwt" + "github.com/golang-jwt/jwt/v5" ) type KeySet struct { @@ -15,17 +16,23 @@ type KeySet struct { func NewKeySet(secret string) *KeySet { return &KeySet{ - m: jwt.GetSigningMethod("HS256"), + m: jwt.SigningMethodHS256, secret: secret, } } -func (ks *KeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) { - parts := strings.Split(jwt, ".") - err := ks.m.Verify(strings.Join(parts[0:2], "."), parts[2], []byte(ks.secret)) +func (ks *KeySet) VerifySignature(ctx context.Context, rawJWT string) ([]byte, error) { + _, err := jwt.Parse(rawJWT, func(token *jwt.Token) (interface{}, error) { + // Don't forget to validate the alg is what you expect: + if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + return []byte(ks.secret), nil + }) if err != nil { return nil, err } + parts := strings.Split(rawJWT, ".") payload, err := base64.RawURLEncoding.DecodeString(parts[1]) return payload, err } diff --git a/schema.yml b/schema.yml index a78de9ceed..f90d8ce4ef 100644 --- a/schema.yml +++ b/schema.yml @@ -44167,6 +44167,12 @@ components: type: array items: $ref: '#/components/schemas/LogEvent' + expires: + type: string + format: date-time + nullable: true + expiring: + type: boolean required: - description - duration diff --git a/web/src/admin/system-tasks/SystemTaskListPage.ts b/web/src/admin/system-tasks/SystemTaskListPage.ts index e05a59c249..9947bc0d7c 100644 --- a/web/src/admin/system-tasks/SystemTaskListPage.ts +++ b/web/src/admin/system-tasks/SystemTaskListPage.ts @@ -90,6 +90,27 @@ export class SystemTaskListPage extends TablePage { +
+
+ ${msg("Expiry")} +
+
+
+ ${item.expiring + ? html` + + ${getRelativeTime(item.expires || new Date())} + + ` + : msg("-")} +
+
+
${msg("Messages")}