diff --git a/packages/zapier/README.md b/packages/zapier/README.md index c7785dd..c1fe364 100644 --- a/packages/zapier/README.md +++ b/packages/zapier/README.md @@ -110,6 +110,40 @@ https://zapier.com/dashboard/auth/oauth/return/App/ and the app must grant the scopes the integration requests (see `SCOPES` in `src/authentication.ts`). Then connect an account in the Zap editor. +### Releasing a new version + +Pushing to `main` runs `zapier push`, which **overwrites the same version** — fine +while a version is unpublished, and blocked by a CI guard once it is promoted and +has real users. When the guard trips, bump `version` in `package.json` and follow +the full sequence; `push` alone leaves everyone on the old version. + +```bash +npx zapier-platform versions # confirm state + Zap Users +# bump version in package.json, merge -> CI pushes the new version + +npx zapier-platform env:get # ← DO NOT SKIP (see below) +npx zapier-platform promote # new users get it from here on +npx zapier-platform migrate 100% # move the users who already exist +``` + +Two traps, both silent: + +- **Env is per-version and does not follow a version bump.** A same-version push + preserves it, a new version starts from whatever is (not) set. Promoting a + version with no `CLIENT_ID` breaks OAuth for everyone, far worse than whatever + you were fixing. Always `env:get` first, and `env:set` the three vars if it + comes back empty. +- **`promote` does not move existing users** — it only decides what new users + install. Without `migrate`, everyone already on the old version stays there, + which usually means they never receive the fix you just shipped. + +Changing `SCOPES` needs one more thing on top: scopes are frozen when a user +authorizes, so `migrate` swaps their code but not their token. Every existing +user has to reconnect before a newly-requested scope takes effect. `triggers/bases.ts` +turns the resulting 403 into `ExpiredAuthError` so Zapier asks them to, instead of +leaving the dropdown mysteriously empty — keep that pattern for any future scope +that gates a dropdown. + ## Testing ```bash diff --git a/packages/zapier/package-lock.json b/packages/zapier/package-lock.json index 39caaec..efc172c 100644 --- a/packages/zapier/package-lock.json +++ b/packages/zapier/package-lock.json @@ -1,12 +1,12 @@ { "name": "@teable/zapier", - "version": "1.0.0", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@teable/zapier", - "version": "1.0.0", + "version": "1.1.0", "dependencies": { "zapier-platform-core": "19.0.0" }, diff --git a/packages/zapier/package.json b/packages/zapier/package.json index 51dbb74..ab5be40 100644 --- a/packages/zapier/package.json +++ b/packages/zapier/package.json @@ -1,6 +1,6 @@ { "name": "@teable/zapier", - "version": "1.0.0", + "version": "1.1.0", "description": "Teable is a no-code database platform for building collaborative apps, managing structured data, and automating workflows.", "main": "dist/index.js", "private": true, diff --git a/packages/zapier/src/authentication.ts b/packages/zapier/src/authentication.ts index fefa141..8a60f45 100644 --- a/packages/zapier/src/authentication.ts +++ b/packages/zapier/src/authentication.ts @@ -20,8 +20,15 @@ const expiresAt = (expiresIn: number | undefined): number => // Scopes requested from Teable (format: resource|action). Must be a subset of // what the OAuth App was granted in Teable → Settings → OAuth Apps. +// +// NOTE on `base|read_all`: the Base dropdown calls GET /api/base/access/all, +// which the backend guards with `base|read_all` — NOT `base|read`. Requesting +// only `base|read` makes that endpoint return 403 restricted_resource, so the +// dropdown comes back empty and no Zap can be configured. `base|read` is kept +// for the per-base endpoints (GET /api/base/:baseId and friends). const SCOPES = [ 'base|read', + 'base|read_all', 'table|read', 'field|read', 'view|read', diff --git a/packages/zapier/src/lib/errors.ts b/packages/zapier/src/lib/errors.ts new file mode 100644 index 0000000..e741824 --- /dev/null +++ b/packages/zapier/src/lib/errors.ts @@ -0,0 +1,17 @@ +// handleErrors (see src/index.ts) turns a failed Teable response into +// z.errors.Error, whose constructor JSON-stringifies { message, code, status } +// into the Error's own message. This reads that status back out so a caller can +// react to one specific failure without repeating the request. +const statusOf = (error: unknown): number | undefined => { + if (!(error instanceof Error)) return undefined; + try { + const parsed = JSON.parse(error.message) as { status?: unknown }; + return typeof parsed.status === 'number' ? parsed.status : undefined; + } catch { + // Anything not produced by z.errors.Error — a network failure, a bug in our + // own code — has a plain-text message and simply has no status. + return undefined; + } +}; + +export { statusOf }; diff --git a/packages/zapier/src/triggers/bases.ts b/packages/zapier/src/triggers/bases.ts index 4d7a091..4588e5a 100644 --- a/packages/zapier/src/triggers/bases.ts +++ b/packages/zapier/src/triggers/bases.ts @@ -1,11 +1,32 @@ import type { ZObject, Bundle } from 'zapier-platform-core'; import { apiUrl } from '../lib/client'; +import { statusOf } from '../lib/errors'; import type { TeableBase, DropdownItem } from '../lib/types'; // Hidden trigger that powers the "Base" dropdown. GET /api/base/access/all // returns every base the token can see, across spaces. const perform = async (z: ZObject, bundle: Bundle): Promise => { - const response = await z.request({ url: apiUrl(bundle, '/base/access/all') }); + let response; + try { + response = await z.request({ url: apiUrl(bundle, '/base/access/all') }); + } catch (error) { + // A 403 here means the connection was authorized before we requested the + // `base|read_all` scope (see SCOPES in src/authentication.ts). Scopes are + // fixed at the moment the user authorizes, so neither a retry nor a token + // refresh can recover — the account has to be reconnected. + // + // ExpiredAuthError is what gets that across: Zapier marks the connection as + // needing attention and walks the user through reconnecting. Letting the + // raw 403 through instead leaves the dropdown silently empty, which reads + // as "Teable has no bases" and sends people hunting through their Teable + // permissions or Zapier support for something neither one can fix. + if (statusOf(error) === 403) { + throw new z.errors.ExpiredAuthError( + 'Your Teable connection is missing the "view all bases" permission. Please reconnect your Teable account to continue.', + ); + } + throw error; + } return (response.data || []).map((base) => ({ id: base.id, name: base.name, diff --git a/packages/zapier/test/unit.test.ts b/packages/zapier/test/unit.test.ts index 7945dac..e83ea1d 100644 --- a/packages/zapier/test/unit.test.ts +++ b/packages/zapier/test/unit.test.ts @@ -2,10 +2,12 @@ // so they always run (and are safe in CI). Logic-only coverage of the bits most // likely to break: URL building, record flattening, field collection. -import type { ZObject, HttpRequestOptionsWithUrl } from 'zapier-platform-core'; +import type { ZObject, Bundle, HttpRequestOptionsWithUrl } from 'zapier-platform-core'; import App from '../src'; +import bases from '../src/triggers/bases'; import { apiBase, apiUrl } from '../src/lib/client'; +import { statusOf } from '../src/lib/errors'; import { flatten, byTimeDesc } from '../src/lib/records'; import type { FlatRecord } from '../src/lib/records'; import { collectFieldsObject } from '../src/lib/fields'; @@ -43,6 +45,101 @@ describe('lib/client apiBase (driven by TEABLE_INSTANCE_URL)', () => { }); }); +// Every dropdown is powered by an endpoint the backend guards with a specific +// permission, and a missing scope fails silently: the request 403s and the +// dropdown just renders empty. `base|read_all` is the one that bit us — the Base +// dropdown calls GET /api/base/access/all, which is guarded by `base|read_all`, +// not by `base|read`. Pin the whole set so a scope can't be dropped again. +describe('authentication OAuth scopes', () => { + const scopes = App.authentication.oauth2Config.authorizeUrl.params.scope.split(' '); + + it.each([ + ['base|read_all', 'GET /api/base/access/all — the Base dropdown'], + ['table|read', 'GET /api/base/:baseId/table — the Table dropdown'], + ['view|read', 'GET /api/table/:tableId/view — the View dropdown'], + ['field|read', 'GET /api/table/:tableId/field — the field inputs'], + ['record|read', 'GET /api/table/:tableId/record — triggers and searches'], + ['record|create', 'POST /api/table/:tableId/record'], + ['record|update', 'PATCH /api/table/:tableId/record/:recordId + attachment upload'], + ['record|delete', 'DELETE /api/table/:tableId/record/:recordId'], + ['user|email_read', 'GET /api/auth/user — the connection label'], + ])('requests %s (%s)', (scope) => { + expect(scopes).toContain(scope); + }); +}); + +describe('lib/errors statusOf', () => { + // Exactly what z.errors.Error(message, code, status) produces. + const appError = (status: number) => + new Error( + JSON.stringify({ message: 'Teable: Forbidden resource', code: 'TeableApiError', status }), + ); + + it('reads the status back out of a z.errors.Error', () => { + expect(statusOf(appError(403))).toBe(403); + }); + + it('returns undefined for a plain error (network failure, our own bug)', () => { + expect(statusOf(new Error('socket hang up'))).toBeUndefined(); + }); + + it('returns undefined when the message is JSON but carries no status', () => { + expect(statusOf(new Error(JSON.stringify({ message: 'nope' })))).toBeUndefined(); + }); + + it('returns undefined for a non-Error throw', () => { + expect(statusOf('403')).toBeUndefined(); + }); +}); + +// The Base dropdown is the one place a missing scope surfaces, and a bare 403 +// there is indistinguishable from "this account has no bases" — which is what +// sent the original reporter to Zapier support for days. It has to ask for a +// reconnect instead, because a token's scopes are fixed when it is granted. +describe('triggers/bases 403 handling', () => { + class ExpiredAuthError extends Error {} + const zWith = (request: () => Promise) => + ({ request, errors: { ExpiredAuthError } }) as unknown as ZObject; + const bundle = {} as Bundle; + const perform = bases.operation.perform; + + it('asks the user to reconnect when the base list is forbidden', async () => { + const z = zWith(() => + Promise.reject( + new Error(JSON.stringify({ message: 'x', code: 'TeableApiError', status: 403 })), + ), + ); + await expect(perform(z, bundle)).rejects.toThrow(ExpiredAuthError); + await expect(perform(z, bundle)).rejects.toThrow(/reconnect your Teable account/i); + }); + + it('leaves every other failure alone', async () => { + const boom = new Error(JSON.stringify({ message: 'x', code: 'TeableApiError', status: 500 })); + await expect( + perform( + zWith(() => Promise.reject(boom)), + bundle, + ), + ).rejects.toThrow(boom); + }); + + it('maps bases to dropdown items on success', async () => { + const z = zWith(() => + Promise.resolve({ data: [{ id: 'bse1', name: 'CRM', extra: 'ignored' }] }), + ); + await expect(perform(z, bundle)).resolves.toEqual([{ id: 'bse1', name: 'CRM' }]); + }); + + it('tolerates an empty body', async () => { + await expect( + perform( + zWith(() => Promise.resolve({})), + bundle, + ), + ).resolves.toEqual([]); + }); +}); + // The preemptive-refresh middleware is the first beforeRequest hook. It must // throw RefreshAuthError for a stale token BEFORE the request goes out (so the // API never logs a 401), and must stay out of the way everywhere else.