Skip to content
Open
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
8 changes: 8 additions & 0 deletions frontend/src/components/Config/CreateTargetDialog.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,12 @@ export const useCreateTargetDialogStyles = makeStyles({
flexDirection: 'column',
gap: tokens.spacingVerticalL,
},
warningMessage: {
width: '100%',
},
warningMessageBody: {
whiteSpace: 'normal',
overflowWrap: 'anywhere',
wordBreak: 'break-word',
},
})
176 changes: 176 additions & 0 deletions frontend/src/components/Config/CreateTargetDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -497,4 +497,180 @@ describe("CreateTargetDialog", () => {

expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
});

it("should hide the API Key field and omit api_key/include auth_mode when Entra is selected", async () => {
const onCreated = jest.fn();
const user = userEvent.setup();
mockedTargetsApi.createTarget.mockResolvedValue({
target_registry_name: "openai_chat_entra",
target_type: "OpenAIChatTarget",
});

render(
<TestWrapper>
<CreateTargetDialog {...defaultProps} onCreated={onCreated} />
</TestWrapper>
);

await selectTargetType(user, "OpenAIChatTarget");

const endpointInput = screen.getByPlaceholderText(
"https://your-resource.openai.azure.com/"
);
fireEvent.change(endpointInput, {
target: { value: "https://my-resource.openai.azure.com/" },
});

// check that API Key field is visible by default.
expect(
screen.getByPlaceholderText("API key (stored in memory only)")
).toBeInTheDocument();

// Select Entra option.
await user.click(
screen.getByRole("radio", {
name: /Microsoft Entra Authentication/,
})
);

// check that API Key field is hidden when Entra mode is selected.
expect(
screen.queryByPlaceholderText("API key (stored in memory only)")
).not.toBeInTheDocument();

await user.click(screen.getByText("Create Target"));

await waitFor(() => {
expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({
type: "OpenAIChatTarget",
params: {
endpoint: "https://my-resource.openai.azure.com/",
},
auth_mode: "entra",
});
expect(onCreated).toHaveBeenCalled();
});
});

it("should clear a previously-typed API key when switching to Entra", async () => {
const onCreated = jest.fn();
const user = userEvent.setup();
mockedTargetsApi.createTarget.mockResolvedValue({
target_registry_name: "openai_chat_entra",
target_type: "OpenAIChatTarget",
});

render(
<TestWrapper>
<CreateTargetDialog {...defaultProps} onCreated={onCreated} />
</TestWrapper>
);

await selectTargetType(user, "OpenAIChatTarget");

const endpointInput = screen.getByPlaceholderText(
"https://your-resource.openai.azure.com/"
);
fireEvent.change(endpointInput, {
target: { value: "https://my-resource.openai.azure.com/" },
});

// Type a key, then switch to Entra option.
fireEvent.change(
screen.getByPlaceholderText("API key (stored in memory only)"),
{ target: { value: "sk-typed-before-switch" } }
);

await user.click(
screen.getByRole("radio", { name: /Microsoft Entra Authentication/ })
);

await user.click(screen.getByText("Create Target"));

await waitFor(() => {
const call = mockedTargetsApi.createTarget.mock.calls[0][0];
expect(call.auth_mode).toBe("entra");
expect(call.params).not.toHaveProperty("api_key");
});
});

it("should warn the user when Entra is selected for a non-Azure OpenAI endpoint", async () => {
const user = userEvent.setup();

render(
<TestWrapper>
<CreateTargetDialog {...defaultProps} />
</TestWrapper>
);

await selectTargetType(user, "OpenAIChatTarget");

const endpointInput = screen.getByPlaceholderText(
"https://your-resource.openai.azure.com/"
);
fireEvent.change(endpointInput, { target: { value: "https://api.openai.com/" } });

await user.click(
screen.getByRole("radio", { name: /Microsoft Entra Authentication/ })
);

expect(
screen.getByText(/Entra auth only works with Azure OpenAI/)
).toBeInTheDocument();
});

it("should NOT warn when Entra is selected for a recognized Azure endpoint", async () => {
const user = userEvent.setup();

render(
<TestWrapper>
<CreateTargetDialog {...defaultProps} />
</TestWrapper>
);

await selectTargetType(user, "OpenAIChatTarget");

const endpointInput = screen.getByPlaceholderText(
"https://your-resource.openai.azure.com/"
);
fireEvent.change(endpointInput, {
target: { value: "https://my-resource.openai.azure.com/" },
});

await user.click(
screen.getByRole("radio", { name: /Microsoft Entra Authentication/ })
);

expect(
screen.queryByText(/Entra auth only works with Azure OpenAI/)
).not.toBeInTheDocument();
});

it("should disable Create Target and skip API call for Entra + non-Azure OpenAI endpoint", async () => {
const user = userEvent.setup();

render(
<TestWrapper>
<CreateTargetDialog {...defaultProps} />
</TestWrapper>
);

await selectTargetType(user, "OpenAIChatTarget");

const endpointInput = screen.getByPlaceholderText(
"https://your-resource.openai.azure.com/"
);
fireEvent.change(endpointInput, { target: { value: "https://api.test.com/" } });

await user.click(
screen.getByRole("radio", { name: /Microsoft Entra Authentication/ })
);

const createButton = screen.getByText("Create Target").closest("button");
expect(createButton).toBeDisabled();

await user.click(screen.getByText("Create Target"));

expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled();
});
});
78 changes: 68 additions & 10 deletions frontend/src/components/Config/CreateTargetDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
Button,
Input,
Label,
Radio,
RadioGroup,
Select,
Switch,
Text,
Expand All @@ -32,6 +34,26 @@ const TARGET_TYPE_CONFIG: Record<string, 'openai' | 'azureml'> = {

const SUPPORTED_TARGET_TYPES = Object.keys(TARGET_TYPE_CONFIG)

type AuthMode = 'api_key' | 'entra'

// Mirrors backend's hostname-suffix check (list in target_service.py).
// The backend still does the check and will reject unsupported endpoints, but this allows us to show a warning in the UI if the user selects Microsoft Entra authentication with a non-Azure OpenAI endpoint.
const AZURE_OPENAI_HOSTNAME_SUFFIXES = [
'.openai.azure.com',
'.ai.azure.com',
'.services.ai.azure.com',
'.cognitiveservices.azure.com',
]

function isAzureOpenAiEndpoint(endpoint: string): boolean {
try {
const host = new URL(endpoint).hostname.toLowerCase()
return AZURE_OPENAI_HOSTNAME_SUFFIXES.some((s) => host.endsWith(s))
} catch {
return false
}
}

interface CreateTargetDialogProps {
open: boolean
onClose: () => void
Expand All @@ -45,6 +67,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT
const [modelName, setModelName] = useState('')
const [hasDifferentUnderlying, setHasDifferentUnderlying] = useState(false)
const [underlyingModel, setUnderlyingModel] = useState('')
const [authMode, setAuthMode] = useState<AuthMode>('api_key')
const [apiKey, setApiKey] = useState('')
const [maxNewTokens, setMaxNewTokens] = useState('400')
const [temperature, setTemperature] = useState('1.0')
Expand All @@ -54,14 +77,19 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT
const [error, setError] = useState<string | null>(null)
const [fieldErrors, setFieldErrors] = useState<{ targetType?: string; endpoint?: string }>({})

const isAzureML = TARGET_TYPE_CONFIG[targetType] === 'azureml'
const targetKind = TARGET_TYPE_CONFIG[targetType]
const isAzureML = targetKind === 'azureml'
const isOpenAi = targetKind === 'openai'
const isEntra = authMode === 'entra'
const showNonAzureEntraWarning = isEntra && isOpenAi && endpoint !== '' && !isAzureOpenAiEndpoint(endpoint)

const resetForm = () => {
setTargetType('')
setEndpoint('')
setModelName('')
setHasDifferentUnderlying(false)
setUnderlyingModel('')
setAuthMode('api_key')
setApiKey('')
setMaxNewTokens('400')
setTemperature('1.0')
Expand Down Expand Up @@ -94,7 +122,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT
endpoint,
}
if (modelName) params.model_name = modelName
if (apiKey) params.api_key = apiKey
if (!isEntra && apiKey) params.api_key = apiKey

if (hasDifferentUnderlying && underlyingModel) params.underlying_model = underlyingModel

Expand All @@ -112,6 +140,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT
await targetsApi.createTarget({
type: targetType,
params,
...(isEntra ? { auth_mode: 'entra' as const } : {}),
})
resetForm()
onCreated()
Expand Down Expand Up @@ -243,15 +272,40 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT
</>
)}

<Field label="API Key">
<Input
type="password"
placeholder="API key (stored in memory only)"
value={apiKey}
onChange={(_, data) => setApiKey(data.value)}
/>
<Field label="Authentication">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Authentication field is rendered unconditionally — the Entra radio shows for every entry in TARGET_TYPE_CONFIG, and even before any type is picked. Today that's fine because all 7 entries are Entra-capable, but the moment someone adds a non-Azure target (HuggingFace, Ollama, etc.) to the dropdown, picking Entra → Submit will hit the backend's "does not support Entra" raise and surface as a confusing 500.

I'd make Entra capability an explicit per-type property rather than an implicit assumption:

interface TargetTypeConfig { kind: 'openai' | 'azureml'; supportsEntra: boolean }
const TARGET_TYPE_CONFIG: Record<string, TargetTypeConfig> = {
  OpenAIChatTarget:  { kind: 'openai',  supportsEntra: true },
  // ...
  AzureMLChatTarget: { kind: 'azureml', supportsEntra: true },
}

Then hide the whole Authentication field (and fall back to the plain API Key input) when !targetConfig?.supportsEntra. Also worth gating it on targetType !== '' so we don't show auth controls before a type is selected.

<RadioGroup
value={authMode}
onChange={(_, data) => {
const next = data.value as AuthMode
setAuthMode(next)
if (next === 'entra') setApiKey('')
}}
>
<Radio value="api_key" label="API Key" />
<Radio value="entra" label="Microsoft Entra Authentication" />
</RadioGroup>
</Field>

{showNonAzureEntraWarning && (
<MessageBar intent="warning" className={styles.warningMessage}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things on this warning block:

  1. The MessageBar is intent="warning" but the body text starts with "Error:". Pick one — either drop the Error: prefix or switch to intent="error".
  2. The warning is purely informational — disabled={submitting || !targetType || !endpoint} on the Create Target button doesn't account for it, so the user can still click Create and get a server round-trip failure. Either include showNonAzureEntraWarning in the disabled condition, or surface this as a validationState="error" on the Endpoint field so submission is blocked the same way other field errors are.

<MessageBarBody className={styles.warningMessageBody}>
Error: Entra auth only works with Azure OpenAI / AI Foundry endpoints (for example,
*.openai.azure.com or *.ai.azure.com).
</MessageBarBody>
</MessageBar>
)}

{!isEntra && (
<Field label="API Key">
<Input
type="password"
placeholder="API key (stored in memory only)"
value={apiKey}
onChange={(_, data) => setApiKey(data.value)}
/>
</Field>
)}

<Label size="small" style={{ color: tokens.colorNeutralForeground3 }}>
Targets can also be auto-populated by adding an initializer (e.g. <code>airt</code>) to your{' '}
<code>~/.pyrit/.pyrit_conf</code> file, which reads endpoints from your <code>.env</code> and{' '}
Expand All @@ -266,7 +320,11 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT
<Button appearance="secondary" onClick={handleClose} disabled={submitting}>
Cancel
</Button>
<Button appearance="primary" onClick={handleSubmit} disabled={submitting || !targetType || !endpoint}>
<Button
appearance="primary"
onClick={handleSubmit}
disabled={submitting || !targetType || !endpoint || showNonAzureEntraWarning}
>
{submitting ? 'Creating...' : 'Create Target'}
</Button>
</DialogActions>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export interface TargetListResponse {
export interface CreateTargetRequest {
type: string
params: Record<string, unknown>
auth_mode?: 'api_key' | 'entra'
}

// --- Converters ---
Expand Down
10 changes: 9 additions & 1 deletion pyrit/backend/models/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
This module defines the Instance models for runtime target management.
"""

from typing import Any, Optional
from typing import Any, Literal, Optional

from pydantic import BaseModel, Field

Expand Down Expand Up @@ -80,3 +80,11 @@ class CreateTargetRequest(BaseModel):

type: str = Field(..., description="Target type (e.g., 'OpenAIChatTarget')")
params: dict[str, Any] = Field(default_factory=dict, description="Target constructor parameters")
auth_mode: Literal["api_key", "entra"] = Field(
"api_key",
description=(
"Authentication mode. 'api_key' uses the api_key in params (default)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing separator between the concatenated strings — this renders as …(default)'entra' uses…. Add a period + space at the end of this line: "…(default). ".

"'entra' uses Microsoft Entra ID; requires an Azure endpoint and is "
"supported by OpenAI-family targets and AzureMLChatTarget."
),
)
Loading
Loading