Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/cimd-org-redirect-uri.md
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 9 additions & 8 deletions packages/core/api/src/server/oauth-client-metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand All @@ -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");
});
Expand Down Expand Up @@ -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", {
Expand All @@ -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"]);
});
});
23 changes: 7 additions & 16 deletions packages/core/api/src/server/oauth-client-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
Loading