Skip to content

Commit e4be2f6

Browse files
committed
Fill OAuth connection labels from account identity instead of the owner default
1 parent fc1e589 commit e4be2f6

10 files changed

Lines changed: 376 additions & 52 deletions

File tree

e2e/scenarios/google-health-checks.test.ts

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ const connectGoogleAccount = (input: {
116116
readonly target: TargetShape;
117117
readonly integration: IntegrationSlug;
118118
readonly oauthClient: OAuthClientSlug;
119+
readonly newConnection?: boolean;
119120
}) =>
120121
Effect.gen(function* () {
121122
const redirectUri = new URL("/api/oauth/callback", input.target.baseUrl).toString();
@@ -146,6 +147,25 @@ const connectGoogleAccount = (input: {
146147
},
147148
});
148149

150+
return yield* runGoogleOAuthFlow(input, redirectUri);
151+
});
152+
153+
/** Run one authorize+complete round through the already-registered app. Split
154+
* from `connectGoogleAccount` so a scenario can connect a SECOND account
155+
* through the same app (`newConnection: true`) without re-registering it. */
156+
const runGoogleOAuthFlow = (
157+
input: {
158+
readonly client: Client;
159+
readonly target: TargetShape;
160+
readonly integration: IntegrationSlug;
161+
readonly oauthClient: OAuthClientSlug;
162+
readonly newConnection?: boolean;
163+
},
164+
redirectUriOverride?: string,
165+
) =>
166+
Effect.gen(function* () {
167+
const redirectUri =
168+
redirectUriOverride ?? new URL("/api/oauth/callback", input.target.baseUrl).toString();
149169
const started = yield* input.client.oauth.start({
150170
payload: {
151171
client: input.oauthClient,
@@ -155,6 +175,7 @@ const connectGoogleAccount = (input: {
155175
integration: input.integration,
156176
template: GOOGLE_AUTH_TEMPLATE,
157177
redirectUri,
178+
...(input.newConnection === true ? { newConnection: true } : {}),
158179
},
159180
});
160181
expect(started.status, "OAuth starts with an emulator redirect").toBe("redirect");
@@ -167,6 +188,7 @@ const connectGoogleAccount = (input: {
167188
expect(completed.integration, "OAuth completion creates the connection").toBe(
168189
input.integration,
169190
);
191+
return completed;
170192
});
171193

172194
scenario(
@@ -334,13 +356,53 @@ scenario(
334356
refreshed.find((connection) => connection.name === CONNECTION)?.identityLabel,
335357
"Google Sheets still shows the OAuth grant identity after no-probe health",
336358
).toBe(GOOGLE_EMULATOR_ACCOUNT_EMAIL);
359+
360+
// Reconnecting the SAME connection must not clobber a curated label
361+
// with the grant identity.
362+
yield* client.connections.update({
363+
params: { owner: "org", integration: slug, name: CONNECTION },
364+
payload: { identityLabel: "Finance account" },
365+
});
366+
yield* runGoogleOAuthFlow({ client, target, integration: slug, oauthClient });
367+
const reconnected = yield* client.connections.list({
368+
query: { owner: "org", integration: slug },
369+
});
370+
expect(
371+
reconnected.find((connection) => connection.name === CONNECTION)?.identityLabel,
372+
"reconnect keeps the curated label over the grant identity",
373+
).toBe("Finance account");
374+
375+
// A `newConnection` connect under a taken name mints a SECOND
376+
// connection with a suffixed name instead of replacing the first.
377+
const second = yield* runGoogleOAuthFlow({
378+
client,
379+
target,
380+
integration: slug,
381+
oauthClient,
382+
newConnection: true,
383+
});
384+
expect(String(second.name), "second account mints a suffixed connection").toBe(
385+
`${String(CONNECTION)}2`,
386+
);
387+
expect(second.identityLabel, "second account label comes from the grant identity").toBe(
388+
GOOGLE_EMULATOR_ACCOUNT_EMAIL,
389+
);
390+
const both = yield* client.connections.list({
391+
query: { owner: "org", integration: slug },
392+
});
393+
expect(
394+
both.map((connection) => String(connection.name)).sort(),
395+
"both accounts coexist",
396+
).toEqual([String(CONNECTION), `${String(CONNECTION)}2`]);
337397
}),
338398
Effect.gen(function* () {
339-
yield* client.connections
340-
.remove({
341-
params: { owner: "org", integration: slug, name: CONNECTION },
342-
})
343-
.pipe(Effect.ignore);
399+
for (const name of [CONNECTION, ConnectionName.make(`${String(CONNECTION)}2`)]) {
400+
yield* client.connections
401+
.remove({
402+
params: { owner: "org", integration: slug, name },
403+
})
404+
.pipe(Effect.ignore);
405+
}
344406
yield* client.oauth
345407
.removeClient({ params: { slug: oauthClient }, payload: { owner: "org" } })
346408
.pipe(Effect.ignore);

packages/core/api/src/handlers/oauth.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ export const OAuthHandlers = HttpApiBuilder.group(ExecutorApi, "oauth", (handler
161161
integration: payload.integration,
162162
template: payload.template,
163163
identityLabel: payload.identityLabel,
164+
newConnection: payload.newConnection,
164165
redirectUri: payload.redirectUri,
165166
});
166167
return startResultToResponse(result);

packages/core/api/src/oauth/api.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ const StartPayload = Schema.Struct({
160160
integration: IntegrationSlug,
161161
template: AuthTemplateSlug,
162162
identityLabel: Schema.optional(Schema.NullOr(Schema.String)),
163+
/** Mint a NEW connection: a taken `name` resolves to the next free suffixed
164+
* name server-side instead of re-minting the existing row. */
165+
newConnection: Schema.optional(Schema.Boolean),
163166
redirectUri: Schema.optional(Schema.NullOr(Schema.String)),
164167
});
165168

packages/core/sdk/src/executor.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2489,14 +2489,22 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
24892489
integration: input.integration,
24902490
name,
24912491
};
2492+
// Label precedence: an explicit (user-chosen) label always wins; a
2493+
// derived label (OIDC claims) only FILLS an empty slot. Like
2494+
// `description` below, a reconnect or token refresh must not erase a
2495+
// label the user curated. Resolved once, used by every write below.
2496+
let identityLabel: string | null = null;
24922497
yield* transaction(
24932498
Effect.gen(function* () {
24942499
const existing = yield* findConnectionRow(ref);
2500+
const existingLabel = existing?.identity_label?.trim() ? existing.identity_label : null;
2501+
identityLabel =
2502+
input.identityLabel ?? existingLabel ?? input.derivedIdentityLabel ?? null;
24952503
const set: Record<string, unknown> = {
24962504
template: String(input.template),
24972505
provider: input.provider,
24982506
item_ids: { [PRIMARY_INPUT_VARIABLE]: input.itemId },
2499-
identity_label: input.identityLabel ?? null,
2507+
identity_label: identityLabel,
25002508
oauth_client: String(input.oauthClient),
25012509
oauth_client_owner: input.oauthClientOwner,
25022510
refresh_item_id: input.refreshItemId,
@@ -2529,7 +2537,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
25292537
template: String(input.template),
25302538
provider: input.provider,
25312539
item_ids: { [PRIMARY_INPUT_VARIABLE]: input.itemId },
2532-
identity_label: input.identityLabel ?? null,
2540+
identity_label: identityLabel,
25332541
// Curated description: never stamped by a mint — a reconnect
25342542
// or token refresh must not erase what the user wrote.
25352543
description: null,
@@ -2568,7 +2576,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
25682576
template: String(input.template),
25692577
provider: input.provider,
25702578
item_ids: { [PRIMARY_INPUT_VARIABLE]: input.itemId },
2571-
identity_label: input.identityLabel ?? null,
2579+
identity_label: identityLabel,
25722580
description: null,
25732581
oauth_client: String(input.oauthClient),
25742582
oauth_client_owner: input.oauthClientOwner,
@@ -3848,6 +3856,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
38483856
ownedKeys: (owner: Owner) => ownedKeys(owner),
38493857
defaultWritableProvider,
38503858
mintOAuthConnection: (input: MintOAuthConnectionInput) => mintOAuthConnection(input),
3859+
connectionNameTaken: (ref) => findConnectionRow(ref).pipe(Effect.map((row) => row !== null)),
38513860
// One integration-row read + one projector run. Resolve the method this
38523861
// template selects exactly as the runtime's `selectAuthMethod` does —
38533862
// exact slug match, else the sole declared method (single-method

packages/core/sdk/src/oauth-client.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,15 @@ export interface OAuthStartInput {
127127
readonly name: ConnectionName;
128128
readonly integration: IntegrationSlug;
129129
readonly template: AuthTemplateSlug;
130+
/** Display label for the minted connection. When omitted, the server derives
131+
* one from the provider's OIDC account claims (the account email) when the
132+
* grant carries them; a re-mint of an existing connection keeps its stored
133+
* label. Send a value only for a label the user actually chose. */
130134
readonly identityLabel?: string | null;
135+
/** Mint a NEW connection: when `name` is already taken, the mint picks the
136+
* next free suffixed name (`personalGmail2`) instead of re-minting the
137+
* existing row. Omit for reconnect flows, which target the existing row. */
138+
readonly newConnection?: boolean;
131139
/** Browser-facing callback URL for this flow. Defaults to the executor's configured redirectUri. */
132140
readonly redirectUri?: string | null;
133141
}

packages/core/sdk/src/oauth-flow.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,115 @@ describe("oauth.start / oauth.complete", () => {
384384
),
385385
);
386386

387+
it.effect("newConnection resolves a taken name to the next free suffix", () =>
388+
Effect.scoped(
389+
Effect.gen(function* () {
390+
const server = yield* serveOAuthTestServer({
391+
scopes: ["openid", "email", "profile", "read"],
392+
idTokenClaims: { email: "alice@example.com", sub: "user-1" },
393+
});
394+
const { executor } = yield* makeTestWorkspaceHarness({ plugins });
395+
yield* executor.acme.seed(["openid", "email", "profile", "read"]);
396+
397+
yield* executor.oauth.createClient({
398+
owner: "org",
399+
slug: CLIENT,
400+
authorizationUrl: server.authorizationEndpoint,
401+
tokenUrl: server.tokenEndpoint,
402+
grant: "authorization_code",
403+
clientId: "test-client",
404+
clientSecret: "test-secret",
405+
});
406+
407+
const runFlow = Effect.gen(function* () {
408+
const started = yield* executor.oauth.start({
409+
owner: "org",
410+
client: CLIENT,
411+
clientOwner: "org",
412+
name: ConnectionName.make("main"),
413+
integration: INTEG,
414+
template: TEMPLATE,
415+
newConnection: true,
416+
});
417+
expect(started.status).toBe("redirect");
418+
if (started.status !== "redirect") return null;
419+
const callback = yield* server.completeAuthorizationCodeFlow({
420+
authorizationUrl: started.authorizationUrl,
421+
});
422+
return yield* executor.oauth.complete({
423+
state: started.state,
424+
code: callback.code,
425+
});
426+
});
427+
428+
const first = yield* runFlow;
429+
const second = yield* runFlow;
430+
expect(String(first?.name)).toBe("main");
431+
expect(String(second?.name)).toBe("main2");
432+
433+
const connections = yield* executor.connections.list({ integration: INTEG });
434+
expect(connections.map((connection) => String(connection.name)).sort()).toEqual([
435+
"main",
436+
"main2",
437+
]);
438+
}),
439+
),
440+
);
441+
442+
it.effect("preserves a curated label when reconnecting without an explicit label", () =>
443+
Effect.scoped(
444+
Effect.gen(function* () {
445+
const server = yield* serveOAuthTestServer({
446+
scopes: ["openid", "email", "profile", "read"],
447+
idTokenClaims: { email: "alice@example.com", sub: "user-1" },
448+
});
449+
const { executor } = yield* makeTestWorkspaceHarness({ plugins });
450+
yield* executor.acme.seed(["openid", "email", "profile", "read"]);
451+
452+
yield* executor.oauth.createClient({
453+
owner: "org",
454+
slug: CLIENT,
455+
authorizationUrl: server.authorizationEndpoint,
456+
tokenUrl: server.tokenEndpoint,
457+
grant: "authorization_code",
458+
clientId: "test-client",
459+
clientSecret: "test-secret",
460+
});
461+
462+
const runFlow = Effect.gen(function* () {
463+
const started = yield* executor.oauth.start({
464+
owner: "org",
465+
client: CLIENT,
466+
clientOwner: "org",
467+
name: ConnectionName.make("main"),
468+
integration: INTEG,
469+
template: TEMPLATE,
470+
});
471+
expect(started.status).toBe("redirect");
472+
if (started.status !== "redirect") return null;
473+
const callback = yield* server.completeAuthorizationCodeFlow({
474+
authorizationUrl: started.authorizationUrl,
475+
});
476+
return yield* executor.oauth.complete({
477+
state: started.state,
478+
code: callback.code,
479+
});
480+
});
481+
482+
const first = yield* runFlow;
483+
expect(first?.identityLabel).toBe("alice@example.com");
484+
485+
// The user renames the connection, then reconnects (no typed label).
486+
yield* executor.connections.update(
487+
{ owner: "org", integration: INTEG, name: ConnectionName.make("main") },
488+
{ identityLabel: "Finance account" },
489+
);
490+
const second = yield* runFlow;
491+
expect(second?.identityLabel).toBe("Finance account");
492+
}),
493+
),
494+
);
495+
387496
it.effect("does not overwrite connection identity from id_token claims on refresh", () =>
388497
Effect.scoped(
389498
Effect.gen(function* () {

0 commit comments

Comments
 (0)