diff --git a/apps/wallet/src/components/external-canisters/CanisterSetupDialog.vue b/apps/wallet/src/components/external-canisters/CanisterSetupDialog.vue index d2eacb301..40f2a01b4 100644 --- a/apps/wallet/src/components/external-canisters/CanisterSetupDialog.vue +++ b/apps/wallet/src/components/external-canisters/CanisterSetupDialog.vue @@ -147,6 +147,7 @@ const save = async (): Promise => { const saveChangesToExistingExternalCanister = async (canisterId: Principal): Promise => { const settings: Partial = {}; + settings.name = [assertAndReturn(wizard.value.configuration.name, 'name')]; settings.labels = wizard.value.configuration.labels ? [wizard.value.configuration.labels] : []; settings.state = wizard.value.configuration.state ? [mapExternalCanisterStateEnumToVariant(wizard.value.configuration.state)] @@ -156,21 +157,23 @@ const saveChangesToExistingExternalCanister = async (canisterId: Principal): Pro : ['']; settings.permissions = [ { - read: assertAndReturn(wizard.value.permission.read, 'read permission'), - change: assertAndReturn(wizard.value.permission.change, 'change permission'), - calls: [], + read: [assertAndReturn(wizard.value.permission.read, 'read permission')], + change: [assertAndReturn(wizard.value.permission.change, 'change permission')], + calls: [], // optional field, not updating calls through this dialog }, ]; settings.request_policies = [ { - calls: [], - change: wizard.value.approvalPolicy.change - .filter(item => item.rule !== undefined) - .map(item => ({ - policy_id: item.policy_id ? [item.policy_id] : [], - rule: item.rule as RequestPolicyRule, - })), + calls: [], // optional field, not updating calls through this dialog + change: [ + wizard.value.approvalPolicy.change + .filter(item => item.rule !== undefined) + .map(item => ({ + policy_id: item.policy_id ? [item.policy_id] : [], + rule: item.rule as RequestPolicyRule, + })), + ], }, ]; diff --git a/apps/wallet/src/generated/station/station.did b/apps/wallet/src/generated/station/station.did index 9cd097207..eb567dbfb 100644 --- a/apps/wallet/src/generated/station/station.did +++ b/apps/wallet/src/generated/station/station.did @@ -653,9 +653,9 @@ type CreateExternalCanisterOperationInput = record { // The labels of the external canister. labels : opt vec text; // What operations are allowed on the canister. - permissions : ExternalCanisterPermissionsInput; + permissions : ExternalCanisterPermissionsCreateInput; // The request policies for the canister. - request_policies : ExternalCanisterRequestPoliciesInput; + request_policies : ExternalCanisterRequestPoliciesCreateInput; }; type ConfigureExternalCanisterSettingsInput = record { @@ -665,9 +665,9 @@ type ConfigureExternalCanisterSettingsInput = record { // The labels of the external canister. labels : opt vec text; // What operations are allowed on the canister. - permissions : opt ExternalCanisterPermissionsInput; + permissions : opt ExternalCanisterPermissionsUpdateInput; // The request policies for the canister. - request_policies : opt ExternalCanisterRequestPoliciesInput; + request_policies : opt ExternalCanisterRequestPoliciesUpdateInput; // The state of the external canister. state : opt ExternalCanisterState; }; @@ -2310,8 +2310,32 @@ type ExternalCanisterPermissions = record { calls : vec ExternalCanisterCallPermission; }; +// The create input type for setting the permissions for the external canister. +type ExternalCanisterPermissionsCreateInput = ExternalCanisterPermissions; + +// The input type for setting call permissions of an existing external canister. +type ExternalCanisterChangeCallPermissionsInput = variant { + // Replaces all the call permissions with the provided list, if the list is empty + // all the call permissions will be removed. + ReplaceAllBy : vec ExternalCanisterCallPermission; + // Override the call permissions from the specified execution methods. + OverrideSpecifiedByExecutionMethods : vec ExternalCanisterCallPermission; + // Remove call permissions of the specified execution methods. + RemoveByExecutionMethods : vec text; +}; + // The input type for setting the permissions for the external canister. -type ExternalCanisterPermissionsInput = ExternalCanisterPermissions; +type ExternalCanisterPermissionsUpdateInput = record { + // Who can read information about the canister (e.g. canister status), + // changes to this permission can be made by the `change` permission. + read : opt Allow; + // Who can make changes to the canister, includes: + // - changing the permissions + // - install operations + change : opt Allow; + // The permissions for calling methods on the canister. + calls : opt ExternalCanisterChangeCallPermissionsInput; +}; // The request policy rules for the external canister. type ExternalCanisterRequestPolicies = record { @@ -2321,14 +2345,31 @@ type ExternalCanisterRequestPolicies = record { calls : vec ExternalCanisterCallRequestPolicyRule; }; -// The input type for setting the request policies for the external canister. -type ExternalCanisterRequestPoliciesInput = record { +// The input type for setting the request policies for a new external canister. +type ExternalCanisterRequestPoliciesCreateInput = record { // The request policy rules for the canister change operation. change : vec ExternalCanisterChangeRequestPolicyRuleInput; // The request policy rules for the calling methods on the canister. calls : vec ExternalCanisterCallRequestPolicyRuleInput; }; +type ExternalCanisterChangeCallRequestPoliciesInput = variant { + // Replaces all the call request policies with the provided list. + ReplaceAllBy : vec ExternalCanisterCallRequestPolicyRuleInput; + // Remove call request policies by the provided ids. + RemoveByPolicyIds : vec UUID; + // Override the request policies for the specified execution methods. + OverrideSpecifiedByExecutionMethods : vec ExternalCanisterCallRequestPolicyRuleInput; +}; + +// The input type for setting the request policies for an existing external canister. +type ExternalCanisterRequestPoliciesUpdateInput = record { + // The request policy rules for the canister change operation. + change : opt vec ExternalCanisterChangeRequestPolicyRuleInput; + // The request policy rules for the calling methods on the canister. + calls : opt ExternalCanisterChangeCallRequestPoliciesInput; +}; + // An external canister that the station can interact with. type ExternalCanister = record { // The id of the resource in the station. diff --git a/apps/wallet/src/generated/station/station.did.d.ts b/apps/wallet/src/generated/station/station.did.d.ts index 6f53f4c11..21568f739 100644 --- a/apps/wallet/src/generated/station/station.did.d.ts +++ b/apps/wallet/src/generated/station/station.did.d.ts @@ -204,11 +204,11 @@ export type ConfigureExternalCanisterOperationKind = { 'SoftDelete' : null } | { 'Delete' : null } | { 'NativeSettings' : DefiniteCanisterSettingsInput }; export interface ConfigureExternalCanisterSettingsInput { - 'permissions' : [] | [ExternalCanisterPermissionsInput], + 'permissions' : [] | [ExternalCanisterPermissionsUpdateInput], 'name' : [] | [string], 'labels' : [] | [Array], 'description' : [] | [string], - 'request_policies' : [] | [ExternalCanisterRequestPoliciesInput], + 'request_policies' : [] | [ExternalCanisterRequestPoliciesUpdateInput], 'state' : [] | [ExternalCanisterState], } export interface CreateExternalCanisterOperation { @@ -216,12 +216,12 @@ export interface CreateExternalCanisterOperation { 'input' : CreateExternalCanisterOperationInput, } export interface CreateExternalCanisterOperationInput { - 'permissions' : ExternalCanisterPermissionsInput, + 'permissions' : ExternalCanisterPermissionsCreateInput, 'kind' : CreateExternalCanisterOperationKind, 'name' : string, 'labels' : [] | [Array], 'description' : [] | [string], - 'request_policies' : ExternalCanisterRequestPoliciesInput, + 'request_policies' : ExternalCanisterRequestPoliciesCreateInput, } export type CreateExternalCanisterOperationKind = { 'AddExisting' : CreateExternalCanisterOperationKindAddExisting @@ -405,6 +405,22 @@ export interface ExternalCanisterCallerPrivileges { 'can_call' : Array, 'can_fund' : boolean, } +export type ExternalCanisterChangeCallPermissionsInput = { + 'OverrideSpecifiedByExecutionMethods' : Array< + ExternalCanisterCallPermission + > + } | + { 'RemoveByExecutionMethods' : Array } | + { 'ReplaceAllBy' : Array }; +export type ExternalCanisterChangeCallRequestPoliciesInput = { + 'RemoveByPolicyIds' : Array + } | + { + 'OverrideSpecifiedByExecutionMethods' : Array< + ExternalCanisterCallRequestPolicyRuleInput + > + } | + { 'ReplaceAllBy' : Array }; export interface ExternalCanisterChangeRequestPolicyRule { 'rule' : RequestPolicyRule, 'policy_id' : UUID, @@ -420,15 +436,24 @@ export interface ExternalCanisterPermissions { 'read' : Allow, 'change' : Allow, } -export type ExternalCanisterPermissionsInput = ExternalCanisterPermissions; +export type ExternalCanisterPermissionsCreateInput = ExternalCanisterPermissions; +export interface ExternalCanisterPermissionsUpdateInput { + 'calls' : [] | [ExternalCanisterChangeCallPermissionsInput], + 'read' : [] | [Allow], + 'change' : [] | [Allow], +} export interface ExternalCanisterRequestPolicies { 'calls' : Array, 'change' : Array, } -export interface ExternalCanisterRequestPoliciesInput { +export interface ExternalCanisterRequestPoliciesCreateInput { 'calls' : Array, 'change' : Array, } +export interface ExternalCanisterRequestPoliciesUpdateInput { + 'calls' : [] | [ExternalCanisterChangeCallRequestPoliciesInput], + 'change' : [] | [Array], +} export type ExternalCanisterResourceAction = { 'Call' : CallExternalCanisterResourceTarget } | diff --git a/apps/wallet/src/generated/station/station.did.js b/apps/wallet/src/generated/station/station.did.js index 04ca38261..e6021c794 100644 --- a/apps/wallet/src/generated/station/station.did.js +++ b/apps/wallet/src/generated/station/station.did.js @@ -191,12 +191,18 @@ export const idlFactory = ({ IDL }) => { 'allow' : Allow, 'validation_method' : ValidationMethodResourceTarget, }); - const ExternalCanisterPermissions = IDL.Record({ - 'calls' : IDL.Vec(ExternalCanisterCallPermission), - 'read' : Allow, - 'change' : Allow, + const ExternalCanisterChangeCallPermissionsInput = IDL.Variant({ + 'OverrideSpecifiedByExecutionMethods' : IDL.Vec( + ExternalCanisterCallPermission + ), + 'RemoveByExecutionMethods' : IDL.Vec(IDL.Text), + 'ReplaceAllBy' : IDL.Vec(ExternalCanisterCallPermission), + }); + const ExternalCanisterPermissionsUpdateInput = IDL.Record({ + 'calls' : IDL.Opt(ExternalCanisterChangeCallPermissionsInput), + 'read' : IDL.Opt(Allow), + 'change' : IDL.Opt(Allow), }); - const ExternalCanisterPermissionsInput = ExternalCanisterPermissions; const UserSpecifier = IDL.Variant({ 'Id' : IDL.Vec(UUID), 'Any' : IDL.Null, @@ -232,24 +238,31 @@ export const idlFactory = ({ IDL }) => { 'validation_method' : ValidationMethodResourceTarget, 'policy_id' : IDL.Opt(UUID), }); + const ExternalCanisterChangeCallRequestPoliciesInput = IDL.Variant({ + 'RemoveByPolicyIds' : IDL.Vec(UUID), + 'OverrideSpecifiedByExecutionMethods' : IDL.Vec( + ExternalCanisterCallRequestPolicyRuleInput + ), + 'ReplaceAllBy' : IDL.Vec(ExternalCanisterCallRequestPolicyRuleInput), + }); const ExternalCanisterChangeRequestPolicyRuleInput = IDL.Record({ 'rule' : RequestPolicyRule, 'policy_id' : IDL.Opt(UUID), }); - const ExternalCanisterRequestPoliciesInput = IDL.Record({ - 'calls' : IDL.Vec(ExternalCanisterCallRequestPolicyRuleInput), - 'change' : IDL.Vec(ExternalCanisterChangeRequestPolicyRuleInput), + const ExternalCanisterRequestPoliciesUpdateInput = IDL.Record({ + 'calls' : IDL.Opt(ExternalCanisterChangeCallRequestPoliciesInput), + 'change' : IDL.Opt(IDL.Vec(ExternalCanisterChangeRequestPolicyRuleInput)), }); const ExternalCanisterState = IDL.Variant({ 'Active' : IDL.Null, 'Archived' : IDL.Null, }); const ConfigureExternalCanisterSettingsInput = IDL.Record({ - 'permissions' : IDL.Opt(ExternalCanisterPermissionsInput), + 'permissions' : IDL.Opt(ExternalCanisterPermissionsUpdateInput), 'name' : IDL.Opt(IDL.Text), 'labels' : IDL.Opt(IDL.Vec(IDL.Text)), 'description' : IDL.Opt(IDL.Text), - 'request_policies' : IDL.Opt(ExternalCanisterRequestPoliciesInput), + 'request_policies' : IDL.Opt(ExternalCanisterRequestPoliciesUpdateInput), 'state' : IDL.Opt(ExternalCanisterState), }); const DefiniteCanisterSettingsInput = IDL.Record({ @@ -354,6 +367,12 @@ export const idlFactory = ({ IDL }) => { const RemoveAddressBookEntryOperationInput = IDL.Record({ 'address_book_entry_id' : UUID, }); + const ExternalCanisterPermissions = IDL.Record({ + 'calls' : IDL.Vec(ExternalCanisterCallPermission), + 'read' : Allow, + 'change' : Allow, + }); + const ExternalCanisterPermissionsCreateInput = ExternalCanisterPermissions; const CreateExternalCanisterOperationKindAddExisting = IDL.Record({ 'canister_id' : IDL.Principal, }); @@ -364,13 +383,17 @@ export const idlFactory = ({ IDL }) => { 'AddExisting' : CreateExternalCanisterOperationKindAddExisting, 'CreateNew' : CreateExternalCanisterOperationKindCreateNew, }); + const ExternalCanisterRequestPoliciesCreateInput = IDL.Record({ + 'calls' : IDL.Vec(ExternalCanisterCallRequestPolicyRuleInput), + 'change' : IDL.Vec(ExternalCanisterChangeRequestPolicyRuleInput), + }); const CreateExternalCanisterOperationInput = IDL.Record({ - 'permissions' : ExternalCanisterPermissionsInput, + 'permissions' : ExternalCanisterPermissionsCreateInput, 'kind' : CreateExternalCanisterOperationKind, 'name' : IDL.Text, 'labels' : IDL.Opt(IDL.Vec(IDL.Text)), 'description' : IDL.Opt(IDL.Text), - 'request_policies' : ExternalCanisterRequestPoliciesInput, + 'request_policies' : ExternalCanisterRequestPoliciesCreateInput, }); const ChangeAddressBookMetadata = IDL.Variant({ 'OverrideSpecifiedBy' : IDL.Vec(AddressBookMetadata), diff --git a/apps/wallet/src/pages/ExternalCanisterDetailPage.vue b/apps/wallet/src/pages/ExternalCanisterDetailPage.vue index 9ea6d6d30..51e7b4b34 100644 --- a/apps/wallet/src/pages/ExternalCanisterDetailPage.vue +++ b/apps/wallet/src/pages/ExternalCanisterDetailPage.vue @@ -200,7 +200,7 @@ color="default" variant="tonal" class="ml-1 px-2" - :disabled="canisterDetails.moduleHash.loading" + :disabled="!canisterDetails.status.value" :append-icon="mdiDatabaseCog" @click="dialogs.install = true" > diff --git a/core/station/api/spec.did b/core/station/api/spec.did index 9cd097207..eb567dbfb 100644 --- a/core/station/api/spec.did +++ b/core/station/api/spec.did @@ -653,9 +653,9 @@ type CreateExternalCanisterOperationInput = record { // The labels of the external canister. labels : opt vec text; // What operations are allowed on the canister. - permissions : ExternalCanisterPermissionsInput; + permissions : ExternalCanisterPermissionsCreateInput; // The request policies for the canister. - request_policies : ExternalCanisterRequestPoliciesInput; + request_policies : ExternalCanisterRequestPoliciesCreateInput; }; type ConfigureExternalCanisterSettingsInput = record { @@ -665,9 +665,9 @@ type ConfigureExternalCanisterSettingsInput = record { // The labels of the external canister. labels : opt vec text; // What operations are allowed on the canister. - permissions : opt ExternalCanisterPermissionsInput; + permissions : opt ExternalCanisterPermissionsUpdateInput; // The request policies for the canister. - request_policies : opt ExternalCanisterRequestPoliciesInput; + request_policies : opt ExternalCanisterRequestPoliciesUpdateInput; // The state of the external canister. state : opt ExternalCanisterState; }; @@ -2310,8 +2310,32 @@ type ExternalCanisterPermissions = record { calls : vec ExternalCanisterCallPermission; }; +// The create input type for setting the permissions for the external canister. +type ExternalCanisterPermissionsCreateInput = ExternalCanisterPermissions; + +// The input type for setting call permissions of an existing external canister. +type ExternalCanisterChangeCallPermissionsInput = variant { + // Replaces all the call permissions with the provided list, if the list is empty + // all the call permissions will be removed. + ReplaceAllBy : vec ExternalCanisterCallPermission; + // Override the call permissions from the specified execution methods. + OverrideSpecifiedByExecutionMethods : vec ExternalCanisterCallPermission; + // Remove call permissions of the specified execution methods. + RemoveByExecutionMethods : vec text; +}; + // The input type for setting the permissions for the external canister. -type ExternalCanisterPermissionsInput = ExternalCanisterPermissions; +type ExternalCanisterPermissionsUpdateInput = record { + // Who can read information about the canister (e.g. canister status), + // changes to this permission can be made by the `change` permission. + read : opt Allow; + // Who can make changes to the canister, includes: + // - changing the permissions + // - install operations + change : opt Allow; + // The permissions for calling methods on the canister. + calls : opt ExternalCanisterChangeCallPermissionsInput; +}; // The request policy rules for the external canister. type ExternalCanisterRequestPolicies = record { @@ -2321,14 +2345,31 @@ type ExternalCanisterRequestPolicies = record { calls : vec ExternalCanisterCallRequestPolicyRule; }; -// The input type for setting the request policies for the external canister. -type ExternalCanisterRequestPoliciesInput = record { +// The input type for setting the request policies for a new external canister. +type ExternalCanisterRequestPoliciesCreateInput = record { // The request policy rules for the canister change operation. change : vec ExternalCanisterChangeRequestPolicyRuleInput; // The request policy rules for the calling methods on the canister. calls : vec ExternalCanisterCallRequestPolicyRuleInput; }; +type ExternalCanisterChangeCallRequestPoliciesInput = variant { + // Replaces all the call request policies with the provided list. + ReplaceAllBy : vec ExternalCanisterCallRequestPolicyRuleInput; + // Remove call request policies by the provided ids. + RemoveByPolicyIds : vec UUID; + // Override the request policies for the specified execution methods. + OverrideSpecifiedByExecutionMethods : vec ExternalCanisterCallRequestPolicyRuleInput; +}; + +// The input type for setting the request policies for an existing external canister. +type ExternalCanisterRequestPoliciesUpdateInput = record { + // The request policy rules for the canister change operation. + change : opt vec ExternalCanisterChangeRequestPolicyRuleInput; + // The request policy rules for the calling methods on the canister. + calls : opt ExternalCanisterChangeCallRequestPoliciesInput; +}; + // An external canister that the station can interact with. type ExternalCanister = record { // The id of the resource in the station. diff --git a/core/station/api/src/external_canister.rs b/core/station/api/src/external_canister.rs index f2a80397d..05ef3d1df 100644 --- a/core/station/api/src/external_canister.rs +++ b/core/station/api/src/external_canister.rs @@ -3,9 +3,6 @@ use crate::{ SortDirection, TimestampRfc3339, UuidDTO, ValidationMethodResourceTargetDTO, }; use candid::{CandidType, Deserialize, Nat, Principal}; - -pub type ExternalCanisterPermissionsInput = ExternalCanisterPermissionsDTO; - // Taken from https://internetcomputer.org/docs/current/references/ic-interface-spec/#ic-create_canister #[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] pub struct DefiniteCanisterSettingsInput { @@ -43,8 +40,24 @@ pub struct CreateExternalCanisterOperationInput { pub name: String, pub description: Option, pub labels: Option>, - pub permissions: ExternalCanisterPermissionsInput, - pub request_policies: ExternalCanisterRequestPoliciesInput, + pub permissions: ExternalCanisterPermissionsCreateInput, + pub request_policies: ExternalCanisterRequestPoliciesCreateInput, +} + +pub type ExternalCanisterPermissionsCreateInput = ExternalCanisterPermissionsDTO; + +#[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] +pub struct ExternalCanisterPermissionsUpdateInput { + pub read: Option, + pub change: Option, + pub calls: Option, +} + +#[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] +pub enum ExternalCanisterChangeCallPermissionsInput { + ReplaceAllBy(Vec), + OverrideSpecifiedByExecutionMethods(Vec), + RemoveByExecutionMethods(Vec), } #[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] @@ -85,8 +98,8 @@ pub struct ConfigureExternalCanisterSettingsInput { pub description: Option, pub labels: Option>, pub state: Option, - pub permissions: Option, - pub request_policies: Option, + pub permissions: Option, + pub request_policies: Option, } #[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] @@ -171,11 +184,24 @@ pub struct ExternalCanisterRequestPoliciesDTO { } #[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] -pub struct ExternalCanisterRequestPoliciesInput { +pub struct ExternalCanisterRequestPoliciesCreateInput { pub change: Vec, pub calls: Vec, } +#[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] +pub struct ExternalCanisterRequestPoliciesUpdateInput { + pub change: Option>, + pub calls: Option, +} + +#[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] +pub enum ExternalCanisterChangeCallRequestPoliciesInput { + ReplaceAllBy(Vec), + RemoveByPolicyIds(Vec), + OverrideSpecifiedByExecutionMethods(Vec), +} + #[derive(CandidType, serde::Serialize, Deserialize, Debug, Clone)] pub struct ExternalCanisterDTO { pub id: UuidDTO, diff --git a/core/station/impl/src/errors/external_canister.rs b/core/station/impl/src/errors/external_canister.rs index 5cca0faad..406c9886d 100644 --- a/core/station/impl/src/errors/external_canister.rs +++ b/core/station/impl/src/errors/external_canister.rs @@ -17,7 +17,7 @@ pub enum ExternalCanisterError { #[error(r#"The external canister operation failed due to {reason}"#)] Failed { reason: String }, /// The external canister has failed validation. - #[error(r#"The external canister has failed validation."#)] + #[error(r#"The external canister has failed validation with reason `{info}`."#)] ValidationError { info: String }, } @@ -50,6 +50,9 @@ impl From for ExternalCanisterError { ExternalCanisterValidationError::InvalidExternalCanister { principal } => { ExternalCanisterError::InvalidExternalCanister { principal } } + ExternalCanisterValidationError::ValidationError { info } => { + ExternalCanisterError::ValidationError { info } + } } } } diff --git a/core/station/impl/src/errors/request.rs b/core/station/impl/src/errors/request.rs index eec439079..67de6fd07 100644 --- a/core/station/impl/src/errors/request.rs +++ b/core/station/impl/src/errors/request.rs @@ -92,6 +92,9 @@ impl From for RequestError { info: format!("Invalid external canister {}", principal), } } + ExternalCanisterValidationError::ValidationError { info } => { + RequestError::ValidationError { info } + } } } } diff --git a/core/station/impl/src/errors/request_policy.rs b/core/station/impl/src/errors/request_policy.rs index 3d52d1a85..c533bed69 100644 --- a/core/station/impl/src/errors/request_policy.rs +++ b/core/station/impl/src/errors/request_policy.rs @@ -43,6 +43,9 @@ impl From for RequestPolicyError { info: format!("Invalid external canister {}", principal), } } + ExternalCanisterValidationError::ValidationError { info } => { + RequestPolicyError::ValidationError { info } + } } } } diff --git a/core/station/impl/src/errors/validation.rs b/core/station/impl/src/errors/validation.rs index e6939f7f8..93ede9bd0 100644 --- a/core/station/impl/src/errors/validation.rs +++ b/core/station/impl/src/errors/validation.rs @@ -59,10 +59,12 @@ impl DetailableError for RecordValidationError { } } -#[derive(Debug, Error)] +#[derive(Debug, Error, Eq, PartialEq)] pub enum ExternalCanisterValidationError { #[error(r#"The principal {principal} is an invalid external canister."#)] InvalidExternalCanister { principal: Principal }, + #[error(r#"The external canister has failed validation with reason `{info}`."#)] + ValidationError { info: String }, } impl DetailableError for ExternalCanisterValidationError { @@ -74,6 +76,10 @@ impl DetailableError for ExternalCanisterValidationError { details.insert("principal".to_string(), principal.to_string()); Some(details) } + ExternalCanisterValidationError::ValidationError { info } => { + details.insert("info".to_string(), info.to_string()); + Some(details) + } } } } diff --git a/core/station/impl/src/mappers/request_operation.rs b/core/station/impl/src/mappers/request_operation.rs index 448d392e9..660b0d65c 100644 --- a/core/station/impl/src/mappers/request_operation.rs +++ b/core/station/impl/src/mappers/request_operation.rs @@ -23,8 +23,10 @@ use crate::{ EditPermissionOperationInput, EditRequestPolicyOperation, EditRequestPolicyOperationInput, EditUserGroupOperation, EditUserOperation, EditUserOperationInput, ExternalCanisterCallPermission, ExternalCanisterCallRequestPolicyRuleInput, - ExternalCanisterChangeRequestPolicyRuleInput, ExternalCanisterPermissionsInput, - ExternalCanisterRequestPoliciesInput, FundExternalCanisterOperation, + ExternalCanisterChangeCallPermissionsInput, ExternalCanisterChangeCallRequestPoliciesInput, + ExternalCanisterChangeRequestPolicyRuleInput, ExternalCanisterPermissionsCreateInput, + ExternalCanisterPermissionsUpdateInput, ExternalCanisterRequestPoliciesCreateInput, + ExternalCanisterRequestPoliciesUpdateInput, FundExternalCanisterOperation, ManageSystemInfoOperation, ManageSystemInfoOperationInput, RemoveAddressBookEntryOperation, RemoveRequestPolicyOperation, RemoveRequestPolicyOperationInput, RemoveUserGroupOperation, RequestOperation, SetDisasterRecoveryOperation, SetDisasterRecoveryOperationInput, @@ -578,6 +580,76 @@ impl From for station_api::DefiniteCanisterSettin } } +impl From + for ExternalCanisterRequestPoliciesUpdateInput +{ + fn from( + input: station_api::ExternalCanisterRequestPoliciesUpdateInput, + ) -> ExternalCanisterRequestPoliciesUpdateInput { + ExternalCanisterRequestPoliciesUpdateInput { + change: input + .change + .map(|change| change.into_iter().map(Into::into).collect()), + calls: input.calls.map(Into::into), + } + } +} + +impl From + for station_api::ExternalCanisterRequestPoliciesUpdateInput +{ + fn from( + input: ExternalCanisterRequestPoliciesUpdateInput, + ) -> station_api::ExternalCanisterRequestPoliciesUpdateInput { + station_api::ExternalCanisterRequestPoliciesUpdateInput { + change: input + .change + .map(|change| change.into_iter().map(Into::into).collect()), + calls: input.calls.map(Into::into), + } + } +} + +impl From + for ExternalCanisterChangeCallRequestPoliciesInput +{ + fn from( + input: station_api::ExternalCanisterChangeCallRequestPoliciesInput, + ) -> ExternalCanisterChangeCallRequestPoliciesInput { + match input { + station_api::ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(input) => { + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(input.into_iter().map(Into::into).collect()) + } + station_api::ExternalCanisterChangeCallRequestPoliciesInput::OverrideSpecifiedByExecutionMethods(input) => { + ExternalCanisterChangeCallRequestPoliciesInput::OverrideSpecifiedByExecutionMethods(input.into_iter().map(Into::into).collect()) + } + station_api::ExternalCanisterChangeCallRequestPoliciesInput::RemoveByPolicyIds(ids) => { + ExternalCanisterChangeCallRequestPoliciesInput::RemoveByPolicyIds(ids.into_iter().map(|id| *HelperMapper::to_uuid(id).expect("Invalid policy id").as_bytes()).collect()) + } + } + } +} + +impl From + for station_api::ExternalCanisterChangeCallRequestPoliciesInput +{ + fn from( + input: ExternalCanisterChangeCallRequestPoliciesInput, + ) -> station_api::ExternalCanisterChangeCallRequestPoliciesInput { + match input { + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(input) => { + station_api::ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(input.into_iter().map(Into::into).collect()) + } + ExternalCanisterChangeCallRequestPoliciesInput::OverrideSpecifiedByExecutionMethods(input) => { + station_api::ExternalCanisterChangeCallRequestPoliciesInput::OverrideSpecifiedByExecutionMethods(input.into_iter().map(Into::into).collect()) + } + ExternalCanisterChangeCallRequestPoliciesInput::RemoveByPolicyIds(ids) => { + station_api::ExternalCanisterChangeCallRequestPoliciesInput::RemoveByPolicyIds(ids.into_iter().map(|id| Uuid::from_bytes(id).hyphenated().to_string()).collect()) + } + } + } +} + impl From for ExternalCanisterCallPermission { fn from( input: station_api::ExternalCanisterCallPermissionDTO, @@ -602,11 +674,13 @@ impl From for station_api::ExternalCanisterCallP } } -impl From for ExternalCanisterPermissionsInput { +impl From + for ExternalCanisterPermissionsCreateInput +{ fn from( - input: station_api::ExternalCanisterPermissionsInput, - ) -> ExternalCanisterPermissionsInput { - ExternalCanisterPermissionsInput { + input: station_api::ExternalCanisterPermissionsCreateInput, + ) -> ExternalCanisterPermissionsCreateInput { + ExternalCanisterPermissionsCreateInput { read: input.read.into(), change: input.change.into(), calls: input.calls.into_iter().map(Into::into).collect(), @@ -614,11 +688,13 @@ impl From for ExternalCanisterPer } } -impl From for station_api::ExternalCanisterPermissionsInput { +impl From + for station_api::ExternalCanisterPermissionsCreateInput +{ fn from( - input: ExternalCanisterPermissionsInput, - ) -> station_api::ExternalCanisterPermissionsInput { - station_api::ExternalCanisterPermissionsInput { + input: ExternalCanisterPermissionsCreateInput, + ) -> station_api::ExternalCanisterPermissionsCreateInput { + station_api::ExternalCanisterPermissionsCreateInput { read: input.read.into(), change: input.change.into(), calls: input.calls.into_iter().map(Into::into).collect(), @@ -626,6 +702,86 @@ impl From for station_api::ExternalCanisterPer } } +impl From + for ExternalCanisterPermissionsUpdateInput +{ + fn from( + input: station_api::ExternalCanisterPermissionsUpdateInput, + ) -> ExternalCanisterPermissionsUpdateInput { + ExternalCanisterPermissionsUpdateInput { + read: input.read.map(|read| read.into()), + change: input.change.map(|change| change.into()), + calls: input.calls.map(Into::into), + } + } +} + +impl From + for station_api::ExternalCanisterPermissionsUpdateInput +{ + fn from( + input: ExternalCanisterPermissionsUpdateInput, + ) -> station_api::ExternalCanisterPermissionsUpdateInput { + station_api::ExternalCanisterPermissionsUpdateInput { + read: input.read.map(|read| read.into()), + change: input.change.map(|change| change.into()), + calls: input.calls.map(Into::into), + } + } +} + +impl From + for ExternalCanisterChangeCallPermissionsInput +{ + fn from( + input: station_api::ExternalCanisterChangeCallPermissionsInput, + ) -> ExternalCanisterChangeCallPermissionsInput { + match input { + station_api::ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy(input) => { + ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy( + input.into_iter().map(Into::into).collect(), + ) + } + station_api::ExternalCanisterChangeCallPermissionsInput::OverrideSpecifiedByExecutionMethods( + input, + ) => ExternalCanisterChangeCallPermissionsInput::OverrideSpecifiedByExecutionMethods( + input.into_iter().map(Into::into).collect(), + ), + station_api::ExternalCanisterChangeCallPermissionsInput::RemoveByExecutionMethods(methods) => { + ExternalCanisterChangeCallPermissionsInput::RemoveByExecutionMethods( + methods.into_iter().map(Into::into).collect(), + ) + } + } + } +} + +impl From + for station_api::ExternalCanisterChangeCallPermissionsInput +{ + fn from( + input: ExternalCanisterChangeCallPermissionsInput, + ) -> station_api::ExternalCanisterChangeCallPermissionsInput { + match input { + ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy(input) => { + station_api::ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy( + input.into_iter().map(Into::into).collect(), + ) + } + ExternalCanisterChangeCallPermissionsInput::OverrideSpecifiedByExecutionMethods(input) => { + station_api::ExternalCanisterChangeCallPermissionsInput::OverrideSpecifiedByExecutionMethods( + input.into_iter().map(Into::into).collect(), + ) + } + ExternalCanisterChangeCallPermissionsInput::RemoveByExecutionMethods(methods) => { + station_api::ExternalCanisterChangeCallPermissionsInput::RemoveByExecutionMethods( + methods.into_iter().map(Into::into).collect(), + ) + } + } + } +} + impl From for ExternalCanisterCallRequestPolicyRuleInput { @@ -694,26 +850,26 @@ impl From } } -impl From - for ExternalCanisterRequestPoliciesInput +impl From + for ExternalCanisterRequestPoliciesCreateInput { fn from( - input: station_api::ExternalCanisterRequestPoliciesInput, - ) -> ExternalCanisterRequestPoliciesInput { - ExternalCanisterRequestPoliciesInput { + input: station_api::ExternalCanisterRequestPoliciesCreateInput, + ) -> ExternalCanisterRequestPoliciesCreateInput { + ExternalCanisterRequestPoliciesCreateInput { change: input.change.into_iter().map(Into::into).collect(), calls: input.calls.into_iter().map(Into::into).collect(), } } } -impl From - for station_api::ExternalCanisterRequestPoliciesInput +impl From + for station_api::ExternalCanisterRequestPoliciesCreateInput { fn from( - input: ExternalCanisterRequestPoliciesInput, - ) -> station_api::ExternalCanisterRequestPoliciesInput { - station_api::ExternalCanisterRequestPoliciesInput { + input: ExternalCanisterRequestPoliciesCreateInput, + ) -> station_api::ExternalCanisterRequestPoliciesCreateInput { + station_api::ExternalCanisterRequestPoliciesCreateInput { change: input.change.into_iter().map(Into::into).collect(), calls: input.calls.into_iter().map(Into::into).collect(), } diff --git a/core/station/impl/src/migration.rs b/core/station/impl/src/migration.rs index a81ea2063..a275dcbec 100644 --- a/core/station/impl/src/migration.rs +++ b/core/station/impl/src/migration.rs @@ -5,13 +5,13 @@ use crate::models::request_specifier::RequestSpecifier; use crate::models::resource::{ExternalCanisterResourceAction, Resource, SystemResourceAction}; use crate::models::{ Account, AccountKey, AddressBookEntry, AddressBookEntryKey, ExternalCanister, - ExternalCanisterKey, Request, RequestKey, RequestOperation, RequestPolicy, User, UserGroup, - UserKey, + ExternalCanisterKey, ListRequestsOperationType, Request, RequestKey, RequestOperation, + RequestPolicy, User, UserGroup, UserKey, }; use crate::repositories::permission::{PermissionRepository, PERMISSION_REPOSITORY}; use crate::repositories::{ AccountRepository, AddressBookRepository, ExternalCanisterRepository, RequestPolicyRepository, - RequestRepository, UserGroupRepository, UserRepository, ACCOUNT_REPOSITORY, + RequestRepository, RequestWhereClause, UserGroupRepository, UserRepository, ACCOUNT_REPOSITORY, ADDRESS_BOOK_REPOSITORY, EXTERNAL_CANISTER_REPOSITORY, REQUEST_POLICY_REPOSITORY, USER_GROUP_REPOSITORY, USER_REPOSITORY, }; @@ -46,6 +46,8 @@ impl MigrationHandler { let stored_version = system_info.get_stable_memory_version(); if stored_version == STABLE_MEMORY_VERSION { + // Run the post-run checks that need to be run on every upgrade. + post_run(); return; } @@ -61,6 +63,33 @@ impl MigrationHandler { // Update the stable memory version to the latest version. system_info.set_stable_memory_version(STABLE_MEMORY_VERSION); write_system_info(system_info); + + // Run the post-run checks that need to be run on every upgrade. + post_run(); + } +} + +/// If there is a check that needs to be run on every upgrade, regardless if the memory version has changed, +/// it should be added here. +fn post_run() { + // Deserialization of the all requests to make sure an incompatible memory will panic and avoids + // putting the station in an inconsistent state. + // + // This is a temporary addition only for the next release since we've added a breaking change to + // the `ConfigureExternalCanisterSettingsInput` which had a new API not yet used in production. + let where_clause = RequestWhereClause { + operation_types: vec![ListRequestsOperationType::ConfigureExternalCanister(None)], + ..Default::default() + }; + + let ids = REQUEST_REPOSITORY + .find_ids_where(where_clause, None) + .expect("Failed to search for requests with the external canister operation types"); + + for id in ids { + REQUEST_REPOSITORY + .get(&RequestKey { id }) + .expect("Failed to deserialize the request from the stable memory"); } } diff --git a/core/station/impl/src/models/external_canister.rs b/core/station/impl/src/models/external_canister.rs index 9da585ca3..d5da21408 100644 --- a/core/station/impl/src/models/external_canister.rs +++ b/core/station/impl/src/models/external_canister.rs @@ -1,9 +1,20 @@ use super::permission::Allow; -use super::resource::ValidationMethodResourceTarget; -use super::{ConfigureExternalCanisterSettingsInput, RequestPolicyRule}; -use crate::errors::ExternalCanisterError; +use super::request_specifier::RequestSpecifier; +use super::resource::{ + CallExternalCanisterResourceTarget, ExecutionMethodResourceTarget, ExternalCanisterId, + ValidationMethodResourceTarget, +}; +use super::{ + CanisterMethod, ConfigureExternalCanisterSettingsInput, CreateExternalCanisterOperationInput, + CreateExternalCanisterOperationKind, ExternalCanisterChangeCallRequestPoliciesInput, + ExternalCanisterRequestPoliciesCreateInput, ExternalCanisterRequestPoliciesUpdateInput, + RequestPolicy, RequestPolicyRule, +}; +use crate::errors::{ExternalCanisterError, ExternalCanisterValidationError}; +use crate::repositories::REQUEST_POLICY_REPOSITORY; use candid::Principal; -use orbit_essentials::model::ModelKey; +use orbit_essentials::model::{ContextualModel, ModelKey}; +use orbit_essentials::repository::Repository; use orbit_essentials::storable; use orbit_essentials::{ model::{ModelValidator, ModelValidatorResult}, @@ -12,6 +23,7 @@ use orbit_essentials::{ use station_api::GetExternalCanisterFiltersResponse; use std::collections::BTreeSet; use std::hash::Hash; +use uuid::Uuid; /// The external canister id, which is a UUID. pub type ExternalCanisterEntryId = UUID; @@ -219,6 +231,176 @@ impl ModelValidator for ExternalCanister { } } +fn assert_policy_is_associated_with_target_canister( + policy_id: &UUID, + is_associated: F, +) -> ModelValidatorResult +where + F: Fn(&RequestPolicy) -> bool, +{ + let policy = REQUEST_POLICY_REPOSITORY.get(policy_id).ok_or_else(|| { + ExternalCanisterValidationError::ValidationError { + info: format!( + "The request policy with id '{}' does not exist.", + Uuid::from_bytes(*policy_id).hyphenated() + ), + } + })?; + + // validates if the policy matches the expected variant and canister_id or throws an error + if !is_associated(&policy) { + Err(ExternalCanisterValidationError::ValidationError { + info: format!( + "The request policy with id '{}' is not associated with the external canister.", + Uuid::from_bytes(*policy_id).hyphenated() + ), + })?; + } + + Ok(()) +} + +fn validate_change_policies_are_associated_with_target_canister( + canister_id: &Principal, + policy_ids: &[UUID], +) -> ModelValidatorResult { + for policy_id in policy_ids { + // validates if the policy matches the expected change variant and canister_id or throws an error + assert_policy_is_associated_with_target_canister(policy_id, |policy| { + matches!( + policy.specifier, + RequestSpecifier::ChangeExternalCanister(ExternalCanisterId::Canister(id)) if id == *canister_id + ) + })?; + } + + Ok(()) +} + +fn validate_calls_policies_are_associated_with_target_canister( + canister_id: &Principal, + policy_ids: &[UUID], +) -> ModelValidatorResult { + for policy_id in policy_ids { + // validates if the policy matches the expected call variant and canister_id or throws an error + assert_policy_is_associated_with_target_canister(policy_id, |policy| { + matches!( + policy.specifier, + RequestSpecifier::CallExternalCanister(CallExternalCanisterResourceTarget { + execution_method: ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { + canister_id: id, + method_name: _, + } + ), + .. + }) if id == *canister_id + ) + })?; + } + + Ok(()) +} + +impl ModelValidator + for ContextualModel +{ + fn validate(&self) -> ModelValidatorResult { + let canister_id = &self.context; + + if let Some(change_policies) = &self.model.change { + let policy_ids = change_policies + .iter() + .filter_map(|p| p.policy_id) + .collect::>(); + + validate_change_policies_are_associated_with_target_canister(canister_id, &policy_ids)?; + } + + if let Some(change_calls_policies_operation) = &self.model.calls { + let policy_ids = match &change_calls_policies_operation { + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(policies) => policies + .iter() + .filter_map(|p| p.policy_id) + .collect::>(), + ExternalCanisterChangeCallRequestPoliciesInput::RemoveByPolicyIds(policy_ids) => { + policy_ids.clone() + } + ExternalCanisterChangeCallRequestPoliciesInput::OverrideSpecifiedByExecutionMethods( + policies, + ) => policies + .iter() + .filter_map(|p| p.policy_id) + .collect::>(), + }; + + validate_calls_policies_are_associated_with_target_canister(canister_id, &policy_ids)?; + } + + Ok(()) + } +} + +impl ModelValidator + for ContextualModel +{ + fn validate(&self) -> ModelValidatorResult { + let canister_id = &self.context; + + validate_change_policies_are_associated_with_target_canister( + canister_id, + &self + .model + .change + .iter() + .filter_map(|p| p.policy_id) + .collect::>(), + )?; + + validate_calls_policies_are_associated_with_target_canister( + canister_id, + &self + .model + .calls + .iter() + .filter_map(|p| p.policy_id) + .collect::>(), + )?; + + Ok(()) + } +} + +impl ModelValidator for CreateExternalCanisterOperationInput { + fn validate(&self) -> ModelValidatorResult { + match &self.kind { + CreateExternalCanisterOperationKind::AddExisting(existing) => { + ContextualModel::new(self.request_policies.clone(), existing.canister_id) + .validate()?; + } + CreateExternalCanisterOperationKind::CreateNew(_) => { + if self + .request_policies + .change + .iter() + .any(|p| p.policy_id.is_some()) + || self + .request_policies + .calls + .iter() + .any(|p| p.policy_id.is_some()) + { + return Err(ExternalCanisterValidationError::ValidationError { + info: "The request policies cannot have policy ids when creating a new external canister.".to_string(), + }); + } + } + } + + Ok(()) + } +} + #[cfg(any(test, feature = "canbench"))] pub mod external_canister_test_utils { use super::*; @@ -245,6 +427,12 @@ pub mod external_canister_test_utils { #[cfg(test)] mod tests { + use crate::models::{ + request_policy_test_utils::mock_request_policy, + CreateExternalCanisterOperationKindCreateNew, ExternalCanisterCallRequestPolicyRuleInput, + ExternalCanisterChangeRequestPolicyRuleInput, ExternalCanisterPermissionsCreateInput, + }; + use super::*; use external_canister_test_utils::mock_external_canister; use ic_stable_structures::Storable; @@ -384,4 +572,180 @@ mod tests { assert_eq!(model.labels, vec!["new".to_string()]); assert_eq!(model.state, ExternalCanisterState::Archived); } + + #[test] + fn fail_validation_adding_policies_associated_with_another_external_canister() { + let mut request_policy = mock_request_policy(); + request_policy.specifier = RequestSpecifier::ChangeExternalCanister( + ExternalCanisterId::Canister(Principal::from_slice(&[1; 29])), + ); + + REQUEST_POLICY_REPOSITORY.insert(request_policy.id, request_policy.clone()); + + let validation_change_policies = + validate_change_policies_are_associated_with_target_canister( + &Principal::from_slice(&[2; 29]), + &[request_policy.id], + ); + + assert!(validation_change_policies.is_err()); + assert_eq!( + validation_change_policies.unwrap_err(), + ExternalCanisterValidationError::ValidationError { + info: format!( + "The request policy with id '{}' is not associated with the external canister.", + Uuid::from_bytes(request_policy.id).hyphenated() + ) + } + ); + + let validation_calls_policies = validate_calls_policies_are_associated_with_target_canister( + &Principal::from_slice(&[2; 29]), + &[request_policy.id], + ); + + assert!(validation_calls_policies.is_err()); + assert_eq!( + validation_calls_policies.unwrap_err(), + ExternalCanisterValidationError::ValidationError { + info: format!( + "The request policy with id '{}' is not associated with the external canister.", + Uuid::from_bytes(request_policy.id).hyphenated() + ) + } + ); + } + + #[test] + fn fail_validation_adding_policies_associated_with_another_specifier() { + let mut request_policy = mock_request_policy(); + request_policy.specifier = RequestSpecifier::AddAccount; + + REQUEST_POLICY_REPOSITORY.insert(request_policy.id, request_policy.clone()); + + let validation_change_policies = + validate_change_policies_are_associated_with_target_canister( + &Principal::from_slice(&[1; 29]), + &[request_policy.id], + ); + + assert!(validation_change_policies.is_err()); + assert_eq!( + validation_change_policies.unwrap_err(), + ExternalCanisterValidationError::ValidationError { + info: format!( + "The request policy with id '{}' is not associated with the external canister.", + Uuid::from_bytes(request_policy.id).hyphenated() + ) + } + ); + + let validation_calls_policies = validate_calls_policies_are_associated_with_target_canister( + &Principal::from_slice(&[1; 29]), + &[request_policy.id], + ); + + assert!(validation_calls_policies.is_err()); + assert_eq!( + validation_calls_policies.unwrap_err(), + ExternalCanisterValidationError::ValidationError { + info: format!( + "The request policy with id '{}' is not associated with the external canister.", + Uuid::from_bytes(request_policy.id).hyphenated() + ) + } + ); + } + + #[test] + fn passes_validation_adding_policies_for_the_target_external_canister() { + let mut request_policy = mock_request_policy(); + request_policy.specifier = RequestSpecifier::ChangeExternalCanister( + ExternalCanisterId::Canister(Principal::from_slice(&[1; 29])), + ); + + REQUEST_POLICY_REPOSITORY.insert(request_policy.id, request_policy.clone()); + + let validation_change_policies = + validate_change_policies_are_associated_with_target_canister( + &Principal::from_slice(&[1; 29]), + &[request_policy.id], + ); + + assert!(validation_change_policies.is_ok()); + + let mut request_policy = mock_request_policy(); + + request_policy.specifier = + RequestSpecifier::CallExternalCanister(CallExternalCanisterResourceTarget { + execution_method: ExecutionMethodResourceTarget::ExecutionMethod(CanisterMethod { + canister_id: Principal::from_slice(&[2; 29]), + method_name: "test".to_string(), + }), + validation_method: ValidationMethodResourceTarget::No, + }); + + REQUEST_POLICY_REPOSITORY.insert(request_policy.id, request_policy.clone()); + + let validation_calls_policies = validate_calls_policies_are_associated_with_target_canister( + &Principal::from_slice(&[2; 29]), + &[request_policy.id], + ); + + assert!(validation_calls_policies.is_ok()); + } + + #[test] + fn fail_creating_new_canister_with_existing_policy_ids() { + let mut input = CreateExternalCanisterOperationInput { + kind: CreateExternalCanisterOperationKind::CreateNew( + CreateExternalCanisterOperationKindCreateNew { + initial_cycles: None, + }, + ), + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: vec![ExternalCanisterChangeRequestPolicyRuleInput { + policy_id: Some([1; 16]), + rule: RequestPolicyRule::AutoApproved, + }], + calls: vec![], + }, + name: "Test canister".to_string(), + description: None, + labels: None, + permissions: ExternalCanisterPermissionsCreateInput { + read: Allow::authenticated(), + change: Allow::authenticated(), + calls: vec![], + }, + }; + + let validation = input.validate(); + + assert!(validation.is_err()); + assert_eq!( + validation.unwrap_err(), + ExternalCanisterValidationError::ValidationError { + info: "The request policies cannot have policy ids when creating a new external canister.".to_string() + } + ); + + input.request_policies.change = vec![]; + input.request_policies.calls = vec![ExternalCanisterCallRequestPolicyRuleInput { + policy_id: Some([1; 16]), + rule: RequestPolicyRule::AutoApproved, + validation_method: ValidationMethodResourceTarget::No, + execution_method: "test".to_string(), + }]; + + let validation = input.validate(); + + assert!(validation.is_err()); + assert_eq!( + validation.unwrap_err(), + ExternalCanisterValidationError::ValidationError { + info: "The request policies cannot have policy ids when creating a new external canister.".to_string() + } + ); + } } diff --git a/core/station/impl/src/models/request.rs b/core/station/impl/src/models/request.rs index 1f5522ee0..d3963bfdf 100644 --- a/core/station/impl/src/models/request.rs +++ b/core/station/impl/src/models/request.rs @@ -1,7 +1,7 @@ use super::request_policy_rule::{RequestEvaluationResult, RequestPolicyRuleInput}; use super::{ - DisplayUser, EvaluationStatus, RequestApproval, RequestApprovalStatus, RequestOperation, - RequestStatus, UserId, UserKey, + ConfigureExternalCanisterOperationKind, DisplayUser, EvaluationStatus, RequestApproval, + RequestApprovalStatus, RequestOperation, RequestStatus, UserId, UserKey, }; use crate::core::evaluation::{ Evaluate, REQUEST_APPROVE_RIGHTS_REQUEST_POLICY_RULE_EVALUATOR, REQUEST_POLICY_RULE_EVALUATOR, @@ -20,7 +20,7 @@ use crate::errors::{EvaluateError, RequestError, ValidationError}; use crate::models::resource::{ExecutionMethodResourceTarget, ValidationMethodResourceTarget}; use crate::repositories::USER_REPOSITORY; use candid::{CandidType, Deserialize}; -use orbit_essentials::model::ModelKey; +use orbit_essentials::model::{ContextualModel, ModelKey}; use orbit_essentials::repository::Repository; use orbit_essentials::storable; use orbit_essentials::{ @@ -214,9 +214,19 @@ fn validate_request_operation_foreign_keys( } RequestOperation::SystemUpgrade(_) => (), RequestOperation::ChangeExternalCanister(_) => (), - RequestOperation::ConfigureExternalCanister(_) => (), + RequestOperation::ConfigureExternalCanister(op) => { + let canister_id = op.canister_id; + if let ConfigureExternalCanisterOperationKind::Settings(settings) = &op.kind { + if let Some(updated_request_policies) = &settings.request_policies { + ContextualModel::new(updated_request_policies.clone(), canister_id) + .validate()?; + } + } + } RequestOperation::FundExternalCanister(_) => (), - RequestOperation::CreateExternalCanister(_) => (), + RequestOperation::CreateExternalCanister(op) => { + op.input.validate()?; + } RequestOperation::CallExternalCanister(op) => { let validation_method_target: ValidationMethodResourceTarget = op.input.validation_method.clone().into(); diff --git a/core/station/impl/src/models/request_operation.rs b/core/station/impl/src/models/request_operation.rs index 9ca213e74..8b4f8fa0b 100644 --- a/core/station/impl/src/models/request_operation.rs +++ b/core/station/impl/src/models/request_operation.rs @@ -348,12 +348,28 @@ pub struct ChangeExternalCanisterOperation { #[storable] #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub struct ExternalCanisterPermissionsInput { +pub struct ExternalCanisterPermissionsCreateInput { pub read: Allow, pub change: Allow, pub calls: Vec, } +#[storable] +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct ExternalCanisterPermissionsUpdateInput { + pub read: Option, + pub change: Option, + pub calls: Option, +} + +#[storable] +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum ExternalCanisterChangeCallPermissionsInput { + ReplaceAllBy(Vec), + OverrideSpecifiedByExecutionMethods(Vec), + RemoveByExecutionMethods(Vec), +} + #[storable] #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct ExternalCanisterCallRequestPolicyRuleInput { @@ -372,11 +388,26 @@ pub struct ExternalCanisterChangeRequestPolicyRuleInput { #[storable] #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub struct ExternalCanisterRequestPoliciesInput { +pub struct ExternalCanisterRequestPoliciesCreateInput { pub change: Vec, pub calls: Vec, } +#[storable] +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct ExternalCanisterRequestPoliciesUpdateInput { + pub change: Option>, + pub calls: Option, +} + +#[storable] +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum ExternalCanisterChangeCallRequestPoliciesInput { + ReplaceAllBy(Vec), + RemoveByPolicyIds(Vec), + OverrideSpecifiedByExecutionMethods(Vec), +} + #[storable] #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct CreateExternalCanisterOperationKindCreateNew { @@ -403,8 +434,8 @@ pub struct CreateExternalCanisterOperationInput { pub name: String, pub description: Option, pub labels: Option>, - pub permissions: ExternalCanisterPermissionsInput, - pub request_policies: ExternalCanisterRequestPoliciesInput, + pub permissions: ExternalCanisterPermissionsCreateInput, + pub request_policies: ExternalCanisterRequestPoliciesCreateInput, } #[storable] @@ -470,8 +501,8 @@ pub struct ConfigureExternalCanisterSettingsInput { pub description: Option, pub labels: Option>, pub state: Option, - pub permissions: Option, - pub request_policies: Option, + pub permissions: Option, + pub request_policies: Option, } #[storable] diff --git a/core/station/impl/src/repositories/indexes/request_policy_resource_index.rs b/core/station/impl/src/repositories/indexes/request_policy_resource_index.rs index b68bbc9e6..85d64c43e 100644 --- a/core/station/impl/src/repositories/indexes/request_policy_resource_index.rs +++ b/core/station/impl/src/repositories/indexes/request_policy_resource_index.rs @@ -84,6 +84,36 @@ impl IndexRepository for RequestPolicyResource } } +#[derive(Clone, Default)] +pub struct ExternalCanisterPoliciesList { + pub change: Vec, + pub calls: Vec, +} + +impl ExternalCanisterPoliciesList { + pub fn new() -> Self { + Self { + change: Vec::new(), + calls: Vec::new(), + } + } + + pub fn len(&self) -> usize { + self.change.len() + self.calls.len() + } + + pub fn is_empty(&self) -> bool { + self.change.is_empty() && self.calls.is_empty() + } + + pub fn all(&self) -> Vec { + let mut all = Vec::new(); + all.extend(self.change.iter()); + all.extend(self.calls.iter()); + all + } +} + impl RequestPolicyResourceIndexRepository { /// Finds all external canister policies related to the specified canister id. /// @@ -91,11 +121,14 @@ impl RequestPolicyResourceIndexRepository { /// /// - `Change` related policies. /// - `Call` related policies. - pub fn find_external_canister_policies(&self, canister_id: &Principal) -> Vec { + pub fn find_external_canister_policies( + &self, + canister_id: &Principal, + ) -> ExternalCanisterPoliciesList { DB.with(|db| { - let mut policies = Vec::new(); + let mut policies = ExternalCanisterPoliciesList::new(); // Find all change related policies for the specified canister id. - policies.extend( + policies.change.extend( db.borrow() .range( (RequestPolicyResourceIndex { @@ -119,7 +152,7 @@ impl RequestPolicyResourceIndexRepository { ); // Find all call related policies for the specified canister id. - policies.extend( + policies.calls.extend( db.borrow() .range( (RequestPolicyResourceIndex { @@ -158,6 +191,49 @@ impl RequestPolicyResourceIndexRepository { policies }) } + + // Find all external canister call policies related to the specified canister id and execution method. + pub fn find_external_canister_call_policies_by_execution_method( + &self, + canister_id: &Principal, + execution_method: &String, + ) -> Vec { + DB.with(|db| { + db.borrow() + .range( + (RequestPolicyResourceIndex { + resource: Resource::ExternalCanister(ExternalCanisterResourceAction::Call( + CallExternalCanisterResourceTarget { + execution_method: ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { + canister_id: *canister_id, + method_name: execution_method.clone(), + }, + ), + validation_method: ValidationMethodResourceTarget::No, + }, + )), + policy_id: [u8::MIN; 16], + }).., + ) + .take_while(|(index, _)| { + matches!( + &index.resource, + Resource::ExternalCanister(ExternalCanisterResourceAction::Call( + CallExternalCanisterResourceTarget { + execution_method: ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { canister_id: id, method_name: method } + ), + .. + } + )) + if id == canister_id && execution_method == method + ) + }) + .map(|(index, _)| index.policy_id) + .collect::>() + }) + } } #[cfg(test)] @@ -248,4 +324,51 @@ mod tests { assert_eq!(policies.len(), 5); } + + #[test] + fn test_find_external_canister_call_policies_by_execution_method() { + let repository = RequestPolicyResourceIndexRepository::default(); + let mut expected_method_ids = Vec::new(); + for i in 0..20 { + let index = RequestPolicyResourceIndex { + resource: Resource::ExternalCanister(ExternalCanisterResourceAction::Call( + CallExternalCanisterResourceTarget { + execution_method: ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { + canister_id: Principal::management_canister(), + method_name: format!("method_{}", i), + }, + ), + validation_method: if i % 2 == 0 { + ValidationMethodResourceTarget::No + } else { + ValidationMethodResourceTarget::ValidationMethod(CanisterMethod { + canister_id: Principal::management_canister(), + method_name: format!("validation_method_{}", i), + }) + }, + }, + )), + policy_id: [i; 16], + }; + + repository.insert(index); + + if i % 2 == 0 { + expected_method_ids.push([i; 16]); + } + } + + expected_method_ids.reverse(); + + for i in (0..20).step_by(2) { + let policies = repository.find_external_canister_call_policies_by_execution_method( + &Principal::management_canister(), + &format!("method_{}", i), + ); + + assert_eq!(policies.len(), 1); + assert_eq!(policies[0], expected_method_ids.pop().unwrap()); + } + } } diff --git a/core/station/impl/src/repositories/request.rs b/core/station/impl/src/repositories/request.rs index 8ca145d55..13f4e6b52 100644 --- a/core/station/impl/src/repositories/request.rs +++ b/core/station/impl/src/repositories/request.rs @@ -418,7 +418,7 @@ impl RequestRepository { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct RequestWhereClause { pub created_dt_from: Option, pub created_dt_to: Option, diff --git a/core/station/impl/src/repositories/request_policy.rs b/core/station/impl/src/repositories/request_policy.rs index 81a8e0051..fe045ec98 100644 --- a/core/station/impl/src/repositories/request_policy.rs +++ b/core/station/impl/src/repositories/request_policy.rs @@ -1,4 +1,6 @@ -use super::indexes::request_policy_resource_index::RequestPolicyResourceIndexRepository; +use super::indexes::request_policy_resource_index::{ + ExternalCanisterPoliciesList, RequestPolicyResourceIndexRepository, +}; use crate::{ core::{ metrics::REQUEST_POLICY_METRICS, with_memory_manager, Memory, REQUEST_POLICIES_MEMORY_ID, @@ -116,10 +118,23 @@ impl RequestPolicyRepository { /// /// - `Change` related policies. /// - `Call` related policies. - pub fn find_external_canister_policies(&self, canister_id: &Principal) -> Vec { + pub fn find_external_canister_policies( + &self, + canister_id: &Principal, + ) -> ExternalCanisterPoliciesList { self.resource_index .find_external_canister_policies(canister_id) } + + /// Finds all external canister call policies related to the specified canister id and execution method. + pub fn find_external_canister_call_policies_by_execution_method( + &self, + canister_id: &Principal, + execution_method: &String, + ) -> Vec { + self.resource_index + .find_external_canister_call_policies_by_execution_method(canister_id, execution_method) + } } #[cfg(test)] diff --git a/core/station/impl/src/services/external_canister.rs b/core/station/impl/src/services/external_canister.rs index 687d6cc09..806a76378 100644 --- a/core/station/impl/src/services/external_canister.rs +++ b/core/station/impl/src/services/external_canister.rs @@ -7,6 +7,7 @@ use crate::core::validation::EnsureExternalCanister; use crate::core::CallContext; use crate::errors::ExternalCanisterError; use crate::mappers::ExternalCanisterMapper; +use crate::models::permission::Permission; use crate::models::request_specifier::RequestSpecifier; use crate::models::resource::{ CallExternalCanisterResourceTarget, ExecutionMethodResourceTarget, ExternalCanisterId, @@ -17,11 +18,12 @@ use crate::models::{ CreateExternalCanisterOperationInput, CreateExternalCanisterOperationKind, DefiniteCanisterSettingsInput, EditPermissionOperationInput, EditRequestPolicyOperationInput, ExternalCanister, ExternalCanisterAvailableFilters, ExternalCanisterCallPermission, - ExternalCanisterCallRequestPolicyRule, ExternalCanisterCallerMethodsPrivileges, - ExternalCanisterCallerPrivileges, ExternalCanisterChangeRequestPolicyRule, - ExternalCanisterEntryId, ExternalCanisterKey, ExternalCanisterPermissions, - ExternalCanisterPermissionsInput, ExternalCanisterRequestPolicies, - ExternalCanisterRequestPoliciesInput, RequestPolicy, + ExternalCanisterCallRequestPolicyRule, ExternalCanisterCallRequestPolicyRuleInput, + ExternalCanisterCallerMethodsPrivileges, ExternalCanisterCallerPrivileges, + ExternalCanisterChangeCallPermissionsInput, ExternalCanisterChangeCallRequestPoliciesInput, + ExternalCanisterChangeRequestPolicyRule, ExternalCanisterEntryId, ExternalCanisterKey, + ExternalCanisterPermissions, ExternalCanisterPermissionsUpdateInput, + ExternalCanisterRequestPolicies, ExternalCanisterRequestPoliciesUpdateInput, RequestPolicy, }; use crate::repositories::permission::{PermissionRepository, PERMISSION_REPOSITORY}; use crate::repositories::{ @@ -45,7 +47,7 @@ use station_api::{ GetExternalCanisterFiltersInput, GetExternalCanisterFiltersResponseNameEntry, ListExternalCanistersInput, }; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use uuid::Uuid; @@ -134,6 +136,7 @@ impl ExternalCanisterService { let policies = self .request_policy_repository .find_external_canister_policies(canister_id) + .all() .iter() .filter_map(|policy_id| self.request_policy_repository.get(policy_id)) .collect::>(); @@ -479,7 +482,7 @@ impl ExternalCanisterService { &self, input: CreateExternalCanisterOperationInput, ) -> ServiceResult { - self.check_unique_name(input.name.clone().as_str(), None)?; + self.check_unique_name(input.name.as_str(), None)?; let external_canister = match &input.kind { CreateExternalCanisterOperationKind::CreateNew(opts) => { let mut external_canister = ExternalCanisterMapper::from_create_input( @@ -517,196 +520,282 @@ impl ExternalCanisterService { self.external_canister_repository .insert(external_canister.key(), external_canister.clone()); - self.configure_external_canister_permissions(&external_canister, input.permissions)?; + self.configure_external_canister_permissions( + &external_canister, + // maps from create to update type to reuse the same permission configuration logic + ExternalCanisterPermissionsUpdateInput { + read: Some(input.permissions.read), + change: Some(input.permissions.change), + calls: Some(ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy( + input.permissions.calls, + )), + }, + ) + .map_err(|err| { + // remove the external canister if the permission configuration failed + self.external_canister_repository + .remove(&external_canister.key()); + + err + })?; self.configure_external_canister_request_policies( &external_canister, - input.request_policies, - )?; + // maps from create to update type to reuse the same request policy configuration logic + ExternalCanisterRequestPoliciesUpdateInput { + change: Some(input.request_policies.change), + calls: Some( + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy( + input.request_policies.calls, + ), + ), + }, + ) + .map_err(|err| { + // remove the external canister if the request policy configuration failed + self.external_canister_repository + .remove(&external_canister.key()); + + err + })?; Ok(external_canister) } - /// Updates the permissions of the external canister. - fn configure_external_canister_permissions( + /// Edits an external canister's settings. + pub fn edit_external_canister( &self, - external_canister: &ExternalCanister, - input: ExternalCanisterPermissionsInput, - ) -> ServiceResult<()> { - // read permission - self.permission_service - .edit_permission(EditPermissionOperationInput { - auth_scope: Some(input.read.auth_scope), - users: Some(input.read.users), - user_groups: Some(input.read.user_groups), - resource: Resource::ExternalCanister(ExternalCanisterResourceAction::Read( - ExternalCanisterId::Canister(external_canister.canister_id), - )), - })?; + id: &ExternalCanisterEntryId, + input: ConfigureExternalCanisterSettingsInput, + ) -> ServiceResult { + let mut external_canister = self.get_external_canister(id)?; - // change permission - self.permission_service - .edit_permission(EditPermissionOperationInput { - auth_scope: Some(input.change.auth_scope), - users: Some(input.change.users), - user_groups: Some(input.change.user_groups), - resource: Resource::ExternalCanister(ExternalCanisterResourceAction::Change( - ExternalCanisterId::Canister(external_canister.canister_id), - )), - })?; + if let Some(name) = &input.name { + self.check_unique_name(name.as_str(), Some(external_canister.id))?; + } - // calls permissions - let mut updated_calls_resources = Vec::new(); - for call in input.calls { - let call_resource = Resource::ExternalCanister(ExternalCanisterResourceAction::Call( - CallExternalCanisterResourceTarget { - execution_method: ExecutionMethodResourceTarget::ExecutionMethod( - CanisterMethod { - canister_id: external_canister.canister_id, - method_name: call.execution_method, - }, - ), - validation_method: call.validation_method, - }, - )); + external_canister.update_with(input.clone()); + external_canister.validate()?; - self.permission_service - .edit_permission(EditPermissionOperationInput { - auth_scope: Some(call.allow.auth_scope), - users: Some(call.allow.users), - user_groups: Some(call.allow.user_groups), - resource: call_resource.clone(), - })?; + self.external_canister_repository + .insert(external_canister.key(), external_canister.clone()); - updated_calls_resources.push(call_resource); + if let Some(updated_permissions) = input.permissions { + self.configure_external_canister_permissions(&external_canister, updated_permissions)?; } - // removes outdated permissions - self.permission_repository - .find_external_canister_call_permissions(&external_canister.canister_id) - .iter() - .filter(|permission| !updated_calls_resources.contains(&permission.resource)) - .for_each(|permission| { - self.permission_service - .remove_permission(&permission.resource); - }); + if let Some(updated_request_policies) = input.request_policies { + self.configure_external_canister_request_policies( + &external_canister, + updated_request_policies, + )?; + } - Ok(()) + Ok(external_canister) } + // Updates the request policies of the external canister. fn configure_external_canister_request_policies( &self, external_canister: &ExternalCanister, - input: ExternalCanisterRequestPoliciesInput, + input: ExternalCanisterRequestPoliciesUpdateInput, ) -> ServiceResult<()> { - let current_policies: HashSet = self + let current_policies = self .request_policy_repository - .find_external_canister_policies(&external_canister.canister_id) - .into_iter() - .collect(); - - // if the updated list of policies does not contain the current policy, remove it from the system - let policies_to_remove = current_policies - .iter() - .filter(|policy_id| { - !input - .calls - .iter() - .any(|policy| policy.policy_id == Some(**policy_id)) - || !input - .change - .iter() - .any(|policy| policy.policy_id == Some(**policy_id)) - }) - .collect::>(); + .find_external_canister_policies(&external_canister.canister_id); - for policy_id in policies_to_remove { - self.request_policy_service - .remove_request_policy(policy_id)?; - } + // manage the policies for the `Change` requests + if let Some(updated_change_policies) = input.change { + // first remove all existing policies that are not in the updated list + let current_change_policy_ids: HashSet<_> = + current_policies.change.iter().cloned().collect(); + let updated_change_policy_ids: HashSet<_> = updated_change_policies + .iter() + .filter_map(|policy| policy.policy_id) + .collect(); + + for removed_policy_id in + current_change_policy_ids.difference(&updated_change_policy_ids) + { + self.request_policy_service + .remove_request_policy(removed_policy_id)?; + } - // add or update the `Change` policies - for policy in input.change { - match policy.policy_id { - Some(policy_id) => { - if !current_policies.contains(&policy_id) { - print(format!( - "Policy with id {} not found for external canister {}", - Uuid::from_bytes(policy_id).hyphenated(), - external_canister.canister_id.to_text() - )); + // then add or update the `Change` policies + for updated_change_policy in updated_change_policies { + match updated_change_policy.policy_id { + Some(policy_id) => { + // IMPORTANT: makes sure the policy exists and is associated with the target external canister + if !current_change_policy_ids.contains(&policy_id) { + return Err(ExternalCanisterError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(policy_id).hyphenated() + ), + })?; + } - continue; + self.request_policy_service.edit_request_policy( + EditRequestPolicyOperationInput { + policy_id, + rule: Some(updated_change_policy.rule), + specifier: None, + }, + )?; + } + None => { + self.request_policy_service.add_request_policy( + AddRequestPolicyOperationInput { + rule: updated_change_policy.rule, + specifier: RequestSpecifier::ChangeExternalCanister( + ExternalCanisterId::Canister(external_canister.canister_id), + ), + }, + )?; } + } + } + } - self.request_policy_service.edit_request_policy( - EditRequestPolicyOperationInput { - policy_id, - rule: Some(policy.rule), - specifier: None, + // manage the policies for the `Call` requests + if let Some(updated_call_policies) = input.calls { + let current_calls_policy_ids: HashSet<_> = current_policies.calls.iter().collect(); + + match &updated_call_policies { + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(calls) => self + .maybe_mutate_canister_calls_request_policies( + external_canister, + ¤t_policies.calls, + calls, + )?, + ExternalCanisterChangeCallRequestPoliciesInput::OverrideSpecifiedByExecutionMethods( + calls, + ) => { + // first aggregates the calls by the execution method + let calls_by_execution_method = calls.iter().fold( + HashMap::new(), + |mut acc: HashMap< + String, + Vec, + >, + call| { + acc.entry(call.execution_method.clone()) + .or_default() + .push(call.clone()); + acc }, - )?; + ); + + for (method, calls) in calls_by_execution_method { + self.maybe_mutate_canister_calls_request_policies( + external_canister, + &self + .request_policy_repository + .find_external_canister_call_policies_by_execution_method( + &external_canister.canister_id, + &method, + ), + &calls, + )?; + } } - None => { - self.request_policy_service.add_request_policy( - AddRequestPolicyOperationInput { - rule: policy.rule, - specifier: RequestSpecifier::ChangeExternalCanister( - ExternalCanisterId::Canister(external_canister.canister_id), - ), - }, - )?; + ExternalCanisterChangeCallRequestPoliciesInput::RemoveByPolicyIds( + policy_ids_to_remove, + ) => { + for policy_id in policy_ids_to_remove { + // IMPORTANT: makes sure the policy exists and is associated with the target external canister + if !current_calls_policy_ids.contains(policy_id) { + return Err(ExternalCanisterError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(*policy_id).hyphenated() + ), + })?; + } + + self.request_policy_service + .remove_request_policy(policy_id)?; + } } - } + }; } - // add or update the `Call` policies - for policy in input.calls { - match policy.policy_id { - Some(policy_id) => { - if !current_policies.contains(&policy_id) { - print(format!( - "Policy with id {} not found for external canister {}", - Uuid::from_bytes(policy_id).hyphenated(), - external_canister.canister_id.to_text() - )); + Ok(()) + } + + // Given the current and updated call policies, this function will remove the outdated policies and + // add or update the new ones for the external canister. + fn maybe_mutate_canister_calls_request_policies( + &self, + external_canister: &ExternalCanister, + existing_calls_policy_ids: &[UUID], + updated_calls: &[ExternalCanisterCallRequestPolicyRuleInput], + ) -> ServiceResult<()> { + let current_calls_policy_ids: HashSet<_> = + existing_calls_policy_ids.iter().cloned().collect(); + let updated_calls_policy_ids: HashSet<_> = updated_calls + .iter() + .filter_map(|call| call.policy_id) + .collect(); + + for removed_policy_id in current_calls_policy_ids.difference(&updated_calls_policy_ids) { + self.request_policy_service + .remove_request_policy(removed_policy_id)?; + } - continue; + for updated_call_policy in updated_calls { + match &updated_call_policy.policy_id { + Some(policy_id) => { + // IMPORTANT: makes sure the policy exists and is associated with the target external canister + if !current_calls_policy_ids.contains(policy_id) { + Err(ExternalCanisterError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(*policy_id).hyphenated() + ), + })?; } self.request_policy_service.edit_request_policy( EditRequestPolicyOperationInput { - policy_id, - rule: Some(policy.rule), + policy_id: *policy_id, + rule: Some(updated_call_policy.rule.clone()), specifier: None, }, )?; } None => { if let ValidationMethodResourceTarget::ValidationMethod(validation) = - &policy.validation_method + &updated_call_policy.validation_method { if validation.canister_id == external_canister.canister_id - && validation.method_name == policy.execution_method + && validation.method_name == updated_call_policy.execution_method { Err(ExternalCanisterError::ValidationError { - info: format!("The validation method `{}` cannot be the same as the execution method.", policy.execution_method), + info: format!( + "The validation method `{}` cannot be the same as the execution method.", + updated_call_policy.execution_method + ), })?; } } self.request_policy_service.add_request_policy( AddRequestPolicyOperationInput { - rule: policy.rule, + rule: updated_call_policy.rule.clone(), specifier: RequestSpecifier::CallExternalCanister( CallExternalCanisterResourceTarget { execution_method: ExecutionMethodResourceTarget::ExecutionMethod( CanisterMethod { canister_id: external_canister.canister_id, - method_name: policy.execution_method, + method_name: updated_call_policy + .execution_method + .clone(), }, ), - validation_method: policy.validation_method, + validation_method: updated_call_policy + .validation_method + .clone(), }, ), }, @@ -718,32 +807,164 @@ impl ExternalCanisterService { Ok(()) } - /// Edits an external canister's settings. - pub fn edit_external_canister( + /// Updates the permissions of the external canister. + fn configure_external_canister_permissions( &self, - id: &ExternalCanisterEntryId, - input: ConfigureExternalCanisterSettingsInput, - ) -> ServiceResult { - let mut external_canister = self.get_external_canister(id)?; + external_canister: &ExternalCanister, + input: ExternalCanisterPermissionsUpdateInput, + ) -> ServiceResult<()> { + // read permission + if let Some(permission) = input.read { + self.permission_service + .edit_permission(EditPermissionOperationInput { + auth_scope: Some(permission.auth_scope), + users: Some(permission.users), + user_groups: Some(permission.user_groups), + resource: Resource::ExternalCanister(ExternalCanisterResourceAction::Read( + ExternalCanisterId::Canister(external_canister.canister_id), + )), + })?; + } - external_canister.update_with(input.clone()); - external_canister.validate()?; + // change permission (for updating the external canister settings, gives admin access to the external canister) + if let Some(permission) = input.change { + self.permission_service + .edit_permission(EditPermissionOperationInput { + auth_scope: Some(permission.auth_scope), + users: Some(permission.users), + user_groups: Some(permission.user_groups), + resource: Resource::ExternalCanister(ExternalCanisterResourceAction::Change( + ExternalCanisterId::Canister(external_canister.canister_id), + )), + })?; + } - self.external_canister_repository - .insert(external_canister.key(), external_canister.clone()); + // calls permissions (for calling methods of the external canister) + if let Some(calls_permissions) = input.calls { + match calls_permissions { + ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy(calls) => { + self.maybe_mutate_canister_calls_permissions( + external_canister, + &calls, + // always remove all existing call permissions to override them + |_| true, + )?; + } + ExternalCanisterChangeCallPermissionsInput::OverrideSpecifiedByExecutionMethods( + calls, + ) => { + let update_call_methods: HashSet<_> = calls + .iter() + .map(|call| call.execution_method.clone()) + .collect(); + + self.maybe_mutate_canister_calls_permissions( + external_canister, + &calls, + |permission| { + matches!( + &permission.resource, + Resource::ExternalCanister( + ExternalCanisterResourceAction::Call( + CallExternalCanisterResourceTarget { + execution_method: + ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { + canister_id: _, + method_name, + }, + ), + validation_method: _, + }, + ), + ) if update_call_methods.contains(method_name) + ) + }, + )?; + } + ExternalCanisterChangeCallPermissionsInput::RemoveByExecutionMethods(methods) => { + // removes the all call permissions associated with the methods specified of the external canister + let methods: HashSet<_> = methods.iter().cloned().collect(); + self.permission_repository + .find_external_canister_call_permissions(&external_canister.canister_id) + .iter() + .filter(|permission| { + matches!( + &permission.resource, + Resource::ExternalCanister( + ExternalCanisterResourceAction::Call( + CallExternalCanisterResourceTarget { + execution_method: + ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { + canister_id: _, + method_name, + }, + ), + validation_method: _, + }, + ), + ) if methods.contains(method_name) + ) + }) + .for_each(|permission| { + self.permission_service + .remove_permission(&permission.resource); + }); + } + } + } - if let Some(updated_permissions) = input.permissions { - self.configure_external_canister_permissions(&external_canister, updated_permissions)?; + Ok(()) + } + + pub fn maybe_mutate_canister_calls_permissions( + &self, + external_canister: &ExternalCanister, + updated_calls: &[ExternalCanisterCallPermission], + is_affected_permission: F, + ) -> ServiceResult<()> + where + F: Fn(&Permission) -> bool, + { + let current_calls_permissions = self + .permission_repository + .find_external_canister_call_permissions(&external_canister.canister_id); + + // first remove all existing call permissions that are affected by the updated permission set + for permission in current_calls_permissions + .iter() + .filter(|permission| is_affected_permission(permission)) + { + self.permission_service + .remove_permission(&permission.resource); } - if let Some(updated_request_policies) = input.request_policies { - self.configure_external_canister_request_policies( - &external_canister, - updated_request_policies, - )?; + // adds the new call permissions + for updated_call_permission in updated_calls { + let updated_call_permission = updated_call_permission.clone(); + let call_resource = Resource::ExternalCanister(ExternalCanisterResourceAction::Call( + CallExternalCanisterResourceTarget { + execution_method: ExecutionMethodResourceTarget::ExecutionMethod( + CanisterMethod { + canister_id: external_canister.canister_id, + method_name: updated_call_permission.execution_method, + }, + ), + validation_method: updated_call_permission.validation_method, + }, + )); + + self.permission_service + .edit_permission(EditPermissionOperationInput { + auth_scope: Some(updated_call_permission.allow.auth_scope), + users: Some(updated_call_permission.allow.users), + user_groups: Some(updated_call_permission.allow.user_groups), + resource: call_resource, + })?; } - Ok(external_canister) + Ok(()) } /// Adds cycles to the external canister, the cycles are taken from the station's balance. @@ -808,6 +1029,7 @@ impl ExternalCanisterService { // Remove all request policies related to the external canister. self.request_policy_repository .find_external_canister_policies(&external_canister.canister_id) + .all() .iter() .for_each(|policy_id| { if let Err(err) = self.request_policy_service.remove_request_policy(policy_id) { @@ -940,20 +1162,21 @@ impl ExternalCanisterService { #[cfg(test)] mod tests { - use orbit_essentials::api::ApiError; - use super::*; use crate::{ core::test_utils, + errors::ExternalCanisterValidationError, models::{ permission::{Allow, AuthScope}, resource::ValidationMethodResourceTarget, CreateExternalCanisterOperationKindAddExisting, ExternalCanisterCallPermission, - ExternalCanisterCallRequestPolicyRuleInput, - ExternalCanisterChangeRequestPolicyRuleInput, ExternalCanisterPermissionsInput, - ExternalCanisterRequestPoliciesInput, RequestPolicyRule, + ExternalCanisterCallRequestPolicyRuleInput, ExternalCanisterChangeCallPermissionsInput, + ExternalCanisterChangeCallRequestPoliciesInput, + ExternalCanisterChangeRequestPolicyRuleInput, ExternalCanisterPermissionsCreateInput, + ExternalCanisterRequestPoliciesCreateInput, RequestPolicyRule, }, }; + use orbit_essentials::api::ApiError; fn setup() { test_utils::init_canister_system(); @@ -967,7 +1190,7 @@ mod tests { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: vec![ @@ -988,7 +1211,7 @@ mod tests { }, ], }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: vec![ExternalCanisterChangeRequestPolicyRuleInput { policy_id: None, rule: RequestPolicyRule::AutoApproved, @@ -1051,7 +1274,7 @@ mod tests { assert_eq!(request_policies.len(), 2); - for policy in request_policies { + for policy in request_policies.all() { let policy = REQUEST_POLICY_REPOSITORY.get(&policy).unwrap(); assert_eq!(policy.rule, RequestPolicyRule::AutoApproved); @@ -1059,7 +1282,7 @@ mod tests { } #[tokio::test] - async fn add_external_canister_with_non_compatible_policy_is_ignored() { + async fn add_external_canister_with_non_compatible_policy_fails_validation() { setup(); let incompatible_policy = REQUEST_POLICY_SERVICE .add_request_policy(AddRequestPolicyOperationInput { @@ -1070,21 +1293,117 @@ mod tests { }) .unwrap(); + let mut create_input = CreateExternalCanisterOperationInput { + name: "test".to_string(), + description: None, + labels: None, + permissions: ExternalCanisterPermissionsCreateInput { + read: Allow::authenticated(), + change: Allow::authenticated(), + calls: Vec::new(), + }, + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: vec![ExternalCanisterChangeRequestPolicyRuleInput { + policy_id: Some(incompatible_policy.id), + rule: RequestPolicyRule::AutoApproved, + }], + calls: Vec::new(), + }, + kind: CreateExternalCanisterOperationKind::AddExisting( + CreateExternalCanisterOperationKindAddExisting { + canister_id: Principal::from_slice(&[10; 29]), + }, + ), + }; + + let result = EXTERNAL_CANISTER_SERVICE + .add_external_canister(create_input.clone()) + .await; + + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ApiError::from(ExternalCanisterValidationError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(incompatible_policy.id).hyphenated() + ) + }) + ); + + let incompatible_policy = REQUEST_POLICY_SERVICE + .add_request_policy(AddRequestPolicyOperationInput { + rule: RequestPolicyRule::AutoApproved, + specifier: RequestSpecifier::AddAccount, + }) + .unwrap(); + + create_input.request_policies.change = vec![ExternalCanisterChangeRequestPolicyRuleInput { + policy_id: Some(incompatible_policy.id), + rule: RequestPolicyRule::AutoApproved, + }]; + + let result = EXTERNAL_CANISTER_SERVICE + .add_external_canister(create_input.clone()) + .await; + + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ApiError::from(ExternalCanisterValidationError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(incompatible_policy.id).hyphenated() + ) + }) + ); + + create_input.request_policies.change = vec![]; + create_input.request_policies.calls = vec![ExternalCanisterCallRequestPolicyRuleInput { + policy_id: Some(incompatible_policy.id), + execution_method: "test".to_string(), + validation_method: ValidationMethodResourceTarget::No, + rule: RequestPolicyRule::AutoApproved, + }]; + + let result = EXTERNAL_CANISTER_SERVICE + .add_external_canister(create_input.clone()) + .await; + + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ApiError::from(ExternalCanisterValidationError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(incompatible_policy.id).hyphenated() + ) + }) + ); + } + + #[tokio::test] + async fn edit_external_canister_with_non_compatible_policy_fails_validation() { + setup(); + let incompatible_policy = REQUEST_POLICY_SERVICE + .add_request_policy(AddRequestPolicyOperationInput { + rule: RequestPolicyRule::AutoApproved, + specifier: RequestSpecifier::AddAccount, + }) + .unwrap(); + let external_canister = EXTERNAL_CANISTER_SERVICE .add_external_canister(CreateExternalCanisterOperationInput { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: Vec::new(), }, - request_policies: ExternalCanisterRequestPoliciesInput { - change: vec![ExternalCanisterChangeRequestPolicyRuleInput { - policy_id: Some(incompatible_policy.id), - rule: RequestPolicyRule::AutoApproved, - }], + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: Vec::new(), calls: Vec::new(), }, kind: CreateExternalCanisterOperationKind::AddExisting( @@ -1096,20 +1415,60 @@ mod tests { .await .unwrap(); - let request_policies = REQUEST_POLICY_REPOSITORY - .find_external_canister_policies(&external_canister.canister_id); + let result = EXTERNAL_CANISTER_SERVICE.edit_external_canister( + &external_canister.id, + ConfigureExternalCanisterSettingsInput { + request_policies: Some(ExternalCanisterRequestPoliciesUpdateInput { + change: Some(vec![ExternalCanisterChangeRequestPolicyRuleInput { + policy_id: Some(incompatible_policy.id), + rule: RequestPolicyRule::AutoApproved, + }]), + calls: None, + }), + ..Default::default() + }, + ); - assert!(request_policies.is_empty()); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ApiError::from(ExternalCanisterValidationError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(incompatible_policy.id).hyphenated() + ) + }) + ); - let policy = REQUEST_POLICY_REPOSITORY - .get(&incompatible_policy.id) - .unwrap(); + let result = EXTERNAL_CANISTER_SERVICE.edit_external_canister( + &external_canister.id, + ConfigureExternalCanisterSettingsInput { + request_policies: Some(ExternalCanisterRequestPoliciesUpdateInput { + change: None, + calls: Some( + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy(vec![ + ExternalCanisterCallRequestPolicyRuleInput { + policy_id: Some(incompatible_policy.id), + execution_method: "test".to_string(), + validation_method: ValidationMethodResourceTarget::No, + rule: RequestPolicyRule::AutoApproved, + }, + ]), + ), + }), + ..Default::default() + }, + ); + assert!(result.is_err()); assert_eq!( - policy.specifier, - RequestSpecifier::ChangeExternalCanister(ExternalCanisterId::Canister( - Principal::from_slice(&[1; 29]) - ),) + result.unwrap_err(), + ApiError::from(ExternalCanisterValidationError::ValidationError { + info: format!( + "The policy with id {} does not exist for the external canister.", + Uuid::from_bytes(incompatible_policy.id).hyphenated() + ) + }) ); } @@ -1121,12 +1480,12 @@ mod tests { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: Vec::new(), }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: Vec::new(), calls: vec![ExternalCanisterCallRequestPolicyRuleInput { policy_id: None, @@ -1167,12 +1526,12 @@ mod tests { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: Vec::new(), }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: Vec::new(), calls: Vec::new(), }, @@ -1201,12 +1560,12 @@ mod tests { name: format!("test{}", i), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: Vec::new(), }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: Vec::new(), calls: Vec::new(), }, @@ -1234,12 +1593,12 @@ mod tests { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: Vec::new(), }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: Vec::new(), calls: Vec::new(), }, @@ -1268,7 +1627,7 @@ mod tests { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: vec![ExternalCanisterCallPermission { @@ -1277,7 +1636,7 @@ mod tests { validation_method: ValidationMethodResourceTarget::No, }], }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: Vec::new(), calls: Vec::new(), }, @@ -1290,26 +1649,33 @@ mod tests { .await .unwrap(); - let updated_canister = EXTERNAL_CANISTER_SERVICE - .edit_external_canister( - &canister.id, - ConfigureExternalCanisterSettingsInput { - name: Some("test2".to_string()), - description: None, - labels: None, - state: None, - permissions: Some(ExternalCanisterPermissionsInput { - read: Allow::authenticated(), - change: Allow::authenticated(), - calls: Vec::new(), - }), - request_policies: Some(ExternalCanisterRequestPoliciesInput { - change: Vec::new(), - calls: Vec::new(), - }), - }, - ) - .unwrap(); + let updated_canister = + EXTERNAL_CANISTER_SERVICE + .edit_external_canister( + &canister.id, + ConfigureExternalCanisterSettingsInput { + name: Some("test2".to_string()), + description: None, + labels: None, + state: None, + permissions: Some(ExternalCanisterPermissionsUpdateInput { + read: Some(Allow::authenticated()), + change: Some(Allow::authenticated()), + calls: Some(ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy( + Vec::new(), + )), + }), + request_policies: Some(ExternalCanisterRequestPoliciesUpdateInput { + change: Some(Vec::new()), + calls: Some( + ExternalCanisterChangeCallRequestPoliciesInput::ReplaceAllBy( + Vec::new(), + ), + ), + }), + }, + ) + .unwrap(); assert_eq!(updated_canister.name, "test2"); @@ -1328,7 +1694,7 @@ mod tests { name: format!("test{}", i), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: vec![ @@ -1349,7 +1715,7 @@ mod tests { }, ], }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: Vec::new(), calls: Vec::new(), }, @@ -1380,12 +1746,12 @@ mod tests { name: format!("test{}", i), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { read: Allow::authenticated(), change: Allow::authenticated(), calls: Vec::new(), }, - request_policies: ExternalCanisterRequestPoliciesInput { + request_policies: ExternalCanisterRequestPoliciesCreateInput { change: vec![ ExternalCanisterChangeRequestPolicyRuleInput { policy_id: None, @@ -1419,6 +1785,132 @@ mod tests { assert_eq!(policies.calls.len(), 1); assert_eq!(policies.change.len(), 2); } + + #[tokio::test] + async fn external_canister_name_is_unique() { + setup(); + let _ = EXTERNAL_CANISTER_SERVICE + .add_external_canister(CreateExternalCanisterOperationInput { + name: "test".to_string(), + description: None, + labels: None, + permissions: ExternalCanisterPermissionsCreateInput { + read: Allow::authenticated(), + change: Allow::authenticated(), + calls: Vec::new(), + }, + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: Vec::new(), + calls: Vec::new(), + }, + kind: CreateExternalCanisterOperationKind::AddExisting( + CreateExternalCanisterOperationKindAddExisting { + canister_id: Principal::from_slice(&[10; 29]), + }, + ), + }) + .await + .unwrap(); + + let result = EXTERNAL_CANISTER_SERVICE + .add_external_canister(CreateExternalCanisterOperationInput { + name: "test".to_string(), + description: None, + labels: None, + permissions: ExternalCanisterPermissionsCreateInput { + read: Allow::authenticated(), + change: Allow::authenticated(), + calls: Vec::new(), + }, + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: Vec::new(), + calls: Vec::new(), + }, + kind: CreateExternalCanisterOperationKind::AddExisting( + CreateExternalCanisterOperationKindAddExisting { + canister_id: Principal::from_slice(&[11; 29]), + }, + ), + }) + .await; + + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ApiError::from(ExternalCanisterError::ValidationError { + info: "The name 'test' is already in use.".to_string() + }) + ); + } + + #[tokio::test] + async fn edits_to_external_canister_name_still_checks_uniqueness() { + setup(); + + let mut external_canisters = Vec::new(); + for i in 0..2 { + external_canisters.push( + EXTERNAL_CANISTER_SERVICE + .add_external_canister(CreateExternalCanisterOperationInput { + name: format!("test{}", i), + description: None, + labels: None, + permissions: ExternalCanisterPermissionsCreateInput { + read: Allow::authenticated(), + change: Allow::authenticated(), + calls: Vec::new(), + }, + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: Vec::new(), + calls: Vec::new(), + }, + kind: CreateExternalCanisterOperationKind::AddExisting( + CreateExternalCanisterOperationKindAddExisting { + canister_id: Principal::from_slice(&[i; 29]), + }, + ), + }) + .await + .unwrap(), + ); + } + + let edit_same_is_ok = EXTERNAL_CANISTER_SERVICE + .edit_external_canister( + &external_canisters[0].id, + ConfigureExternalCanisterSettingsInput { + name: Some(external_canisters[0].name.to_string()), + description: None, + labels: None, + state: None, + permissions: None, + request_policies: None, + }, + ) + .unwrap(); + + assert_eq!(edit_same_is_ok.name, external_canisters[0].name); + + let edit_non_unique_name_fails = EXTERNAL_CANISTER_SERVICE.edit_external_canister( + &external_canisters[0].id, + ConfigureExternalCanisterSettingsInput { + name: Some("test1".to_string()), + description: None, + labels: None, + state: None, + permissions: None, + request_policies: None, + }, + ); + + assert!(edit_non_unique_name_fails.is_err()); + assert_eq!( + edit_non_unique_name_fails.unwrap_err(), + ApiError::from(ExternalCanisterError::ValidationError { + info: "The name 'test1' is already in use.".to_string() + }) + ); + } } #[cfg(feature = "canbench")] @@ -1505,7 +1997,7 @@ mod external_canister_test_utils { use super::*; use crate::models::{ external_canister_test_utils::mock_external_canister, permission::Allow, - ExternalCanisterState, + ExternalCanisterChangeCallPermissionsInput, ExternalCanisterState, }; pub fn add_test_external_canisters( @@ -1533,10 +2025,12 @@ mod external_canister_test_utils { EXTERNAL_CANISTER_REPOSITORY.insert(external_canister.key(), external_canister.clone()); let mut input = ConfigureExternalCanisterSettingsInput::default(); - input.permissions = Some(ExternalCanisterPermissionsInput { - calls, - read: allow.clone(), - change: allow.clone(), + input.permissions = Some(ExternalCanisterPermissionsUpdateInput { + calls: Some(ExternalCanisterChangeCallPermissionsInput::ReplaceAllBy( + calls, + )), + read: Some(allow.clone()), + change: Some(allow.clone()), }); EXTERNAL_CANISTER_SERVICE diff --git a/libs/orbit-essentials/src/model.rs b/libs/orbit-essentials/src/model.rs index 1f0b2b41e..4a8fbdeb9 100644 --- a/libs/orbit-essentials/src/model.rs +++ b/libs/orbit-essentials/src/model.rs @@ -8,6 +8,18 @@ pub trait ModelValidator { fn validate(&self) -> ModelValidatorResult; } +#[derive(Debug, Clone)] +pub struct ContextualModel { + pub model: M, + pub context: C, +} + +impl ContextualModel { + pub fn new(model: M, context: C) -> Self { + Self { model, context } + } +} + /// A trait for models to expose their key. pub trait ModelKey { fn key(&self) -> Key; diff --git a/tests/integration/src/external_canister_tests.rs b/tests/integration/src/external_canister_tests.rs index 49706a16b..2cca29f5a 100644 --- a/tests/integration/src/external_canister_tests.rs +++ b/tests/integration/src/external_canister_tests.rs @@ -17,8 +17,8 @@ use station_api::{ ChangeExternalCanisterOperationInput, CreateExternalCanisterOperationInput, CreateExternalCanisterOperationKindCreateNewDTO, CreateExternalCanisterOperationKindDTO, EditPermissionOperationInput, ExecutionMethodResourceTargetDTO, ExternalCanisterIdDTO, - ExternalCanisterPermissionsInput, ExternalCanisterRequestPoliciesInput, HealthStatus, - ListRequestsInput, ListRequestsOperationTypeDTO, ListRequestsResponse, QuorumDTO, + ExternalCanisterPermissionsCreateInput, ExternalCanisterRequestPoliciesCreateInput, + HealthStatus, ListRequestsInput, ListRequestsOperationTypeDTO, ListRequestsResponse, QuorumDTO, RequestApprovalStatusDTO, RequestOperationDTO, RequestOperationInput, RequestPolicyRuleDTO, RequestSpecifierDTO, RequestStatusDTO, UserSpecifierDTO, ValidationMethodResourceTargetDTO, }; @@ -379,7 +379,7 @@ fn create_external_canister_and_check_status() { name: "test".to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { calls: vec![], read: AllowDTO { auth_scope: station_api::AuthScopeDTO::Restricted, @@ -392,8 +392,8 @@ fn create_external_canister_and_check_status() { users: vec![], }, }, - request_policies: ExternalCanisterRequestPoliciesInput { - change: Vec::new(), + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: vec![], calls: vec![], }, }); @@ -1152,7 +1152,7 @@ fn create_external_canister_with_too_many_cycles() { name: name.to_string(), description: None, labels: None, - permissions: ExternalCanisterPermissionsInput { + permissions: ExternalCanisterPermissionsCreateInput { calls: vec![], read: AllowDTO { auth_scope: station_api::AuthScopeDTO::Restricted, @@ -1165,8 +1165,8 @@ fn create_external_canister_with_too_many_cycles() { users: vec![], }, }, - request_policies: ExternalCanisterRequestPoliciesInput { - change: Vec::new(), + request_policies: ExternalCanisterRequestPoliciesCreateInput { + change: vec![], calls: vec![], }, }) diff --git a/tests/integration/src/test_data/system_upgrade.rs b/tests/integration/src/test_data/system_upgrade.rs index b0a70187b..2c00093ab 100644 --- a/tests/integration/src/test_data/system_upgrade.rs +++ b/tests/integration/src/test_data/system_upgrade.rs @@ -54,7 +54,7 @@ pub fn perform_station_update( ); // wait with extra ticks since the canister is stopped by the upgrade process - for _ in 0..10 { + for _ in 0..20 { env.tick(); env.advance_time(Duration::from_secs(1)); }