Auth derivation previewed the raw spec while the add stored the overridden one, so a preset's scope overrides were derived away; and registry overrides wrongly beat preset overrides. One spec plan (preset overrides first, mirroring the full page) now feeds both the preview and the add, and the composition is a pure exported function with tests pinning the contract: override-changed OAuth scopes survive alongside an appended registry key, and a spec-declared key suppresses the registry's.
Unified · Split
| @@ -0,0 +1,103 @@ | |||
| 1 | import { describe, expect, it } from "@effect/vitest"; | ||
| 2 | import { Option } from "effect"; | ||
| 3 | |||
| 4 | import { decodeOpenApiSpecOverrides } from "../sdk/spec-overrides"; | ||
| 5 | import { composeQuickAddAuth, quickAddSpecPlan } from "./integration-plugin"; | ||
| 6 | |||
| 7 | // The quick add's composition is where review found bugs twice: first it | ||
| 8 | // suppressed spec-declared methods, then it derived auth from the unmodified | ||
| 9 | // document while adding the overridden one. These pin the contract. | ||
| 10 | |||
| 11 | const scopeOverride = decodeOpenApiSpecOverrides([ | ||
| 12 | { | ||
| 13 | op: "replace", | ||
| 14 | path: "/components/securitySchemes/OAuth2/flows/authorizationCode/scopes", | ||
| 15 | value: { "files:read": "" }, | ||
| 16 | }, | ||
| 17 | ])!; | ||
| 18 | |||
| 19 | describe("quickAddSpecPlan", () => { | ||
| 20 | it("uses the same effective overrides for preview and add, preset first", () => { | ||
| 21 | const registryOverrides = decodeOpenApiSpecOverrides([ | ||
| 22 | { op: "remove", path: "/components/securitySchemes/Cookie" }, | ||
| 23 | ]); | ||
| 24 | const plan = quickAddSpecPlan( | ||
| 25 | { | ||
| 26 | id: "figma", | ||
| 27 | name: "Figma", | ||
| 28 | summary: "", | ||
| 29 | specFormat: "plain-ish", | ||
| 30 | specOverrides: [...scopeOverride], | ||
| 31 | }, | ||
| 32 | registryOverrides, | ||
| 33 | ); | ||
| 34 | // Preset overrides win over the registry's, mirroring the full add page. | ||
| 35 | expect(plan.specOverrides).toEqual(scopeOverride); | ||
| 36 | expect(plan.specFormat).toBe("plain-ish"); | ||
| 37 | }); | ||
| 38 | |||
| 39 | it("falls through to registry overrides when the preset has none", () => { | ||
| 40 | const registryOverrides = decodeOpenApiSpecOverrides([ | ||
| 41 | { op: "remove", path: "/components/securitySchemes/Cookie" }, | ||
| 42 | ]); | ||
| 43 | const plan = quickAddSpecPlan(undefined, registryOverrides); | ||
| 44 | expect(plan.specOverrides).toEqual(registryOverrides); | ||
| 45 | expect(plan.specFormat).toBeUndefined(); | ||
| 46 | }); | ||
| 47 | }); | ||
| 48 | |||
| 49 | describe("composeQuickAddAuth", () => { | ||
| 50 | const registryPlacement = { | ||
| 51 | carrier: "header", | ||
| 52 | name: "Authorization", | ||
| 53 | prefix: "Bearer ", | ||
| 54 | } as const; | ||
| 55 | |||
| 56 | it("keeps override-derived OAuth scopes and appends the registry key", () => { | ||
| 57 | // The preview reflects the OVERRIDDEN document (quickAddSpecPlan sends | ||
| 58 | // the same overrides to preview): its oauth preset carries the replaced | ||
| 59 | // scopes. The composed template must retain them verbatim and add the | ||
| 60 | // registry header only because no key method was detected. | ||
| 61 | const template = composeQuickAddAuth([], registryPlacement, { | ||
| 62 | headerPresets: [], | ||
| 63 | oauth2Presets: [ | ||
| 64 | { | ||
| 65 | label: "OAuth2", | ||
| 66 | securitySchemeName: "OAuth2", | ||
| 67 | flow: "authorizationCode" as const, | ||
| 68 | authorizationUrl: Option.some("https://example.com/authorize"), | ||
| 69 | tokenUrl: "https://example.com/token", | ||
| 70 | resource: Option.none(), | ||
| 71 | refreshUrl: Option.none(), | ||
| 72 | // The overridden document's scopes — the whole point: the preview | ||
| 73 | // reflects the SAME effective spec the add stores. | ||
| 74 | scopes: { "files:read": "" }, | ||
| 75 | identityScopes: [] as const, | ||
| 76 | }, | ||
| 77 | ], | ||
| 78 | servers: [{ url: "https://api.example.com" }], | ||
| 79 | }); | ||
| 80 | expect(template).toHaveLength(2); | ||
| 81 | const oauth = template.find((method) => "kind" in method && method.kind === "oauth2"); | ||
| 82 | expect(oauth && "scopes" in oauth ? oauth.scopes : undefined).toEqual(["files:read"]); | ||
| 83 | const key = template.find((method) => !("kind" in method) || method.kind !== "oauth2"); | ||
| 84 | expect(JSON.stringify(key)).toContain("Authorization"); | ||
| 85 | }); | ||
| 86 | |||
| 87 | it("does not append the registry key when the spec already declares one", () => { | ||
| 88 | const template = composeQuickAddAuth([], registryPlacement, { | ||
| 89 | headerPresets: [ | ||
| 90 | { | ||
| 91 | label: "API key", | ||
| 92 | headers: { "X-API-Key": null }, | ||
| 93 | secretHeaders: ["X-API-Key"], | ||
| 94 | secretQueryParams: [], | ||
| 95 | }, | ||
| 96 | ], | ||
| 97 | oauth2Presets: [], | ||
| 98 | servers: [{ url: "https://api.example.com" }], | ||
| 99 | }); | ||
| 100 | expect(template).toHaveLength(1); | ||
| 101 | expect(JSON.stringify(template[0])).toContain("X-API-Key"); | ||
| 102 | }); | ||
| 103 | }); | ||
| @@ -9,7 +9,7 @@ import type { · 8 unmodified lines | |||
| 9 | } from "@executor-js/sdk/client"; | 9 | } from "@executor-js/sdk/client"; |
| 10 | import { AuthTemplateSlug } from "@executor-js/sdk/shared"; | 10 | import { AuthTemplateSlug } from "@executor-js/sdk/shared"; |
| 11 | import { slugifyNamespace } from "@executor-js/react/plugins/integration-identity"; | 11 | import { slugifyNamespace } from "@executor-js/react/plugins/integration-identity"; |
| 12 | import { placementFromHeaderPattern } from "@executor-js/react/lib/auth-placements"; | 12 | import { placementFromHeaderPattern, type Placement } from "@executor-js/react/lib/auth-placements"; |
| 13 | import { integrationWriteKeys } from "@executor-js/react/api/reactivity-keys"; | 13 | import { integrationWriteKeys } from "@executor-js/react/api/reactivity-keys"; |
| 14 | import { openApiPresets } from "../sdk/presets"; | 14 | import { openApiPresets } from "../sdk/presets"; |
| 15 | import { detectedAuthenticationTemplates } from "../sdk/derive-auth"; | 15 | import { detectedAuthenticationTemplates } from "../sdk/derive-auth"; |
| @@ -44,6 +44,67 @@ const presetForSpecUrl = ( · 28 unmodified lines | |||
| 44 | * configuration: omitting `authenticationTemplate` and `baseUrl` tells the | 44 | * configuration: omitting `authenticationTemplate` and `baseUrl` tells the |
| 45 | * server to derive both from the document, exactly what the add page's | 45 | * server to derive both from the document, exactly what the add page's |
| 46 | * untouched defaults submit. Registry spec overrides ride along. */ | 46 | * untouched defaults submit. Registry spec overrides ride along. */ |
| 47 | |||
| 48 | /** The spec inputs a quick add must hold CONSISTENT between preview and add: | ||
| 49 | * deriving auth from a document other than the one being added is how a | ||
| 50 | * preset's scope overrides got silently dropped. Preset overrides win over | ||
| 51 | * the registry's, mirroring the full add page. */ | ||
| 52 | export const quickAddSpecPlan = ( | ||
| 53 | preset: IntegrationPreset | undefined, | ||
| 54 | registryOverrides: ReturnType<typeof decodeOpenApiSpecOverrides>, | ||
| 55 | ): { | ||
| 56 | readonly specFormat?: string; | ||
| 57 | readonly specOverrides?: NonNullable<ReturnType<typeof decodeOpenApiSpecOverrides>>; | ||
| 58 | } => { | ||
| 59 | const presetOverrides = preset?.specOverrides | ||
| 60 | ? decodeOpenApiSpecOverrides(preset.specOverrides) | ||
| 61 | : undefined; | ||
| 62 | const effective = | ||
| 63 | presetOverrides && presetOverrides.length > 0 | ||
| 64 | ? presetOverrides | ||
| 65 | : registryOverrides && registryOverrides.length > 0 | ||
| 66 | ? registryOverrides | ||
| 67 | : undefined; | ||
| 68 | return { | ||
| 69 | ...(preset?.specFormat ? { specFormat: preset.specFormat } : {}), | ||
| 70 | ...(effective ? { specOverrides: effective } : {}), | ||
| 71 | }; | ||
| 72 | }; | ||
| 73 | |||
| 74 | /** The full add page's method policy as one pure step: preset OAuth wins | ||
| 75 | * outright; else every preview-detected method is preserved and the | ||
| 76 | * registry's key placement is appended only when the detected set has no | ||
| 77 | * key method (GitHub declares no security at all). `preview` MUST be the | ||
| 78 | * summary of the same effective document the add will store. */ | ||
| 79 | export const composeQuickAddAuth = ( | ||
| 80 | presetMethods: readonly ReturnType<typeof openApiWireAuthInput>[], | ||
| 81 | registryPlacement: Placement | null, | ||
| 82 | preview: { | ||
| 83 | readonly headerPresets: Parameters<typeof detectedAuthenticationTemplates>[0]; | ||
| 84 | readonly oauth2Presets: Parameters<typeof detectedAuthenticationTemplates>[1]; | ||
| 85 | readonly servers: readonly { readonly url: string }[]; | ||
| 86 | } | null, | ||
| 87 | ): readonly ReturnType<typeof openApiWireAuthInput>[] => { | ||
| 88 | if (presetMethods.length > 0) { | ||
| 89 | return registryPlacement | ||
| 90 | ? [...presetMethods, openApiWireAuthInput(templateFromPlacements([registryPlacement]))] | ||
| 91 | : presetMethods; | ||
| 92 | } | ||
| 93 | if (!registryPlacement || preview === null) return presetMethods; | ||
| 94 | const detected = detectedAuthenticationTemplates( | ||
| 95 | preview.headerPresets, | ||
| 96 | preview.oauth2Presets, | ||
| 97 | preview.servers[0]?.url ?? "", | ||
| 98 | ); | ||
| 99 | const detectedHasApiKey = detected.some((template) => template.kind === "apikey"); | ||
| 100 | return [ | ||
| 101 | ...detected.map(openApiWireAuthInput), | ||
| 102 | ...(detectedHasApiKey | ||
| 103 | ? [] | ||
| 104 | : [openApiWireAuthInput(templateFromPlacements([registryPlacement]))]), | ||
| 105 | ]; | ||
| 106 | }; | ||
| 107 | |||
| 47 | function makeUseQuickAdd(presets: readonly IntegrationPreset[]) { | 108 | function makeUseQuickAdd(presets: readonly IntegrationPreset[]) { |
| 48 | return function useOpenApiQuickAdd(): ( | 109 | return function useOpenApiQuickAdd(): ( |
| 49 | input: IntegrationQuickAddInput, | 110 | input: IntegrationQuickAddInput, |
| @@ -64,13 +125,6 @@ function makeUseQuickAdd(presets: readonly IntegrationPreset[]) { · 14 unmodified lines | |||
| 64 | return { ok: false, reason: "unparseable spec overrides" }; | 125 | return { ok: false, reason: "unparseable spec overrides" }; |
| 65 | } | 126 | } |
| 66 | const preset = presetForSpecUrl(presets, input.url); | 127 | const preset = presetForSpecUrl(presets, input.url); |
| 67 | // The FULL add page's method policy, replicated exactly: a preset's | ||
| 68 | // OAuth template wins outright; otherwise EVERY spec-detected method is | ||
| 69 | // preserved (an explicit template suppresses server-side derivation, so | ||
| 70 | // sending only the registry header would erase a spec's declared OAuth | ||
| 71 | // and key schemes); the registry's header pattern is appended only when | ||
| 72 | // the detected set has no API-key method (GitHub declares no security | ||
| 73 | // at all, and a PAT header is how most calls actually authenticate). | ||
| 74 | const registryPlacement = input.authHeader | 128 | const registryPlacement = input.authHeader |
| 75 | ? placementFromHeaderPattern(input.authHeader) | 129 | ? placementFromHeaderPattern(input.authHeader) |
| 76 | : null; | 130 | : null; |
| @@ -85,53 +139,38 @@ function makeUseQuickAdd(presets: readonly IntegrationPreset[]) { · 8 unmodified lines | |||
| 85 | ] | 139 | ] |
| 86 | : [], | 140 | : [], |
| 87 | ); | 141 | ); |
| 88 | let authenticationTemplate = presetMethods; | 142 | // ONE spec plan for both preview and add: deriving auth from a |
| 143 | // different effective document than the one stored is how a preset's | ||
| 144 | // scope overrides got silently dropped. | ||
| 145 | const specPlan = quickAddSpecPlan(preset, specOverrides); | ||
| 146 | let preview: Parameters<typeof composeQuickAddAuth>[2] = null; | ||
| 89 | if (presetMethods.length === 0 && registryPlacement) { | 147 | if (presetMethods.length === 0 && registryPlacement) { |
| 90 | // Composing with spec knowledge needs the spec: one preview call, | 148 | // Composing with spec knowledge needs the spec: one preview call, |
| 91 | // only on this path (a plain registry row with no auth facts still | 149 | // only on this path (a plain registry row with no auth facts still |
| 92 | // adds with zero extra round trips). | 150 | // adds with zero extra round trips). |
| 93 | const previewExit = await doPreview({ payload: { spec: input.url } }); | 151 | const previewExit = await doPreview({ |
| 152 | payload: { spec: input.url, ...specPlan }, | ||
| 153 | }); | ||
| 94 | if (Exit.isFailure(previewExit)) return { ok: false, reason: "preview failed" }; | 154 | if (Exit.isFailure(previewExit)) return { ok: false, reason: "preview failed" }; |
| 95 | const previewSummary = previewExit.value; | 155 | preview = previewExit.value; |
| 96 | const detected = detectedAuthenticationTemplates( | ||
| 97 | previewSummary.headerPresets, | ||
| 98 | previewSummary.oauth2Presets, | ||
| 99 | previewSummary.servers[0]?.url ?? "", | ||
| 100 | ); | ||
| 101 | const detectedHasApiKey = detected.some((template) => template.kind === "apikey"); | ||
| 102 | authenticationTemplate = [ | ||
| 103 | ...detected.map(openApiWireAuthInput), | ||
| 104 | ...(detectedHasApiKey | ||
| 105 | ? [] | ||
| 106 | : [openApiWireAuthInput(templateFromPlacements([registryPlacement]))]), | ||
| 107 | ]; | ||
| 108 | } else if (presetMethods.length > 0 && registryPlacement) { | ||
| 109 | authenticationTemplate = [ | ||
| 110 | ...presetMethods, | ||
| 111 | openApiWireAuthInput(templateFromPlacements([registryPlacement])), | ||
| 112 | ]; | ||
| 113 | } | 156 | } |
| 114 | // Preset overrides arrive as the same untyped JSON shape the registry | 157 | const authenticationTemplate = composeQuickAddAuth( |
| 115 | // sends; decode both through the one boundary. | 158 | presetMethods, |
| 116 | const presetOverrides = | 159 | registryPlacement, |
| 117 | specOverrides && specOverrides.length > 0 | 160 | preview, |
| 118 | ? specOverrides | 161 | ); |
| 119 | : preset?.specOverrides | ||
| 120 | ? decodeOpenApiSpecOverrides(preset.specOverrides) | ||
| 121 | : undefined; | ||
| 122 | const exit = await doAdd({ | 162 | const exit = await doAdd({ |
| 123 | payload: { | 163 | payload: { |
| 124 | spec: { kind: "url" as const, url: input.url }, | 164 | spec: { kind: "url" as const, url: input.url }, |
| 125 | slug, | 165 | slug, |
| 126 | name: input.name, | 166 | name: input.name, |
| 127 | ...(input.domain ? { displayDomain: input.domain } : {}), | 167 | ...(input.domain ? { displayDomain: input.domain } : {}), |
| 128 | ...(preset?.specFormat ? { specFormat: preset.specFormat } : {}), | 168 | ...specPlan, |
| 129 | ...(preset?.family ? { family: preset.family } : {}), | 169 | ...(preset?.family ? { family: preset.family } : {}), |
| 130 | ...(preset?.healthCheck ? { healthCheck: preset.healthCheck } : {}), | 170 | ...(preset?.healthCheck ? { healthCheck: preset.healthCheck } : {}), |
| 131 | ...(presetOverrides && presetOverrides.length > 0 | 171 | ...(authenticationTemplate.length > 0 |
| 132 | ? { specOverrides: presetOverrides } | 172 | ? { authenticationTemplate: [...authenticationTemplate] } |
| 133 | : {}), | 173 | : {}), |
| 134 | ...(authenticationTemplate.length > 0 ? { authenticationTemplate } : {}), | ||
| 135 | }, | 174 | }, |
| 136 | reactivityKeys: integrationWriteKeys, | 175 | reactivityKeys: integrationWriteKeys, |
| 137 | }); | 176 | }); |