Skip to content

Commit c779611

Browse files
feat(automation): connector connection read answers redirectUri; a client-less reconnect reuses the kept installer client (#29701)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8NQ1AWCdjDL4Yzk8V1Cf7d6)_ Fixes #29463 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - 改了什么:连接器连接读取接口(`GET /api/v1/automation/connectors/NAME/connection`)现在返回本部署的 OAuth 回调地址 `redirectUri`;安装方自带客户端的实例,重连时不填客户端就沿用已保存的那个。 - 为什么改:管理员要先把回调地址登记到厂商那里才能拿到客户端,控制台此前读不到这个地址;重连时控制台也拿不回已存的密钥,只能让人重新输入。 - 风险与回滚:只读接口多一个可选字段,没有配置基础地址时字段缺省、不报错;发起连接的 503 不变。重连沿用仅限同一组织的已存客户端,撤销后不再沿用。回滚即撤回本 PR。 - 先看:`packages/services/service-automation/src/plugin.ts` 的 `derivedConnectorOAuthRedirectUri`、`getConnectorConnection`、`keptInstallerClient` 三处;其次 `connector-connection-door.test.ts` 新增的四个用例。 - 两分钟验证:`pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/connector-connection-door.test.ts`,预期 34 个用例全部通过。 - 需要你做的:无,席位落地。 ## What changes **Ask 1: the connection read answers `redirectUri`.** `ServiceAutomationPlugin` gets one derivation, `derivedConnectorOAuthRedirectUri()`: the `connectorOAuthRedirectUri` plugin option, else `/api/v1/automation/connectors/oauth/callback` under `OS_AUTH_URL`, else under `OS_BASE_URL`, else `undefined`. `connectorOAuthRedirectUri()` (the begin path) now calls it and keeps its `503` byte-for-byte when it answers `undefined`. `getConnectorConnection` sets `redirectUri` on its answer whenever the derivation answers, whatever `status` says, and leaves the key out otherwise (never a refusal, never the request's origin), as the `ConnectorConnectionState.redirectUri` TSDoc landed by #29618 promises. A `client_credentials` instance has no connection: its read stays the existing `400 VALIDATION_ERROR`, so it never carries the key. The revoke and callback answers are unchanged. **Ask 2: a client-less reconnect reuses the kept installer client.** `installerClientOfBegin` is now async and takes the organization: a begin naming no client on a `clientSource: 'installer'` instance reads the organization's connection (`keptInstallerClient`) and, when it keeps an installer client, authorizes, exchanges and re-stores as that client. With none kept (a first connect, another organization, or after a revoke) it is refused `400` with the same text as before. A begin naming a client replaces the kept one, as before. A store that cannot read or decrypt the kept client refuses with the store's existing `503`, never "keeps none". **Docs:** `content/docs/automation/connectors.mdx` names `redirectUri` on the status read (the one place a client, the Console included, reads it from) and says a reconnect keeps the stored client unless it brings a new one. ## Why An administrator registering a vendor OAuth client for a `clientSource: 'installer'` instance needs this deployment's redirect URI before any client exists, and the Console could only guess it from `window.location`. And the Console never reads a stored secret back, so a reconnect had to re-enter it (card Ask 2, ruling B of #28702). ## Tests - `connector-connection-door.test.ts` (service-automation), 34/34: - new: the read answers the option, then `OS_AUTH_URL` over `OS_BASE_URL`; no base (and a scheme-less base) leaves the key out while the read still answers and a begin is still `503`; the URI shown equals the `redirect_uri` a connect names; a connected read still carries it. - new: a `client_credentials` instance's read is refused `400 VALIDATION_ERROR` and carries no redirect URI. - new: a reconnect without a client after a connect with one names the stored `client_id`, exchanges as it (HTTP Basic), re-stores the same encrypted client at revision 2; another organization connecting first, and the same organization after a revoke, are still refused `400`. - new: a kept public client is reused as it is (id in the form, no secret). - updated: the first test's not-connected read now also expects `redirectUri` (the boot passes the plugin option). The existing tests pinning the old refusal ("a connect without a client is refused 400") and replacement ("a reconnect replaces it") are unchanged and still pass: Ask 2 changes neither. - `connector-installer-oauth-client.dogfood.test.ts` (real HTTP door on the showcase), 4/4: SERVED asserts the read's `redirectUri` is the deployment origin plus the callback path; CONNECTED asserts the begin's `redirect_uri` equals the read's `redirectUri`, and that a `POST` with no body after the connect is `200` naming the stored `client_id`, never the secret. ## Acceptance notes - The `ConnectorConnectionState.redirectUri` TSDoc in `packages/spec/src/contracts/automation-service.ts` still says "Declared ahead of the service that answers it: until the automation service sets it, no answer carries it." That sentence is false once this lands; spec edits are `domain:spec`'s, so it is left to that lane (also the matching bullet in the unreleased `.changeset/29574-connector-redirect-uri.md`). - `redirectUri` is set on the read leg only, as the TSDoc names; the revoke and callback answers do not carry it. - With no data engine, a client-less begin on an installer instance now answers the store's `503` instead of the `400` (it can no longer tell whether a client is kept). - The Console does not show the URI yet: #28869 item 2 is the consumer. ## Verification <details><summary>Local runs at d7e2726</summary> - `pnpm --filter @objectstack/service-automation typecheck`: exit 0. - `pnpm --filter @objectstack/service-automation test`: Test Files 262 passed, Tests 3398 passed, lock VERDICT command-exit 0. - Single dogfood file under the lock after building the dogfood closure: Test Files 1 passed, Tests 4 passed. - Ablation (via `scripts/ablation-replace.mjs`, src-resolved relative import, restored by the tool, `git diff HEAD` empty after): read leg `return state;` turns 2 tests red (the first test and the precedence test); `if (kept) return kept;` replaced by `void kept;` turns both reuse tests red. - `dispatch-gates --commands`: 90 families, all exit 0 (three first answered exit 3 PREREQUISITE and were re-run green after `git fetch origin 621a487` and building client-react, studio, embedder-openai, knowledge-memory, knowledge-ragflow, service-cluster-redis and service-knowledge); `--ran` reconciliation: 90 derived, 90 run, 0 NOT-MEASURED, 0 UNRUN. - `pnpm lint`: exit 0. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. Card thread session_01JQwxbXC31EnuxG3osEW4Fc, dispatched by `domain:server#1` · os-warren. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JQwxbXC31EnuxG3osEW4Fc --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbXC31EnuxG3osEW4Fc)_ <!-- signed: domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4898162 commit c779611

5 files changed

Lines changed: 196 additions & 24 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
feat(automation): the connector connection read answers the deployment's OAuth `redirectUri`, and a client-less reconnect of an installer-client instance reuses the kept client
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
Two answers an administrator connecting an `oauth2` `authorization_code` instance with `clientSource: 'installer'` needs from the connector connection door (`/api/v1/automation/connectors/NAME/connection`):
11+
12+
- **`GET` answers `redirectUri`.** The connection read (`getConnectorConnection`) now sets `ConnectorConnectionState.redirectUri`, the redirect URI the administrator registers with the vendor before any client exists, whatever the connection's `status`. It is the same derivation a begun connect uses: the automation plugin's `connectorOAuthRedirectUri` option, else `/api/v1/automation/connectors/oauth/callback` under `OS_AUTH_URL`, else under `OS_BASE_URL`. With no base to derive it from, the key is absent and the read still answers its status; it is never guessed from the request. A begun connect in that case is refused `503` as before.
13+
- **A reconnect without a client keeps the stored one.** A `POST` that names no client, on an instance that takes the installer's client, now reuses the client the organization's connection keeps (the secret is never read back, so the Console cannot resend it). With no kept client (a first connect, or after a revoke) it is refused `400 VALIDATION_ERROR` as before, and a `POST` that names a client replaces the kept one as before.

‎content/docs/automation/connectors.mdx‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ a rotating refresh token is never spent twice.
684684
it holds.
685685

686686
```http
687+
GET /api/v1/automation/connectors/:name/connection → { status, …, redirectUri }
687688
POST /api/v1/automation/connectors/:name/connection → { authorizationUrl, expiresAt }
688689
GET /api/v1/automation/connectors/oauth/callback?code=…&state=… (the vendor redirects here)
689690
DELETE /api/v1/automation/connectors/:name/connection → revoke
@@ -692,13 +693,20 @@ a rotating refresh token is never spent twice.
692693
An instance declaring `clientSource: 'installer'` takes its client in the
693694
connect's body, `{ "clientId": "…", "clientSecret": "…" }` (omit
694695
`clientSecret` for a public client). Register that client with the vendor
695-
first, with the redirect URI `<origin>/api/v1/automation/connectors/oauth/callback`.
696-
The authorization request, the code exchange and every refresh of that
697-
organization's connection then go out as that client, and a reconnect may
698-
bring a new one. The connection keeps it encrypted; the status read shows
699-
`client: { source: 'installer', clientId, hasSecret }`, never the secret. A
700-
connect of such an instance without a client, a client offered to any other
701-
instance, and any other body key are refused `400 VALIDATION_ERROR`.
696+
first, with the redirect URI `<origin>/api/v1/automation/connectors/oauth/callback`:
697+
the status read answers that exact URI as `redirectUri` before any client
698+
exists. That is the one place a client, the Console included, reads it
699+
from, never the browser's own origin (wrong behind a proxy, or when
700+
`OS_AUTH_URL` / `OS_BASE_URL` name another origin). The authorization request, the code exchange and every
701+
refresh of that organization's connection then go out as that client. A
702+
reconnect keeps the stored client unless it brings a new one: a connect
703+
that names no client reuses the one the organization's connection keeps
704+
(the secret is never read back, so it need not be entered again), and one
705+
that names a client replaces it. The connection keeps it encrypted; the
706+
status read shows `client: { source: 'installer', clientId, hasSecret }`,
707+
never the secret. A first connect of such an instance without a client (or
708+
one after a revoke, which removes the kept client), a client offered to any
709+
other instance, and any other body key are refused `400 VALIDATION_ERROR`.
702710

703711
**In the Console**, **Setup → Integrations → Connector Connections** drives
704712
these calls; the entry is shown to holders of `manage_connector_connections`.
@@ -710,7 +718,11 @@ a rotating refresh token is never spent twice.
710718
revokes after a confirmation.
711719

712720
`GET …/:name/connection` answers the organization's status
713-
(`not_connected`, `connected`, `reconnect_required`), never a token. The
721+
(`not_connected`, `connected`, `reconnect_required`), never a token, and,
722+
whatever the status, `redirectUri`: the deployment's redirect URI to
723+
register with the vendor. It is absent when the deployment has no base to
724+
derive it from (the read still answers; a connect is then refused `503`),
725+
never guessed from the request. The
714726
`state` and the PKCE verifier are bound to the user and organization that
715727
began the connect and are used once; a callback from anyone else is refused
716728
before its code is exchanged. While the connect waits (ten minutes at most)

‎packages/qa/dogfood/test/connector-installer-oauth-client.dogfood.test.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@
1111
// - ⛔ REFUSED: a begin with no client, and a begin body carrying a key other
1212
// than `clientId` / `clientSecret`, answer `400 VALIDATION_ERROR` and store
1313
// nothing; no refusal repeats a value the caller sent.
14+
// - SERVED: the connection's read answers `redirectUri`, the URI to register
15+
// with the vendor before any client exists, derived from the deployment's
16+
// fixed origin; a connect then names exactly that URI.
1417
// - CONNECTED: a begin with the administrator's client sends the browser to
1518
// the vendor as that client; the callback exchanges the code as it (its
1619
// secret in HTTP Basic); the connection's read shows which client is set,
1720
// never its secret; the stored row keeps the id in plain text and the
18-
// secret only behind the masked column; revoke removes it.
21+
// secret only behind the masked column; a reconnect naming no client
22+
// reuses the kept one; revoke removes it.
1923
//
2024
// Every credential below is an obviously-fake placeholder, and every
2125
// assertion on the secret is its absence from what a door served.
@@ -131,7 +135,12 @@ describe("connector connection door: an `oauth2` instance that takes the install
131135
expect(JSON.stringify(entry)).not.toContain('clientId');
132136
const read = await stack.apiAs(admin, 'GET', door);
133137
expect(read.status, await read.clone().text()).toBe(200);
134-
expect(dataOf(await read.json())).toMatchObject({ connector: NAME, status: 'not_connected' });
138+
const state = dataOf(await read.json());
139+
expect(state).toMatchObject({ connector: NAME, status: 'not_connected' });
140+
// The URI an administrator registers with the vendor, answered before any
141+
// client exists: the deployment's origin plus the callback path.
142+
const origin = String(process.env.OS_AUTH_URL || process.env.OS_BASE_URL).trim().replace(/\/+$/, '');
143+
expect(state.redirectUri).toBe(`${origin}/api/v1/automation/connectors/oauth/callback`);
135144
});
136145

137146
it('⛔ a begin with no client is refused 400 VALIDATION_ERROR, naming the vendor registration step; nothing is stored', async () => {
@@ -164,6 +173,9 @@ describe("connector connection door: an `oauth2` instance that takes the install
164173
const authorizationUrl = new URL(dataOf(JSON.parse(beginText)).authorizationUrl);
165174
expect(authorizationUrl.origin + authorizationUrl.pathname).toBe(`${idpUrl}/authorize`);
166175
expect(authorizationUrl.searchParams.get('client_id')).toBe(INSTALLER_ID);
176+
// The connect names the very redirect URI the read showed.
177+
const shown = dataOf(await (await stack.apiAs(admin, 'GET', door)).json()).redirectUri;
178+
expect(authorizationUrl.searchParams.get('redirect_uri')).toBe(shown);
167179
const state = authorizationUrl.searchParams.get('state');
168180
expect(state).toBeTruthy();
169181

@@ -193,6 +205,14 @@ describe("connector connection door: an `oauth2` instance that takes the install
193205
expect(row).toMatchObject({ connector_name: NAME, client_id: INSTALLER_ID });
194206
expect(JSON.stringify(row)).not.toContain(INSTALLER_SECRET);
195207

208+
// A reconnect naming no client reuses the kept one: the Console never
209+
// reads the secret back, so it could never resend it.
210+
const reconnect = await stack.apiAs(admin, 'POST', door);
211+
const reconnectText = await reconnect.text();
212+
expect(reconnect.status, reconnectText.slice(0, 400)).toBe(200);
213+
expect(reconnectText).not.toContain(INSTALLER_SECRET);
214+
expect(new URL(dataOf(JSON.parse(reconnectText)).authorizationUrl).searchParams.get('client_id')).toBe(INSTALLER_ID);
215+
196216
const revoked = await stack.apiAs(admin, 'DELETE', door);
197217
expect(revoked.status, await revoked.clone().text()).toBe(200);
198218
expect(dataOf(await revoked.json())).toMatchObject({ connector: NAME, status: 'not_connected' });

‎packages/services/service-automation/src/connector-connection-door.test.ts‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ describe('ADR-0143 D4/D5 — connect an instance nobody authorized yet', () => {
288288
expect(provider.state.materialized).toHaveLength(0);
289289
expect(engine.getConnectorDegradedReason('crm')).toContain("connector instance 'crm' is not connected");
290290
expect(await engine.getConnectorConnection({ connector: 'crm', context: ADMIN_A as never }))
291-
.toEqual({ connector: 'crm', organizationId: 'org_a', status: 'not_connected' });
291+
.toEqual({ connector: 'crm', organizationId: 'org_a', status: 'not_connected', redirectUri: REDIRECT_URI });
292292

293293
const { begun, done } = await connect();
294294
const url = new URL(begun.authorizationUrl);
@@ -440,6 +440,54 @@ describe('ADR-0143 D4/D5 — connect an instance nobody authorized yet', () => {
440440
if (saved.b === undefined) delete process.env.OS_BASE_URL; else process.env.OS_BASE_URL = saved.b;
441441
}
442442
});
443+
it('the read answers the redirect URI to register with the vendor: the option, else OS_AUTH_URL, else OS_BASE_URL; with no base it is absent, never a refusal', async () => {
444+
const CALLBACK = '/api/v1/automation/connectors/oauth/callback';
445+
const saved = { a: process.env.OS_AUTH_URL, b: process.env.OS_BASE_URL };
446+
delete process.env.OS_AUTH_URL;
447+
delete process.env.OS_BASE_URL;
448+
try {
449+
const { engine, connect } = await boot([crm()], { redirectUri: null });
450+
const read = () => engine.getConnectorConnection({ connector: 'crm', context: ADMIN_A as never });
451+
452+
// No base: the status still answers, the key is absent (not a
453+
// guess, not a 503) — and a begin is still the 503 it was.
454+
const bare = await read();
455+
expect(bare).toEqual({ connector: 'crm', organizationId: 'org_a', status: 'not_connected' });
456+
expect('redirectUri' in bare).toBe(false);
457+
await expect(engine.beginConnectorConnection({ connector: 'crm', context: ADMIN_A as never }))
458+
.rejects.toMatchObject({ status: 503, code: 'SERVICE_UNAVAILABLE' });
459+
// A base that is no absolute http(s) URL derives nothing either.
460+
process.env.OS_BASE_URL = 'os.example.com';
461+
expect('redirectUri' in (await read())).toBe(false);
462+
463+
process.env.OS_BASE_URL = 'https://base.example.com/';
464+
expect((await read()).redirectUri).toBe(`https://base.example.com${CALLBACK}`);
465+
process.env.OS_AUTH_URL = 'https://auth.example.com';
466+
expect((await read()).redirectUri).toBe(`https://auth.example.com${CALLBACK}`);
467+
468+
// The URI the read shows is the one a connect names, and a
469+
// connected read still answers it.
470+
const { begun, done } = await connect();
471+
expect(new URL(begun.authorizationUrl).searchParams.get('redirect_uri')).toBe(`https://auth.example.com${CALLBACK}`);
472+
await done();
473+
expect(await read()).toMatchObject({ status: 'connected', redirectUri: `https://auth.example.com${CALLBACK}` });
474+
475+
// The plugin option outranks both variables.
476+
const { engine: optioned } = await boot([crm()]);
477+
expect((await optioned.getConnectorConnection({ connector: 'crm', context: ADMIN_A as never })).redirectUri).toBe(REDIRECT_URI);
478+
} finally {
479+
if (saved.a === undefined) delete process.env.OS_AUTH_URL; else process.env.OS_AUTH_URL = saved.a;
480+
if (saved.b === undefined) delete process.env.OS_BASE_URL; else process.env.OS_BASE_URL = saved.b;
481+
}
482+
});
483+
484+
it('⛔ a client_credentials instance has no connection, so its read is refused 400 and answers no redirect URI', async () => {
485+
const clientCredentials = crm({ name: 'svc', auth: { ...crm().auth, grant: 'client_credentials', authorizationUrl: undefined } });
486+
const { engine } = await boot([crm(), clientCredentials]);
487+
const err = (await engine.getConnectorConnection({ connector: 'svc', context: ADMIN_A as never }).catch((e: unknown) => e)) as Record<string, unknown>;
488+
expect(err).toMatchObject({ status: 400, code: 'VALIDATION_ERROR', message: expect.stringContaining('does not use the oauth2 authorization_code grant') });
489+
expect(String(err.message)).not.toContain('oauth/callback');
490+
});
443491
});
444492

445493
// ─── D4 dispatch ─────────────────────────────────────────────────────
@@ -732,6 +780,42 @@ describe("ADR-0143 D2.1 — an instance that takes the installer's OAuth client"
732780
.toEqual({ source: 'installer', clientId: 'installer-client-a2', hasSecret: false });
733781
});
734782

783+
it("a reconnect without a client reuses the one the organization's connection keeps; another organization's, or a revoked one, is never reused", async () => {
784+
const { engine, ql } = await boot([installerCrm()]);
785+
await (await connectWith(engine, INSTALLER_CLIENT)).done();
786+
787+
// Reconnect naming no client: no 400 — the authorization request, the
788+
// code exchange and the stored row all use the kept client.
789+
const { begun, done } = await connectWith(engine, undefined);
790+
expect(new URL(begun.authorizationUrl).searchParams.get('client_id')).toBe(INSTALLER_ID);
791+
expect(JSON.stringify(begun)).not.toContain(INSTALLER_SECRET);
792+
expect(await done()).toMatchObject({ status: 'connected', client: { source: 'installer', clientId: INSTALLER_ID, hasSecret: true } });
793+
expect(idp.requests).toHaveLength(2);
794+
expect(idp.requests[1].authorization).toBe(basicOf(INSTALLER_ID, INSTALLER_SECRET));
795+
const [row] = await rowsOf(ql);
796+
expect(row).toMatchObject({ organization_id: 'org_a', client_id: INSTALLER_ID, revision: 2 });
797+
expect(JSON.parse(await (ql as any).resolveSecretField(CONNECTOR_CONNECTION_OBJECT, row.id, 'registered_client')))
798+
.toEqual({ source: 'installer', clientId: INSTALLER_ID, clientSecret: INSTALLER_SECRET });
799+
800+
// The client belongs to org_a's connection: org_b, connecting first, is refused as before.
801+
await expect(engine.beginConnectorConnection({ connector: 'crm', context: ADMIN_B as never }))
802+
.rejects.toMatchObject({ status: 400, code: 'VALIDATION_ERROR', message: expect.stringContaining('Register an OAuth client') });
803+
// Revoked, nothing is kept to reuse.
804+
await engine.revokeConnectorConnection({ connector: 'crm', context: ADMIN_A as never });
805+
await expect(engine.beginConnectorConnection({ connector: 'crm', context: ADMIN_A as never }))
806+
.rejects.toMatchObject({ status: 400, code: 'VALIDATION_ERROR', message: expect.stringContaining('Register an OAuth client') });
807+
expect(idp.requests).toHaveLength(2);
808+
});
809+
810+
it('a reconnect without a client reuses a kept public client as it is: its id in the form, no secret', async () => {
811+
const { engine } = await boot([installerCrm()]);
812+
await (await connectWith(engine, { clientId: INSTALLER_ID })).done();
813+
const state = await (await connectWith(engine, undefined)).done();
814+
expect(state.client).toEqual({ source: 'installer', clientId: INSTALLER_ID, hasSecret: false });
815+
expect(idp.requests[1].authorization).toBeUndefined();
816+
expect(idp.requests[1].form.client_id).toBe(INSTALLER_ID);
817+
});
818+
735819
it('⛔ a connect without a client is refused 400, naming the instance and the vendor registration step; nothing is kept', async () => {
736820
const { engine, ql } = await boot([installerCrm()]);
737821
const err = (await engine.beginConnectorConnection({ connector: 'crm', context: ADMIN_A as never }).catch((e: Error) => e)) as Error & { status?: number; code?: string };

0 commit comments

Comments
 (0)