diff --git a/Cargo.lock b/Cargo.lock index 94ed1f9fb..7559f123d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -740,6 +740,7 @@ dependencies = [ "toml 0.8.23", "tracing", "tracing-subscriber", + "url", ] [[package]] diff --git a/crates/aionui-mcp/Cargo.toml b/crates/aionui-mcp/Cargo.toml index b9381f165..53fe0da17 100644 --- a/crates/aionui-mcp/Cargo.toml +++ b/crates/aionui-mcp/Cargo.toml @@ -19,6 +19,7 @@ thiserror.workspace = true oauth2.workspace = true open.workspace = true toml.workspace = true +url.workspace = true tokio.workspace = true tracing.workspace = true reqwest.workspace = true diff --git a/crates/aionui-mcp/src/error.rs b/crates/aionui-mcp/src/error.rs index 675f40514..1cced6583 100644 --- a/crates/aionui-mcp/src/error.rs +++ b/crates/aionui-mcp/src/error.rs @@ -25,6 +25,12 @@ pub enum McpError { #[error("OAuth error: {0}")] OAuth(String), + /// OAuth endpoint discovery failed — the server did not publish usable + /// authorization metadata. Caused by remote/config state, not an internal + /// fault, so it must not surface as a 500. + #[error("{0}")] + OAuthDiscovery(String), + #[error("{0}")] Database(#[from] aionui_db::DbError), diff --git a/crates/aionui-mcp/src/lib.rs b/crates/aionui-mcp/src/lib.rs index 6ea9d814b..3efae5893 100644 --- a/crates/aionui-mcp/src/lib.rs +++ b/crates/aionui-mcp/src/lib.rs @@ -5,6 +5,7 @@ pub mod adapter; pub mod adapters; pub mod connection_test; pub mod error; +pub mod oauth_discovery; pub mod oauth_service; pub mod routes; pub mod service; diff --git a/crates/aionui-mcp/src/oauth_discovery.rs b/crates/aionui-mcp/src/oauth_discovery.rs new file mode 100644 index 000000000..14170909e --- /dev/null +++ b/crates/aionui-mcp/src/oauth_discovery.rs @@ -0,0 +1,272 @@ +//! OAuth endpoint discovery and dynamic client registration for MCP servers. +//! +//! Implements the discovery chain the MCP authorization spec requires: +//! +//! 1. RFC 9728 protected-resource metadata to locate the authorization server +//! 2. RFC 8414 / OIDC authorization-server metadata for the endpoints +//! 3. RFC 7591 dynamic client registration when the server advertises it +//! +//! The well-known probe order matters: RFC 8414 inserts `.well-known/...` +//! between the origin and the resource path (`/sse` -> `/.well-known/x/sse`), +//! which is *not* the same as appending it to the full URL. Appending to the +//! full path is what many public MCP servers reject, so both spellings are +//! tried, origin-relative first. + +use serde::Deserialize; +use url::Url; + +use crate::error::McpError; + +/// OAuth Authorization Server Metadata (RFC 8414) — subset of fields we need. +#[derive(Debug, Clone, Deserialize)] +pub struct OAuthServerMetadata { + pub authorization_endpoint: String, + pub token_endpoint: String, + /// RFC 7591 dynamic client registration endpoint, when supported. + #[serde(default)] + pub registration_endpoint: Option, +} + +/// OAuth Protected Resource Metadata (RFC 9728) — subset of fields we need. +#[derive(Debug, Deserialize)] +struct ProtectedResourceMetadata { + #[serde(default)] + authorization_servers: Vec, +} + +/// Credentials for the OAuth client used against one MCP server. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ClientCredentials { + pub client_id: String, + pub client_secret: Option, +} + +/// Successful RFC 7591 registration response — subset of fields we need. +#[derive(Debug, Deserialize)] +struct RegistrationResponse { + client_id: String, + #[serde(default)] + client_secret: Option, +} + +/// Build the candidate well-known URLs for a metadata document, in probe order. +/// +/// For `https://host/sse` and suffix `oauth-authorization-server` this yields: +/// +/// 1. `https://host/.well-known/oauth-authorization-server/sse` (RFC 8414) +/// 2. `https://host/.well-known/oauth-authorization-server` (path-less form) +/// 3. `https://host/sse/.well-known/oauth-authorization-server` (legacy) +/// +/// Returns an empty vec when `server_url` cannot be parsed as a URL. +pub fn well_known_candidates(server_url: &str, suffix: &str) -> Vec { + let Ok(url) = Url::parse(server_url) else { + return Vec::new(); + }; + let Some(origin) = url_origin(&url) else { + return Vec::new(); + }; + + let path = url.path().trim_end_matches('/'); + let mut candidates = Vec::new(); + + if !path.is_empty() { + candidates.push(format!("{origin}/.well-known/{suffix}{path}")); + } + candidates.push(format!("{origin}/.well-known/{suffix}")); + if !path.is_empty() { + candidates.push(format!("{origin}{path}/.well-known/{suffix}")); + } + + candidates +} + +/// Serialize a URL's origin as `scheme://host[:port]`. +/// +/// `Url::origin()` is not used because it opaquefies non-special schemes; we +/// only ever probe http(s) MCP endpoints and want a plain string. +fn url_origin(url: &Url) -> Option { + let scheme = url.scheme(); + let host = url.host_str()?; + match url.port() { + Some(port) => Some(format!("{scheme}://{host}:{port}")), + None => Some(format!("{scheme}://{host}")), + } +} + +/// Discovery and registration over a shared HTTP client. +pub struct OAuthDiscovery<'a> { + http_client: &'a reqwest::Client, +} + +impl<'a> OAuthDiscovery<'a> { + pub fn new(http_client: &'a reqwest::Client) -> Self { + Self { http_client } + } + + /// Discover the authorization server metadata for an MCP server URL. + /// + /// Follows the RFC 9728 pointer when present, then probes RFC 8414 and + /// OIDC well-known locations. Returns the first document that parses. + pub async fn discover(&self, server_url: &str) -> Result { + // RFC 9728: the resource may name its authorization server(s). + let mut issuers: Vec = Vec::new(); + for candidate in well_known_candidates(server_url, "oauth-protected-resource") { + if let Some(resource) = self.fetch::(&candidate).await { + issuers = resource.authorization_servers; + break; + } + } + + // Probe the advertised issuers first, then the resource URL itself. + let mut probe_bases: Vec = issuers; + probe_bases.push(server_url.to_string()); + + for base in &probe_bases { + for suffix in ["oauth-authorization-server", "openid-configuration"] { + for candidate in well_known_candidates(base, suffix) { + if let Some(metadata) = self.fetch::(&candidate).await { + return Ok(metadata); + } + } + } + } + + Err(McpError::OAuthDiscovery(format!( + "Could not discover OAuth endpoints for '{server_url}'. \ + The server did not publish RFC 8414 authorization-server metadata, \ + OpenID configuration, or RFC 9728 protected-resource metadata at any \ + well-known location." + ))) + } + + /// Register a public OAuth client via RFC 7591 dynamic client registration. + pub async fn register_client( + &self, + registration_endpoint: &str, + redirect_uri: &str, + ) -> Result { + let body = serde_json::json!({ + "client_name": "AionUi", + "redirect_uris": [redirect_uri], + "grant_types": ["authorization_code", "refresh_token"], + "response_types": ["code"], + "token_endpoint_auth_method": "none", + }); + + let resp = self + .http_client + .post(registration_endpoint) + .json(&body) + .send() + .await + .map_err(|e| McpError::OAuth(format!("Client registration request failed: {e}")))?; + + let status = resp.status(); + if !status.is_success() { + return Err(McpError::OAuth(format!( + "Client registration at '{registration_endpoint}' returned {status}" + ))); + } + + let registered: RegistrationResponse = resp + .json() + .await + .map_err(|e| McpError::OAuth(format!("Failed to parse client registration response: {e}")))?; + + Ok(ClientCredentials { + client_id: registered.client_id, + client_secret: registered.client_secret, + }) + } + + /// GET and deserialize a metadata document, returning `None` on any failure. + async fn fetch(&self, url: &str) -> Option { + let resp = self.http_client.get(url).send().await.ok()?; + if !resp.status().is_success() { + return None; + } + resp.json::().await.ok() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn candidates_put_origin_relative_path_suffix_first() { + let candidates = well_known_candidates("https://mcp.monday.com/sse", "oauth-authorization-server"); + assert_eq!( + candidates, + vec![ + "https://mcp.monday.com/.well-known/oauth-authorization-server/sse", + "https://mcp.monday.com/.well-known/oauth-authorization-server", + "https://mcp.monday.com/sse/.well-known/oauth-authorization-server", + ] + ); + } + + /// Regression guard for the reported bug: the legacy "append to full path" + /// spelling must not be the only thing we try, because public MCP servers + /// answer 401 there. + #[test] + fn candidates_are_not_only_the_legacy_appended_form() { + let legacy = "https://mcp.monday.com/sse/.well-known/oauth-authorization-server"; + let candidates = well_known_candidates("https://mcp.monday.com/sse", "oauth-authorization-server"); + assert!(candidates.len() > 1); + assert_ne!(candidates[0], legacy); + assert!(candidates.contains(&legacy.to_string())); + } + + #[test] + fn candidates_for_root_url_have_no_duplicate_path_forms() { + let candidates = well_known_candidates("https://example.com/", "openid-configuration"); + assert_eq!(candidates, vec!["https://example.com/.well-known/openid-configuration"]); + } + + #[test] + fn candidates_preserve_non_default_port() { + let candidates = well_known_candidates("http://127.0.0.1:8931/mcp", "oauth-authorization-server"); + assert_eq!( + candidates[0], + "http://127.0.0.1:8931/.well-known/oauth-authorization-server/mcp" + ); + } + + #[test] + fn candidates_for_nested_path() { + let candidates = well_known_candidates("https://example.com/api/v1/mcp", "oauth-protected-resource"); + assert_eq!( + candidates[0], + "https://example.com/.well-known/oauth-protected-resource/api/v1/mcp" + ); + } + + #[test] + fn candidates_empty_for_unparseable_url() { + assert!(well_known_candidates("not a url", "oauth-authorization-server").is_empty()); + } + + #[test] + fn metadata_parses_without_registration_endpoint() { + let metadata: OAuthServerMetadata = serde_json::from_str( + r#"{"authorization_endpoint":"https://a.example/auth","token_endpoint":"https://a.example/token"}"#, + ) + .unwrap(); + assert_eq!(metadata.registration_endpoint, None); + } + + #[test] + fn metadata_parses_with_registration_endpoint_and_extra_fields() { + let metadata: OAuthServerMetadata = serde_json::from_str( + r#"{"issuer":"https://a.example","authorization_endpoint":"https://a.example/auth", + "token_endpoint":"https://a.example/token", + "registration_endpoint":"https://a.example/register"}"#, + ) + .unwrap(); + assert_eq!( + metadata.registration_endpoint.as_deref(), + Some("https://a.example/register") + ); + } +} diff --git a/crates/aionui-mcp/src/oauth_service.rs b/crates/aionui-mcp/src/oauth_service.rs index dddce6ed0..f6b7f26e3 100644 --- a/crates/aionui-mcp/src/oauth_service.rs +++ b/crates/aionui-mcp/src/oauth_service.rs @@ -7,16 +7,16 @@ use aionui_common::{TimestampMs, now_ms}; use aionui_db::{IOAuthTokenRepository, UpsertOAuthTokenParams}; use oauth2::basic::BasicClient; use oauth2::{ - AuthUrl, AuthorizationCode, ClientId, CsrfToken, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, - TokenResponse, TokenUrl, + AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, + RefreshToken, TokenResponse, TokenUrl, }; -use serde::Deserialize; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpListener; use tokio::sync::Mutex; use tracing::{debug, warn}; use crate::error::McpError; +use crate::oauth_discovery::{ClientCredentials, OAuthDiscovery, OAuthServerMetadata}; // --------------------------------------------------------------------------- // Constants @@ -31,17 +31,6 @@ const DEFAULT_CLIENT_ID: &str = "aionui"; /// Token expiry safety margin (refresh 5 minutes before expiration). const EXPIRY_MARGIN_MS: i64 = 5 * 60 * 1000; -// --------------------------------------------------------------------------- -// Discovery response -// --------------------------------------------------------------------------- - -/// OAuth Authorization Server Metadata (RFC 8414) — subset of fields we need. -#[derive(Debug, Deserialize)] -struct OAuthServerMetadata { - authorization_endpoint: String, - token_endpoint: String, -} - // --------------------------------------------------------------------------- // Pending login state // --------------------------------------------------------------------------- @@ -56,6 +45,10 @@ struct PendingLogin { auth_url: String, token_url: String, redirect_url: String, + /// Credentials the authorize request was made with. The token exchange must + /// reuse exactly these — a dynamically registered client_id differs per + /// server and per registration. + credentials: ClientCredentials, } // --------------------------------------------------------------------------- @@ -72,6 +65,16 @@ pub struct McpOAuthService { http_client: reqwest::Client, /// Mutex protecting pending login state by (user_id, oauth_state). pending: Arc>>, + /// Dynamically registered client credentials, keyed by server URL. + /// + /// Cached in-process so the authorize/exchange/refresh legs of one login + /// share a client_id without re-registering on every call. + /// + /// Not persisted: after a restart the cache is empty, so a refresh against + /// a registration-only server falls back to the static client ID and fails, + /// requiring a fresh login. Persisting the registration alongside the token + /// row would remove that re-login and is the natural follow-up. + registered_clients: Arc>>, } impl McpOAuthService { @@ -80,6 +83,7 @@ impl McpOAuthService { token_repo, http_client, pending: Arc::new(Mutex::new(HashMap::new())), + registered_clients: Arc::new(Mutex::new(HashMap::new())), } } @@ -206,10 +210,10 @@ impl McpOAuthService { let auth_url_str = metadata.authorization_endpoint.clone(); let token_url_str = metadata.token_endpoint.clone(); - let auth_url = AuthUrl::new(metadata.authorization_endpoint) + let auth_url = AuthUrl::new(metadata.authorization_endpoint.clone()) .map_err(|e| McpError::OAuth(format!("Invalid auth URL: {e}")))?; - let token_url = - TokenUrl::new(metadata.token_endpoint).map_err(|e| McpError::OAuth(format!("Invalid token URL: {e}")))?; + let token_url = TokenUrl::new(metadata.token_endpoint.clone()) + .map_err(|e| McpError::OAuth(format!("Invalid token URL: {e}")))?; let listener = TcpListener::bind("127.0.0.1:0") .await @@ -223,10 +227,17 @@ impl McpOAuthService { let redirect = RedirectUrl::new(redirect_url_str.clone()) .map_err(|e| McpError::OAuth(format!("Invalid redirect URL: {e}")))?; - let client = BasicClient::new(ClientId::new(DEFAULT_CLIENT_ID.to_string())) + let credentials = self + .resolve_client_credentials(server_url, &metadata, &redirect_url_str) + .await?; + + let mut client = BasicClient::new(ClientId::new(credentials.client_id.clone())) .set_auth_uri(auth_url) .set_token_uri(token_url) .set_redirect_uri(redirect); + if let Some(ref secret) = credentials.client_secret { + client = client.set_client_secret(ClientSecret::new(secret.clone())); + } let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); @@ -246,6 +257,7 @@ impl McpOAuthService { auth_url: auth_url_str, token_url: token_url_str, redirect_url: redirect_url_str, + credentials, }, ); } @@ -269,48 +281,66 @@ impl McpOAuthService { Ok(true) } - /// Discover OAuth authorization server metadata. + /// Discover OAuth authorization server metadata for an MCP server URL. /// - /// Tries `.well-known/oauth-authorization-server` first, - /// falls back to `.well-known/openid-configuration`. + /// See [`crate::oauth_discovery`] for the probe order; the important part + /// is that `.well-known` paths are resolved against the server's *origin* + /// with the resource path as a suffix (RFC 8414), not appended to the full + /// URL — appending is rejected by many public MCP servers. async fn discover_endpoints(&self, server_url: &str) -> Result { - let base = server_url.trim_end_matches('/'); + OAuthDiscovery::new(&self.http_client).discover(server_url).await + } - let well_known_url = format!("{base}/.well-known/oauth-authorization-server"); - if let Ok(metadata) = self.fetch_metadata(&well_known_url).await { - debug!(server_url, "Discovered OAuth metadata via RFC 8414"); - return Ok(metadata); + /// Resolve the OAuth client to use for a server. + /// + /// Prefers a previously registered client, then RFC 7591 dynamic client + /// registration when the server advertises a registration endpoint, and + /// finally the static fallback client ID for servers that pre-provision it. + async fn resolve_client_credentials( + &self, + server_url: &str, + metadata: &OAuthServerMetadata, + redirect_uri: &str, + ) -> Result { + if let Some(existing) = self.registered_clients.lock().await.get(server_url) { + return Ok(existing.clone()); } - let oidc_url = format!("{base}/.well-known/openid-configuration"); - if let Ok(metadata) = self.fetch_metadata(&oidc_url).await { - debug!(server_url, "Discovered OAuth metadata via OIDC"); - return Ok(metadata); - } + let Some(registration_endpoint) = metadata.registration_endpoint.as_deref() else { + debug!( + server_url, + "No registration endpoint advertised; using static client ID" + ); + return Ok(ClientCredentials { + client_id: DEFAULT_CLIENT_ID.to_string(), + client_secret: None, + }); + }; - Err(McpError::OAuth(format!( - "Failed to discover OAuth endpoints for '{server_url}': \ - no .well-known/oauth-authorization-server or \ - .well-known/openid-configuration found" - ))) - } + let credentials = OAuthDiscovery::new(&self.http_client) + .register_client(registration_endpoint, redirect_uri) + .await?; + debug!(server_url, "Registered OAuth client via dynamic client registration"); - /// Fetch and parse OAuth server metadata from a URL. - async fn fetch_metadata(&self, url: &str) -> Result { - let resp = self - .http_client - .get(url) - .send() + self.registered_clients + .lock() .await - .map_err(|e| McpError::OAuth(format!("HTTP request failed: {e}")))?; + .insert(server_url.to_string(), credentials.clone()); - if !resp.status().is_success() { - return Err(McpError::OAuth(format!("Metadata endpoint returned {}", resp.status()))); - } + Ok(credentials) + } - resp.json() + /// Return the registered client for a server, or the static fallback. + async fn cached_client_credentials(&self, server_url: &str) -> ClientCredentials { + self.registered_clients + .lock() .await - .map_err(|e| McpError::OAuth(format!("Failed to parse metadata: {e}"))) + .get(server_url) + .cloned() + .unwrap_or_else(|| ClientCredentials { + client_id: DEFAULT_CLIENT_ID.to_string(), + client_secret: None, + }) } /// Wait for the OAuth callback redirect on the given listener. @@ -392,7 +422,7 @@ impl McpOAuthService { code: String, state: String, ) -> Result<(), McpError> { - let (auth_url_str, token_url_str, redirect_url_str, pkce_verifier) = { + let (auth_url_str, token_url_str, redirect_url_str, pkce_verifier, credentials) = { let mut guard = self.pending.lock().await; let pending = guard .remove(&(user_id.to_string(), state)) @@ -402,6 +432,7 @@ impl McpOAuthService { pending.token_url, pending.redirect_url, pending.pkce_verifier, + pending.credentials, ) }; @@ -410,10 +441,13 @@ impl McpOAuthService { let redirect = RedirectUrl::new(redirect_url_str).map_err(|e| McpError::OAuth(format!("Invalid redirect URL: {e}")))?; - let client = BasicClient::new(ClientId::new(DEFAULT_CLIENT_ID.to_string())) + let mut client = BasicClient::new(ClientId::new(credentials.client_id)) .set_auth_uri(auth_url) .set_token_uri(token_url) .set_redirect_uri(redirect); + if let Some(secret) = credentials.client_secret { + client = client.set_client_secret(ClientSecret::new(secret)); + } let http_client = Self::build_no_redirect_client()?; @@ -440,7 +474,11 @@ impl McpOAuthService { let token_url = TokenUrl::new(metadata.token_endpoint).map_err(|e| McpError::OAuth(format!("Invalid token URL: {e}")))?; - let client = BasicClient::new(ClientId::new(DEFAULT_CLIENT_ID.to_string())).set_token_uri(token_url); + let credentials = self.cached_client_credentials(server_url).await; + let mut client = BasicClient::new(ClientId::new(credentials.client_id)).set_token_uri(token_url); + if let Some(secret) = credentials.client_secret { + client = client.set_client_secret(ClientSecret::new(secret)); + } let http_client = Self::build_no_redirect_client()?; @@ -706,6 +744,10 @@ mod tests { auth_url: "https://auth.example.com/authorize".to_string(), token_url: "https://auth.example.com/token".to_string(), redirect_url: "http://127.0.0.1/callback".to_string(), + credentials: ClientCredentials { + client_id: DEFAULT_CLIENT_ID.to_string(), + client_secret: None, + }, }, ); } diff --git a/crates/aionui-mcp/src/routes.rs b/crates/aionui-mcp/src/routes.rs index 671818ab1..2f8a051ce 100644 --- a/crates/aionui-mcp/src/routes.rs +++ b/crates/aionui-mcp/src/routes.rs @@ -33,6 +33,13 @@ impl From for ApiError { McpError::AgentOperationFailed(msg) => ApiError::Internal(msg), McpError::ConnectionFailed(msg) => ApiError::BadGateway(msg), McpError::OAuth(msg) => ApiError::Internal(format!("OAuth error: {msg}")), + // Discovery failures are caused by the remote server or the + // configured URL, not by an internal fault. Use a coded error so + // the actionable reason reaches the user instead of the generic + // "Internal server error." / "Upstream service unavailable." + McpError::OAuthDiscovery(msg) => { + ApiError::coded(StatusCode::BAD_GATEWAY, "MCP_OAUTH_DISCOVERY_FAILED", msg, None) + } McpError::Database(db_err) => ApiError::Internal(db_err.to_string()), McpError::Json(e) => ApiError::Internal(format!("JSON error: {e}")), } @@ -370,6 +377,16 @@ mod error_mapping_tests { assert!(matches!(err, ApiError::BadRequest(_))); } + /// Discovery failures must not be laundered into an opaque 500 — the user + /// needs to see which server/URL failed to publish OAuth metadata. + #[test] + fn oauth_discovery_maps_to_bad_gateway_and_keeps_message() { + let err = ApiError::from(McpError::OAuthDiscovery("no metadata at example.com".into())); + assert_eq!(err.status_code(), StatusCode::BAD_GATEWAY); + assert_eq!(err.error_code(), "MCP_OAUTH_DISCOVERY_FAILED"); + assert!(err.public_message().contains("no metadata at example.com")); + } + #[test] fn agent_operation_failed_maps_to_internal() { let err = ApiError::from(McpError::AgentOperationFailed("exit code 1".into())); diff --git a/crates/aionui-mcp/tests/oauth_discovery_live.rs b/crates/aionui-mcp/tests/oauth_discovery_live.rs new file mode 100644 index 000000000..316e760cd --- /dev/null +++ b/crates/aionui-mcp/tests/oauth_discovery_live.rs @@ -0,0 +1,35 @@ +//! Live discovery checks against real public MCP servers. +//! +//! Ignored by default (network-dependent). Run with: +//! `cargo test -p aionui-mcp --test oauth_discovery_live -- --ignored` +//! +//! These guard the exact regression from the bug report: the old code appended +//! `.well-known/...` to the full URL path, which these servers answer with 401. + +use aionui_mcp::oauth_discovery::OAuthDiscovery; + +#[tokio::test] +#[ignore = "requires network access to mcp.monday.com"] +async fn discovers_monday_sse_endpoints() { + let client = reqwest::Client::new(); + let metadata = OAuthDiscovery::new(&client) + .discover("https://mcp.monday.com/sse") + .await + .expect("discovery must succeed for monday.com MCP server"); + + assert!( + metadata.authorization_endpoint.starts_with("https://"), + "unexpected authorization_endpoint: {}", + metadata.authorization_endpoint + ); + assert!( + metadata.token_endpoint.starts_with("https://"), + "unexpected token_endpoint: {}", + metadata.token_endpoint + ); + // monday requires dynamic client registration. + assert!( + metadata.registration_endpoint.is_some(), + "expected a registration endpoint to be advertised" + ); +}