diff --git a/.changeset/setup-nav-jwks-dead-entry.md b/.changeset/setup-nav-jwks-dead-entry.md new file mode 100644 index 0000000000..e33a14b74c --- /dev/null +++ b/.changeset/setup-nav-jwks-dead-entry.md @@ -0,0 +1,56 @@ +--- +"@objectstack/platform-objects": patch +--- + +fix(platform-objects): drop the dead Setup › Advanced › Signing Keys (JWKS) nav entry (#7544) + +`Setup › Advanced › Signing Keys (JWKS)` could never load, for **any** persona. +`sys_jwks` declares `enable.apiEnabled: false` / `apiMethods: []`, so the list +request answers `OBJECT_API_DISABLED` (404) — and the console masked that as a +generic "No identity records" empty state, so the surface read as *"you have no +signing keys"* rather than *"this page cannot work"*. + +**Why the gate it carried could not help.** The entry was contributed with +`requiredPermissions: ['manage_platform_settings']`, and an in-code comment +claimed a non-admin's list "403s server-side" — which reads as though an admin +could list the keys. None could. `apiAccessDenialFromEnable` (`rest-server.ts`) +is a **pure function of the object's `enable` block**: it takes no user, no +permissions and no context, so the 404 is identical for every persona, platform +admin included. A permission gate on the entry and an API-disabled object are +independent conditions, and no combination of the first prunes the second. + +**The repair is the entry, not the object.** `sys_jwks` rows are the +environment's JWT signing keys (`private_key` — private key material); opening a +read path onto them over the generic data API would be a credential disclosure. +`enable` is unchanged, and a test now pins that it stays `apiEnabled: false` / +`apiMethods: []` (fails CLOSED since #3391) and `access: { default: 'private' }` +(ADR-0066 ④). better-auth continues to read the keys through its adapter under a +system context, so token signing and verification are unaffected. + +This matches how the same class is already handled two lines below in +`setup-nav.contributions.ts`: `sys_verification` and `sys_device_code` omit +`list` and therefore get no browse entry. `sys_jwks` was the only one of the +repo's seven API-disabled objects that still had a nav entry — the six +`sys_oauth_*` token/consent stores never had one. + +Also landed with the removal: + +- The four `apps.setup.navigation.nav_jwks` labels move into the + `DEAD_SETUP_NAV_IDS` tombstone (`setup-nav-dead-key-tombstone.test.ts`), which + refuses a label with no declaring nav item and states the order for re-adding + one. The `sys_jwks` **object** labels in the generated bundles are untouched — + the object still exists. +- A new invariant in `platform-objects.test.ts`: every contributed + `type: 'object'` Setup entry must target an object that can actually serve a + `list`, judged through the same single derivation source the REST gate uses + (`resolveEffectiveApiMethods` / `isApiOperationAllowed`, #3391). It asserts the + control too — `nav_api_keys` → `sys_api_key` still lists, so a fix that pruned + both would fail. + +**Not addressed here** (reported on #7544 instead): nav gating has no declaration +that can express "prune when the destination cannot serve". `filterAppForUser` +gates `requiredPermissions` and `requiresService` server-side and deliberately +leaves `requiresObject` to the client, and nothing anywhere consults +`enable.apiEnabled` — so re-pointing this entry at a `requiresObject` gate would +not have pruned it either. Closing that gap is a contract-face change and belongs +in its own card. diff --git a/content/docs/ui/setup-app.mdx b/content/docs/ui/setup-app.mdx index 03f582286e..573072894d 100644 --- a/content/docs/ui/setup-app.mdx +++ b/content/docs/ui/setup-app.mdx @@ -56,7 +56,7 @@ anchors are: | **Configuration** (`group_configuration`) | All Settings · Localization · Company · Branding · Authentication · Email · File Storage · AI & Embedder · Knowledge · Feature Flags — `platform-objects` | | **Diagnostics** (`group_diagnostics`) | Sessions · Notification Events — `platform-objects`; Audit Logs — `plugin-audit` | | **Integrations** (`group_integrations`) | `plugin-webhooks` | -| **Advanced** (`group_advanced`) | OAuth Applications · Signing Keys (JWKS) · Identity Links · User Preferences — `platform-objects` | +| **Advanced** (`group_advanced`) | OAuth Applications · Identity Links · User Preferences — `platform-objects` | The exact rendered menu depends on which capability plugins are loaded. A few notable entries: @@ -65,13 +65,17 @@ A few notable entries: into `group_diagnostics`. - **OAuth Applications** (`sys_oauth_application`) — third-party OAuth client registrations, contributed into `group_advanced`. -- **Signing Keys (JWKS)** (`sys_jwks`) — JWKS keys used for OIDC / JWT - signing, contributed into `group_advanced`. The nav item is gated on the - `manage_platform_settings` capability, and the object itself is - `access: { default: 'private' }` (ADR-0066 ④) — signing keys are never - covered by the wildcard grant, so non-admins are denied server-side. - (`sys_verification` and `sys_device_code` are deliberately **not** in the - nav: sensitive, ephemeral secrets — not browsable, and also `private`.) +- **Signing Keys (JWKS)** (`sys_jwks`) — **not** in the nav (#7544). The + object is the environment's JWT signing-key store (`private_key`) and + declares `enable.apiEnabled: false` / `apiMethods: []`, so the generic data + API answers `OBJECT_API_DISABLED` (404) on every list — for **every** + persona, platform admin included, since that gate reads only the object's + `enable` block and takes no user or permissions. The entry that used to be + here carried a `manage_platform_settings` gate, which could not prune it: + a permission gate and an API-disabled object are independent conditions. + (`sys_verification` and `sys_device_code` are likewise deliberately absent: + sensitive, ephemeral secrets — not browsable, and also `private`. All three + are pinned in `setup-nav-dead-key-tombstone.test.ts`.) - **Audit Logs** (`sys_audit_log`) — contributed by `plugin-audit` into `group_diagnostics`. (The `sys_activity` and `sys_comment` objects also live in `plugin-audit`, but they are not contributed as Setup nav diff --git a/docs/qa/platform-checklist/areas/platform-core.json b/docs/qa/platform-checklist/areas/platform-core.json index 683faf0ff7..4e3061b322 100644 --- a/docs/qa/platform-checklist/areas/platform-core.json +++ b/docs/qa/platform-checklist/areas/platform-core.json @@ -482,9 +482,9 @@ "Setup detail: confirm each settings URL entry (nav_settings_* → /apps/setup/system/settings/) opens the settings namespace form (localization/company/branding/auth/mail/storage/ai/knowledge/feature_flags), and Users / Organization / Business Units / Teams / Sessions / OAuth Applications / Identity Links / User Preferences render", "Studio detail: confirm each metadata:resource list (object/app/view/page/dashboard/report/dataset/action/hook/flow/agent/tool/skill/email_template) renders, and the component surfaces render (App Builder studio:builder, All Metadata Types metadata:directory, Packages developer:packages, API Console developer:api-console, Flow Runs developer:flow-runs, Public Forms developer:public-forms)", "Account detail: confirm Profile (account:profile_card), Notifications (sys_inbox_message/mine), Approvals (sys_approval_request/my_pending), Linked Accounts (sys_account), Active Sessions (sys_session/mine), API Keys (sys_api_key/mine), OAuth Applications (sys_oauth_application/mine) each render", - "confirm the gated entries resolve to ABSENT-not-erroring for the admin: nav_organizations (requiresService org-scoping) is absent in single-org mode; nav_jwks Signing Keys (requiredPermissions manage_platform_settings, sys_jwks private per ADR-0066) is PRESENT for admin; SSO Providers is absent unless OS_SSO_ENABLED (knownGap)", + "confirm the gated entries resolve to ABSENT-not-erroring for the admin: nav_organizations (requiresService org-scoping) is absent in single-org mode; nav_jwks Signing Keys is ABSENT for EVERY persona, admin included — the entry was removed in #7544 because sys_jwks declares apiEnabled:false, so its list answers OBJECT_API_DISABLED (404) for all callers and no permission gate could prune it; SSO Providers is absent unless OS_SSO_ENABLED (knownGap)", "sign in as the plain member: attempt to open Setup (com.objectstack.setup) and Studio (com.objectstack.studio) — capture the app-level refusal (App.requiredPermissions setup.access / studio.access); open Account (declares no requiredPermissions) — capture it opening", - "as the member, GET /api/v1/meta/app for a reachable app and confirm every manage_platform_settings-gated entry (JWKS, API Keys, the manage_platform_settings settings URLs) is ABSENT from the member's payload — the server prunes, the client does not merely hide", + "as the member, GET /api/v1/meta/app for a reachable app and confirm every manage_platform_settings-gated entry (API Keys, the manage_platform_settings settings URLs) is ABSENT from the member's payload — the server prunes, the client does not merely hide. JWKS is deliberately NOT in this list any more: it is absent for admins too, so finding it absent for a member proves nothing about the gate (#7544)", "cross-check served-vs-rendered: diff each app's meta/app nav destinations against what actually rendered so nothing is silently outside the walk" ], "acceptance": [ @@ -541,7 +541,7 @@ "packages/platform-objects/src/apps/setup.app.ts (shell group anchors + requiredPermissions setup.access)", "packages/platform-objects/src/apps/studio.app.ts (static nav; requiredPermissions studio.access)", "packages/platform-objects/src/apps/account.app.ts (no requiredPermissions; hidden from App Switcher; requiresObject/requiresService entry gates)", - "packages/platform-objects/src/apps/setup-nav.contributions.ts (nav_jwks manage_platform_settings; nav_organizations requiresService org-scoping; nav_api_keys manage_platform_settings)", + "packages/platform-objects/src/apps/setup-nav.contributions.ts (nav_organizations requiresService org-scoping; nav_api_keys manage_platform_settings; no nav_jwks — removed in #7544)", "packages/plugins/plugin-auth/src/auth-plugin.ts (SSO Providers nav_sso_providers contributed only when isSsoWired())", "ADR-0048 (Setup/Studio/Account as one-app packages com.objectstack.{setup,studio,account}); ADR-0029 (nav contributions); ADR-0066 (sys_jwks private)" ], diff --git a/packages/platform-objects/src/apps/setup-nav.contributions.ts b/packages/platform-objects/src/apps/setup-nav.contributions.ts index 3fff63011c..110762315e 100644 --- a/packages/platform-objects/src/apps/setup-nav.contributions.ts +++ b/packages/platform-objects/src/apps/setup-nav.contributions.ts @@ -134,11 +134,30 @@ export const SETUP_NAV_CONTRIBUTIONS: NavigationContribution[] = [ priority: BASE_PRIORITY, items: [ { id: 'nav_oauth_apps', type: 'object', label: 'OAuth Applications', objectName: 'sys_oauth_application', icon: 'app-window' }, - // nav_jwks is capability-gated (like nav_api_keys): sys_jwks is - // `access.default:'private'` (ADR-0066 ④ — signing keys), so a - // non-admin's list request 403s server-side; gating the nav item keeps - // the menu honest instead of showing an entry that can only error. - { id: 'nav_jwks', type: 'object', label: 'Signing Keys (JWKS)', objectName: 'sys_jwks', icon: 'key-round', requiredPermissions: ['manage_platform_settings'] }, + // No `nav_jwks` here (#7544). `sys_jwks` is the environment's JWT SIGNING + // KEY store (`private_key` — private key material), and it declares + // `enable.apiEnabled: false` / `apiMethods: []`, so the generic data API + // answers `OBJECT_API_DISABLED` (404) on every list request. + // + // ⚠️ That 404 is NOT a permission outcome and no permission gate can + // prune it. `apiAccessDenialFromEnable` (rest-server.ts) is a PURE + // function of the object's `enable` block — it takes no user, no + // permissions and no context — so the 404 is identical for every + // persona, platform admin included. The entry this replaces carried + // `requiredPermissions: ['manage_platform_settings']` and a comment + // claiming a non-admin "403s server-side", which read as though an ADMIN + // could list the keys. None could: the page was dead for everyone, and + // the console masked the 404 as a generic empty state, so the surface + // read as "you have no signing keys" rather than "this page cannot work". + // + // ⛔ The repair is the entry, never the object: opening a read path onto + // private signing keys over the generic data API would be a credential + // disclosure. `apiMethods: []` fails CLOSED by design (#3391). + // + // The same reasoning already governs the two entries below, and it is why + // the other six API-disabled objects (the `sys_oauth_*` token/consent + // stores) have no nav entry either — `sys_jwks` was the only one that did. + // // `sys_verification` (email/phone tokens) and `sys_device_code` (OAuth // device-grant codes) deliberately omit `list` from their `apiMethods` // (sensitive, ephemeral secrets — not browsable), so an object/list-view diff --git a/packages/platform-objects/src/apps/translations/en.ts b/packages/platform-objects/src/apps/translations/en.ts index 727a749b6a..76ebcd17e9 100644 --- a/packages/platform-objects/src/apps/translations/en.ts +++ b/packages/platform-objects/src/apps/translations/en.ts @@ -139,12 +139,13 @@ export const en: TranslationData = { // No `nav_verifications` / `nav_device_codes` here on purpose: // `sys_verification` and `sys_device_code` omit `list` from their // `apiMethods`, so `setup-nav.contributions.ts` deliberately declares - // no browse entry for them (#2266). Nor `nav_metadata` — that entry - // moved to Studio as `nav_metadata_directory`. Re-adding a label here - // means re-adding the nav item first; the tombstone in + // no browse entry for them (#2266). Nor `nav_jwks` — `sys_jwks` is + // `apiEnabled: false`, so its list 404s for EVERY persona and the entry + // was removed (#7544). Nor `nav_metadata` — that entry moved to Studio + // as `nav_metadata_directory`. Re-adding a label here means re-adding + // the nav item first; the tombstone in // `setup-nav-dead-key-tombstone.test.ts` states the whole rule (#6660). nav_oauth_apps: { label: 'OAuth Applications' }, - nav_jwks: { label: 'Signing Keys (JWKS)' }, nav_accounts: { label: 'Identity Links' }, nav_user_preferences: { label: 'User Preferences' }, }, diff --git a/packages/platform-objects/src/apps/translations/es-ES.ts b/packages/platform-objects/src/apps/translations/es-ES.ts index 884f9429b7..add5867851 100644 --- a/packages/platform-objects/src/apps/translations/es-ES.ts +++ b/packages/platform-objects/src/apps/translations/es-ES.ts @@ -98,7 +98,6 @@ export const esES: TranslationData = { nav_datasources: { label: 'Fuentes de datos' }, nav_oauth_apps: { label: 'Aplicaciones OAuth' }, - nav_jwks: { label: 'Claves de Firma (JWKS)' }, nav_accounts: { label: 'Enlaces de Identidad' }, nav_user_preferences: { label: 'Preferencias de Usuario' }, }, diff --git a/packages/platform-objects/src/apps/translations/ja-JP.ts b/packages/platform-objects/src/apps/translations/ja-JP.ts index 103fc727c0..1d8632b131 100644 --- a/packages/platform-objects/src/apps/translations/ja-JP.ts +++ b/packages/platform-objects/src/apps/translations/ja-JP.ts @@ -98,7 +98,6 @@ export const jaJP: TranslationData = { nav_datasources: { label: 'データソース' }, nav_oauth_apps: { label: 'OAuth アプリケーション' }, - nav_jwks: { label: '署名キー (JWKS)' }, nav_accounts: { label: 'ID 連携' }, nav_user_preferences: { label: 'ユーザー設定' }, }, diff --git a/packages/platform-objects/src/apps/translations/setup-nav-dead-key-tombstone.test.ts b/packages/platform-objects/src/apps/translations/setup-nav-dead-key-tombstone.test.ts index 47302af087..d1a74755ee 100644 --- a/packages/platform-objects/src/apps/translations/setup-nav-dead-key-tombstone.test.ts +++ b/packages/platform-objects/src/apps/translations/setup-nav-dead-key-tombstone.test.ts @@ -21,10 +21,11 @@ // deliberately NOT built (#6659's triage): it is a separate maintainer-facing // call, not a prerequisite for labelling the ids it cannot see. // -// This file makes no general claim. It pins exactly four ids that were checked +// This file makes no general claim. It pins exactly five ids that were checked // ONE BY ONE against a repo-wide grep — `id: ''` returned zero hits for -// each of them on `61282f906`, against a control probe (`nav_webhooks`) that -// returned five — and each of which has a recorded reason to be gone: +// each of them on `61282f906` (`nav_jwks` on `68a1edb`), against a control +// probe (`nav_webhooks`) that returned five — and each of which has a recorded +// reason to be gone: // // nav_approval_processes the process engine was retired in favour of the // approval flow node (#1408, ADR-0019 P4/P5) @@ -34,16 +35,26 @@ // ever render "failed to load" (#2266, and the // comment that records it in // `setup-nav.contributions.ts`) +// nav_jwks `sys_jwks` (JWT signing PRIVATE keys) declares +// `apiEnabled: false` / `apiMethods: []`, so its list +// answers `OBJECT_API_DISABLED` (404). Unlike the two +// above this entry EXISTED and was dead for every +// persona — `apiAccessDenialFromEnable` is a pure +// function of `enable`, so the admin-looking +// `requiredPermissions` gate it carried could never +// prune it (#7544) // nav_metadata moved to Studio as `nav_metadata_directory` when // the Studio app was split out (482eb67cc) // // --------------------------------------------------------------------------- // What to do when this test goes red // --------------------------------------------------------------------------- -// It goes red on exactly one event: one of the four ids comes back. That is not -// automatically wrong — re-adding `nav_verifications` or `nav_device_codes` is a -// deliberate security decision (it requires enabling `list` on the object -// first), and `nav_approval_processes` could return with a new owner. The rule +// It goes red on exactly one event: one of the five ids comes back. That is not +// automatically wrong — re-adding `nav_verifications`, `nav_device_codes` or +// `nav_jwks` is a deliberate security decision (each requires enabling the read +// on the object first — for `sys_jwks`, opening a data-API read path onto +// private signing-key material, which is why it is the least likely of the +// three), and `nav_approval_processes` could return with a new owner. The rule // is the ORDER: the declaring nav item comes back first, the label second, and // the id's line is deleted from `DEAD_SETUP_NAV_IDS` in that same commit. A // label with no declaring nav item is what this tombstone exists to refuse. @@ -61,6 +72,7 @@ const LOCALES = { en, 'zh-CN': zhCN, 'ja-JP': jaJP, 'es-ES': esES } as const; const DEAD_SETUP_NAV_IDS = [ 'nav_approval_processes', 'nav_device_codes', + 'nav_jwks', 'nav_metadata', 'nav_verifications', ] as const; diff --git a/packages/platform-objects/src/apps/translations/zh-CN.ts b/packages/platform-objects/src/apps/translations/zh-CN.ts index 5a5f914c0d..a0260ab1be 100644 --- a/packages/platform-objects/src/apps/translations/zh-CN.ts +++ b/packages/platform-objects/src/apps/translations/zh-CN.ts @@ -108,7 +108,6 @@ export const zhCN: TranslationData = { nav_datasources: { label: '数据源' }, nav_oauth_apps: { label: 'OAuth 应用' }, - nav_jwks: { label: '签名密钥 (JWKS)' }, nav_accounts: { label: '身份链接' }, nav_user_preferences: { label: '用户偏好' }, }, diff --git a/packages/platform-objects/src/platform-objects.test.ts b/packages/platform-objects/src/platform-objects.test.ts index adfb8d2be7..9c300ce7ff 100644 --- a/packages/platform-objects/src/platform-objects.test.ts +++ b/packages/platform-objects/src/platform-objects.test.ts @@ -332,6 +332,116 @@ describe('@objectstack/platform-objects', () => { expect(contrib).toBeUndefined(); } }); + + // ───────────────────────────────────────────────────────────────────── + // [#7544] A `type: 'object'` entry promises a browsable list. Whether the + // destination can serve one is decided by the OBJECT's own `enable` block + // — `apiEnabled: false` → 404 `OBJECT_API_DISABLED`, an `apiMethods` + // whitelist without `list` → 405 — resolved here through the same single + // derivation source `rest-server.ts` gates on (#3391), in the same order. + // + // ⚠️ That decision takes NO user and NO permissions + // (`apiAccessDenialFromEnable` is a pure function of `enable`), which is + // why a `requiredPermissions` gate on the ENTRY cannot prune an entry + // whose OBJECT is API-disabled: it is dead for every persona, platform + // admin included. `nav_jwks` → `sys_jwks` was exactly that, and is gone. + // ───────────────────────────────────────────────────────────────────── + describe('object entries can actually serve a list (#7544)', () => { + // Objects this package owns whose `enable` block is therefore importable + // here: every current Setup nav target, PLUS `sys_jwks`. Keeping the + // removed one listed is what makes the gate diagnose rather than merely + // notice — a re-added `nav_jwks` fails on "cannot serve a list", the + // actual reason, instead of falling into the unclassified branch below + // and reporting only that this file has never heard of the object. + const OWNED_NAV_OBJECTS: Record = { + sys_account: SysAccount, + sys_api_key: SysApiKey, + sys_business_unit: SysBusinessUnit, + sys_invitation: SysInvitation, + sys_jwks: SysJwks, + sys_oauth_application: SysOauthApplication, + sys_organization: SysOrganization, + sys_session: SysSession, + sys_team: SysTeam, + sys_user: SysUser, + sys_user_preference: SysUserPreference, + }; + // Targets owned by another package (contributed there, gated by + // `requiresObject`), so nothing here can read their `enable`. Named + // rather than skipped: a NEW unresolvable target must be classified by + // hand instead of silently dropping out of this gate's coverage. + const FOREIGN_NAV_OBJECTS = new Set(['sys_notification']); + + const objectEntries = (): Array<{ id: string; objectName: string }> => { + const out: Array<{ id: string; objectName: string }> = []; + const walk = (items: readonly unknown[]): void => { + for (const raw of items) { + const e = raw as { id?: string; type?: string; objectName?: string; children?: unknown[] }; + if (e?.type === 'object' && e.objectName) { + out.push({ id: e.id ?? '(unnamed)', objectName: e.objectName }); + } + if (Array.isArray(e?.children)) walk(e.children); + } + }; + for (const c of SETUP_NAV_CONTRIBUTIONS) walk(c.items); + return out; + }; + + /** The rest-server gate order, over an object's declared `enable`. */ + const canList = (enable: unknown): boolean => { + if ((enable as { apiEnabled?: unknown } | undefined)?.apiEnabled === false) return false; + return isApiOperationAllowed(resolveEffectiveApiMethods(enable as never), 'list'); + }; + + it('every contributed object entry targets a listable object', () => { + const entries = objectEntries(); + + // Control: the walk really reads the contributions, so the assertions + // below cannot pass by vacuity. + expect(entries.length).toBeGreaterThan(0); + expect(entries.map((e) => e.id)).toContain('nav_api_keys'); + + const unclassified = entries + .filter((e) => !(e.objectName in OWNED_NAV_OBJECTS) && !FOREIGN_NAV_OBJECTS.has(e.objectName)) + .map((e) => `${e.id} → ${e.objectName}`); + expect( + unclassified, + 'new nav target this package cannot check — add it to OWNED_NAV_OBJECTS or FOREIGN_NAV_OBJECTS', + ).toEqual([]); + + const dead = entries + .filter((e) => e.objectName in OWNED_NAV_OBJECTS) + .filter((e) => !canList(OWNED_NAV_OBJECTS[e.objectName]!.enable)) + .map((e) => `${e.id} → ${e.objectName}`); + expect(dead, 'nav entries whose destination answers 4xx on list — see #7544').toEqual([]); + }); + + it('nav_jwks is gone, and the nav_api_keys control still lists', () => { + const entries = objectEntries(); + + // The reported defect: no entry advertises the unreachable + // destination any more, under this id or any other. + expect(entries.find((e) => e.id === 'nav_jwks')).toBeUndefined(); + expect(entries.some((e) => e.objectName === 'sys_jwks')).toBe(false); + + // The NEGATIVE direction — the card's own control, from the same QA + // run: `nav_api_keys` → `sys_api_key` rides the same machinery and + // returns no 4xx. A fix that pruned this too would be over-reach, and + // must fail here. + expect(entries.find((e) => e.id === 'nav_api_keys')?.objectName).toBe('sys_api_key'); + expect(canList(SysApiKey.enable)).toBe(true); + }); + + it('the repair is the nav entry — sys_jwks stays API-disabled and private', () => { + // ⛔ Opening a data-API read path onto private JWT signing-key material + // would be a credential disclosure. `apiMethods: []` fails CLOSED + // (#3391); the `private` posture is pinned above (ADR-0066 ④). + expect(SysJwks.enable?.apiEnabled).toBe(false); + expect(SysJwks.enable?.apiMethods).toEqual([]); + expect(canList(SysJwks.enable)).toBe(false); + expect((SysJwks as { access?: { default?: string } }).access?.default).toBe('private'); + }); + }); }); });