Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions src/lib/elicit-credentials.ts
Original file line number Diff line number Diff line change
@@ -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<string, string | undefined>,
fields: CredentialField[],
message: string,
): Promise<Record<string, string | undefined>> {
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;
}
45 changes: 42 additions & 3 deletions src/tools/accessibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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,
);
Expand Down
9 changes: 8 additions & 1 deletion src/tools/testmanagement-utils/create-lca-steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,21 @@ 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"),
password: z.string().describe("Password for authentication"),
})
.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()
Expand Down
37 changes: 36 additions & 1 deletion src/tools/testmanagement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 {
Expand Down
118 changes: 118 additions & 0 deletions tests/lib/elicit-credentials.test.ts
Original file line number Diff line number Diff line change
@@ -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 });
});
});
36 changes: 35 additions & 1 deletion tests/tools/accessibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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" },
Expand Down
Loading
Loading