History

Gate Google OAuth listings by user flag (#1947)

Rhys Sullivan committed 2 months ago

Commit 31a80421 parent 5e4da2a8 files · +274 −66Browse files

Unified · Split

8 files
  1. .changeset/first-party-oauth-listing-policy.md +5 −0
  2. apps/cloud/src/analytics/google-oauth-listing.test.ts +89 −0
  3. apps/cloud/src/analytics/google-oauth-listing.ts +44 −0
  4. apps/cloud/src/engine/first-party-oauth-clients.test.ts +9 −4
  5. apps/cloud/src/engine/first-party-oauth-clients.ts +12 −6
  6. packages/core/sdk/src/oauth-client.ts +8 −0
  7. packages/core/sdk/src/oauth-first-party.test.ts +46 −1
  8. packages/core/sdk/src/oauth-service.ts +61 −55
.changeset/first-party-oauth-listing-policy.md+5−0
@@ -0,0 +1,5 @@
1+---
2+"@executor-js/sdk": patch
3+---
4+
5+Allow hosts to control first-party OAuth app listings for each acting user without disrupting existing connections.
apps/cloud/src/analytics/google-oauth-listing.test.ts+89−0
@@ -0,0 +1,89 @@
1+import { describe, expect, it } from "@effect/vitest";
2+import { Effect } from "effect";
3+
4+import { GOOGLE_OAUTH_REVIEW_FLAG, makeGoogleOAuthListing } from "./google-oauth-listing";
5+
6+const context = { userId: "review-user", organizationId: "review-org" };
7+
8+describe("Google OAuth review listing", () => {
9+ it.effect("targets the authenticated user and re-evaluates after revocation", () =>
10+ Effect.gen(function* () {
11+ let enabled = true;
12+ const policy = makeGoogleOAuthListing({
13+ projectKey: "public-project-key",
14+ host: "https://flags.example.com",
15+ fetch: async (url, init) => {
16+ expect(url).toBe("https://flags.example.com/flags?v=2");
17+ expect(await new Response(String(init?.body)).json()).toMatchObject({
18+ distinct_id: context.userId,
19+ person_properties: { executor_user_id: context.userId },
20+ flag_keys_to_evaluate: [GOOGLE_OAUTH_REVIEW_FLAG],
21+ });
22+ expect(init?.signal).toBeInstanceOf(AbortSignal);
23+ return Response.json({ flags: { [GOOGLE_OAUTH_REVIEW_FLAG]: { enabled } } });
24+ },
25+ });
26+ expect(yield* policy(context)).toBe(true);
27+ enabled = false;
28+ expect(yield* policy(context)).toBe(false);
29+ }),
30+ );
31+
32+ for (const body of [
33+ { flags: {} },
34+ { flags: { [GOOGLE_OAUTH_REVIEW_FLAG]: { enabled: "true" } } },
35+ {},
36+ ]) {
37+ it.effect(`withholds an absent or invalid flag: ${JSON.stringify(body)}`, () =>
38+ Effect.gen(function* () {
39+ const policy = makeGoogleOAuthListing({
40+ projectKey: "key",
41+ host: "https://flags.example.com",
42+ fetch: async () => Response.json(body),
43+ });
44+ expect(yield* policy(context)).toBe(false);
45+ }),
46+ );
47+ }
48+
49+ it.effect("withholds on HTTP and transport failures", () =>
50+ Effect.gen(function* () {
51+ const unavailable = makeGoogleOAuthListing({
52+ projectKey: "key",
53+ host: "https://flags.example.com",
54+ fetch: async () => new Response(null, { status: 503 }),
55+ });
56+ const failed = makeGoogleOAuthListing({
57+ projectKey: "key",
58+ host: "https://flags.example.com",
59+ fetch: () =>
60+ Effect.runPromise(Effect.fail(new DOMException("Network unavailable", "NetworkError"))),
61+ });
62+ expect(yield* unavailable(context)).toBe(false);
63+ expect(yield* failed(context)).toBe(false);
64+ }),
65+ );
66+
67+ it.effect("does not evaluate without a user or project key", () =>
68+ Effect.gen(function* () {
69+ let requests = 0;
70+ const fetch: typeof globalThis.fetch = async () => {
71+ requests += 1;
72+ return Response.json({ flags: {} });
73+ };
74+ const configured = makeGoogleOAuthListing({
75+ projectKey: "key",
76+ host: "https://flags.example.com",
77+ fetch,
78+ });
79+ const unconfigured = makeGoogleOAuthListing({
80+ projectKey: undefined,
81+ host: "https://flags.example.com",
82+ fetch,
83+ });
84+ expect(yield* configured({ ...context, userId: null })).toBe(false);
85+ expect(yield* unconfigured(context)).toBe(false);
86+ expect(requests).toBe(0);
87+ }),
88+ );
89+});
apps/cloud/src/analytics/google-oauth-listing.ts+44−0
@@ -0,0 +1,44 @@
1+import { Effect, Schema } from "effect";
2+
3+import type { FirstPartyOAuthClientConfig } from "@executor-js/sdk";
4+
5+/** Server-side rollout for offering the built-in Google OAuth app. Configure
6+ * the PostHog flag's runtime as "all": direct HTTP clients are not recognized
7+ * as server SDKs. The caller identity still comes only from the server. */
8+export const GOOGLE_OAUTH_REVIEW_FLAG = "google-oauth-review";
9+
10+const FlagsResponse = Schema.Struct({
11+ flags: Schema.Record(Schema.String, Schema.Struct({ enabled: Schema.Boolean })),
12+});
13+
14+/** Evaluate each listing against the authenticated user. Missing configuration,
15+ * timeouts, and invalid responses withhold the app. No verdict is cached. */
16+export const makeGoogleOAuthListing =
17+ (config: {
18+ readonly projectKey: string | undefined;
19+ readonly host: string;
20+ readonly fetch: typeof globalThis.fetch;
21+ }): NonNullable<FirstPartyOAuthClientConfig["isListed"]> =>
22+ (context) =>
23+ Effect.gen(function* () {
24+ if (!config.projectKey || context.userId === null) return false;
25+ const response = yield* Effect.tryPromise(() =>
26+ config.fetch(`${config.host}/flags?v=2`, {
27+ method: "POST",
28+ headers: { "content-type": "application/json", "user-agent": "executor-cloud" },
29+ body: JSON.stringify({
30+ api_key: config.projectKey,
31+ distinct_id: context.userId,
32+ // Supplied by the server so targeting needs no prior browser identify.
33+ person_properties: { executor_user_id: context.userId },
34+ groups: { organization: context.organizationId },
35+ flag_keys_to_evaluate: [GOOGLE_OAUTH_REVIEW_FLAG],
36+ }),
37+ signal: AbortSignal.timeout(1_000),
38+ }),
39+ );
40+ if (!response.ok) return false;
41+ const body: unknown = yield* Effect.tryPromise(() => response.json());
42+ const parsed = yield* Schema.decodeUnknownEffect(FlagsResponse)(body);
43+ return parsed.flags[GOOGLE_OAUTH_REVIEW_FLAG]?.enabled === true;
44+ }).pipe(Effect.catch(() => Effect.succeed(false)));
apps/cloud/src/engine/first-party-oauth-clients.test.ts+9−4
@@ -1,4 +1,5 @@
11 import { describe, expect, it } from "@effect/vitest";
2+import { Effect } from "effect";
23
34 import {
45 firstPartyOAuthClientsFor,
@@ -99,7 +100,7 @@ describe("cloud first-party OAuth clients", () => { · 94 unmodified lines
99100 // The reviewed consumer scope boundary of the Executor-owned Google app.
100101 //
101102 // These assertions used to live in `e2e/scenarios/first-party-oauth.test.ts`,
102−// read off `listClients`. The app is now `unlisted`, so it has no read surface
103+// read off `listClients`. The app is now gated, so it has no public read surface
103104 // to introspect — the bundle is only observable on the config it is built from,
104105 // which is here. The e2e still owns the BEHAVIOUR the boundary produces (which
105106 // scopes an `oauth.start` requests, and that admin scopes are refused).
@@ -109,13 +110,17 @@ describe("cloud first-party Google app", () => { · 3 unmodified lines
109110 const google = () =>
110111 firstPartyOAuthClientsFor(completeEnv).find((client) => client.name === "google");
111112
112− it("declares the Google app but withholds it from every listing", () => {
113+ it("declares the Google app but withholds it without a configured rollout", async () => {
113114 const client = google();
114115 expect(client, "the env-declared first-party Google app is configured").toBeDefined();
115116 // The entry MUST stay declared: `loadClient` resolves it by slug for every
116− // existing connection's refresh and reconnect. `unlisted` is what stops it
117+ // existing connection's refresh and reconnect. The listing policy stops it
117118 // being offered for new connections.
118− expect(client?.unlisted).toBe(true);
119+ expect(client?.isListed).toBeDefined();
120+ if (client?.isListed === undefined) return;
121+ expect(
122+ await Effect.runPromise(client.isListed({ userId: "test-user", organizationId: "test-org" })),
123+ ).toBe(false);
119124 });
120125
121126 it("covers the reviewed consumer bundle", () => {
apps/cloud/src/engine/first-party-oauth-clients.ts+12−6
@@ -7,9 +7,14 @@ import { · 6 unmodified lines
77 import { slackMcpUserScopes } from "@executor-js/react/lib/slack-mcp-oauth";
88 import { IntegrationSlug, type FirstPartyOAuthClientConfig } from "@executor-js/sdk";
99
10+import { makeGoogleOAuthListing } from "../analytics/google-oauth-listing";
11+import { POSTHOG_INGEST_HOST } from "../edge/passthrough";
12+
1013 /** Cloud secret bindings that enable host-operated OAuth clients. A provider
1114 * is absent unless both values in its pair are present. */
1215 export interface FirstPartyOAuthClientEnv {
16+ readonly VITE_PUBLIC_POSTHOG_KEY?: string;
17+ readonly VITE_PUBLIC_POSTHOG_HOST?: string;
1318 readonly FIRST_PARTY_AIRTABLE_CLIENT_ID?: string;
1419 readonly FIRST_PARTY_AIRTABLE_CLIENT_SECRET?: string;
1520 readonly FIRST_PARTY_ATLASSIAN_CLIENT_ID?: string;
@@ -286,12 +291,13 @@ export const firstPartyOAuthClientsFor = ( · 270 unmodified lines
286291 authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth",
287292 tokenUrl: "https://oauth2.googleapis.com/token",
288293 allowedScopes: GOOGLE_ALLOWED_SCOPES,
289− // Withdrawn from the connect picker: no new connection is offered the
290− // Executor-owned Google app. The entry stays declared on purpose — every
291− // connection already minted against it keeps refreshing and reconnecting
292− // through it. Deleting this block, or unsetting the env vars, would strand
293− // those connections instead.
294− unlisted: true,
294+ // Offer the app only to the review rollout. Resolution remains available
295+ // for existing connections regardless of the current listing decision.
296+ isListed: makeGoogleOAuthListing({
297+ projectKey: env.VITE_PUBLIC_POSTHOG_KEY,
298+ host: env.VITE_PUBLIC_POSTHOG_HOST ?? `https://${POSTHOG_INGEST_HOST}`,
299+ fetch: (input, init) => globalThis.fetch(input, init),
300+ }),
295301 }),
296302 ...client(env.FIRST_PARTY_HUBSPOT_CLIENT_ID, env.FIRST_PARTY_HUBSPOT_CLIENT_SECRET, {
297303 name: "hubspot",
packages/core/sdk/src/oauth-client.ts+8−0
@@ -243,6 +243,14 @@ export interface FirstPartyOAuthClientConfig { · 242 unmodified lines
243243 * instead removes the config entry itself, which strands every existing
244244 * connection on a client the host can no longer resolve. */
245245 readonly unlisted?: boolean;
246+ /** Optional host policy for offering this app to the acting user. Evaluated
247+ * on each listing; false withholds the app without disrupting existing
248+ * connections. `unlisted: true` always withholds it. This controls discovery,
249+ * not authorization to resolve an already-known first-party client slug. */
250+ readonly isListed?: (context: {
251+ readonly userId: string | null;
252+ readonly organizationId: string;
253+ }) => Effect.Effect<boolean>;
246254 /** OAuth scopes this deployment permits the app to request. Omit to allow
247255 * every scope declared by a matching integration. For declared scopes,
248256 * start and completion fail unless every requested scope belongs to this
packages/core/sdk/src/oauth-first-party.test.ts+46−1
@@ -324,13 +324,58 @@ describe("first-party oauth clients", () => { · 323 unmodified lines
324324 ),
325325 );
326326
327+ it.effect("listing policies use the acting identity and re-evaluate within an executor", () =>
328+ Effect.scoped(
329+ Effect.gen(function* () {
330+ const server = yield* serveOAuthTestServer({ scopes: ["read"] });
331+ let enabled = true;
332+ const { executor } = yield* makeTestWorkspaceHarness({
333+ plugins,
334+ subject: "review-user",
335+ tenant: "review-org",
336+ firstPartyOAuthClients: [
337+ {
338+ ...firstPartyClientFor(server),
339+ isListed: (context) =>
340+ Effect.sync(() => {
341+ expect(context).toEqual({ userId: "review-user", organizationId: "review-org" });
342+ return enabled;
343+ }),
344+ },
345+ ],
346+ });
347+ expect((yield* executor.oauth.listClients()).map((client) => String(client.slug))).toEqual([
348+ "first-party:acme",
349+ ]);
350+ enabled = false;
351+ expect(yield* executor.oauth.listClients()).toEqual([]);
352+ yield* executor.acme.seed();
353+ const started = yield* executor.oauth.start({
354+ owner: "org",
355+ client: FIRST_PARTY,
356+ clientOwner: "org",
357+ name: ConnectionName.make("main"),
358+ integration: INTEG,
359+ template: TEMPLATE,
360+ });
361+ expect(started.status).toBe("redirect");
362+ }),
363+ ),
364+ );
365+
327366 it.effect("an unlisted first-party app is withheld from listings but still refreshes", () =>
328367 Effect.scoped(
329368 Effect.gen(function* () {
330369 const server = yield* serveOAuthTestServer({ scopes: ["read"] });
331370 const harness = yield* makeTestWorkspaceHarness({
332371 plugins,
333− firstPartyOAuthClients: [{ ...firstPartyClientFor(server), unlisted: true }],
372+ firstPartyOAuthClients: [
373+ {
374+ ...firstPartyClientFor(server),
375+ unlisted: true,
376+ isListed: () => Effect.succeed(true),
377+ },
378+ ],
334379 });
335380 const { executor, config } = harness;
336381 yield* executor.acme.seed();
packages/core/sdk/src/oauth-service.ts+61−55
@@ -1477,19 +1477,25 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 1476 unmodified lines
14771477 // tenant's org rows + this subject's own user rows, so no explicit filter is
14781478 // needed. The `client_secret` column is deliberately never projected.
14791479 // -----------------------------------------------------------------------
1480− const listClients = (): Effect.Effect<readonly OAuthClientSummary[], StorageFailure> => {
1481− // First-party apps lead the list: config-resolved, visible to every caller,
1482− // and projected exactly like stored rows — clientId only, never the secret.
1483− // Owner is reported as "org" (the widest visibility the summary shape can
1484− // express); the flow itself ignores owner for first-party slugs.
1485− //
1486− // `unlisted` apps are withheld here and ONLY here: listing is what offers an
1487− // app for a NEW connection, so this is the whole of "stop offering it".
1488− // `loadClient` still resolves them, keeping every existing connection's
1489− // refresh and reconnect intact.
1490− const firstPartySummaries: readonly OAuthClientSummary[] = [...firstPartyBySlug.values()]
1491− .filter((config) => config.unlisted !== true)
1492− .map((config) => ({
1480+ const listClients = (): Effect.Effect<readonly OAuthClientSummary[], StorageFailure> =>
1481+ Effect.gen(function* () {
1482+ // First-party apps lead the list: config-resolved, filtered by host policy,
1483+ // and projected exactly like stored rows — clientId only, never the secret.
1484+ // Owner is reported as "org" (the widest visibility the summary shape can
1485+ // express); the flow itself ignores owner for first-party slugs.
1486+ //
1487+ // `unlisted` apps are withheld here and ONLY here: listing is what offers an
1488+ // app for a NEW connection, so this is the whole of "stop offering it".
1489+ // `loadClient` still resolves them, keeping every existing connection's
1490+ // refresh and reconnect intact.
1491+ const listed = yield* Effect.filter([...firstPartyBySlug.values()], (config) =>
1492+ config.unlisted === true
1493+ ? Effect.succeed(false)
1494+ : config.isListed === undefined
1495+ ? Effect.succeed(true)
1496+ : config.isListed({ userId: deps.subject, organizationId: deps.tenant }),
1497+ );
1498+ const firstPartySummaries: readonly OAuthClientSummary[] = listed.map((config) => ({
14931499 owner: "org",
14941500 slug: firstPartyOAuthClientSlug(config.name),
14951501 grant: "authorization_code",
@@ -1506,49 +1512,49 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 10 unmodified lines
15061512 ...(config.allowedScopes !== undefined ? { allowedScopes: config.allowedScopes } : {}),
15071513 },
15081514 }));
1509− return deps.fuma
1510− .use("oauth_client.findMany", (db) => looseDb(db).findMany("oauth_client", {}))
1511− .pipe(
1512− Effect.flatMap((rows) =>
1513− Effect.forEach(rows, (row) => {
1514− const grant = parseGrant(row.grant);
1515− // EXPLICIT — a row with an unknown grant is corrupt; surface it
1516− // loudly rather than silently displaying it as authorization_code.
1517− if (grant === null) {
1518− return Effect.fail(
1519− new StorageError({
1520− message: `oauth_client ${String(row.slug)} has an unknown grant: ${String(row.grant)}`,
1521− cause: undefined,
1522− }),
1523− );
1524− }
1525− const tokenEndpointAuthMethod = parseStoredTokenEndpointAuthMethod(
1526− row.token_endpoint_auth_method,
1527− );
1528− if (tokenEndpointAuthMethod === null) {
1529− return Effect.fail(
1530− new StorageError({
1531− message: `oauth_client ${String(row.slug)} has an unknown token endpoint auth method: ${String(row.token_endpoint_auth_method)}`,
1532− cause: undefined,
1533− }),
1515+ return yield* deps.fuma
1516+ .use("oauth_client.findMany", (db) => looseDb(db).findMany("oauth_client", {}))
1517+ .pipe(
1518+ Effect.flatMap((rows) =>
1519+ Effect.forEach(rows, (row) => {
1520+ const grant = parseGrant(row.grant);
1521+ // EXPLICIT — a row with an unknown grant is corrupt; surface it
1522+ // loudly rather than silently displaying it as authorization_code.
1523+ if (grant === null) {
1524+ return Effect.fail(
1525+ new StorageError({
1526+ message: `oauth_client ${String(row.slug)} has an unknown grant: ${String(row.grant)}`,
1527+ cause: undefined,
1528+ }),
1529+ );
1530+ }
1531+ const tokenEndpointAuthMethod = parseStoredTokenEndpointAuthMethod(
1532+ row.token_endpoint_auth_method,
15341533 );
1535− }
1536− return Effect.succeed({
1537− owner: String(row.owner) as Owner,
1538− slug: OAuthClientSlug.make(String(row.slug)),
1539− grant,
1540− authorizationUrl: String(row.authorization_url),
1541− tokenUrl: String(row.token_url),
1542− resource: row.resource == null ? null : String(row.resource),
1543− clientId: String(row.client_id),
1544− ...(tokenEndpointAuthMethod === undefined ? {} : { tokenEndpointAuthMethod }),
1545− origin: parseOAuthClientOrigin(row),
1546− } satisfies OAuthClientSummary);
1547− }),
1548− ),
1549− Effect.map((stored) => [...firstPartySummaries, ...stored]),
1550− );
1551− };
1534+ if (tokenEndpointAuthMethod === null) {
1535+ return Effect.fail(
1536+ new StorageError({
1537+ message: `oauth_client ${String(row.slug)} has an unknown token endpoint auth method: ${String(row.token_endpoint_auth_method)}`,
1538+ cause: undefined,
1539+ }),
1540+ );
1541+ }
1542+ return Effect.succeed({
1543+ owner: String(row.owner) as Owner,
1544+ slug: OAuthClientSlug.make(String(row.slug)),
1545+ grant,
1546+ authorizationUrl: String(row.authorization_url),
1547+ tokenUrl: String(row.token_url),
1548+ resource: row.resource == null ? null : String(row.resource),
1549+ clientId: String(row.client_id),
1550+ ...(tokenEndpointAuthMethod === undefined ? {} : { tokenEndpointAuthMethod }),
1551+ origin: parseOAuthClientOrigin(row),
1552+ } satisfies OAuthClientSummary);
1553+ }),
1554+ ),
1555+ Effect.map((stored) => [...firstPartySummaries, ...stored]),
1556+ );
1557+ });
15521558
15531559 // -----------------------------------------------------------------------
15541560 // Load an oauth_client row by (owner, slug).

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t