Skip to content

Commit 10dae19

Browse files
feat(backend): create controller guard for old update calls (#5028)
# Motivation Some `update` calls in the `backend` do not necessary require `allowed_users` to access them. Mainly they are related to migration. So, we allow just the controllers to access them, until the time comes that we totally deprecate them. # Changes - Create guard caller_is_controller just for controllers. - Use new guard in `update` calls: - `top_up_cycles_ledger` - `set_guards` - `bulk_up` - `migrate_user_data_to` - `migration_stop_timer` - `step_migration` # Tests Current tests are sufficient. --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 48851dc commit 10dae19

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

src/backend/src/guards.rs

+9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ pub fn caller_is_not_anonymous() -> Result<(), String> {
1111
}
1212
}
1313

14+
pub fn caller_is_controller() -> Result<(), String> {
15+
let caller = caller();
16+
if is_controller(&caller) {
17+
Ok(())
18+
} else {
19+
Err("Caller is not a controller.".to_string())
20+
}
21+
}
22+
1423
pub fn caller_is_allowed() -> Result<(), String> {
1524
let caller = caller();
1625
if read_config(|s| s.allowed_callers.contains(&caller)) || is_controller(&caller) {

src/backend/src/lib.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use user_profile_model::UserProfileModel;
5151

5252
use crate::{
5353
assertions::{assert_token_enabled_is_some, assert_token_symbol_length},
54-
guards::{caller_is_allowed, may_read_user_data, may_write_user_data},
54+
guards::{caller_is_allowed, caller_is_controller, may_read_user_data, may_write_user_data},
5555
oisy_user::oisy_user_creation_timestamps,
5656
token::{add_to_user_token, remove_from_user_token},
5757
user_profile::add_hidden_dapp_id,
@@ -216,14 +216,14 @@ pub fn post_upgrade(arg: Option<Arg>) {
216216
#[query(guard = "caller_is_allowed")]
217217
#[must_use]
218218
pub fn config() -> Config {
219-
read_config(std::clone::Clone::clone)
219+
read_config(Clone::clone)
220220
}
221221

222222
/// Adds cycles to the cycles ledger, if it is below a certain threshold.
223223
///
224224
/// # Errors
225225
/// Error conditions are enumerated by: `TopUpCyclesLedgerError`
226-
#[update(guard = "caller_is_allowed")]
226+
#[update(guard = "caller_is_controller")]
227227
pub async fn top_up_cycles_ledger(
228228
request: Option<TopUpCyclesLedgerRequest>,
229229
) -> TopUpCyclesLedgerResult {
@@ -656,7 +656,7 @@ pub fn migration() -> Option<MigrationReport> {
656656

657657
/// Sets the lock state of the canister APIs. This can be used to enable or disable the APIs, or to
658658
/// enable an API in read-only mode.
659-
#[update(guard = "caller_is_allowed")]
659+
#[update(guard = "caller_is_controller")]
660660
pub fn set_guards(guards: Guards) {
661661
mutate_state(|state| modify_state_config(state, |config| config.api = Some(guards)));
662662
}
@@ -675,7 +675,7 @@ pub fn stats() -> Stats {
675675
///
676676
/// Note: In case of conflict, existing data is overwritten. This situation is expected to occur
677677
/// only if a migration failed and had to be restarted.
678-
#[update(guard = "caller_is_allowed")]
678+
#[update(guard = "caller_is_controller")]
679679
#[allow(clippy::needless_pass_by_value)]
680680
pub fn bulk_up(data: Vec<u8>) {
681681
migrate::bulk_up(&data);
@@ -685,7 +685,7 @@ pub fn bulk_up(data: Vec<u8>) {
685685
///
686686
/// # Errors
687687
/// - There is a current migration in progress to a different canister.
688-
#[update(guard = "caller_is_allowed")]
688+
#[update(guard = "caller_is_controller")]
689689
pub fn migrate_user_data_to(to: Principal) -> Result<MigrationReport, String> {
690690
mutate_state(|s| {
691691
if let Some(migration) = &s.migration {
@@ -713,7 +713,7 @@ pub fn migrate_user_data_to(to: Principal) -> Result<MigrationReport, String> {
713713
///
714714
/// # Errors
715715
/// - There is no migration in progress.
716-
#[update(guard = "caller_is_allowed")]
716+
#[update(guard = "caller_is_controller")]
717717
pub fn migration_stop_timer() -> Result<(), String> {
718718
mutate_state(|s| {
719719
if let Some(migration) = &s.migration {
@@ -728,7 +728,7 @@ pub fn migration_stop_timer() -> Result<(), String> {
728728
/// Steps the migration.
729729
///
730730
/// On error, the migration is marked as failed and the timer is cleared.
731-
#[update(guard = "caller_is_allowed")]
731+
#[update(guard = "caller_is_controller")]
732732
pub async fn step_migration() {
733733
let result = migrate::step_migration().await;
734734
eprintln!("Stepped migration: {:?}", result);

src/backend/tests/it/signer.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ use crate::utils::{
1212
};
1313

1414
#[test]
15-
fn test_topup_cannot_be_called_if_not_allowed() {
15+
fn test_topup_cannot_be_called_if_not_controller() {
1616
let pic_setup = setup();
1717
// A random unauthorized user.
1818
let caller = Principal::from_text(VC_HOLDER).unwrap();
1919

2020
let response = pic_setup.update::<TopUpCyclesLedgerResult>(caller, "top_up_cycles_ledger", ());
2121

22-
assert_eq!(response, Err("Caller is not allowed.".to_string()));
22+
assert_eq!(response, Err("Caller is not a controller.".to_string()));
2323
}
2424

2525
#[test]

0 commit comments

Comments
 (0)