Merge #97310

97310: server: fix logout to always fanout to all tenants r=stevendanna a=dhartunian

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

Co-authored-by: David Hartunian <[email protected]>

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)

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("{}"))