diff --git a/ui/actions/providers/dynamic-provider-credentials.test.ts b/ui/actions/providers/dynamic-provider-credentials.test.ts index f8ab619bc8..3f3024a772 100644 --- a/ui/actions/providers/dynamic-provider-credentials.test.ts +++ b/ui/actions/providers/dynamic-provider-credentials.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import openaiSchema from "@/lib/provider-credentials/fixtures/openai-credential-schema.json"; import templateSchema from "@/lib/provider-credentials/fixtures/template-credential-schema.json"; +import unionSchema from "@/lib/provider-credentials/fixtures/union-credential-schema.json"; const { fetchMock, getProviderSchemas, getAuthHeaders, revalidatePath } = vi.hoisted(() => ({ fetchMock: vi.fn(), @@ -136,6 +137,61 @@ describe("dynamic provider credential actions", () => { }); expect(fetchMock).toHaveBeenCalledTimes(1); }); + describe("when the secret type's schema offers several variants", () => { + const basic = { + auth_method: "basic", + host: "api.acme.test", + username: "fixture-user", + password: "fixture-password-not-a-secret", + }; + beforeEach(() => { + getProviderSchemas.mockResolvedValue({ + status: "success", + providerType: "acme", + secretTypes: { static: unionSchema }, + }); + }); + + it("sends a secret that one variant accepts", async () => { + // Given + fetchMock + .mockResolvedValueOnce(response(account())) + .mockResolvedValueOnce(response({ data: { id: "saved" } }, 201)); + + // When + const result = await saveDynamicProviderCredentials({ + ...input, + secretType: "static", + secret: basic, + }); + + // Then + expect(result).toEqual({ status: "saved", secretId: "saved" }); + expect( + JSON.parse(fetchMock.mock.calls[1][1].body).data.attributes, + ).toEqual({ secret_type: "static", secret: basic }); + }); + + it.each([ + ["fields of two variants", { ...basic, token: "fixture-token" }], + ["another variant's discriminator", { ...basic, auth_method: "token" }], + ["a missing required field", { ...basic, password: "" }], + ])("does not write a secret with %s", async (_name, secret) => { + // Given + fetchMock.mockResolvedValueOnce(response(account())); + + // When + const result = await saveDynamicProviderCredentials({ + ...input, + secretType: "static", + secret, + }); + + // Then + expect(result.status).toBe("invalid"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + }); it.each([ { ...input, secretType: "invented" }, { ...input, secret: { token: "" } }, diff --git a/ui/actions/providers/dynamic-provider-credentials.ts b/ui/actions/providers/dynamic-provider-credentials.ts index 79975fa2e7..7506b657ca 100644 --- a/ui/actions/providers/dynamic-provider-credentials.ts +++ b/ui/actions/providers/dynamic-provider-credentials.ts @@ -4,8 +4,8 @@ import { revalidatePath } from "next/cache"; import { z } from "zod"; import { apiBaseUrl, getAuthHeaders } from "@/lib"; -import { parseRegistryCredentialSchema } from "@/lib/provider-credentials/provider-credential-schema"; -import { validateCredentialValues } from "@/lib/provider-credentials/provider-credential-values"; +import { parseRegistryCredentialVariants } from "@/lib/provider-credentials/provider-credential-schema"; +import { validateCredentialVariants } from "@/lib/provider-credentials/provider-credential-values"; import { isKnownProviderType } from "@/types/providers"; import { getProviderSchemas } from "./provider-schemas"; @@ -66,11 +66,11 @@ export async function saveDynamicProviderCredentials( !Object.hasOwn(schemas.secretTypes, secretType) ) return { status: "schema_unavailable" }; - const schema = parseRegistryCredentialSchema( + const variants = parseRegistryCredentialVariants( schemas.secretTypes[secretType], ); - if (!schema) return { status: "schema_unavailable" }; - const validated = validateCredentialValues(schema, secret); + if (!variants) return { status: "schema_unavailable" }; + const validated = validateCredentialVariants(variants, secret); if (!validated.valid) return { status: "invalid", errors: validated.errors }; diff --git a/ui/components/providers/wizard/steps/dynamic-credentials-step.test.tsx b/ui/components/providers/wizard/steps/dynamic-credentials-step.test.tsx index ce28a075b1..f91e5af082 100644 --- a/ui/components/providers/wizard/steps/dynamic-credentials-step.test.tsx +++ b/ui/components/providers/wizard/steps/dynamic-credentials-step.test.tsx @@ -10,6 +10,7 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import openaiSchema from "@/lib/provider-credentials/fixtures/openai-credential-schema.json"; import templateSchema from "@/lib/provider-credentials/fixtures/template-credential-schema.json"; +import unionSchema from "@/lib/provider-credentials/fixtures/union-credential-schema.json"; import { useProviderWizardStore } from "@/store/provider-wizard/store"; import type { ProviderSchemasResult } from "@/types/provider-schema"; @@ -183,6 +184,77 @@ describe("dynamic credentials in the provider wizard", () => { }, }); }); + it("shows only the chosen variant's fields when a schema offers several", async () => { + // Given + const user = userEvent.setup(); + getProviderSchemas.mockResolvedValue({ + status: "success", + providerType: "acme", + secretTypes: { static: unionSchema }, + }); + render(); + await screen.findByLabelText(/API Host/); + const method = screen.getByRole("combobox", { + name: "Authentication method", + }); + + // Then + expect(method).toHaveTextContent("API token"); + expect(screen.getByLabelText(/^Token/)).toBeVisible(); + expect(screen.queryByLabelText(/Username/)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/Password/)).not.toBeInTheDocument(); + expect( + screen.queryByLabelText(/Authentication Method/), + ).not.toBeInTheDocument(); + + // When: a token typed before switching away is gone on return. + await user.type(screen.getByLabelText(/^Token/), "previous-variant-token"); + await user.click(method); + await user.click( + screen.getByRole("option", { name: "Username and password" }), + ); + await user.click(method); + await user.click(screen.getByRole("option", { name: "API token" })); + + // Then + expect(screen.getByLabelText(/^Token/)).toHaveValue(""); + + // When + await user.click(method); + await user.click( + screen.getByRole("option", { name: "Username and password" }), + ); + await user.type(screen.getByLabelText(/API Host/), "api.acme.test"); + await user.type(screen.getByLabelText(/Username/), "fixture-user"); + await user.type( + screen.getByLabelText(/Password/), + "fixture-password-not-a-secret", + ); + + // Then + expect(screen.queryByLabelText(/^Token/)).not.toBeInTheDocument(); + + // When + act(() => + screen + .getByLabelText(/Username/) + .closest("form")! + .requestSubmit(), + ); + + // Then: the variant's fixed discriminator travels with its own fields only. + await waitFor(() => expect(props.onNext).toHaveBeenCalledOnce()); + expect(saveDynamicProviderCredentials).toHaveBeenCalledWith({ + providerId: "account", + secretType: "static", + secret: { + auth_method: "basic", + host: "api.acme.test", + username: "fixture-user", + password: "fixture-password-not-a-secret", + }, + }); + }); it.each<{ result: ProviderSchemasResult; title: string }>([ { result: { status: "success", providerType: "acme", secretTypes: {} }, diff --git a/ui/components/providers/wizard/steps/dynamic-credentials-step.tsx b/ui/components/providers/wizard/steps/dynamic-credentials-step.tsx index 35fc5f0daa..3d2c48941d 100644 --- a/ui/components/providers/wizard/steps/dynamic-credentials-step.tsx +++ b/ui/components/providers/wizard/steps/dynamic-credentials-step.tsx @@ -20,7 +20,7 @@ import { Skeleton } from "@/components/shadcn/skeleton/skeleton"; import { useToast } from "@/components/shadcn/toast"; import { StatusAlert } from "@/components/shared/status-alert"; import { - parseRegistryCredentialSchema, + parseRegistryCredentialVariants, type RegistryCredentialSchema, } from "@/lib/provider-credentials/provider-credential-schema"; import { @@ -28,7 +28,10 @@ import { validateCredentialValues, } from "@/lib/provider-credentials/provider-credential-values"; import { useProviderWizardStore } from "@/store/provider-wizard/store"; -import type { ProviderSchemasResult } from "@/types/provider-schema"; +import type { + ProviderSchemasResult, + ProviderSecretTypes, +} from "@/types/provider-schema"; import { WIZARD_FOOTER_ACTION_TYPE, @@ -43,6 +46,39 @@ interface DynamicCredentialsStepProps { onFooterChange: (config: WizardFooterConfig) => void; } +interface CredentialMethod { + id: string; + secretType: string; + label: string; + schema: RegistryCredentialSchema | null; +} + +// Each variant of a secret type's schema is a method of its own. +function listCredentialMethods( + secretTypes: ProviderSecretTypes, +): CredentialMethod[] { + return Object.entries(secretTypes).flatMap(([secretType, value]) => { + const label = secretType.replaceAll("_", " "); + const variants = parseRegistryCredentialVariants(value); + if (!variants || variants.length === 1) { + return [ + { + id: secretType, + secretType, + label, + schema: variants?.[0].schema ?? null, + }, + ]; + } + return variants.map((variant, index) => ({ + id: `${secretType}/${index}`, + secretType, + label: variant.label ?? `${label} ${index + 1}`, + schema: variant.schema, + })); + }); +} + function credentialFormError(status: ProviderSchemasResult["status"]) { switch (status) { case "access_denied": @@ -233,12 +269,11 @@ function DynamicCredentialsContent(props: DynamicCredentialsStepProps) { }, [providerType, attempt]); const methods = - schemas?.status === "success" ? Object.keys(schemas.secretTypes) : []; - const secretType = selectedMethod || methods[0]; - const schema = - schemas?.status === "success" && secretType - ? parseRegistryCredentialSchema(schemas.secretTypes[secretType]) - : null; + schemas?.status === "success" + ? listCredentialMethods(schemas.secretTypes) + : []; + const method = methods.find(({ id }) => id === selectedMethod) ?? methods[0]; + const schema = method?.schema ?? null; useEffect(() => { if (!schema) onFooterChange({ @@ -277,7 +312,7 @@ function DynamicCredentialsContent(props: DynamicCredentialsStepProps) { Authentication method )} - {schema ? ( + {method && schema ? ( diff --git a/ui/components/providers/workflow/provider-credential-fields.tsx b/ui/components/providers/workflow/provider-credential-fields.tsx index 157f4a0d7e..230f36b0e4 100644 --- a/ui/components/providers/workflow/provider-credential-fields.tsx +++ b/ui/components/providers/workflow/provider-credential-fields.tsx @@ -38,6 +38,7 @@ export function RegistryCredentialFields({ return (
{schema.fields.map((field, index) => { + if (field.kind === "constant") return null; const error = errors[field.name]; const fieldId = `registry-credential-${instanceId}-${index}`; const id = `${fieldId}-control`; diff --git a/ui/lib/provider-credentials/fixtures/union-credential-schema.json b/ui/lib/provider-credentials/fixtures/union-credential-schema.json new file mode 100644 index 0000000000..55d0024f68 --- /dev/null +++ b/ui/lib/provider-credentials/fixtures/union-credential-schema.json @@ -0,0 +1,71 @@ +{ + "description": "Acme API credentials.", + "discriminator": { + "mapping": { + "basic": "#/$defs/AcmeBasicCredentials", + "token": "#/$defs/AcmeTokenCredentials" + }, + "propertyName": "auth_method" + }, + "oneOf": [ + { + "additionalProperties": false, + "description": "A long-lived API token issued from the console.", + "properties": { + "auth_method": { + "const": "token", + "default": "token", + "title": "Authentication Method", + "type": "string" + }, + "host": { + "description": "Hostname of the API.", + "examples": ["api.acme.test"], + "title": "API Host", + "type": "string" + }, + "token": { + "description": "API token.", + "format": "password", + "title": "Token", + "type": "string", + "writeOnly": true + } + }, + "required": ["host", "token"], + "title": "API token", + "type": "object" + }, + { + "additionalProperties": false, + "description": "A username and password exchanged for a session token.", + "properties": { + "auth_method": { + "const": "basic", + "title": "Authentication Method", + "type": "string" + }, + "host": { + "description": "Hostname of the API.", + "examples": ["api.acme.test"], + "title": "API Host", + "type": "string" + }, + "username": { + "title": "Username", + "type": "string" + }, + "password": { + "format": "password", + "title": "Password", + "type": "string", + "writeOnly": true + } + }, + "required": ["auth_method", "host", "username", "password"], + "title": "Username and password", + "type": "object" + } + ], + "title": "AcmeStaticCredentials" +} diff --git a/ui/lib/provider-credentials/provider-credential-schema.test.ts b/ui/lib/provider-credentials/provider-credential-schema.test.ts index eac0ac044f..73a60eb944 100644 --- a/ui/lib/provider-credentials/provider-credential-schema.test.ts +++ b/ui/lib/provider-credentials/provider-credential-schema.test.ts @@ -2,8 +2,10 @@ import { describe, expect, it } from "vitest"; import openaiSchema from "./fixtures/openai-credential-schema.json"; import templateSchema from "./fixtures/template-credential-schema.json"; +import unionSchema from "./fixtures/union-credential-schema.json"; import { parseRegistryCredentialSchema, + parseRegistryCredentialVariants, REGISTRY_CREDENTIAL_SCHEMA_LIMITS, } from "./provider-credential-schema"; @@ -157,10 +159,22 @@ describe("parseRegistryCredentialSchema", () => { ["definitions", { definitions: {} }], ["combinators", { anyOf: [] }], ["additional properties", { additionalProperties: true }], + ["typed additional properties", { additionalProperties: {} }], ])("rejects risky root keywords: %s", (_name, keyword) => { expect(parseRegistryCredentialSchema({ ...schema, ...keyword })).toBeNull(); }); + it("accepts the closed object pydantic emits for extra='forbid'", () => { + // Given / When + const result = parseRegistryCredentialSchema({ + ...schema, + additionalProperties: false, + }); + + // Then + expect(result).toEqual(parseRegistryCredentialSchema(schema)); + }); + it.each([ ["nested objects", { type: "object", properties: {} }], ["arrays", { type: "array" }], @@ -264,3 +278,98 @@ describe("parseRegistryCredentialSchema", () => { ]); }); }); + +describe("parseRegistryCredentialVariants", () => { + it("splits a pydantic union into variants that keep only their own fields", () => { + // Given / When + const result = parseRegistryCredentialVariants(unionSchema); + + // Then + expect( + result?.map(({ label, schema }) => [ + label, + schema.fields.map(({ name, kind }) => [name, kind]), + ]), + ).toEqual([ + [ + "API token", + [ + ["auth_method", "constant"], + ["host", "text"], + ["token", "password"], + ], + ], + [ + "Username and password", + [ + ["auth_method", "constant"], + ["host", "text"], + ["username", "text"], + ["password", "password"], + ], + ], + ]); + expect(result?.[1].schema.fields[0].defaultValue).toBe("basic"); + }); + + it("accepts anyOf, which pydantic emits for a union without discriminator", () => { + // Given + const { oneOf, discriminator: _discriminator, ...root } = unionSchema; + + // When + const result = parseRegistryCredentialVariants({ ...root, anyOf: oneOf }); + + // Then + expect(result?.map(({ label }) => label)).toEqual([ + "API token", + "Username and password", + ]); + }); + + it.each([ + ["an unsupported variant", { oneOf: [templateSchema, { type: "array" }] }], + ["no variants", { oneOf: [] }], + ["variants that are not a list", { oneOf: templateSchema }], + ["both combinators", { oneOf: [templateSchema], anyOf: [templateSchema] }], + ["unknown root keywords", { oneOf: [templateSchema], $defs: {} }], + [ + "a malformed discriminator", + { oneOf: [templateSchema], discriminator: 1 }, + ], + [ + "too many variants", + { + oneOf: Array.from( + { length: REGISTRY_CREDENTIAL_SCHEMA_LIMITS.MAX_VARIANTS + 1 }, + () => templateSchema, + ), + }, + ], + ])("rejects a union with %s", (_name, union) => { + expect(parseRegistryCredentialVariants(union)).toBeNull(); + }); + + it.each([ + { type: "string", const: "token", default: "basic" }, + { type: "string", const: "" }, + { type: "string", const: "token", enum: ["token"] }, + { type: "integer", const: 1 }, + ])("rejects a discriminator that is not one fixed string: %j", (property) => { + expect( + parseRegistryCredentialSchema({ + type: "object", + properties: { auth_method: property }, + }), + ).toBeNull(); + }); + + it("returns a flat schema as its only variant", () => { + // Given / When + const result = parseRegistryCredentialVariants(templateSchema); + + // Then + expect(result).toEqual([ + { schema: parseRegistryCredentialSchema(templateSchema) }, + ]); + }); +}); diff --git a/ui/lib/provider-credentials/provider-credential-schema.ts b/ui/lib/provider-credentials/provider-credential-schema.ts index ed1e4ee0e7..cc02591203 100644 --- a/ui/lib/provider-credentials/provider-credential-schema.ts +++ b/ui/lib/provider-credentials/provider-credential-schema.ts @@ -5,6 +5,7 @@ const FIELD_KIND = { TEXTAREA: "textarea", CHECKBOX: "checkbox", INTEGER: "integer", + CONSTANT: "constant", } as const; export const REGISTRY_CREDENTIAL_SCHEMA_LIMITS = { @@ -12,6 +13,7 @@ export const REGISTRY_CREDENTIAL_SCHEMA_LIMITS = { MAX_NAME_LENGTH: 50, MAX_TEXT_LENGTH: 200, MAX_ENUM_OPTIONS: 20, + MAX_VARIANTS: 8, } as const; type FieldKind = (typeof FIELD_KIND)[keyof typeof FIELD_KIND]; @@ -34,7 +36,18 @@ export interface RegistryCredentialSchema { readonly fields: readonly RegistryCredentialField[]; } -const ROOT = new Set("type title description properties required".split(" ")); +/** One alternative of a `oneOf`/`anyOf` schema, or the whole flat schema. */ +export interface RegistryCredentialVariant { + readonly label?: string; + readonly schema: RegistryCredentialSchema; +} + +const ROOT = new Set( + "type title description properties required additionalProperties".split(" "), +); +const UNION_ROOT = new Set( + "title description oneOf anyOf discriminator".split(" "), +); const FIELD = new Set( "title description type format writeOnly enum default examples x-prowler-widget".split( " ", @@ -44,6 +57,9 @@ const BOOLEAN_FIELD = new Set("title description type default".split(" ")); const INTEGER_FIELD = new Set( "title description type default minimum maximum".split(" "), ); +const CONSTANT_FIELD = new Set( + "title description type const default".split(" "), +); const FORBIDDEN_NAMES = new Set(["__proto__", "prototype", "constructor"]); const FIELD_NAME = /^[A-Za-z][A-Za-z0-9_-]*$/; @@ -87,7 +103,14 @@ function hasOnly( export function parseRegistryCredentialSchema( value: unknown, ): RegistryCredentialSchema | null { - if (!isRecord(value) || !hasOnly(value, ROOT) || value.type !== "object") { + if ( + !isRecord(value) || + !hasOnly(value, ROOT) || + value.type !== "object" || + // Only a closed object: the form never sends undeclared keys anyway. + (value.additionalProperties !== undefined && + value.additionalProperties !== false) + ) { return null; } if ( @@ -189,6 +212,22 @@ export function parseRegistryCredentialSchema( }); continue; } + if (property.type === "string" && property.const !== undefined) { + // A union's discriminator: fixed by the variant, never typed by the user. + if ( + !hasOnly(property, CONSTANT_FIELD) || + !isText(property.const) || + (defaultValue !== undefined && defaultValue !== property.const) + ) { + return null; + } + fields.push({ + ...baseField, + kind: FIELD_KIND.CONSTANT, + defaultValue: property.const, + }); + continue; + } if (property.type !== "string" || !hasOnly(property, FIELD)) return null; const format = property.format; @@ -256,3 +295,42 @@ export function parseRegistryCredentialSchema( } return { fields }; } + +export function parseRegistryCredentialVariants( + value: unknown, +): readonly RegistryCredentialVariant[] | null { + const isUnion = + isRecord(value) && + (Object.hasOwn(value, "oneOf") || Object.hasOwn(value, "anyOf")); + if (!isUnion) { + const schema = parseRegistryCredentialSchema(value); + return schema ? [{ schema }] : null; + } + if ( + !hasOnly(value, UNION_ROOT) || + (Object.hasOwn(value, "oneOf") && Object.hasOwn(value, "anyOf")) || + (value.title !== undefined && !isText(value.title)) || + (value.description !== undefined && + typeof value.description !== "string") || + (value.discriminator !== undefined && !isRecord(value.discriminator)) + ) { + return null; + } + + const branches = value.oneOf ?? value.anyOf; + if ( + !Array.isArray(branches) || + branches.length === 0 || + branches.length > REGISTRY_CREDENTIAL_SCHEMA_LIMITS.MAX_VARIANTS + ) { + return null; + } + const variants: RegistryCredentialVariant[] = []; + for (const branch of branches) { + const schema = parseRegistryCredentialSchema(branch); + if (!schema) return null; + const label = isRecord(branch) ? branch.title : undefined; + variants.push({ ...(isText(label) ? { label } : {}), schema }); + } + return variants; +} diff --git a/ui/lib/provider-credentials/provider-credential-values.ts b/ui/lib/provider-credentials/provider-credential-values.ts index 6673adf9ea..3526cd8808 100644 --- a/ui/lib/provider-credentials/provider-credential-values.ts +++ b/ui/lib/provider-credentials/provider-credential-values.ts @@ -1,6 +1,7 @@ import type { RegistryCredentialSchema, RegistryCredentialValue, + RegistryCredentialVariant, } from "./provider-credential-schema"; export function getCredentialDefaults( @@ -17,16 +18,18 @@ export function getCredentialDefaults( ); } -export function validateCredentialValues( - schema: RegistryCredentialSchema, - values: unknown, -): +type CredentialValidation = | { valid: true; secret: Record; errors: Record; } - | { valid: false; errors: Record } { + | { valid: false; errors: Record }; + +export function validateCredentialValues( + schema: RegistryCredentialSchema, + values: unknown, +): CredentialValidation { if (!values || typeof values !== "object" || Array.isArray(values)) return { valid: false, @@ -47,7 +50,17 @@ export function validateCredentialValues( const secret: Record = {}; for (const field of schema.fields) { const value = fields.get(field.name); - if (value === undefined || (value === "" && field.kind !== "checkbox")) { + if (field.kind === "constant") { + // Always sent: it tells the API which variant the secret follows. + if (value !== undefined && value !== field.defaultValue) { + errors[field.name] = `Enter a valid ${field.label}`; + } else if (field.defaultValue !== undefined) { + secret[field.name] = field.defaultValue; + } + } else if ( + value === undefined || + (value === "" && field.kind !== "checkbox") + ) { if (field.required) errors[field.name] = `${field.label} is required`; } else if (field.kind === "checkbox") { if (typeof value !== "boolean") { @@ -83,3 +96,19 @@ export function validateCredentialValues( ? { valid: false, errors } : { valid: true, secret, errors }; } + +/** Valid when one variant accepts the values, as JSON Schema `anyOf` does. */ +export function validateCredentialVariants( + variants: readonly RegistryCredentialVariant[], + values: unknown, +): CredentialValidation { + const results = variants.map(({ schema }) => + validateCredentialValues(schema, values), + ); + // Otherwise report the variant that owns every submitted field. + return ( + results.find(({ valid }) => valid) ?? + results.find(({ errors }) => !errors._form) ?? + results[0] + ); +}