From e619487e1afd20cb5261837852a4a52203f7711c Mon Sep 17 00:00:00 2001 From: Daniel Vigovszky Date: Tue, 3 Dec 2024 13:06:48 +0100 Subject: [PATCH] Verify plugin scopes on installation (#1106) --- golem-common/src/model/plugin.rs | 7 +++ .../src/service/component.rs | 6 +-- .../src/service/plugin.rs | 23 ++++++++++ golem-component-service/src/api/component.rs | 45 ++++++++++++++----- golem-component-service/src/api/mod.rs | 7 +++ .../src/grpcapi/component.rs | 42 ++++++++++++++--- golem-component-service/src/grpcapi/mod.rs | 1 + 7 files changed, 109 insertions(+), 22 deletions(-) diff --git a/golem-common/src/model/plugin.rs b/golem-common/src/model/plugin.rs index 24ced557e..cd17a3191 100644 --- a/golem-common/src/model/plugin.rs +++ b/golem-common/src/model/plugin.rs @@ -39,6 +39,13 @@ impl DefaultPluginScope { pub fn component(component_id: ComponentId) -> Self { DefaultPluginScope::Component(ComponentPluginScope { component_id }) } + + pub fn valid_in_component(&self, component_id: &ComponentId) -> bool { + match self { + DefaultPluginScope::Global(_) => true, + DefaultPluginScope::Component(scope) => &scope.component_id == component_id, + } + } } impl Default for DefaultPluginScope { diff --git a/golem-component-service-base/src/service/component.rs b/golem-component-service-base/src/service/component.rs index f87d1ff78..0cffc460c 100644 --- a/golem-component-service-base/src/service/component.rs +++ b/golem-component-service-base/src/service/component.rs @@ -1441,8 +1441,8 @@ impl ComponentService installation: PluginInstallationCreation, ) -> Result { let namespace = owner.to_string(); - let owner: Owner::Row = owner.clone().into(); - let plugin_owner = owner.into(); + let owner_row: Owner::Row = owner.clone().into(); + let plugin_owner_row = owner_row.into(); let latest = self .component_repo @@ -1464,7 +1464,7 @@ impl ComponentService component_version: latest.version as u64, } .into(), - owner: plugin_owner, + owner: plugin_owner_row, }; let new_component_version = self.component_repo.install_plugin(&record).await?; diff --git a/golem-component-service-base/src/service/plugin.rs b/golem-component-service-base/src/service/plugin.rs index ae2e6e978..7ee473dff 100644 --- a/golem-component-service-base/src/service/plugin.rs +++ b/golem-component-service-base/src/service/plugin.rs @@ -35,6 +35,17 @@ pub enum PluginError { ComponentNotFound { component_id: ComponentId }, #[error("Failed to get available scopes: {error}")] FailedToGetAvailableScopes { error: String }, + #[error("Plugin not found: {plugin_name}@{plugin_version}")] + PluginNotFound { + plugin_name: String, + plugin_version: String, + }, + #[error("Plugin {plugin_name}@{plugin_version} {details}")] + InvalidScope { + plugin_name: String, + plugin_version: String, + details: String, + }, } impl PluginError { @@ -54,6 +65,8 @@ impl SafeDisplay for PluginError { Self::InternalComponentError(inner) => inner.to_safe_string(), Self::ComponentNotFound { .. } => self.to_string(), Self::FailedToGetAvailableScopes { .. } => self.to_string(), + Self::PluginNotFound { .. } => self.to_string(), + Self::InvalidScope { .. } => self.to_string(), } } } @@ -82,6 +95,16 @@ impl From for golem_api_grpc::proto::golem::component::v1::Componen error: value.to_safe_string(), })), }, + PluginError::PluginNotFound { .. } => Self { + error: Some(component_error::Error::NotFound(ErrorBody { + error: value.to_safe_string(), + })), + }, + PluginError::InvalidScope { .. } => Self { + error: Some(component_error::Error::Unauthorized(ErrorBody { + error: value.to_safe_string(), + })), + }, } } } diff --git a/golem-component-service/src/api/component.rs b/golem-component-service/src/api/component.rs index 35d21d484..81c928eaa 100644 --- a/golem-component-service/src/api/component.rs +++ b/golem-component-service/src/api/component.rs @@ -16,7 +16,8 @@ use crate::api::{ComponentError, Result}; use futures_util::TryStreamExt; use golem_common::model::component::DefaultComponentOwner; use golem_common::model::plugin::{ - PluginInstallation, PluginInstallationCreation, PluginInstallationUpdate, + DefaultPluginOwner, DefaultPluginScope, PluginInstallation, PluginInstallationCreation, + PluginInstallationUpdate, }; use golem_common::model::ComponentFilePathWithPermissionsList; use golem_common::model::{ComponentId, ComponentType, Empty, PluginInstallationId}; @@ -25,6 +26,7 @@ use golem_component_service_base::model::{ InitialComponentFilesArchiveAndPermissions, UpdatePayload, }; use golem_component_service_base::service::component::ComponentService; +use golem_component_service_base::service::plugin::{PluginError, PluginService}; use golem_service_base::api_tags::ApiTags; use golem_service_base::model::*; use golem_service_base::poem::TempFileUpload; @@ -38,6 +40,8 @@ use tracing::Instrument; pub struct ComponentApi { pub component_service: Arc + Sync + Send>, + pub plugin_service: + Arc + Sync + Send>, } #[OpenApi(prefix_path = "/v1/components", tag = ApiTags::Component)] @@ -386,18 +390,35 @@ impl ComponentApi { plugin_version = plugin.version.clone() ); - let response = self - .component_service - .create_plugin_installation_for_component( - &DefaultComponentOwner, - &component_id.0, - plugin.0, - ) - .await - .map_err(|e| e.into()) - .map(Json); + let plugin_definition = self + .plugin_service + .get(&DefaultPluginOwner, &plugin.name, &plugin.version) + .await?; + + let response = if let Some(plugin_definition) = plugin_definition { + if plugin_definition.scope.valid_in_component(&component_id.0) { + self.component_service + .create_plugin_installation_for_component( + &DefaultComponentOwner, + &component_id.0, + plugin.0, + ) + .await + } else { + Err(PluginError::InvalidScope { + plugin_name: plugin.name.clone(), + plugin_version: plugin.version.clone(), + details: format!("not available for component {}", component_id.0), + }) + } + } else { + Err(PluginError::PluginNotFound { + plugin_name: plugin.name.clone(), + plugin_version: plugin.version.clone(), + }) + }; - record.result(response) + record.result(response.map_err(|e| e.into()).map(Json)) } /// Updates the priority or parameters of a plugin installation diff --git a/golem-component-service/src/api/mod.rs b/golem-component-service/src/api/mod.rs index 5f272e5dc..dbff28fb5 100644 --- a/golem-component-service/src/api/mod.rs +++ b/golem-component-service/src/api/mod.rs @@ -54,6 +54,7 @@ pub fn make_open_api_service(services: &Services) -> OpenApiService for ComponentError { error: value.to_safe_string(), })) } + PluginError::PluginNotFound { .. } => ComponentError::NotFound(Json(ErrorBody { + error: value.to_safe_string(), + })), + PluginError::InvalidScope { .. } => ComponentError::Unauthorized(Json(ErrorBody { + error: value.to_safe_string(), + })), } } } diff --git a/golem-component-service/src/grpcapi/component.rs b/golem-component-service/src/grpcapi/component.rs index 24e3117b9..23f05d635 100644 --- a/golem-component-service/src/grpcapi/component.rs +++ b/golem-component-service/src/grpcapi/component.rs @@ -47,12 +47,15 @@ use golem_api_grpc::proto::golem::component::{Component, PluginInstallation}; use golem_common::grpc::{proto_component_id_string, proto_plugin_installation_id_string}; use golem_common::model::component::DefaultComponentOwner; use golem_common::model::component_constraint::FunctionConstraintCollection; -use golem_common::model::plugin::{PluginInstallationCreation, PluginInstallationUpdate}; +use golem_common::model::plugin::{ + DefaultPluginOwner, DefaultPluginScope, PluginInstallationCreation, PluginInstallationUpdate, +}; use golem_common::model::{ComponentId, ComponentType}; use golem_common::recorded_grpc_api_request; use golem_component_service_base::api::common::ComponentTraceErrorKind; use golem_component_service_base::model::ComponentConstraints; use golem_component_service_base::service::component; +use golem_component_service_base::service::plugin::{PluginError, PluginService}; use tokio_stream::Stream; use tonic::{Request, Response, Status, Streaming}; @@ -75,6 +78,8 @@ fn internal_error(error: &str) -> ComponentError { pub struct ComponentGrpcApi { pub component_service: Arc + Sync + Send>, + pub plugin_service: + Arc + Sync + Send>, } impl ComponentGrpcApi { @@ -283,15 +288,38 @@ impl ComponentGrpcApi { parameters: request.parameters.clone(), }; - let response = self - .component_service - .create_plugin_installation_for_component( - &DefaultComponentOwner, - &component_id, - plugin_installation_creation, + let plugin_definition = self + .plugin_service + .get( + &DefaultPluginOwner, + &plugin_installation_creation.name, + &plugin_installation_creation.version, ) .await?; + let response = if let Some(plugin_definition) = plugin_definition { + if plugin_definition.scope.valid_in_component(&component_id) { + self.component_service + .create_plugin_installation_for_component( + &DefaultComponentOwner, + &component_id, + plugin_installation_creation.clone(), + ) + .await + } else { + Err(PluginError::InvalidScope { + plugin_name: plugin_installation_creation.name, + plugin_version: plugin_installation_creation.version, + details: format!("not available for component {}", component_id), + }) + } + } else { + Err(PluginError::PluginNotFound { + plugin_name: plugin_installation_creation.name, + plugin_version: plugin_installation_creation.version, + }) + }?; + Ok(response.into()) } diff --git a/golem-component-service/src/grpcapi/mod.rs b/golem-component-service/src/grpcapi/mod.rs index 2c519cde6..fa492e11b 100644 --- a/golem-component-service/src/grpcapi/mod.rs +++ b/golem-component-service/src/grpcapi/mod.rs @@ -56,6 +56,7 @@ pub async fn start_grpc_server( .add_service( ComponentServiceServer::new(ComponentGrpcApi { component_service: services.component_service.clone(), + plugin_service: services.plugin_service.clone(), }) .accept_compressed(CompressionEncoding::Gzip) .send_compressed(CompressionEncoding::Gzip),