fix(composio): callback endpoint should not require Multica auth (MUL-3843) (#4709)

* fix(composio): move OAuth callback out of the Auth group (MUL-3843)

Composio 302-redirects the browser to /api/integrations/composio/callback
at the end of the OAuth flow, but PR #4608 mounted it inside the cookie-auth
middleware group. When the session cookie is absent (expired session,
SameSite=Strict / Safari ITP, private window, self-hosted callback subdomain)
the Auth middleware returned a hard 401 and a JSON blob instead of the
settings redirect, breaking the flow.

Identity never came from the cookie anyway: it is carried by the HMAC-signed
state param that CompleteCallback verifies (signature, expiry, replay) and
cross-checked by verifyAccountOwnership; h.Composio == nil still 503s. So the
callback is registered alongside the other public OAuth/webhook routes; the
other four composio endpoints stay session-gated.

Refs MUL-3843, MUL-3715.

Co-authored-by: multica-agent <github@multica.ai>

* fix(composio): correct stale callback routing comments (MUL-3843)

The package header and ComposioCallback doc comments still described the
callback as sitting under the Auth middleware group. After the route was
moved out (this PR), update both to state it is a public route whose identity
comes from the signed state — addressing review nit from 张大彪.

Refs MUL-3843.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: multica-agent <github@multica.ai>

server/cmd/server/composio_callback_public_test.go

@@ -0,0 +1,68 @@
+package main
+
+import (
+	"io"
+	"net/http"
+	"testing"
+)
+
+// TestComposioCallbackIsPublic_NoCookieNot401 locks in the MUL-3843 fix: the
+// Composio OAuth callback must live OUTSIDE the Auth middleware group, because
+// Composio 302-redirects the user's browser to it and the cookie session is
+// frequently absent (expired session, SameSite=Strict / Safari ITP, private
+// window, self-hosted callback subdomain). Before the fix the route sat under
+// Auth, so a cookie-less browser got a hard 401 and a JSON blob instead of the
+// settings redirect — the exact symptom Yushen hit.
+//
+// With no COMPOSIO_API_KEY configured in the test env, h.Composio == nil, so a
+// cookie-less hit on the callback now reaches the handler and returns 503
+// ("not configured") rather than being short-circuited to 401 by the Auth
+// middleware. The precise non-401 code is incidental; what this test pins is
+// that the request is NOT rejected by auth.
+func TestComposioCallbackIsPublic_NoCookieNot401(t *testing.T) {
+	// Deliberately send NO Authorization header / cookie — simulate the
+	// cookie-stripped browser redirect coming back from Composio.
+	resp, err := http.Get(testServer.URL + "/api/integrations/composio/callback?state=bogus&status=success&connected_account_id=ca_x")
+	if err != nil {
+		t.Fatalf("callback request failed: %v", err)
+	}
+	defer resp.Body.Close()
+	body, _ := io.ReadAll(resp.Body)
+
+	if resp.StatusCode == http.StatusUnauthorized {
+		t.Fatalf("callback returned 401 without a session — it is still behind the Auth group (regression of MUL-3843). body=%s", body)
+	}
+}
+
+// TestComposioNonCallbackEndpointsStayGated is the other half of the invariant:
+// moving the callback out of the Auth group must NOT loosen the four
+// session-scoped endpoints. A cookie-less request to them must still 401.
+func TestComposioNonCallbackEndpointsStayGated(t *testing.T) {
+	gated := []struct {
+		method string
+		path   string
+	}{
+		{http.MethodPost, "/api/integrations/composio/connect/init"},
+		{http.MethodGet, "/api/integrations/composio/toolkits"},
+		{http.MethodGet, "/api/integrations/composio/connections"},
+		{http.MethodDelete, "/api/integrations/composio/connections/11111111-1111-1111-1111-111111111111"},
+	}
+	for _, tc := range gated {
+		t.Run(tc.method+" "+tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(tc.method, testServer.URL+tc.path, nil)
+			if err != nil {
+				t.Fatalf("build request: %v", err)
+			}
+			resp, err := http.DefaultClient.Do(req)
+			if err != nil {
+				t.Fatalf("request failed: %v", err)
+			}
+			defer resp.Body.Close()
+			io.Copy(io.Discard, resp.Body)
+
+			if resp.StatusCode != http.StatusUnauthorized {
+				t.Fatalf("expected 401 without a session, got %d — endpoint is no longer auth-gated", resp.StatusCode)
+			}
+		})
+	}
+}

server/cmd/server/router.go

@@ -612,6 +612,18 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus
 	// HandleCloudBillingStripeWebhook for the rationale).
 	r.Post("/api/webhooks/stripe", h.HandleCloudBillingStripeWebhook)
 
+	// Composio OAuth callback (MUL-3843). NOT under the Auth group on purpose:
+	// Composio 302-redirects the user's browser here at the end of the OAuth
+	// flow, and the cookie session is frequently absent (expired session,
+	// SameSite=Strict / Safari ITP stripping cross-site cookies, private
+	// windows, self-hosted callbacks on a different subdomain). Identity is NOT
+	// taken from the session — it comes from the HMAC-signed `state` query
+	// param, which CompleteCallback verifies (signature, expiry, replay) before
+	// doing anything. h.Composio == nil still returns 503. Keeping it inside the
+	// Auth group made a missing cookie a hard 401, breaking the flow for exactly
+	// the browsers above; the other four composio endpoints stay session-gated.
+	r.Get("/api/integrations/composio/callback", h.ComposioCallback)
+
 	// Daemon API routes (require daemon token or valid user token)
 	r.Route("/api/daemon", func(r chi.Router) {
 		r.Use(middleware.DaemonAuth(queries, patCache, daemonTokenCache, cloudPATVerifier))
@@ -770,12 +782,12 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus
 		r.Post("/api/lark/binding/redeem", h.RedeemLarkBindingToken)
 
 		// Composio integration (MUL-3720). User-scoped (no workspace context):
-		// a connection belongs to a user. The callback is a GET so it works as
-		// a top-level browser redirect under cookie auth (no CSRF gate on GET).
-		// All four return 503 when COMPOSIO_API_KEY is unset.
+		// a connection belongs to a user. These four require a logged-in
+		// session; the OAuth callback is the outlier and lives outside the Auth
+		// group (registered above with the other public OAuth/webhook routes —
+		// see MUL-3843). All return 503 when COMPOSIO_API_KEY is unset.
 		r.Route("/api/integrations/composio", func(r chi.Router) {
 			r.Post("/connect/init", h.ComposioConnectInit)
-			r.Get("/callback", h.ComposioCallback)
 			r.Get("/toolkits", h.ListComposioToolkits)
 			r.Get("/connections", h.ListComposioConnections)
 			r.Delete("/connections/{id}", h.DeleteComposioConnection)

server/internal/handler/integrations_composio.go

@@ -10,11 +10,16 @@ import (
 	composio "github.com/multica-ai/multica/server/internal/integrations/composio"
 )
 
-// Composio integration handlers (MUL-3720, Stage 2 MVP). All routes are
-// user-scoped (requireUserID) and live outside the workspace-membership group —
-// a Composio connection belongs to a user, not a workspace. The whole block
-// returns 503 when h.Composio is nil (COMPOSIO_API_KEY unset), matching the
-// Lark/GitHub "integration not configured" convention.
+// Composio integration handlers (MUL-3720, Stage 2 MVP). A Composio connection
+// belongs to a user, not a workspace, so these handlers live outside the
+// workspace-membership group. The four management endpoints (connect/init,
+// toolkits, connections, delete) are user-scoped (requireUserID) and sit under
+// the Auth middleware. ComposioCallback is the exception: it is a public route
+// (outside the Auth group, see router.go / MUL-3843) because the browser often
+// arrives without a session cookie — its identity comes from the signed state,
+// not requireUserID. The whole block returns 503 when h.Composio is nil
+// (COMPOSIO_API_KEY unset), matching the Lark/GitHub "integration not
+// configured" convention.
 
 // ComposioConnectInitRequest is the POST /connect/init body.
 type ComposioConnectInitRequest struct {
@@ -87,13 +92,15 @@ func (h *Handler) ComposioConnectInit(w http.ResponseWriter, r *http.Request) {
 }
 
 // ComposioCallback (GET /api/integrations/composio/callback) is the browser
-// redirect target Composio sends the user back to after the hosted flow. The
-// signed `state` query param is the source of truth for the user identity, so
-// the request is attributed from it (the route still sits under Auth so the
-// browser session is present, but identity comes from the state). On success
-// the row is upserted and the browser is redirected to the settings page; any
-// failure redirects to the same page with a stable error code so the user is
-// never left on a blank API response.
+// redirect target Composio sends the user back to after the hosted flow. It is
+// registered as a PUBLIC route (outside the Auth middleware group — see
+// router.go / MUL-3843), because the browser frequently lands here without a
+// session cookie (expired session, SameSite/ITP stripping, private window,
+// self-hosted callback subdomain). Identity therefore comes solely from the
+// HMAC-signed `state` query param, which CompleteCallback verifies before
+// doing anything. On success the row is upserted and the browser is redirected
+// to the settings page; any failure redirects to the same page with a stable
+// error code so the user is never left on a blank API response.
 func (h *Handler) ComposioCallback(w http.ResponseWriter, r *http.Request) {
 	if h.Composio == nil {
 		writeError(w, http.StatusServiceUnavailable, "composio integration not configured")