Skip to content

Commit ae8cb60

Browse files
committed
Don't nag reconnect for spec-derived OAuth scope catalogs
An OpenAPI source declares the full per-operation scope catalog (PostHog declares ~200). Those are requested broadly to unlock as many tools as possible, but a provider that narrows the grant to the user's actual access is healthy, not in need of reconnect. Only treat custom (user-configured) scopes as required, never the spec catalog, via reconsentRequiredScopes. Also fixes a garbled reconnect message in the accounts list.
1 parent 88b7008 commit ae8cb60

3 files changed

Lines changed: 136 additions & 26 deletions

File tree

packages/react/src/components/accounts-section.tsx

Lines changed: 68 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { useEffect, useMemo, useState } from "react";
22
import { useAtomValue, useAtomSet } from "@effect/atom-react";
33
import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
44
import * as Exit from "effect/Exit";
5-
import { IntegrationSlug, type Connection, type Owner } from "@executor-js/sdk/shared";
5+
import {
6+
IntegrationSlug,
7+
type Connection,
8+
type Owner,
9+
} from "@executor-js/sdk/shared";
610
import type { IntegrationAccountHandoff } from "@executor-js/sdk/client";
711
import { toast } from "sonner";
812

@@ -22,6 +26,7 @@ import {
2226
connectionNeedsReconsent,
2327
oauthReconnectPayload,
2428
reconnectMode,
29+
reconsentRequiredScopes,
2530
} from "../plugins/oauth-reconnect";
2631
import { useOAuthPopupFlow } from "../plugins/oauth-sign-in";
2732
import { AddAccountModal } from "./add-account-modal";
@@ -79,7 +84,10 @@ function AccountRow(props: {
7984
<CardStackEntryTitle className="flex min-w-0 items-center gap-2">
8085
<span className="truncate">{displayLabel}</span>
8186
{needsReconsent ? (
82-
<Badge variant="outline" className="shrink-0 border-border text-muted-foreground">
87+
<Badge
88+
variant="outline"
89+
className="shrink-0 border-border text-muted-foreground"
90+
>
8391
Reconnect to grant access
8492
</Badge>
8593
) : null}
@@ -91,7 +99,8 @@ function AccountRow(props: {
9199
) : null}
92100
{needsReconsent ? (
93101
<CardStackEntryDescription className="mt-1 text-xs text-muted-foreground">
94-
This integration now needs access this connection wasn't granted.
102+
This connection wasn't granted all the access this integration now
103+
needs.
95104
</CardStackEntryDescription>
96105
) : null}
97106
</CardStackEntryContent>
@@ -120,7 +129,11 @@ function AccountRow(props: {
120129
<DropdownMenuItem className="text-sm" onClick={props.onReconnect}>
121130
Reconnect
122131
</DropdownMenuItem>
123-
<DropdownMenuItem variant="destructive" className="text-sm" onClick={props.onRemove}>
132+
<DropdownMenuItem
133+
variant="destructive"
134+
className="text-sm"
135+
onClick={props.onRemove}
136+
>
124137
Remove
125138
</DropdownMenuItem>
126139
</DropdownMenuContent>
@@ -142,7 +155,9 @@ function OwnerAccounts(props: {
142155
readonly declaredScopes: readonly string[] | undefined;
143156
}) {
144157
const { integration, owner } = props;
145-
const connections = useAtomValue(connectionsForIntegrationAtom({ integration, owner }));
158+
const connections = useAtomValue(
159+
connectionsForIntegrationAtom({ integration, owner }),
160+
);
146161
const doRemove = useAtomSet(removeConnectionOptimistic(owner), {
147162
mode: "promiseExit",
148163
});
@@ -159,7 +174,9 @@ function OwnerAccounts(props: {
159174
startErrorMessage: "Failed to reconnect",
160175
});
161176

162-
const rows: readonly Connection[] = AsyncResult.isSuccess(connections) ? connections.value : [];
177+
const rows: readonly Connection[] = AsyncResult.isSuccess(connections)
178+
? connections.value
179+
: [];
163180
if (rows.length === 0) return null;
164181

165182
const handleReconnect = async (connection: Connection) => {
@@ -168,7 +185,8 @@ function OwnerAccounts(props: {
168185
if (reconnectMode(connection) === "oauth") {
169186
const method = props.methods.find(
170187
(candidate: AuthMethod) =>
171-
candidate.kind === "oauth" && String(candidate.template) === String(connection.template),
188+
candidate.kind === "oauth" &&
189+
String(candidate.template) === String(connection.template),
172190
);
173191
if (
174192
method?.oauth?.supportsDynamicRegistration === true ||
@@ -275,13 +293,18 @@ function OwnerAccounts(props: {
275293

276294
return (
277295
<CardStack>
278-
{props.showOwnerLabels ? <CardStackHeader>{ownerLabel(owner)}</CardStackHeader> : null}
296+
{props.showOwnerLabels ? (
297+
<CardStackHeader>{ownerLabel(owner)}</CardStackHeader>
298+
) : null}
279299
<CardStackContent>
280300
{rows.map((connection: Connection) => (
281301
<AccountRow
282302
key={`${connection.owner}:${connection.integration}:${connection.name}`}
283303
connection={connection}
284-
needsReconsent={connectionNeedsReconsent(connection, props.declaredScopes)}
304+
needsReconsent={connectionNeedsReconsent(
305+
connection,
306+
props.declaredScopes,
307+
)}
285308
showOwnerLabel={props.showOwnerLabels}
286309
onEdit={() => props.onEdit(connection)}
287310
onReconnect={() => void handleReconnect(connection)}
@@ -312,10 +335,14 @@ export function AccountsSection(props: {
312335
removeCustomMethod,
313336
} = props;
314337
const [adding, setAdding] = useState(false);
315-
const [editingConnection, setEditingConnection] = useState<Connection | null>(null);
316-
const [reconnectHandoff, setReconnectHandoff] = useState<IntegrationAccountHandoff | null>(null);
338+
const [editingConnection, setEditingConnection] = useState<Connection | null>(
339+
null,
340+
);
341+
const [reconnectHandoff, setReconnectHandoff] =
342+
useState<IntegrationAccountHandoff | null>(null);
317343
const ownerDisplay = useOwnerDisplay();
318-
const canAddConnection = methods.length > 0 || createCustomMethod !== undefined;
344+
const canAddConnection =
345+
methods.length > 0 || createCustomMethod !== undefined;
319346

320347
useEffect(() => {
321348
if (accountHandoff) {
@@ -326,11 +353,20 @@ export function AccountsSection(props: {
326353
// The integration's declared oauth scopes — what connections need granted. A
327354
// connection granted fewer is flagged to reconnect (e.g. after a service was
328355
// added widened the consent).
329-
const declaredScopes = methods.find((m: AuthMethod) => m.kind === "oauth")?.oauth?.scopes;
356+
//
357+
// Spec-derived oauth scopes are the full per-operation catalog union (e.g. an
358+
// OpenAPI source like PostHog declares hundreds of scopes). Those are requested
359+
// broadly but not individually required: a provider that narrows the grant to
360+
// the user's actual access is healthy, not in need of reconnect. So only treat
361+
// CUSTOM (user-configured) scopes as required here; never the spec catalog.
362+
const oauthMethod = methods.find((m: AuthMethod) => m.kind === "oauth");
363+
const declaredScopes = reconsentRequiredScopes(oauthMethod);
330364

331365
// Read both owners to decide between the grouped view and the empty CTA. The
332366
// grouped sub-components re-read these (effect-atom dedupes) and self-hide.
333-
const orgConnections = useAtomValue(connectionsForIntegrationAtom({ integration, owner: "org" }));
367+
const orgConnections = useAtomValue(
368+
connectionsForIntegrationAtom({ integration, owner: "org" }),
369+
);
334370
const userConnections = useAtomValue(
335371
connectionsForIntegrationAtom({ integration, owner: "user" }),
336372
);
@@ -341,12 +377,18 @@ export function AccountsSection(props: {
341377
useAtomSet(addConnectionOptimistic("user"));
342378

343379
const totalCount = useMemo(() => {
344-
const orgRows = AsyncResult.isSuccess(orgConnections) ? orgConnections.value.length : 0;
345-
const userRows = AsyncResult.isSuccess(userConnections) ? userConnections.value.length : 0;
380+
const orgRows = AsyncResult.isSuccess(orgConnections)
381+
? orgConnections.value.length
382+
: 0;
383+
const userRows = AsyncResult.isSuccess(userConnections)
384+
? userConnections.value.length
385+
: 0;
346386
return orgRows + userRows;
347387
}, [orgConnections, userConnections]);
348388

349-
const loading = !AsyncResult.isSuccess(orgConnections) && !AsyncResult.isSuccess(userConnections);
389+
const loading =
390+
!AsyncResult.isSuccess(orgConnections) &&
391+
!AsyncResult.isSuccess(userConnections);
350392

351393
const modalState = reconnectHandoff ?? accountHandoff;
352394
const modal = (
@@ -378,7 +420,9 @@ export function AccountsSection(props: {
378420
onClick={() => {
379421
trackEvent("connection_add_opened", {
380422
integration_slug: String(integration),
381-
has_oauth_method: methods.some((m: AuthMethod) => m.kind === "oauth"),
423+
has_oauth_method: methods.some(
424+
(m: AuthMethod) => m.kind === "oauth",
425+
),
382426
has_api_key_method: methods.some(
383427
(m: AuthMethod) => m.kind !== "oauth" && m.kind !== "none",
384428
),
@@ -398,7 +442,9 @@ export function AccountsSection(props: {
398442
</div>
399443
) : totalCount === 0 ? (
400444
<div className="rounded-lg border border-dashed border-border/60 px-6 py-8 text-center">
401-
<p className="text-sm font-medium text-foreground">No connections yet</p>
445+
<p className="text-sm font-medium text-foreground">
446+
No connections yet
447+
</p>
402448
<p className="mt-1 text-sm text-muted-foreground">
403449
Add a connection to make this integration's tools available.
404450
</p>
@@ -409,7 +455,9 @@ export function AccountsSection(props: {
409455
onClick={() => {
410456
trackEvent("connection_add_opened", {
411457
integration_slug: String(integration),
412-
has_oauth_method: methods.some((m: AuthMethod) => m.kind === "oauth"),
458+
has_oauth_method: methods.some(
459+
(m: AuthMethod) => m.kind === "oauth",
460+
),
413461
has_api_key_method: methods.some(
414462
(m: AuthMethod) => m.kind !== "oauth" && m.kind !== "none",
415463
),

packages/react/src/plugins/oauth-reconnect.test.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ import {
99
type Connection,
1010
} from "@executor-js/sdk/shared";
1111

12-
import { missingScopes, oauthReconnectPayload, reconnectMode } from "./oauth-reconnect";
12+
import {
13+
missingScopes,
14+
oauthReconnectPayload,
15+
reconnectMode,
16+
reconsentRequiredScopes,
17+
} from "./oauth-reconnect";
1318

1419
const connection = (overrides: Partial<Connection> = {}): Connection => ({
1520
owner: "user",
@@ -109,3 +114,29 @@ describe("missingScopes (Part 2 informational subset warning)", () => {
109114
).toEqual([]);
110115
});
111116
});
117+
118+
describe("reconsentRequiredScopes", () => {
119+
it("treats spec-derived oauth scopes as NOT required (opportunistic catalog)", () => {
120+
// An OpenAPI source (e.g. PostHog) declares the full per-operation scope
121+
// catalog. A narrower grant is healthy and must not nag for reconnect.
122+
expect(
123+
reconsentRequiredScopes({
124+
source: "spec",
125+
oauth: { scopes: ["insight:read", "insight:write", "person:read"] },
126+
}),
127+
).toBeUndefined();
128+
});
129+
130+
it("keeps custom (user-configured) oauth scopes required", () => {
131+
expect(
132+
reconsentRequiredScopes({
133+
source: "custom",
134+
oauth: { scopes: ["read", "write"] },
135+
}),
136+
).toEqual(["read", "write"]);
137+
});
138+
139+
it("returns undefined when there is no oauth method", () => {
140+
expect(reconsentRequiredScopes(undefined)).toBeUndefined();
141+
});
142+
});

packages/react/src/plugins/oauth-reconnect.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export function reconnectMode(connection: Connection): ReconnectMode {
2828
* connection are exactly the branded types `oauth.start` expects, so the same
2929
* connection (owner/integration/name) is re-minted with a fresh refresh token
3030
* and the widened scope union. Returns null for a non-OAuth connection. */
31-
export function oauthReconnectPayload(connection: Connection): OAuthStartPayload | null {
31+
export function oauthReconnectPayload(
32+
connection: Connection,
33+
): OAuthStartPayload | null {
3234
if (connection.oauthClient == null) return null;
3335
return {
3436
client: connection.oauthClient,
@@ -59,7 +61,8 @@ const OAUTH_SCOPE_ALIASES: Readonly<Record<string, string>> = {
5961
"https://www.googleapis.com/auth/userinfo.profile": "profile",
6062
};
6163

62-
const canonicalScope = (scope: string): string => OAUTH_SCOPE_ALIASES[scope] ?? scope;
64+
const canonicalScope = (scope: string): string =>
65+
OAUTH_SCOPE_ALIASES[scope] ?? scope;
6366

6467
/** Normalize a scope list: trim, canonicalize known provider aliases, drop
6568
* empties, de-dupe (order-preserving). A scope set is compared as a SET —
@@ -91,8 +94,12 @@ export function missingScopes(
9194

9295
/** The scopes a connection was actually GRANTED, parsed from its space-delimited
9396
* `oauthScope` record. Empty for static creds / when the AS omitted scope. */
94-
export function connectionGrantedScopes(connection: Connection): readonly string[] {
95-
return connection.oauthScope ? connection.oauthScope.split(/\s+/).filter(Boolean) : [];
97+
export function connectionGrantedScopes(
98+
connection: Connection,
99+
): readonly string[] {
100+
return connection.oauthScope
101+
? connection.oauthScope.split(/\s+/).filter(Boolean)
102+
: [];
96103
}
97104

98105
/** Whether an OAuth connection must RECONNECT to grant newly-needed access: the
@@ -106,5 +113,29 @@ export function connectionNeedsReconsent(
106113
declaredScopes: readonly string[] | undefined,
107114
): boolean {
108115
if (connection.oauthClient == null) return false;
109-
return missingScopes(declaredScopes, connectionGrantedScopes(connection)).length > 0;
116+
return (
117+
missingScopes(declaredScopes, connectionGrantedScopes(connection)).length >
118+
0
119+
);
120+
}
121+
122+
/** The scopes that count as REQUIRED for a connection to be considered fully
123+
* granted, given the integration's oauth auth method.
124+
*
125+
* Spec-derived oauth scopes are the full per-operation catalog union (an OpenAPI
126+
* source like PostHog declares hundreds): requested broadly to unlock as many
127+
* tools as possible, but none individually required. A provider that narrows the
128+
* grant to the user's actual access is healthy, so the spec catalog must NOT
129+
* drive a reconnect prompt. Custom (user-configured) scopes are intentional and
130+
* stay required. Feed the result to `connectionNeedsReconsent`. */
131+
export function reconsentRequiredScopes(
132+
method:
133+
| {
134+
readonly source: "spec" | "custom";
135+
readonly oauth?: { readonly scopes?: readonly string[] };
136+
}
137+
| undefined,
138+
): readonly string[] | undefined {
139+
if (method == null || method.source === "spec") return undefined;
140+
return method.oauth?.scopes;
110141
}

0 commit comments

Comments
 (0)