delete cookie from request instead of setting max age because cookie is never seen by client at this stage before it's passed to backend

CHANGELOG.md

@@ -12,6 +12,8 @@ FIX: The migration `sqlite3/015_v0_4_19_share_unique_name_constraint.sql` has be
 
 FIX: Email addresses have been made case-insensitive. Please note that there is a migration included in this release (`016_v0_4_21_lowercase_email.sql`) which will attempt to ensure that all email addresses in your existing database are stored in lowercase; **if this migration fails you will need to manually remediate the duplicate account entries** (https://github.com/openziti/zrok/issues/517)
 
+FIX: Stop sending authentication cookies to non-authenticated shares (https://github.com/openziti/zrok/issues/512)
+
 ## v0.4.20
 
 CHANGE: OpenZiti SDK updated to `v0.21.2`. All `ziti.ListenOptions` listener options configured to use `WaitForNEstablishedListeners: 1`. When a `zrok share` client or an `sdk.Share` client are connected to an OpenZiti router that supports "listener established" events, then listen calls will not return until the listener is fully established on the OpenZiti network. Previously a `zrok share` client could report that it is fully operational and listening before the listener is fully established on the OpenZiti network; in practice this produced a very small window of time when the share would not be ready to accept requests. This change eliminates this window of time (https://github.com/openziti/zrok/issues/490)

endpoints/publicProxy/http.go

@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"net/http/httputil"
 	"net/url"
+	"slices"
 	"strings"
 	"time"
 )
@@ -157,7 +158,8 @@ func authHandler(handler http.Handler, pcfg *Config, key []byte, ctx ziti.Contex
 						switch scheme {
 						case string(sdk.None):
 							logrus.Debugf("auth scheme none '%v'", shrToken)
-							deleteZrokCookie(w, r)
+							// ensure cookies from other shares are not sent to this share, in case it's malicious
+							deleteZrokCookies(w, r)
 							handler.ServeHTTP(w, r)
 							return
 
@@ -203,7 +205,8 @@ func authHandler(handler http.Handler, pcfg *Config, key []byte, ctx ziti.Contex
 								return
 							}
 
-							deleteZrokCookie(w, r)
+							// ensure cookies from other shares are not sent to this share, in case it's malicious
+							deleteZrokCookies(w, r)
 							handler.ServeHTTP(w, r)
 
 						case string(sdk.Oauth):
@@ -362,11 +365,26 @@ func SetZrokCookie(w http.ResponseWriter, cookieDomain, email, accessToken, prov
 	})
 }
 
-func deleteZrokCookie(w http.ResponseWriter, r *http.Request) {
+func deleteZrokCookies(w http.ResponseWriter, r *http.Request) {
-	cookie, err := r.Cookie("zrok-access")
+	// Get all cookies from the request
-	if err == nil {
+	cookies := r.Cookies()
-		cookie.MaxAge = -1
+	// List of cookies to delete, the pkce cookie might be okay to pass along to the HTTP backend, but zrok-access is
-		http.SetCookie(w, cookie)
+	// not because it can contain the accessToken from any other OAuth enabled shares, so we delete it here when the
+	// current share is not OAuth-enabled. OAuth-enabled shares check the audience claim in the JWT to ensure it matches
+	// the requested share and will send the client back to the OAuth provider if it does not match.
+	deletedCookies := []string{"zrok-access", "pkce"}
+	// Filter the cookies to save
+	filteredCookies := make([]*http.Cookie, 0)
+	for _, cookie := range cookies {
+		if !slices.Contains(deletedCookies, cookie.Name) {
+			filteredCookies = append(filteredCookies, cookie)
+		}
+	}
+
+	// Set the Cookie header to the filtered list of cookies
+	r.Header.Del("Cookie")
+	for _, cookie := range filteredCookies {
+		r.AddCookie(cookie)
 	}
 }