From c82358c7805f53d3c4adcce4d8b3d03b3141ea37 Mon Sep 17 00:00:00 2001
From: David Hartunian <davidh@cockroachlabs.com>
Date: Fri, 17 Feb 2023 16:41:14 -0500
Subject: [PATCH] server: fix logout to always fanout to all tenants

Previously, we added login fanout logic handled through an HTTP layer in
`server_controller_http.go` which lets us fanout logins to multiple tenants. In
the case of a single tenant cluster, this simply means that the
`multitenant-session` cookie contains only a single session, and `tenant` is
set to `system`. However, we run into problems during `/logout` calls because
our routing assumes that if you have a `tenant` cookie set that it should *not*
fanout and simply proxy the request to the correct tenant. In the special case
of Logout, this causes a problem because the tenant's Logout handler only
clears the `session` cookie and not the new `multitenant-session` and `tenant`
cookies.

Since these new cookies are owned by the code in `server_controller_http.go`
and because we don't claim to support per-tenant logout operations yet (#92855)
this commit enforces a fanout on *all* HTTP calls to `/logout`. This ensures
that sessions are cleared in the SQL layer. Then we also make sure to clear the
`session` cookie as well in case this code runs with a legacy session.

Epic: CRDB-12100

Release note: None
---
 pkg/server/authentication_test.go    | 25 +++++++++++++++++++++++++
 pkg/server/server_controller_http.go | 20 +++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/pkg/server/authentication_test.go b/pkg/server/authentication_test.go
index 1e80969cefc3..08bf918d0ff5 100644
--- a/pkg/server/authentication_test.go
+++ b/pkg/server/authentication_test.go
@@ -571,6 +571,31 @@ func TestAuthenticationAPIUserLogin(t *testing.T) {
 	}
 }
 
+func TestLogoutClearsCookies(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+	s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	defer s.Stopper().Stop(context.Background())
+	ts := s.(*TestServer)
+
+	// Log in.
+	authHTTPClient, _, err := ts.getAuthenticatedHTTPClientAndCookie(authenticatedUserName(), true)
+	require.NoError(t, err)
+
+	// Log out.
+	resp, err := authHTTPClient.Get(ts.AdminURL() + logoutPath)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+
+	cookies := resp.Cookies()
+	cNames := make([]string, len(cookies))
+	for i, c := range cookies {
+		require.Equal(t, "", c.Value)
+		cNames[i] = c.Name
+	}
+	require.ElementsMatch(t, cNames, []string{SessionCookieName, MultitenantSessionCookieName, TenantSelectCookieName})
+}
+
 func TestLogout(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
diff --git a/pkg/server/server_controller_http.go b/pkg/server/server_controller_http.go
index 3eec8dcbe06e..8933c6326c00 100644
--- a/pkg/server/server_controller_http.go
+++ b/pkg/server/server_controller_http.go
@@ -56,11 +56,17 @@ func (c *serverController) httpMux(w http.ResponseWriter, r *http.Request) {
 		case loginPath, DemoLoginPath:
 			c.attemptLoginToAllTenants().ServeHTTP(w, r)
 			return
-		case logoutPath:
-			c.attemptLogoutFromAllTenants().ServeHTTP(w, r)
-			return
 		}
 	}
+	// Since we do not support per-tenant logout until
+	// https://github.com/cockroachdb/cockroach/issues/92855
+	// is completed, we should always fanout a logout
+	// request in order to clear the multi-tenant session
+	// cookies properly.
+	if r.URL.Path == logoutPath {
+		c.attemptLogoutFromAllTenants().ServeHTTP(w, r)
+		return
+	}
 	s, err := c.getServer(ctx, tenantName)
 	if err != nil {
 		log.Warningf(ctx, "unable to find server for tenant %q: %v", tenantName, err)
@@ -291,6 +297,14 @@ func (c *serverController) attemptLogoutFromAllTenants() http.Handler {
 			Expires:  timeutil.Unix(0, 0),
 		}
 		http.SetCookie(w, &cookie)
+		cookie = http.Cookie{
+			Name:     SessionCookieName,
+			Value:    "",
+			Path:     "/",
+			HttpOnly: false,
+			Expires:  timeutil.Unix(0, 0),
+		}
+		http.SetCookie(w, &cookie)
 		if r.Header.Get(AcceptHeader) == JSONContentType {
 			w.Header().Add(ContentTypeHeader, JSONContentType)
 			_, err = w.Write([]byte("{}"))