Fix OAuth redirect URI mismatch for org-scoped client-id metadata

Org-scoped client-id metadata documents registered their callback with an
executor_org query param on redirect_uri, but the client sends the bare
callback and the org is carried in the OAuth state (#1147). Providers that
exact-match redirect_uri (PostHog) rejected the authorize request with
"Mismatching redirect URI". Org targets keep their distinct client_id URL
but now register the same bare callback redirect_uri as every other target.

Author: RhysSullivan

.changeset/cimd-org-redirect-uri.md

@@ -0,0 +1,12 @@
+---
+"@executor-js/api": patch
+---
+
+Fix OAuth "Mismatching redirect URI" for org-scoped client-id metadata documents
+
+Org-scoped client-id metadata documents registered their callback as
+`redirect_uri` with an `executor_org` query param, but the client always sends
+the bare callback and the org is carried in the OAuth `state`. Providers that
+compare `redirect_uri` as an exact string (such as PostHog) rejected the
+authorize request. Org targets now keep their distinct `client_id` URL but
+register the same bare callback `redirect_uri` as every other target.

packages/core/api/src/server/oauth-client-metadata.test.ts

@@ -3,7 +3,7 @@ import { describe, expect, it } from "@effect/vitest";
 import { oauthClientIdMetadataDocumentFromRequest } from "./oauth-client-metadata";
 
 describe("OAuth client ID metadata document", () => {
-  it("builds an org-scoped hosted document from a target path", () => {
+  it("builds an org-scoped hosted document with a bare callback redirect_uri", () => {
     const metadata = oauthClientIdMetadataDocumentFromRequest({
       requestUrl: "/api/oauth/client-id-metadata/acme.json",
       webRequest: new Request("http://127.0.0.1:42384/api/oauth/client-id-metadata/acme.json", {
@@ -12,12 +12,13 @@ describe("OAuth client ID metadata document", () => {
       mountPrefix: "/api",
     });
 
+    // The org is identified by the per-org client_id URL, not by a query param
+    // on redirect_uri (which the client never sends, and the callback reads
+    // from `state`). redirect_uri must stay bare for exact-match providers.
     expect(metadata.client_id).toBe(
       "http://100.81.219.45:42384/api/oauth/client-id-metadata/acme.json",
     );
-    expect(metadata.redirect_uris).toEqual([
-      "http://100.81.219.45:42384/api/oauth/callback?executor_org=acme",
-    ]);
+    expect(metadata.redirect_uris).toEqual(["http://100.81.219.45:42384/api/oauth/callback"]);
     expect(metadata.token_endpoint_auth_method).toBe("none");
     expect(metadata.application_type).toBe("web");
   });
@@ -65,7 +66,7 @@ describe("OAuth client ID metadata document", () => {
     expect(metadata.application_type).toBe("native");
   });
 
-  it("still reads the org selector from the legacy query form", () => {
+  it("ignores a legacy executor_org query param when building redirect_uri", () => {
     const metadata = oauthClientIdMetadataDocumentFromRequest({
       requestUrl: "/api/oauth/client-id-metadata.json?executor_org=acme",
       webRequest: new Request("http://127.0.0.1:42384/api/oauth/client-id-metadata.json", {
@@ -74,11 +75,11 @@ describe("OAuth client ID metadata document", () => {
       mountPrefix: "/api",
     });
 
+    // A stray executor_org query param is inert: it stays on the client_id URL
+    // (which is just the request URL) but never leaks into redirect_uri.
     expect(metadata.client_id).toBe(
       "http://100.81.219.45:42384/api/oauth/client-id-metadata.json?executor_org=acme",
     );
-    expect(metadata.redirect_uris).toEqual([
-      "http://100.81.219.45:42384/api/oauth/callback?executor_org=acme",
-    ]);
+    expect(metadata.redirect_uris).toEqual(["http://100.81.219.45:42384/api/oauth/callback"]);
   });
 });

packages/core/api/src/server/oauth-client-metadata.ts

@@ -8,7 +8,6 @@ export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_TARGET_PATH_PREFIX =
   `${OAUTH_CLIENT_ID_METADATA_DOCUMENT_BASE_PATH}/` as const;
 export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_DEFAULT_TARGET = "default" as const;
 export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_LOCAL_TARGET = "local" as const;
-const OAUTH_CALLBACK_ORG_QUERY_PARAM = "executor_org" as const;
 
 type MetadataTarget =
   | typeof OAUTH_CLIENT_ID_METADATA_DOCUMENT_DEFAULT_TARGET
@@ -80,17 +79,6 @@ const metadataTargetFromPath = ({
   return target ? target : undefined;
 };
 
-const orgSlugFromMetadataTarget = (target: MetadataTarget | undefined): string | undefined => {
-  if (!target) return undefined;
-  if (
-    target === OAUTH_CLIENT_ID_METADATA_DOCUMENT_DEFAULT_TARGET ||
-    target === OAUTH_CLIENT_ID_METADATA_DOCUMENT_LOCAL_TARGET
-  ) {
-    return undefined;
-  }
-  return target;
-};
-
 export const oauthClientIdMetadataDocumentUrlFromRequest = ({
   requestUrl,
   webRequest,
@@ -148,11 +136,14 @@ export const oauthClientIdMetadataDocumentFromRequest = ({
     };
   }
 
+  // The org selector travels in the OAuth `state` (see #1147 and apps/cloud
+  // start.ts, which reads it back from state on the callback), never as a
+  // provider-facing query param on redirect_uri. Org targets get a distinct
+  // `client_id` URL, but all targets register the SAME bare callback so the
+  // redirect_uri the client sends matches this document exactly. Providers
+  // (e.g. PostHog) compare redirect_uri as an exact string, so an extra query
+  // param here would fail with "Mismatching redirect URI".
   const redirectUri = new URL(callbackPathWithMountPrefix(mountPrefix), url.origin);
-  const orgSlug =
-    orgSlugFromMetadataTarget(target) ??
-    url.searchParams.get(OAUTH_CALLBACK_ORG_QUERY_PARAM)?.trim();
-  if (orgSlug) redirectUri.searchParams.set(OAUTH_CALLBACK_ORG_QUERY_PARAM, orgSlug);
 
   return {
     client_id: url.toString(),