[http/saml] cookie bugfixes

- strip auth cookies before forwarding to origin
- when no session, send 401 instead of redirect for CORS and websocket requests
- delete unused relaystate cookies

diff --git a/http/route_action_saml.go b/http/route_action_saml.go
index eb0d8295177ce5ae3027d381328f920042ce8005..6594fce58999582d7b2f6fc3723e51ab8d56a2a1 100644 (file)
--- a/http/route_action_saml.go
+++ b/http/route_action_saml.go
@@ -41,9 +41,10 @@ type SAMLServiceProvider struct {
 }
 
 type samlAction struct {
-       sp             *SAMLServiceProvider
-       requireAuth    bool
-       usernameHeader string
+       sp              *SAMLServiceProvider
+       requireAuth     bool
+       usernameHeader  string
+       preserveCookies bool
 }
 
 var samlAttributeReplaceRegexp = regexp.MustCompile(`[^a-z0-9]+`)
@@ -85,6 +86,21 @@ func (sa *samlAction) Handle(w http.ResponseWriter, r *http.Request, next http.H
        }
 
        if sessionErr == samlsp.ErrNoSession && sa.requireAuth {
+               // cleanup previous saml cookies, because these really pile up
+               logger.V(3).Debugf("request tracker is a %T", provider.RequestTracker)
+               if t, ok := provider.RequestTracker.(*samlsp.CookieRequestTracker); ok {
+                       sa.cleanCookies(w, r, t)
+               }
+               if r.Header.Get("sec-fetch-mode") == "cors" || r.Header.Get("sec-fetch-mode") == "websocket" {
+                       // XHRs should not be redirected to auth as this creates a ton of cookie spam
+                       errMsg := "SAML session required"
+                       if r.Header.Get("accept") == "application/json" {
+                               w.Header().Set("content-type", "application/json")
+                               errMsg = `{"error": "` + errMsg + `"}`
+                       }
+                       http.Error(w, errMsg, http.StatusUnauthorized)
+                       return
+               }
                logger.V(3).Debugf("route requires a valid session, redirecting")
 
                provider.HandleStartAuthFlow(w, r)
@@ -128,6 +144,20 @@ func (sa *samlAction) Handle(w http.ResponseWriter, r *http.Request, next http.H
                r.Header.Set("x-saml-anonymous-auth", "1")
        }
 
+       // don't forward saml cookies to the origin unless PreserveCookies is set
+       if !sa.preserveCookies {
+               newReq := r.Clone(r.Context())
+               newReq.Header.Del("Cookie")
+               for _, cookie := range r.Cookies() {
+                       if cookie.Name != "token" && !strings.HasPrefix(cookie.Name, "saml_") {
+                               newReq.AddCookie(cookie)
+                       }
+               }
+
+               logger.V(3).Debugf("new Cookie header with saml cookies filtered: %+v", newReq.Cookies())
+               r = newReq
+       }
+
        next(w, r)
 }
 
@@ -141,6 +171,35 @@ func (sa *samlAction) checkRequest(r *http.Request) error {
        return nil
 }
 
+// cleanCookies deletes cookies set by crewjam/saml.
+//
+// The SP auth handler creates two cookies: one named "token" and one named "saml_" plus a
+// randomized suffix. The problem is, a new randomized-suffix cookie is generated each time an auth
+// redirect is performed. Apps like Grafana continue making requests in the background even after a
+// user's session expires, and this results in dozens or hundreds of cookies being set eventually
+// leading to a 431 (Requested Header Fields Too Large).
+//
+// To mitigate this, we clear any existing saml cookies whenever we send an auth redirect or reject
+// an XHR with a 401.
+func (sa *samlAction) cleanCookies(w http.ResponseWriter, r *http.Request, t *samlsp.CookieRequestTracker) {
+       for _, cookie := range r.Cookies() {
+               if strings.HasPrefix(cookie.Name, "saml_") {
+                       logger := LoggerFromContext(r.Context())
+                       logger.V(3).Debugf("deleting leftover relaystate cookie: %s", cookie.Name)
+
+                       http.SetCookie(w, &http.Cookie{
+                               Name:     cookie.Name,
+                               Value:    "",
+                               MaxAge:   -1,
+                               HttpOnly: true,
+                               SameSite: t.SameSite,
+                               Secure:   t.ServiceProvider.AcsURL.Scheme == "https",
+                               Path:     t.ServiceProvider.AcsURL.Path,
+                       })
+               }
+       }
+}
+
 func (sp *SAMLServiceProvider) Metadata() (*saml.EntityDescriptor, error) {
        var err error
        sp.metadataOnce.Do(func() {
@@ -293,7 +352,8 @@ func samlActionFromRouteYaml(node *yaml.Node) (RouteAction, error) {
        var rawNode struct {
                SP *struct {
                        *SAMLServiceProvider
-                       Require string `yaml:"require"`
+                       Require         string `yaml:"require"`
+                       PreserveCookies bool   `yaml:"preserve_cookies"`
                } `yaml:"saml,omitempty"`
                Auth string `yaml:"auth"`
        }
@@ -309,8 +369,9 @@ func samlActionFromRouteYaml(node *yaml.Node) (RouteAction, error) {
        }
 
        sa := &samlAction{
-               sp:          rawNode.SP.SAMLServiceProvider,
-               requireAuth: require,
+               sp:              rawNode.SP.SAMLServiceProvider,
+               requireAuth:     require,
+               preserveCookies: rawNode.SP.PreserveCookies,
        }
 
        return sa, nil