Skip to content

Commit 9b3c29d

Browse files
fix(plugin-auth): the organization client-address read answers a revoked or expired API key with its own refusal, not a 500 (#31345)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X3cJu9y2sMJ8ikC4hnr8sRo)_ Fixes #31259 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:组织管理员读自身地址的接口 `GET /api/v1/auth/client-address`,在调用方用的 API key 已吊销、已过期或闲置失效时,改为原样返回该 key 自己的拒绝(`403` + `API_KEY_REVOKED` / `API_KEY_EXPIRED` / `API_KEY_INACTIVE` + 原拒绝文案),不再答 `500 INTERNAL_ERROR`、也不再记一条 error 日志。 - 为什么改:这条路由的调用方解析接受 API key,但 catch 只接住了权限库不可达(503),其余一律落到 500;设置接口、REST 对同一把 key 早已答它自己的 403。 - 风险与回滚:只在 catch 里加一个分支,用 `@objectstack/core` 已导出的唯一判别器 `isApiKeyLifecycleRefusalError`,没有复制;503、401、403 与正常 200 的分支都没动。回滚即 revert。 - 先看:`packages/plugins/plugin-auth/src/auth-plugin.ts` 中 `client-address` 路由的 catch(新增的 lifecycle 分支);`packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts` 新增的顶层 describe。 - 两分钟验证:`pnpm --filter @objectstack/plugin-auth exec vitest run src/allowed-ip-ranges.pin.test.ts -t 'lifecycle refuses'`,预期 2 条全绿(吊销 key 答 403 `API_KEY_REVOKED`,过期 key 答 403 `API_KEY_EXPIRED`,均无 error 日志)。 - 需要你做的:无,席位落地。 ## What changes `GET /api/v1/auth/client-address` (the organization administrator's address read, `auth-plugin.ts`) resolves its caller through `resolveOrganizationSettingsCaller`, the settings door's composition with `credentials: 'api-key-and-session'`. That composition propagates the branded `ApiKeyLifecycleRefusalError` for a key that is revoked, expired or inactive. The route's catch relayed only `AuthzStoreUnavailableError` (503), so such a key was answered `500 INTERNAL_ERROR` ("Your client address could not be read.") with an `[AuthPlugin] client-address failed` error log line. The catch now relays that refusal beside the 503 arm, as the error declares it: its `status` (403), `code` and located `message`, in the route's existing `{ success: false, error: { code, message } }` envelope, with no log line. The classifier is the one `@objectstack/core` exports, `isApiKeyLifecycleRefusalError`, already read the same way by the settings routes (`relayApiKeyLifecycleRefusal`), plugin-sharing's shareable-objects route and core's own-row delete routes. No copy, no new dependency edge (plugin-auth already depends on `@objectstack/core`). Unchanged: the 503 store-outage arm, the 401 / 403 admission arms (`judgeOrganizationAddressReader`), the 400 closed-query arm and an admitted organization administrator's 200 answer. The platform-admin twin `GET /api/v1/auth/admin/client-address` reads its caller through `gateAdmin` (session only, no API-key door), so it is not touched. ## Tests New top-level `describe` in the route's shared harness file `allowed-ip-ranges.pin.test.ts` ("an API key its own lifecycle refuses is answered that refusal at the organization address read, never a 500"). It leaves the route's caller resolution unstubbed (new `caller: 'resolved'` option on the rig's `orgRead`), so the real `resolveRequestExecutionContext` composition reads a `sys_api_key` row from a stubbed data engine by the key's at-rest hash: - a revoked key: `403`, `API_KEY_REVOKED`, the message's first sentence `This API key (osk_client…) was revoked.`; - an expired key (`expires_at` 2000-01-01): `403`, `API_KEY_EXPIRED`, first sentence `This API key (osk_client…) expired on 2000-01-01T00:00:00.000Z.`; - both: no `data`, the address never read (`trustsForwardedHeaders` not called), and `logger.error` never called. The existing org-read cases (200 answer across three transports, 401 / 403 / held 403, 503 outage, 400 closed query set) run unchanged as the controls. ## Acceptance notes - The `API_KEY_INACTIVE` arm goes through the same branch; it is not pinned separately here (the card asks for revoked and expired; the settings door's own pin covers the inactive verdict of the shared classifier). - The test rig gains one option (`caller: 'resolved'`); every existing caller of `orgRead` is unchanged. ## Verification Head `e85053f3`. The branch merged `origin/main` once before opening; `origin/main` was still at `f5e23e2f`, so the merge was a no-op. Every heavy run went through `scripts/pm/os-verify-lock.sh`. <details><summary>Red first and ablation</summary> - Red on main's route (pin commit `dc35c35e` before the fix): 2 failed, each `{"status":500,"body":{"success":false,"error":{"code":"INTERNAL_ERROR","message":"Your client address could not be read."}}}` with one `logger.error` call. - Ablation on the fix commit, via `scripts/ablation-replace.mjs`: the anchor `if (isApiKeyLifecycleRefusalError(error)) {` replaced by `if (false && isApiKeyLifecycleRefusalError(error)) {` (anchor 1 → 0, blob `5d15e7c4` → `575d2eaf`): `Tests 2 failed | 44 passed (46)`, the same 500 answer. Restored: blob == HEAD (`5d15e7c4`), `git diff HEAD` empty. The test imports `./auth-plugin.js` from source, so no build sits between the mutation and the run. </details> <details><summary>Suites, type-check, lint</summary> - `pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/allowed-ip-ranges.pin.test.ts`: exit 0, Tests 46 passed (46). - `pnpm --filter @objectstack/plugin-auth test`: `VERDICT command-exit 0`, Test Files 240 passed (240), Tests 4248 passed, 10 skipped. - `pnpm --filter @objectstack/plugin-auth typecheck` (with `tsconfig.examples.json` and `check:test-typecheck`), after `pnpm --filter @objectstack/plugin-auth build`: `VERDICT command-exit 0`. - `pnpm exec eslint packages/plugins/plugin-auth --no-inline-config`: exit 0, no output. </details> <details><summary>Derived gates</summary> On `e85053f3`: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 75 commands. Each ran with stdin from `/dev/null` and its exit captured before any pipe: 75 exit 0. Three first answered exit 3 (prerequisite, nothing measured) and were re-run to exit 0: `check-plugin-teardown-shape.mjs --self-test` (after fetching its pinned fixture commit), `check:dual-build-cjs-loads` (after a full `turbo run build`, `VERDICT command-exit 0`; then 122 entry points across 72 packages load) and `check:type-check-debt` (lock queue timeout while that build held the lock). `--ran`: "75 derived famil(ies) accounted for — 75 run, 0 NOT-MEASURED (a DERIVED zero — all 75 recorded an exit code and none of them is 3)". </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (Not touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes (scoped to plugin-auth). - [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: its generated artifacts are regenerated and committed. (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. (Except the maintainer digest, which os-dev.md writes in Chinese.) - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5kvEQydxRtYLVRTfvo)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d0512c2 commit 9b3c29d

3 files changed

Lines changed: 73 additions & 8 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth): the organization client-address read answers a revoked or expired API key with the key's own refusal, not a 500
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`GET /api/v1/auth/client-address` resolves its caller through the settings door's composition, which admits an API key. A key its own lifecycle refuses (revoked, expired, inactive) was answered `500 INTERNAL_ERROR` with an error log line. The route now relays that refusal as the error declares it: `403` with `API_KEY_REVOKED`, `API_KEY_EXPIRED` or `API_KEY_INACTIVE` and the refusal's located message, as the settings routes and REST already answer the same key, and logs nothing for it. The `503` store-outage answer, the `401` / `403` admission refusals and an organization administrator's answer are unchanged.

‎packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import {
4646
builtinSettingsManifests,
4747
registerSettingsRoutes,
4848
} from '@objectstack/service-settings';
49-
import { AuthzStoreUnavailableError } from '@objectstack/core';
49+
import { AuthzStoreUnavailableError, hashApiKey } from '@objectstack/core';
5050
import { AuthPlugin } from './auth-plugin.js';
5151
import type { AuthManager } from './auth-manager.js';
5252
import type { OrganizationAddressReaderContext } from './client-address-read.js';
@@ -89,9 +89,10 @@ interface Rig {
8989
* `GET /api/v1/auth/client-address` on the same routes. `caller` is what the
9090
* route's resolution of its caller answers (the settings door's composition,
9191
* stubbed at that one seam): an organization administrator unless a test
92-
* says otherwise; an `Error` is thrown from it.
92+
* says otherwise; an `Error` is thrown from it; `'resolved'` leaves that
93+
* seam unstubbed, so the route's own composition resolves the caller.
9394
*/
94-
orgRead(transport?: Transport, opts?: { caller?: OrganizationAddressReaderContext | Error; query?: string }):
95+
orgRead(transport?: Transport, opts?: { caller?: OrganizationAddressReaderContext | Error | 'resolved'; query?: string }):
9596
Promise<{ status: number; body: any; cacheControl: string | null }>;
9697
}
9798

@@ -184,10 +185,12 @@ async function boot(
184185
const orgRead: Rig['orgRead'] = async (transport = {}, opts = {}) => {
185186
const app = mounted();
186187
const caller = 'caller' in opts ? opts.caller : ORG_ADMIN_CALLER;
187-
const resolve = vi.spyOn(plugin as any, 'resolveOrganizationSettingsCaller').mockImplementation(async () => {
188-
if (caller instanceof Error) throw caller;
189-
return caller;
190-
});
188+
const resolve = caller === 'resolved'
189+
? undefined
190+
: vi.spyOn(plugin as any, 'resolveOrganizationSettingsCaller').mockImplementation(async () => {
191+
if (caller instanceof Error) throw caller;
192+
return caller;
193+
});
191194
try {
192195
const res = await app.request(
193196
`http://localhost:3000/api/v1/auth/client-address${opts.query ?? ''}`,
@@ -196,7 +199,7 @@ async function boot(
196199
);
197200
return { status: res.status, body: await res.json(), cacheControl: res.headers.get('cache-control') };
198201
} finally {
199-
resolve.mockRestore();
202+
resolve?.mockRestore();
200203
}
201204
};
202205
return { svc, manager, logger: ctx.logger, save, read, orgRead };
@@ -552,6 +555,51 @@ describe('the organization administrator reads the address their access-policy s
552555
});
553556
});
554557

558+
describe("an API key its own lifecycle refuses is answered that refusal at the organization address read, never a 500", () => {
559+
const KEY = 'osk_client_address_lifecycle_pin';
560+
const LONG_AGO = '2000-01-01T00:00:00.000Z';
561+
562+
/**
563+
* The route's own caller resolution (the settings door's composition) over
564+
* a data engine holding the one `sys_api_key` row `row` describes, read by
565+
* the key's at-rest hash, as the admission reads it.
566+
*/
567+
const readWithKey = async (row: Record<string, unknown>) => {
568+
const rig = await boot();
569+
const keyHash = hashApiKey(KEY);
570+
vi.spyOn(rig.manager, 'getDataEngine').mockReturnValue({
571+
find: async (object: string, q: any) => (
572+
object === 'sys_api_key' && q?.where?.key === keyHash
573+
? [{ id: 'key_1', key: keyHash, prefix: 'osk_client', user_id: 'usr_key_owner', revoked: false, scopes: '[]', ...row }]
574+
: []
575+
),
576+
} as any);
577+
const asked = vi.spyOn(rig.manager, 'trustsForwardedHeaders');
578+
const res = await rig.orgRead({ headers: { 'x-api-key': KEY }, remoteAddress: '203.0.113.9' }, { caller: 'resolved' });
579+
expect(asked).not.toHaveBeenCalled();
580+
expect(res.body.success).toBe(false);
581+
expect(res.body.data).toBeUndefined();
582+
// A refusal the caller was told is no fault: the route logs no error line for it.
583+
expect(rig.logger.error.mock.calls.map(([line]) => line), JSON.stringify(res)).toEqual([]);
584+
return res;
585+
};
586+
587+
it('a revoked key: 403 API_KEY_REVOKED, the refusal\'s own status, code and message', async () => {
588+
const res = await readWithKey({ revoked: true });
589+
expect(res.status).toBe(403);
590+
expect(res.body.error.code).toBe('API_KEY_REVOKED');
591+
// The refusal's own located message (its first sentence names the key), not the route's fault text.
592+
expect(res.body.error.message.startsWith('This API key (osk_client…) was revoked. ')).toBe(true);
593+
});
594+
595+
it('an expired key: 403 API_KEY_EXPIRED, the refusal\'s own status, code and message', async () => {
596+
const res = await readWithKey({ expires_at: LONG_AGO });
597+
expect(res.status).toBe(403);
598+
expect(res.body.error.code).toBe('API_KEY_EXPIRED');
599+
expect(res.body.error.message.startsWith(`This API key (osk_client…) expired on ${LONG_AGO}. `)).toBe(true);
600+
});
601+
});
602+
555603
describe('an allowed IP range whose prefix is not decimal digits within its family is refused at every save door and never matches at the gate', () => {
556604
// An entry is an address, or an address, `/` and a prefix length of one or
557605
// two decimal digits from 0 to 32 (IPv4) or one to three from 0 to 128

‎packages/plugins/plugin-auth/src/auth-plugin.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
isCloudManaged,
1010
resolveRequestExecutionContext,
1111
isAuthzStoreUnavailableError,
12+
isApiKeyLifecycleRefusalError,
1213
AUTHZ_STORE_UNAVAILABLE_CODE,
1314
AUTHZ_STORE_UNAVAILABLE_MESSAGE,
1415
AUTHZ_STORE_UNAVAILABLE_STATUS,
@@ -4100,6 +4101,12 @@ export class AuthPlugin implements Plugin {
41004101
AUTHZ_STORE_UNAVAILABLE_STATUS,
41014102
);
41024103
}
4104+
// An API key its own lifecycle refuses (revoked, expired, inactive) is
4105+
// the caller's refusal, never a fault: relayed as the error declares it,
4106+
// its status, code and located message, as the settings door relays it.
4107+
if (isApiKeyLifecycleRefusalError(error)) {
4108+
return c.json({ success: false, error: { code: error.code, message: error.message } }, error.status);
4109+
}
41034110
const err = error instanceof Error ? error : new Error(String(error));
41044111
ctx.logger.error('[AuthPlugin] client-address failed', err);
41054112
return c.json({ success: false, error: { code: 'INTERNAL_ERROR', message: 'Your client address could not be read.' } }, 500);

0 commit comments

Comments
 (0)