From 132bf1e642232fa1429347282921ad7069fe83d4 Mon Sep 17 00:00:00 2001 From: William Petit Date: Fri, 24 May 2024 16:40:19 +0200 Subject: [PATCH] feat(authn-oidc): allow for dynamic post-logout redirection --- .../layer/authn/oidc/authenticator.go | 123 ++++++++++++------ .../proxy/director/layer/authn/oidc/client.go | 22 +--- .../layer/authn/oidc/layer-options.json | 36 +++-- .../layer/authn/oidc/layer_options.go | 28 ++-- 4 files changed, 128 insertions(+), 81 deletions(-) diff --git a/internal/proxy/director/layer/authn/oidc/authenticator.go b/internal/proxy/director/layer/authn/oidc/authenticator.go index 3b6994c..c060b8a 100644 --- a/internal/proxy/director/layer/authn/oidc/authenticator.go +++ b/internal/proxy/director/layer/authn/oidc/authenticator.go @@ -7,6 +7,8 @@ import ( "fmt" "net/http" "net/url" + "slices" + "strings" "text/template" "time" @@ -35,9 +37,7 @@ func (a *Authenticator) PreAuthentication(w http.ResponseWriter, r *http.Request return errors.WithStack(err) } - baseURL := originalURL.Scheme + "://" + originalURL.Host - - options, err := fromStoreOptions(layer.Options, baseURL) + options, err := fromStoreOptions(layer.Options) if err != nil { return errors.WithStack(err) } @@ -47,7 +47,7 @@ func (a *Authenticator) PreAuthentication(w http.ResponseWriter, r *http.Request logger.Error(ctx, "could not retrieve session", logger.E(errors.WithStack(err))) } - loginCallbackURL, err := a.getLoginCallbackURL(layer.Proxy, layer.Name, options) + loginCallbackURL, err := a.getLoginCallbackURL(originalURL, layer.Proxy, layer.Name, options) if err != nil { return errors.WithStack(err) } @@ -57,20 +57,20 @@ func (a *Authenticator) PreAuthentication(w http.ResponseWriter, r *http.Request return errors.WithStack(err) } - loginCallbackURLPattern, err := a.templatize(options.OIDC.MatchLoginCallbackURL, layer.Proxy, layer.Name) + loginCallbackPathPattern, err := a.templatize(options.OIDC.MatchLoginCallbackPath, layer.Proxy, layer.Name) if err != nil { return errors.WithStack(err) } - logoutURLPattern, err := a.templatize(options.OIDC.MatchLogoutURL, layer.Proxy, layer.Name) + logoutPathPattern, err := a.templatize(options.OIDC.MatchLogoutPath, layer.Proxy, layer.Name) if err != nil { return errors.WithStack(err) } - logger.Debug(ctx, "checking url", logger.F("loginCallbackURLPattern", loginCallbackURLPattern), logger.F("logoutURLPattern", logoutURLPattern)) + logger.Debug(ctx, "checking url", logger.F("loginCallbackPathPattern", loginCallbackPathPattern), logger.F("logoutPathPattern", logoutPathPattern)) switch { - case wildcard.Match(originalURL.String(), loginCallbackURLPattern): + case wildcard.Match(originalURL.Path, loginCallbackPathPattern): if err := client.HandleCallback(w, r, sess); err != nil { return errors.WithStack(err) } @@ -80,10 +80,23 @@ func (a *Authenticator) PreAuthentication(w http.ResponseWriter, r *http.Request metricLabelProxy: string(layer.Proxy), }).Add(1) - case wildcard.Match(originalURL.String(), logoutURLPattern): - postLogoutRedirectURL := options.OIDC.PostLogoutRedirectURL - if options.OIDC.PostLogoutRedirectURL == "" { - postLogoutRedirectURL = originalURL.Scheme + "://" + originalURL.Host + case wildcard.Match(originalURL.Path, logoutPathPattern): + postLogoutRedirectURL := r.URL.Query().Get("redirect") + + if postLogoutRedirectURL != "" { + isAuthorized := slices.Contains(options.OIDC.PostLogoutRedirectURLs, postLogoutRedirectURL) + if !isAuthorized { + http.Error(w, "unauthorized post-logout redirect", http.StatusBadRequest) + return errors.WithStack(authn.ErrSkipRequest) + } + } + + if postLogoutRedirectURL == "" { + if options.OIDC.PublicBaseURL != "" { + postLogoutRedirectURL = options.OIDC.PublicBaseURL + } else { + postLogoutRedirectURL = originalURL.Scheme + "://" + originalURL.Host + } } if err := client.HandleLogout(w, r, sess, postLogoutRedirectURL); err != nil { @@ -103,14 +116,7 @@ func (a *Authenticator) PreAuthentication(w http.ResponseWriter, r *http.Request func (a *Authenticator) Authenticate(w http.ResponseWriter, r *http.Request, layer *store.Layer) (*authn.User, error) { ctx := r.Context() - originalURL, err := director.OriginalURL(ctx) - if err != nil { - return nil, errors.WithStack(err) - } - - baseURL := originalURL.Scheme + "://" + originalURL.Host - - options, err := fromStoreOptions(layer.Options, baseURL) + options, err := fromStoreOptions(layer.Options) if err != nil { return nil, errors.WithStack(err) } @@ -142,7 +148,12 @@ func (a *Authenticator) Authenticate(w http.ResponseWriter, r *http.Request, lay sess.Options.SameSite = http.SameSiteDefaultMode } - loginCallbackURL, err := a.getLoginCallbackURL(layer.Proxy, layer.Name, options) + originalURL, err := director.OriginalURL(ctx) + if err != nil { + return nil, errors.WithStack(err) + } + + loginCallbackURL, err := a.getLoginCallbackURL(originalURL, layer.Proxy, layer.Name, options) if err != nil { return nil, errors.WithStack(err) } @@ -152,7 +163,12 @@ func (a *Authenticator) Authenticate(w http.ResponseWriter, r *http.Request, lay return nil, errors.WithStack(err) } - idToken, err := client.Authenticate(w, r, sess) + postLoginRedirectURL, err := a.mergeURL(originalURL, originalURL.Path, options.OIDC.PublicBaseURL, true) + if err != nil { + return nil, errors.WithStack(err) + } + + idToken, err := client.Authenticate(w, r, sess, postLoginRedirectURL.String()) if err != nil { if errors.Is(err, ErrLoginRequired) { metricLoginRequestsTotal.With(prometheus.Labels{ @@ -166,7 +182,7 @@ func (a *Authenticator) Authenticate(w http.ResponseWriter, r *http.Request, lay return nil, errors.WithStack(err) } - user, err := a.toUser(idToken, layer.Proxy, layer.Name, options, sess) + user, err := a.toUser(originalURL, idToken, layer.Proxy, layer.Name, options, sess) if err != nil { return nil, errors.WithStack(err) } @@ -224,7 +240,7 @@ func (c claims) AsAttrs() map[string]any { return attrs } -func (a *Authenticator) toUser(idToken *oidc.IDToken, proxyName store.ProxyName, layerName store.LayerName, options *LayerOptions, sess *sessions.Session) (*authn.User, error) { +func (a *Authenticator) toUser(originalURL *url.URL, idToken *oidc.IDToken, proxyName store.ProxyName, layerName store.LayerName, options *LayerOptions, sess *sessions.Session) (*authn.User, error) { var claims claims if err := idToken.Claims(&claims); err != nil { @@ -237,7 +253,7 @@ func (a *Authenticator) toUser(idToken *oidc.IDToken, proxyName store.ProxyName, attrs := claims.AsAttrs() - logoutURL, err := a.getLogoutURL(proxyName, layerName, options) + logoutURL, err := a.getLogoutURL(originalURL, proxyName, layerName, options) if err != nil { return nil, errors.WithStack(err) } @@ -261,36 +277,65 @@ func (a *Authenticator) toUser(idToken *oidc.IDToken, proxyName store.ProxyName, return user, nil } -func (a *Authenticator) getLoginCallbackURL(proxyName store.ProxyName, layerName store.LayerName, options *LayerOptions) (*url.URL, error) { - url, err := a.generateURL(options.OIDC.LoginCallbackURL, proxyName, layerName) +func (a *Authenticator) getLoginCallbackURL(originalURL *url.URL, proxyName store.ProxyName, layerName store.LayerName, options *LayerOptions) (*url.URL, error) { + path, err := a.templatize(options.OIDC.LoginCallbackPath, proxyName, layerName) if err != nil { return nil, errors.WithStack(err) } - return url, nil + merged, err := a.mergeURL(originalURL, path, options.OIDC.PublicBaseURL, false) + if err != nil { + return nil, errors.WithStack(err) + } + + return merged, nil } -func (a *Authenticator) getLogoutURL(proxyName store.ProxyName, layerName store.LayerName, options *LayerOptions) (*url.URL, error) { - url, err := a.generateURL(options.OIDC.LogoutURL, proxyName, layerName) +func (a *Authenticator) getLogoutURL(originalURL *url.URL, proxyName store.ProxyName, layerName store.LayerName, options *LayerOptions) (*url.URL, error) { + path, err := a.templatize(options.OIDC.LogoutPath, proxyName, layerName) if err != nil { return nil, errors.WithStack(err) } - return url, nil + merged, err := a.mergeURL(originalURL, path, options.OIDC.PublicBaseURL, true) + if err != nil { + return nil, errors.WithStack(err) + } + + return merged, nil } -func (a *Authenticator) generateURL(rawURLTemplate string, proxyName store.ProxyName, layerName store.LayerName) (*url.URL, error) { - rawURL, err := a.templatize(rawURLTemplate, proxyName, layerName) - if err != nil { - return nil, errors.WithStack(err) +func (a *Authenticator) mergeURL(base *url.URL, path string, overlay string, withQuery bool) (*url.URL, error) { + merged := &url.URL{ + Scheme: base.Scheme, + Host: base.Host, + Path: path, } - url, err := url.Parse(rawURL) - if err != nil { - return nil, errors.WithStack(err) + if withQuery { + merged.RawQuery = base.RawQuery } - return url, nil + if overlay != "" { + overlayURL, err := url.Parse(overlay) + if err != nil { + return nil, errors.WithStack(err) + } + + merged.Scheme = overlayURL.Scheme + merged.Host = overlayURL.Host + merged.Path = overlayURL.Path + strings.TrimPrefix(path, "/") + + for key, values := range overlayURL.Query() { + query := merged.Query() + for _, v := range values { + query.Add(key, v) + } + merged.RawQuery = query.Encode() + } + } + + return merged, nil } func (a *Authenticator) templatize(rawTemplate string, proxyName store.ProxyName, layerName store.LayerName) (string, error) { diff --git a/internal/proxy/director/layer/authn/oidc/client.go b/internal/proxy/director/layer/authn/oidc/client.go index 947a56b..fde7db6 100644 --- a/internal/proxy/director/layer/authn/oidc/client.go +++ b/internal/proxy/director/layer/authn/oidc/client.go @@ -2,12 +2,10 @@ package oidc import ( "bytes" - "fmt" "net/http" "net/url" "strings" - "forge.cadoles.com/cadoles/bouncer/internal/proxy/director" "github.com/coreos/go-oidc/v3/oidc" "github.com/dchest/uniuri" "github.com/gorilla/sessions" @@ -46,12 +44,12 @@ func (c *Client) Provider() *oidc.Provider { return c.provider } -func (c *Client) Authenticate(w http.ResponseWriter, r *http.Request, sess *sessions.Session) (*oidc.IDToken, error) { +func (c *Client) Authenticate(w http.ResponseWriter, r *http.Request, sess *sessions.Session, postLoginRedirectURL string) (*oidc.IDToken, error) { idToken, err := c.getIDToken(r, sess) if err != nil { - logger.Error(r.Context(), "could not retrieve idtoken", logger.E(errors.WithStack(err))) + logger.Warn(r.Context(), "could not retrieve idtoken", logger.E(errors.WithStack(err))) - c.login(w, r, sess) + c.login(w, r, sess, postLoginRedirectURL) return nil, errors.WithStack(ErrLoginRequired) } @@ -59,23 +57,15 @@ func (c *Client) Authenticate(w http.ResponseWriter, r *http.Request, sess *sess return idToken, nil } -func (c *Client) login(w http.ResponseWriter, r *http.Request, sess *sessions.Session) { +func (c *Client) login(w http.ResponseWriter, r *http.Request, sess *sessions.Session, postLoginRedirectURL string) { ctx := r.Context() state := uniuri.New() nonce := uniuri.New() - originalURL, err := director.OriginalURL(ctx) - if err != nil { - logger.Error(ctx, "could not retrieve original url", logger.E(errors.WithStack(err))) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - - return - } - sess.Values[sessionKeyLoginState] = state sess.Values[sessionKeyLoginNonce] = nonce - sess.Values[sessionKeyPostLoginRedirectURL] = fmt.Sprintf("%s?%s", originalURL.Path, originalURL.Query().Encode()) + sess.Values[sessionKeyPostLoginRedirectURL] = postLoginRedirectURL if err := sess.Save(r, w); err != nil { logger.Error(ctx, "could not save session", logger.E(errors.WithStack(err))) @@ -249,7 +239,7 @@ func (c *Client) validate(r *http.Request, sess *sessions.Session) (*oauth2.Toke func (c *Client) getRawIDToken(sess *sessions.Session) (string, error) { rawIDToken, ok := sess.Values[sessionKeyIDToken].(string) if !ok || rawIDToken == "" { - return "", errors.New("invalid id token") + return "", errors.New("id token not found") } return rawIDToken, nil diff --git a/internal/proxy/director/layer/authn/oidc/layer-options.json b/internal/proxy/director/layer/authn/oidc/layer-options.json index 6749fb3..e49ecbe 100644 --- a/internal/proxy/director/layer/authn/oidc/layer-options.json +++ b/internal/proxy/director/layer/authn/oidc/layer-options.json @@ -17,9 +17,13 @@ "title": "URL de base du fournisseur OpenID Connect (racine du .well-known/openid-configuration)", "type": "string" }, - "postLogoutRedirectURL": { - "title": "URL de redirection après déconnexion", - "type": "string" + "postLogoutRedirectURLs": { + "title": "URLs de redirection après déconnexion autorisées", + "description": "La variable d'URL 'redirect=' peut être utilisée pour spécifier une redirection après déconnexion.", + "type": "array", + "item": { + "type": "string" + } }, "scopes": { "title": "Scopes associés au client OpenID Connect", @@ -42,27 +46,33 @@ } } }, - "loginCallbackURL": { + "loginCallbackPath": { "title": "Chemin associé à l'URL de callback OpenID Connect", - "default": ":///.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/callback", + "default": "/.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/callback", "description": "Les marqueurs '{{ .ProxyName }}' et '{{ .LayerName }}' peuvent être utilisés pour injecter le nom du proxy ainsi que celui du layer.", "type": "string" }, - "matchLoginCallbackURL": { - "title": "Patron de correspondance de l'URL interne de callback OpenID Connect", - "default": "*/.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/callback", + "matchLoginCallbackPath": { + "title": "Patron de correspondance du chemin interne de callback OpenID Connect", + "default": "*.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/callback", "description": "Les marqueurs '{{ .ProxyName }}' et '{{ .LayerName }}' peuvent être utilisés pour injecter le nom du proxy ainsi que celui du layer.", "type": "string" }, - "logoutURL": { + "logoutPath": { "title": "Chemin associé à l'URL de déconnexion", - "default": ":///.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/logout", + "default": "/.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/logout", "description": "Les marqueurs '{{ .ProxyName }}' et '{{ .LayerName }}' peuvent être utilisés pour injecter le nom du proxy ainsi que celui du layer.", "type": "string" }, - "matchLogoutURL": { - "title": "Patron de correspondance l'URL interne de déconnexion", - "default": "*/.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/logout", + "publicBaseURL": { + "title": "URL publique de base associée au service distant", + "default": "", + "description": "Peut être utilisé par exemple si il y a discordance de nom d'hôte ou de chemin sur les URLs publiques/internes.", + "type": "string" + }, + "matchLogoutPath": { + "title": "Patron de correspondance du chemin interne de déconnexion", + "default": "*.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/logout", "description": "Les marqueurs '{{ .ProxyName }}' et '{{ .LayerName }}' peuvent être utilisés pour injecter le nom du proxy ainsi que celui du layer.", "type": "string" }, diff --git a/internal/proxy/director/layer/authn/oidc/layer_options.go b/internal/proxy/director/layer/authn/oidc/layer_options.go index ce5b974..d344942 100644 --- a/internal/proxy/director/layer/authn/oidc/layer_options.go +++ b/internal/proxy/director/layer/authn/oidc/layer_options.go @@ -19,13 +19,14 @@ type LayerOptions struct { type OIDCOptions struct { ClientID string `mapstructure:"clientId"` ClientSecret string `mapstructure:"clientSecret"` - LoginCallbackURL string `mapstructure:"loginCallbackURL"` - MatchLoginCallbackURL string `mapstructure:"matchLoginCallbackURL"` - LogoutURL string `mapstructure:"logoutURL"` - MatchLogoutURL string `mapstructure:"matchLogoutURL"` + PublicBaseURL string `mapstructure:"publicBaseURL"` + LoginCallbackPath string `mapstructure:"loginCallbackPath"` + MatchLoginCallbackPath string `mapstructure:"matchLoginCallbackPath"` + LogoutPath string `mapstructure:"logoutPath"` + MatchLogoutPath string `mapstructure:"matchLogoutPath"` IssuerURL string `mapstructure:"issuerURL"` SkipIssuerVerification bool `mapstructure:"skipIssuerVerification"` - PostLogoutRedirectURL string `mapstructure:"postLogoutRedirectURL"` + PostLogoutRedirectURLs []string `mapstructure:"postLogoutRedirectURLs"` TLSInsecureSkipVerify bool `mapstructure:"tlsInsecureSkipVerify"` Scopes []string `mapstructure:"scopes"` AuthParams map[string]string `mapstructure:"authParams"` @@ -41,18 +42,19 @@ type CookieOptions struct { MaxAge time.Duration `mapstructure:"maxAge"` } -func fromStoreOptions(storeOptions store.LayerOptions, baseURL string) (*LayerOptions, error) { - loginCallbackPath := "/.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/callback" - logoutPath := "/.bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/logout" +func fromStoreOptions(storeOptions store.LayerOptions) (*LayerOptions, error) { + loginCallbackPath := ".bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/callback" + logoutPath := ".bouncer/authn/oidc/{{ .ProxyName }}/{{ .LayerName }}/logout" layerOptions := LayerOptions{ LayerOptions: authn.DefaultLayerOptions(), OIDC: OIDCOptions{ - LoginCallbackURL: baseURL + loginCallbackPath, - MatchLoginCallbackURL: "*" + loginCallbackPath + "*", - LogoutURL: baseURL + logoutPath, - MatchLogoutURL: "*" + logoutPath + "*", - Scopes: []string{"openid"}, + PublicBaseURL: "", + LoginCallbackPath: loginCallbackPath, + MatchLoginCallbackPath: "*" + loginCallbackPath, + LogoutPath: logoutPath, + MatchLogoutPath: "*" + logoutPath, + Scopes: []string{"openid"}, }, Cookie: CookieOptions{ Name: defaultCookieName,