Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: controllers cannot be anonymous #306

Merged
merged 2 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/mission_control/src/controllers/mission_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use crate::controllers::store::{delete_controllers, get_admin_controllers, set_c
use crate::store::get_user;
use ic_cdk::id;
use shared::constants::MAX_NUMBER_OF_MISSION_CONTROL_CONTROLLERS;
use shared::controllers::{assert_max_number_of_controllers, into_controller_ids};
use shared::controllers::{
assert_max_number_of_controllers, assert_no_anonymous_controller, into_controller_ids,
};
use shared::ic::update_canister_controllers;
use shared::types::interface::SetController;
use shared::types::state::{ControllerId, ControllerScope, Controllers};
Expand All @@ -22,6 +24,8 @@ pub async fn set_mission_control_controllers(
}
}

assert_no_anonymous_controller(controllers)?;

set_controllers(controllers, controller);

// We update the IC controllers because it is possible that an existing controller was updated.
Expand Down
6 changes: 5 additions & 1 deletion src/orbiter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ use ic_stable_structures::writer::Writer;
#[allow(unused)]
use ic_stable_structures::Memory as _;
use shared::constants::MAX_NUMBER_OF_SATELLITE_CONTROLLERS;
use shared::controllers::{assert_max_number_of_controllers, init_controllers};
use shared::controllers::{
assert_max_number_of_controllers, assert_no_anonymous_controller, init_controllers,
};
use shared::types::interface::{DeleteControllersArgs, SegmentArgs, SetControllersArgs};
use shared::types::state::{ControllerScope, Controllers, SatelliteId};
use std::mem;
Expand Down Expand Up @@ -219,6 +221,8 @@ fn set_controllers(
}
}

assert_no_anonymous_controller(&controllers).unwrap_or_else(|e| trap(&e));

set_controllers_store(&controllers, &controller);
get_controllers()
}
Expand Down
6 changes: 5 additions & 1 deletion src/satellite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ use ic_stable_structures::writer::Writer;
#[allow(unused)]
use ic_stable_structures::Memory as _;
use shared::constants::MAX_NUMBER_OF_SATELLITE_CONTROLLERS;
use shared::controllers::{assert_max_number_of_controllers, init_controllers};
use shared::controllers::{
assert_max_number_of_controllers, assert_no_anonymous_controller, init_controllers,
};
use shared::types::interface::{DeleteControllersArgs, SegmentArgs, SetControllersArgs};
use shared::types::state::{ControllerScope, Controllers};
use std::mem;
Expand Down Expand Up @@ -233,6 +235,8 @@ fn set_controllers(
}
}

assert_no_anonymous_controller(&controllers).unwrap_or_else(|e| trap(&e));

set_controllers_store(&controllers, &controller);
get_controllers()
}
Expand Down
12 changes: 3 additions & 9 deletions src/satellite/src/rules/assert_stores.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ pub fn assert_permission(
match permission {
Permission::Public => true,
Permission::Private => assert_caller(caller, owner),
Permission::Managed => {
assert_caller(caller, owner) || assert_controller(caller, controllers)
}
Permission::Controllers => assert_controller(caller, controllers),
Permission::Managed => assert_caller(caller, owner) || is_controller(caller, controllers),
Permission::Controllers => is_controller(caller, controllers),
}
}

Expand All @@ -33,18 +31,14 @@ pub fn assert_create_permission(
Permission::Public => true,
Permission::Private => assert_not_anonymous(caller),
Permission::Managed => assert_not_anonymous(caller),
Permission::Controllers => assert_controller(caller, controllers),
Permission::Controllers => is_controller(caller, controllers),
}
}

fn assert_caller(caller: Principal, owner: Principal) -> bool {
assert_not_anonymous(caller) && principal_equal(owner, caller)
}

fn assert_controller(caller: Principal, controllers: &Controllers) -> bool {
assert_not_anonymous(caller) && is_controller(caller, controllers)
}

fn assert_not_anonymous(caller: Principal) -> bool {
principal_not_anonymous(caller)
}
Expand Down
33 changes: 23 additions & 10 deletions src/shared/src/controllers.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::env::{CONSOLE, OBSERVATORY};
use crate::types::interface::SetController;
use crate::types::state::{Controller, ControllerId, ControllerScope, Controllers, UserId};
use crate::utils::principal_equal;
use crate::utils::{principal_anonymous, principal_equal, principal_not_anonymous};
use candid::Principal;
use ic_cdk::api::time;
use std::collections::HashMap;
Expand Down Expand Up @@ -56,18 +56,20 @@ pub fn delete_controllers(remove_controllers: &[UserId], controllers: &mut Contr
}

pub fn is_controller(caller: UserId, controllers: &Controllers) -> bool {
controllers
.iter()
.any(|(&controller_id, _)| principal_equal(controller_id, caller))
principal_not_anonymous(caller)
&& controllers
.iter()
.any(|(&controller_id, _)| principal_equal(controller_id, caller))
}

pub fn is_admin_controller(caller: UserId, controllers: &Controllers) -> bool {
controllers
.iter()
.any(|(&controller_id, controller)| match controller.scope {
ControllerScope::Write => false,
ControllerScope::Admin => principal_equal(controller_id, caller),
})
principal_not_anonymous(caller)
&& controllers
.iter()
.any(|(&controller_id, controller)| match controller.scope {
ControllerScope::Write => false,
ControllerScope::Admin => principal_equal(controller_id, caller),
})
}

pub fn into_controller_ids(controllers: &Controllers) -> Vec<ControllerId> {
Expand Down Expand Up @@ -100,6 +102,17 @@ pub fn assert_max_number_of_controllers(
Ok(())
}

pub fn assert_no_anonymous_controller(controllers_ids: &[ControllerId]) -> Result<(), String> {
let has_anonymous = controllers_ids
.iter()
.any(|controller_id| principal_anonymous(controller_id.clone()));

match has_anonymous {
true => Err("Anonymous controller not allowed.".to_string()),
false => Ok(()),
}
}

pub fn caller_is_console(caller: UserId) -> bool {
let console = Principal::from_text(CONSOLE).unwrap();

Expand Down
4 changes: 4 additions & 0 deletions src/shared/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ pub fn principal_not_anonymous(p: Principal) -> bool {
principal_not_equal(p, Principal::anonymous())
}

pub fn principal_anonymous(p: Principal) -> bool {
principal_equal(p, Principal::anonymous())
}

pub fn account_identifier_equal(x: AccountIdentifier, y: AccountIdentifier) -> bool {
x == y
}