fix: Proper MCP / Oauth support (#10965)

* fix: Proper MCP / Oauth support

* [autofix.ci] apply automated fixes

* Fix ruff errors

* Update service.py

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes (attempt 2/3)

* Update AuthModal.test.tsx

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Carlos Coelho <80289056+carlosrcoelho@users.noreply.github.com>
This commit is contained in:
Eric Hare
2025-12-11 05:53:51 -08:00
committed by GitHub
parent 2f9136c17a
commit 397deff1bc
9 changed files with 72 additions and 29 deletions

View File

@ -237,7 +237,7 @@ For more information, see your OAuth provider's documentation.
| **Host** | OAuth server host | MCP Composer default. | `localhost` |
| **Port** | OAuth server port | MCP Composer default. | `9000` |
| **Server URL** | Full OAuth server URL | Combines the MCP Composer default OAuth host and port. | `http://localhost:9000` |
| **Callback Path** | OAuth callback URL on your server | You define this address during OAuth app registration. | `http://localhost:9000/auth/idaas/callback` |
| **Callback URL** | OAuth callback URL on your server | You define this full URL during OAuth app registration. This must match exactly what you register with your OAuth provider. | `http://localhost:9000/auth/idaas/callback` |
| **Client ID** | Your OAuth client identifier | From your OAuth provider. | `Ov23li9vx2grVL61qjb` |
| **Client Secret** | Your OAuth client secret | From your OAuth provider. | `1234567890abcdef1234567890abcdef12345678` |
| **Authorization URL** | OAuth authorization endpoint | From your OAuth provider. | `https://github.com/login/oauth/authorize` |

View File

@ -6977,7 +6977,20 @@
"type": "null"
}
],
"title": "Oauth Callback Path"
"title": "Oauth Callback Path",
"deprecated": true,
"description": "Deprecated: Use oauth_callback_url instead"
},
"oauth_callback_url": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"title": "Oauth Callback Url"
},
"oauth_client_id": {
"anyOf": [

View File

@ -4302,6 +4302,13 @@ components:
- type: string
- type: 'null'
title: Oauth Callback Path
deprecated: true
description: 'Deprecated: Use oauth_callback_url instead'
oauth_callback_url:
anyOf:
- type: string
- type: 'null'
title: Oauth Callback Url
oauth_client_id:
anyOf:
- type: string

View File

@ -426,7 +426,8 @@ class AuthSettings(BaseModel):
oauth_host: str | None = None
oauth_port: str | None = None
oauth_server_url: str | None = None
oauth_callback_path: str | None = None
oauth_callback_path: str | None = None # Deprecated: use oauth_callback_url instead
oauth_callback_url: str | None = None
oauth_client_id: str | None = None
oauth_client_secret: SecretStr | None = None
oauth_auth_url: str | None = None
@ -434,6 +435,13 @@ class AuthSettings(BaseModel):
oauth_mcp_scope: str | None = None
oauth_provider_scope: str | None = None
def model_post_init(self, __context, /) -> None:
"""Normalize oauth_callback_path to oauth_callback_url for backwards compatibility."""
# If oauth_callback_url is not set but oauth_callback_path is, use the path value
if self.oauth_callback_url is None and self.oauth_callback_path is not None:
self.oauth_callback_url = self.oauth_callback_path
# If both are set, oauth_callback_url takes precedence (already set correctly)
class MCPSettings(BaseModel):
"""Model representing MCP settings for a flow."""

View File

@ -99,7 +99,7 @@ describe("AuthModal OAuth Port Synchronization", () => {
expect(serverUrlInput).toHaveValue("http://example.com:8080");
});
it("should auto-sync Callback Path when Port is changed", async () => {
it("should auto-sync Callback URL when Port is changed", async () => {
const user = userEvent.setup();
renderWithTooltip(<AuthModal {...defaultProps} />);
@ -113,14 +113,14 @@ describe("AuthModal OAuth Port Synchronization", () => {
await user.clear(portInput);
await user.type(portInput, "9001");
// Check that Callback Path was auto-updated
const callbackPathInput = screen.getByLabelText(/Callback Path/i);
expect(callbackPathInput).toHaveValue(
// Check that Callback URL was auto-updated
const callbackUrlInput = screen.getByLabelText(/Callback URL/i);
expect(callbackUrlInput).toHaveValue(
"http://localhost:9001/auth/idaas/callback",
);
});
it("should auto-sync both Server URL and Callback Path when Host changes", async () => {
it("should auto-sync both Server URL and Callback URL when Host changes", async () => {
const user = userEvent.setup();
renderWithTooltip(<AuthModal {...defaultProps} />);
@ -142,9 +142,9 @@ describe("AuthModal OAuth Port Synchronization", () => {
const serverUrlInput = screen.getByLabelText(/Server URL/i);
expect(serverUrlInput).toHaveValue("http://192.168.1.100:9002");
// Verify Callback Path
const callbackPathInput = screen.getByLabelText(/Callback Path/i);
expect(callbackPathInput).toHaveValue(
// Verify Callback URL
const callbackUrlInput = screen.getByLabelText(/Callback URL/i);
expect(callbackUrlInput).toHaveValue(
"http://192.168.1.100:9002/auth/idaas/callback",
);
});
@ -190,7 +190,7 @@ describe("AuthModal OAuth Port Synchronization", () => {
oauth_host: "localhost",
oauth_port: "9001",
oauth_server_url: "http://localhost:9001",
oauth_callback_path: "http://localhost:9001/auth/idaas/callback",
oauth_callback_url: "http://localhost:9001/auth/idaas/callback",
oauth_client_id: "test-client-id",
oauth_client_secret: "test-secret",
oauth_auth_url: "http://localhost:9001/auth/authorize",
@ -254,7 +254,7 @@ describe("AuthModal OAuth Port Synchronization", () => {
expect(screen.getByLabelText(/Server URL/i)).toHaveValue(
"http://existing.host.com:8080",
);
expect(screen.getByLabelText(/Callback Path/i)).toHaveValue(
expect(screen.getByLabelText(/Callback URL/i)).toHaveValue(
"http://existing.host.com:8080/auth/idaas/callback",
);
});

View File

@ -34,7 +34,7 @@ const AuthModal = ({
oauthHost?: string;
oauthPort?: string;
oauthServerUrl?: string;
oauthCallbackPath?: string;
oauthCallbackUrl?: string;
oauthClientId?: string;
oauthClientSecret?: string;
oauthAuthUrl?: string;
@ -45,7 +45,11 @@ const AuthModal = ({
oauthHost: authSettings?.oauth_host || "",
oauthPort: authSettings?.oauth_port || "",
oauthServerUrl: authSettings?.oauth_server_url || "",
oauthCallbackPath: authSettings?.oauth_callback_path || "",
// Use oauth_callback_url if available, fallback to oauth_callback_path for backwards compatibility
oauthCallbackUrl:
authSettings?.oauth_callback_url ||
authSettings?.oauth_callback_path ||
"",
oauthClientId: authSettings?.oauth_client_id || "",
oauthClientSecret: authSettings?.oauth_client_secret || "",
oauthAuthUrl: authSettings?.oauth_auth_url || "",
@ -62,7 +66,11 @@ const AuthModal = ({
oauthHost: authSettings.oauth_host || "",
oauthPort: authSettings.oauth_port || "",
oauthServerUrl: authSettings.oauth_server_url || "",
oauthCallbackPath: authSettings.oauth_callback_path || "",
// Use oauth_callback_url if available, fallback to oauth_callback_path for backwards compatibility
oauthCallbackUrl:
authSettings.oauth_callback_url ||
authSettings.oauth_callback_path ||
"",
oauthClientId: authSettings.oauth_client_id || "",
oauthClientSecret: authSettings.oauth_client_secret || "",
oauthAuthUrl: authSettings.oauth_auth_url || "",
@ -94,17 +102,17 @@ const AuthModal = ({
if (port) {
newFields.oauthServerUrl = `http://${host}:${port}`;
// Auto-sync callback path if:
// Auto-sync callback URL if:
// 1. It's empty (initial setup), OR
// 2. It matches the standard format pattern (auto-update when host/port changes)
const isStandardFormat =
!prev.oauthCallbackPath ||
!prev.oauthCallbackUrl ||
/^https?:\/\/[^:/]+:\d+\/auth\/idaas\/callback$/.test(
prev.oauthCallbackPath,
prev.oauthCallbackUrl,
);
if (isStandardFormat) {
newFields.oauthCallbackPath = `http://${host}:${port}/auth/idaas/callback`;
newFields.oauthCallbackUrl = `http://${host}:${port}/auth/idaas/callback`;
}
}
}
@ -120,7 +128,7 @@ const AuthModal = ({
oauth_host: authFields.oauthHost,
oauth_port: authFields.oauthPort,
oauth_server_url: authFields.oauthServerUrl,
oauth_callback_path: authFields.oauthCallbackPath,
oauth_callback_url: authFields.oauthCallbackUrl, // Use new field name
oauth_client_id: authFields.oauthClientId,
oauth_client_secret: authFields.oauthClientSecret,
oauth_auth_url: authFields.oauthAuthUrl,
@ -270,19 +278,19 @@ const AuthModal = ({
</div>
<div className="flex flex-col gap-2">
<Label
htmlFor="oauth-callback-path"
htmlFor="oauth-callback-url"
className="!text-mmd font-medium"
>
Callback Path
Callback URL
</Label>
<Input
id="oauth-callback-path"
id="oauth-callback-url"
type="text"
placeholder="http://localhost:9000/auth/idaas/callback"
value={authFields.oauthCallbackPath || ""}
value={authFields.oauthCallbackUrl || ""}
onChange={(e) =>
handleAuthFieldChange(
"oauthCallbackPath",
"oauthCallbackUrl",
e.target.value,
)
}

View File

@ -3,7 +3,8 @@ export type AuthSettingsType = {
oauth_host?: string;
oauth_port?: string;
oauth_server_url?: string;
oauth_callback_path?: string;
oauth_callback_path?: string; // Deprecated: use oauth_callback_url instead
oauth_callback_url?: string;
oauth_client_id?: string;
oauth_client_secret?: string;
oauth_auth_url?: string;

File diff suppressed because one or more lines are too long

View File

@ -1292,7 +1292,7 @@ class MCPComposerService(Service):
"oauth_host": "OAUTH_HOST",
"oauth_port": "OAUTH_PORT",
"oauth_server_url": "OAUTH_SERVER_URL",
"oauth_callback_path": "OAUTH_CALLBACK_PATH",
"oauth_callback_url": "OAUTH_CALLBACK_URL",
"oauth_client_id": "OAUTH_CLIENT_ID",
"oauth_client_secret": "OAUTH_CLIENT_SECRET", # pragma: allowlist secret
"oauth_auth_url": "OAUTH_AUTH_URL",
@ -1301,6 +1301,12 @@ class MCPComposerService(Service):
"oauth_provider_scope": "OAUTH_PROVIDER_SCOPE",
}
# Backwards compatibility: if oauth_callback_url not set, try oauth_callback_path
if ("oauth_callback_url" not in auth_config or not auth_config.get("oauth_callback_url")) and (
"oauth_callback_path" in auth_config and auth_config.get("oauth_callback_path")
):
auth_config["oauth_callback_url"] = auth_config["oauth_callback_path"]
# Add environment variables as command line arguments
# Only set non-empty values to avoid Pydantic validation errors
for config_key, env_key in oauth_env_mapping.items():