diff --git a/src/lib/elicit-credentials.ts b/src/lib/elicit-credentials.ts new file mode 100644 index 00000000..f70232e4 --- /dev/null +++ b/src/lib/elicit-credentials.ts @@ -0,0 +1,84 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import logger from "../logger.js"; + +export interface CredentialField { + key: string; + title: string; + description: string; +} + +/** + * Collects credential values for a tool WITHOUT routing them through the model. + * + * When the connected client advertises MCP elicitation support, any requested + * field the caller did not already supply is requested directly from the user + * via the client — the value flows user -> client -> server and never appears in + * the LLM's tool-call arguments, context, or logs. + * + * When the client does not support elicitation, or the user declines/cancels, or + * the request errors, the values are returned exactly as provided. This keeps the + * existing argument-based flow working unchanged (backward compatible), and makes + * the helper safe to ship to transports that cannot elicit (it degrades to the + * arg path rather than failing). + */ +export async function elicitCredentialsIfSupported( + server: McpServer, + provided: Record, + fields: CredentialField[], + message: string, +): Promise> { + const missing = fields.filter((field) => !provided[field.key]); + if (missing.length === 0) { + return provided; + } + + // Only attempt elicitation when the client explicitly supports it; otherwise + // fall back to the caller-provided values (existing behavior). + const capabilities = server.server.getClientCapabilities(); + if (!capabilities?.elicitation) { + return provided; + } + + const properties: Record< + string, + { type: "string"; title: string; description: string } + > = {}; + for (const field of missing) { + properties[field.key] = { + type: "string", + title: field.title, + description: field.description, + }; + } + + let result; + try { + result = await server.server.elicitInput({ + mode: "form", + message, + requestedSchema: { + type: "object", + properties, + required: missing.map((field) => field.key), + }, + }); + } catch (error) { + // A client that advertised elicitation but failed to handle it must not + // break the tool — fall back to whatever was provided. + logger.info(`Elicitation request failed; using provided values: ${error}`); + return provided; + } + + if (result.action !== "accept" || !result.content) { + return provided; + } + + const merged = { ...provided }; + for (const field of missing) { + const value = result.content[field.key]; + if (typeof value === "string" && value.length > 0) { + merged[field.key] = value; + } + } + return merged; +} diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index fc76eed9..221b6c8a 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -12,6 +12,7 @@ import { parseAccessibilityReportFromCSV } from "./accessiblity-utils/report-par import { queryAccessibilityRAG } from "./accessiblity-utils/accessibility-rag.js"; import { getBrowserStackAuth } from "../lib/get-auth.js"; import { BrowserStackConfig } from "../lib/types.js"; +import { elicitCredentialsIfSupported } from "../lib/elicit-credentials.js"; import logger from "../logger.js"; interface AuthCredentials { @@ -477,8 +478,18 @@ export default function addAccessibilityTools( "Authentication type: 'form' for form-based auth, 'basic' for HTTP basic auth", ), url: z.string().describe("URL of the authentication page"), - username: z.string().describe("Username for authentication"), - password: z.string().describe("Password for authentication"), + username: z + .string() + .optional() + .describe( + "Site username. Omit to have it requested securely from the user.", + ), + password: z + .string() + .optional() + .describe( + "Site password. Omit to have it requested securely from the user; do not pass real passwords here.", + ), usernameSelector: z .string() .optional() @@ -493,8 +504,36 @@ export default function addAccessibilityTools( .describe("CSS selector for submit button (required for form auth)"), }, async (args) => { + const creds = await elicitCredentialsIfSupported( + server, + { username: args.username, password: args.password }, + [ + { + key: "username", + title: "Site username", + description: `Username for the login being configured ("${args.name}")`, + }, + { + key: "password", + title: "Site password", + description: `Password for the login being configured ("${args.name}")`, + }, + ], + `Enter the login credentials for accessibility auth config "${args.name}".`, + ); + + if (!creds.username || !creds.password) { + return createErrorResponse( + "Username and password are required to create an auth config. Provide them when prompted, or pass them as arguments.", + ); + } + return await executeCreateAuthConfig( - args as AuthConfigArgs, + { + ...args, + username: creds.username, + password: creds.password, + } as AuthConfigArgs, server, config, ); diff --git a/src/tools/testmanagement-utils/create-lca-steps.ts b/src/tools/testmanagement-utils/create-lca-steps.ts index b9bab601..19b894b2 100644 --- a/src/tools/testmanagement-utils/create-lca-steps.ts +++ b/src/tools/testmanagement-utils/create-lca-steps.ts @@ -21,6 +21,13 @@ export const CreateLCAStepsSchema = z.object({ .string() .describe("Identifier of the test case (e.g., 'TC-12345')"), base_url: z.string().describe("Base URL for the test (e.g., 'google.com')"), + requires_authentication: z + .boolean() + .optional() + .default(false) + .describe( + "Set true if the test case requires login. When true, credentials are requested securely from the user — prefer this over passing them.", + ), credentials: z .object({ username: z.string().describe("Username for authentication"), @@ -28,7 +35,7 @@ export const CreateLCAStepsSchema = z.object({ }) .optional() .describe( - "Optional credentials for authentication. Extract from the test case details if provided in it. This is required for the test cases which require authentication.", + "Login credentials. Omit and set requires_authentication instead to have them requested securely from the user; passed here only as a fallback.", ), local_enabled: z .boolean() diff --git a/src/tools/testmanagement.ts b/src/tools/testmanagement.ts index e664ca4c..3bdf56b1 100644 --- a/src/tools/testmanagement.ts +++ b/src/tools/testmanagement.ts @@ -89,6 +89,7 @@ import { } from "./testmanagement-utils/get-sub-testplan.js"; import { BrowserStackConfig } from "../lib/types.js"; +import { elicitCredentialsIfSupported } from "../lib/elicit-credentials.js"; //TODO: Moving the traceMCP and catch block to the parent(server) function @@ -515,7 +516,41 @@ export async function createLCAStepsTool( undefined, config, ); - return await createLCASteps(args, context, config); + + let effectiveArgs = args; + if ( + args.requires_authentication && + (!args.credentials?.username || !args.credentials?.password) + ) { + const creds = await elicitCredentialsIfSupported( + server, + { + username: args.credentials?.username, + password: args.credentials?.password, + }, + [ + { + key: "username", + title: "Login username", + description: `Username for test case ${args.test_case_identifier}`, + }, + { + key: "password", + title: "Login password", + description: `Password for test case ${args.test_case_identifier}`, + }, + ], + `Enter the login credentials for test case ${args.test_case_identifier}.`, + ); + if (creds.username && creds.password) { + effectiveArgs = { + ...args, + credentials: { username: creds.username, password: creds.password }, + }; + } + } + + return await createLCASteps(effectiveArgs, context, config); } catch (err) { trackMCP("createLCASteps", server.server.getClientVersion()!, err, config); return { diff --git a/tests/lib/elicit-credentials.test.ts b/tests/lib/elicit-credentials.test.ts new file mode 100644 index 00000000..4e87fe95 --- /dev/null +++ b/tests/lib/elicit-credentials.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { elicitCredentialsIfSupported } from "../../src/lib/elicit-credentials"; + +vi.mock("../../src/logger", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); + +const FIELDS = [ + { key: "username", title: "Username", description: "The username" }, + { key: "password", title: "Password", description: "The password" }, +]; + +function makeServer(opts: { + elicitationSupported?: boolean; + elicitResult?: any; + elicitThrows?: boolean; +}) { + const elicitInput = vi.fn(); + if (opts.elicitThrows) { + elicitInput.mockRejectedValue(new Error("client error")); + } else { + elicitInput.mockResolvedValue(opts.elicitResult); + } + return { + server: { + getClientCapabilities: vi + .fn() + .mockReturnValue(opts.elicitationSupported ? { elicitation: {} } : {}), + elicitInput, + }, + } as any; +} + +describe("elicitCredentialsIfSupported", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns provided values unchanged when nothing is missing (no elicitation)", async () => { + const server = makeServer({ elicitationSupported: true }); + const out = await elicitCredentialsIfSupported( + server, + { username: "u", password: "p" }, + FIELDS, + "msg", + ); + expect(out).toEqual({ username: "u", password: "p" }); + expect(server.server.elicitInput).not.toHaveBeenCalled(); + }); + + it("falls back to provided values when the client does not support elicitation", async () => { + const server = makeServer({ elicitationSupported: false }); + const out = await elicitCredentialsIfSupported( + server, + { username: undefined, password: undefined }, + FIELDS, + "msg", + ); + expect(out).toEqual({ username: undefined, password: undefined }); + expect(server.server.elicitInput).not.toHaveBeenCalled(); + }); + + it("elicits missing values when supported and the user accepts", async () => { + const server = makeServer({ + elicitationSupported: true, + elicitResult: { action: "accept", content: { username: "eu", password: "ep" } }, + }); + const out = await elicitCredentialsIfSupported( + server, + { username: undefined, password: undefined }, + FIELDS, + "msg", + ); + expect(out).toEqual({ username: "eu", password: "ep" }); + // Only missing fields are requested and marked required. + const req = server.server.elicitInput.mock.calls[0][0].requestedSchema; + expect(req.required).toEqual(["username", "password"]); + }); + + it("only elicits the field that is missing", async () => { + const server = makeServer({ + elicitationSupported: true, + elicitResult: { action: "accept", content: { password: "ep" } }, + }); + const out = await elicitCredentialsIfSupported( + server, + { username: "u", password: undefined }, + FIELDS, + "msg", + ); + expect(out).toEqual({ username: "u", password: "ep" }); + const req = server.server.elicitInput.mock.calls[0][0].requestedSchema; + expect(req.required).toEqual(["password"]); + }); + + it("falls back to provided values when the user declines", async () => { + const server = makeServer({ + elicitationSupported: true, + elicitResult: { action: "decline" }, + }); + const out = await elicitCredentialsIfSupported( + server, + { username: undefined, password: undefined }, + FIELDS, + "msg", + ); + expect(out).toEqual({ username: undefined, password: undefined }); + }); + + it("falls back to provided values when elicitation throws", async () => { + const server = makeServer({ elicitationSupported: true, elicitThrows: true }); + const out = await elicitCredentialsIfSupported( + server, + { username: "u", password: undefined }, + FIELDS, + "msg", + ); + expect(out).toEqual({ username: "u", password: undefined }); + }); +}); diff --git a/tests/tools/accessibility.test.ts b/tests/tools/accessibility.test.ts index 307deac5..61cdb42c 100644 --- a/tests/tools/accessibility.test.ts +++ b/tests/tools/accessibility.test.ts @@ -83,7 +83,12 @@ describe("Accessibility Tools", () => { tool: vi.fn((name: string, _desc: string, _schema: any, handler: (...args: any[]) => any) => { handlers[name] = handler; }), - server: { getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }) }, + server: { + getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }), + // Default: client does NOT support elicitation (arg-based flow). + getClientCapabilities: vi.fn().mockReturnValue({}), + elicitInput: vi.fn(), + }, }; addAccessibilityTools(serverMock, mockConfig); }); @@ -123,6 +128,35 @@ describe("Accessibility Tools", () => { expect(serialized).toContain("auth-1"); }); + it("createAccessibilityAuthConfig — elicits credentials from the user when the client supports it and they are not passed as args", async () => { + serverMock.server.getClientCapabilities.mockReturnValue({ elicitation: {} }); + serverMock.server.elicitInput.mockResolvedValue({ + action: "accept", + content: { username: "elicited-user", password: "elicited-pass" }, + }); + + const result = await handlers["createAccessibilityAuthConfig"]( + { type: "basic", name: "no-args-auth", url: "https://example.com/login" }, + { sendNotification: vi.fn(), _meta: {} }, + ); + + // Elicitation was used, and the config was created without creds in args. + expect(serverMock.server.elicitInput).toHaveBeenCalledTimes(1); + expect(result.isError).toBeFalsy(); + // The elicited secret is never echoed back (allowlisted response). + expect(JSON.stringify(result.content)).not.toContain("elicited-pass"); + }); + + it("createAccessibilityAuthConfig — errors when creds are missing and the client cannot elicit", async () => { + // Default mock: getClientCapabilities returns {} (no elicitation support). + const result = await handlers["createAccessibilityAuthConfig"]( + { type: "basic", name: "no-args-auth", url: "https://example.com/login" }, + { sendNotification: vi.fn(), _meta: {} }, + ); + expect(result.isError).toBe(true); + expect(serverMock.server.elicitInput).not.toHaveBeenCalled(); + }); + it("createAccessibilityAuthConfig — FAIL: form auth without required selectors returns error", async () => { const result = await handlers["createAccessibilityAuthConfig"]( { type: "form", name: "test-form", username: "user", password: "pass", url: "https://example.com" }, diff --git a/tests/tools/testmanagement.test.ts b/tests/tools/testmanagement.test.ts index 2ec69d05..00e91400 100644 --- a/tests/tools/testmanagement.test.ts +++ b/tests/tools/testmanagement.test.ts @@ -682,6 +682,67 @@ describe("createLCAStepsTool", () => { expect(result.isError).toBe(false); }); + it("elicits credentials when requires_authentication is set and the client supports elicitation", async () => { + (createLCASteps as Mock).mockResolvedValue({ content: [], isError: false }); + const localServer = { + server: { + getClientVersion: () => "test", + getClientCapabilities: () => ({ elicitation: {} }), + elicitInput: vi.fn().mockResolvedValue({ + action: "accept", + content: { username: "elicited-user", password: "elicited-pass" }, + }), + }, + } as any; + const args = { ...validArgs, credentials: undefined, requires_authentication: true }; + + await createLCAStepsTool(args as any, mockContext, mockConfig, localServer); + + expect(localServer.server.elicitInput).toHaveBeenCalledTimes(1); + expect(createLCASteps).toHaveBeenCalledWith( + expect.objectContaining({ + credentials: { username: "elicited-user", password: "elicited-pass" }, + }), + mockContext, + mockConfig, + ); + }); + + it("does not elicit when requires_authentication is false", async () => { + (createLCASteps as Mock).mockResolvedValue({ content: [], isError: false }); + const elicitInput = vi.fn(); + const localServer = { + server: { + getClientVersion: () => "test", + getClientCapabilities: () => ({ elicitation: {} }), + elicitInput, + }, + } as any; + const args = { ...validArgs, credentials: undefined, requires_authentication: false }; + + await createLCAStepsTool(args as any, mockContext, mockConfig, localServer); + + expect(elicitInput).not.toHaveBeenCalled(); + }); + + it("falls back to the arg path (no elicit) when requires_authentication but the client cannot elicit", async () => { + (createLCASteps as Mock).mockResolvedValue({ content: [], isError: false }); + const elicitInput = vi.fn(); + const localServer = { + server: { + getClientVersion: () => "test", + getClientCapabilities: () => ({}), + elicitInput, + }, + } as any; + const args = { ...validArgs, credentials: undefined, requires_authentication: true }; + + await createLCAStepsTool(args as any, mockContext, mockConfig, localServer); + + expect(elicitInput).not.toHaveBeenCalled(); + expect(createLCASteps).toHaveBeenCalled(); + }); + it("handles errors when creating LCA steps", async () => { (createLCASteps as Mock).mockRejectedValue(new Error("API Error"));