Skip to content

Commit

Permalink
refactor: rules assertion and utils names (#304)
Browse files Browse the repository at this point in the history
Signed-off-by: David Dal Busco <[email protected]>
  • Loading branch information
peterpeterparker authored Nov 12, 2023
1 parent 82086e3 commit b645468
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 24 deletions.
10 changes: 5 additions & 5 deletions src/satellite/src/db/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::db::types::state::Doc;
use crate::db::utils::filter_values;
use crate::list::utils::list_values;
use crate::msg::{COLLECTION_NOT_EMPTY, ERROR_CANNOT_WRITE};
use crate::rules::assert_stores::{assert_create_permission, assert_permission, public_permission};
use crate::rules::types::rules::{Memory, Permission, Rule};
use crate::rules::utils::{assert_create_rule, assert_rule, public_rule};
use crate::types::core::{CollectionKey, Key};
use crate::types::list::{ListParams, ListResults};
use candid::Principal;
Expand Down Expand Up @@ -84,7 +84,7 @@ fn get_doc_impl(
match value {
None => Ok(None),
Some(value) => {
if !assert_rule(&rule.read, value.owner, caller, controllers) {
if !assert_permission(&rule.read, value.owner, caller, controllers) {
return Ok(None);
}

Expand Down Expand Up @@ -258,15 +258,15 @@ fn assert_write_permission(
user_timestamp: Option<u64>,
) -> Result<(), String> {
// For existing collection and document, check user editing is the caller
if !public_rule(rule) {
if !public_permission(rule) {
match current_doc {
None => {
if !assert_create_rule(rule, caller, controllers) {
if !assert_create_permission(rule, caller, controllers) {
return Err(ERROR_CANNOT_WRITE.to_string());
}
}
Some(current_doc) => {
if !assert_rule(rule, current_doc.owner, caller, controllers) {
if !assert_permission(rule, current_doc.owner, caller, controllers) {
return Err(ERROR_CANNOT_WRITE.to_string());
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/satellite/src/db/utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::db::types::state::Doc;
use crate::list::utils::matcher_regex;
use crate::rules::assert_stores::assert_permission;
use crate::rules::types::rules::Permission;
use crate::rules::utils::assert_rule;
use crate::types::core::Key;
use crate::types::list::ListParams;
use candid::Principal;
Expand All @@ -27,7 +27,7 @@ pub fn filter_values(
filter_key_matcher(&regex_key, key)
&& filter_description_matcher(&regex_description, &doc.description)
&& filter_owner(owner, &doc.owner)
&& assert_rule(rule, doc.owner, caller, controllers)
&& assert_permission(rule, doc.owner, caller, controllers)
})
.map(|(key, doc)| (key.clone(), doc.clone()))
.collect()
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ use shared::controllers::is_controller;
use shared::types::state::Controllers;
use shared::utils::principal_equal;

pub fn assert_rule(
rule: &Permission,
pub fn assert_permission(
permission: &Permission,
owner: Principal,
caller: Principal,
controllers: &Controllers,
) -> bool {
match rule {
match permission {
Permission::Public => true,
Permission::Private => principal_equal(owner, caller),
Permission::Managed => principal_equal(owner, caller) || is_controller(caller, controllers),
Expand All @@ -22,17 +22,21 @@ pub fn assert_rule(

/// If a document or asset is about to be created for the first time, it can be initialized without further rules unless the collection is set as controller and the caller is not a controller.
/// This can be useful e.g. when a collection read permission is set to public but only the administrator can add content.
pub fn assert_create_rule(rule: &Permission, caller: Principal, controllers: &Controllers) -> bool {
match rule {
pub fn assert_create_permission(
permission: &Permission,
caller: Principal,
controllers: &Controllers,
) -> bool {
match permission {
Permission::Public => true,
Permission::Private => true,
Permission::Managed => true,
Permission::Controllers => is_controller(caller, controllers),
}
}

pub fn public_rule(rule: &Permission) -> bool {
matches!(rule, Permission::Public)
pub fn public_permission(permission: &Permission) -> bool {
matches!(permission, Permission::Public)
}

pub fn is_known_user(caller: Principal) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions src/satellite/src/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod assert;
mod assert_rules;
pub mod assert_stores;
pub mod constants;
pub mod impls;
pub mod store;
pub mod types;
pub mod utils;
4 changes: 3 additions & 1 deletion src/satellite/src/rules/store.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::db::store::{delete_collection, init_collection};
use crate::memory::STATE;
use crate::rules::assert::{assert_memory, assert_mutable_permissions, assert_write_permission};
use crate::rules::assert_rules::{
assert_memory, assert_mutable_permissions, assert_write_permission,
};
use crate::rules::constants::SYS_COLLECTION_PREFIX;
use crate::rules::types::interface::{DelRule, SetRule};
use crate::rules::types::rules::{Memory, Rule, Rules};
Expand Down
15 changes: 10 additions & 5 deletions src/satellite/src/storage/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ use shared::types::state::Controllers;
use shared::utils::principal_not_equal;
use std::collections::HashMap;

use crate::rules::assert_stores::{
assert_create_permission, assert_permission, is_known_user, public_permission,
};
use crate::rules::types::rules::{Memory, Rule};
use crate::rules::utils::{assert_create_rule, assert_rule, is_known_user, public_rule};
use crate::storage::constants::{
ASSET_ENCODING_NO_COMPRESSION, BN_WELL_KNOWN_CUSTOM_DOMAINS, ENCODING_CERTIFICATION_ORDER,
ROOT_404_HTML, ROOT_INDEX_HTML,
Expand Down Expand Up @@ -191,7 +193,7 @@ fn delete_asset_impl(
match asset {
None => Err(ERROR_ASSET_NOT_FOUND.to_string()),
Some(asset) => {
if !assert_rule(&rule.write, asset.key.owner, caller, controllers) {
if !assert_permission(&rule.write, asset.key.owner, caller, controllers) {
return Err(ERROR_ASSET_NOT_FOUND.to_string());
}

Expand Down Expand Up @@ -267,7 +269,10 @@ fn secure_create_batch_impl(
) -> Result<u128, String> {
let rule = get_state_rule(&init.collection)?;

if !(public_rule(&rule.write) || is_known_user(caller) || is_controller(caller, controllers)) {
if !(public_permission(&rule.write)
|| is_known_user(caller)
|| is_controller(caller, controllers))
{
return Err(UPLOAD_NOT_ALLOWED.to_string());
}

Expand Down Expand Up @@ -440,7 +445,7 @@ fn secure_commit_chunks(

match current {
None => {
if !assert_create_rule(&rule.write, caller, controllers) {
if !assert_create_permission(&rule.write, caller, controllers) {
return Err(ERROR_CANNOT_COMMIT_BATCH.to_string());
}

Expand All @@ -465,7 +470,7 @@ fn secure_commit_chunks_update(
return Err("Provided collection does not match existing collection.".to_string());
}

if !assert_rule(&rule.write, current.key.owner, caller, controllers) {
if !assert_permission(&rule.write, current.key.owner, caller, controllers) {
return Err(ERROR_CANNOT_COMMIT_BATCH.to_string());
}

Expand Down
4 changes: 2 additions & 2 deletions src/satellite/src/storage/utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::list::utils::matcher_regex;
use crate::rules::assert_stores::assert_permission;
use crate::rules::types::rules::Permission;
use crate::rules::utils::assert_rule;
use crate::storage::types::interface::{AssetEncodingNoContent, AssetNoContent};
use crate::storage::types::state::FullPath;
use crate::storage::types::store::Asset;
Expand Down Expand Up @@ -61,7 +61,7 @@ pub fn filter_values(
&& filter_full_path(&regex_key, asset)
&& filter_description(&regex_description, asset)
&& filter_owner(*owner, asset)
&& assert_rule(rule, asset.key.owner, caller, controllers)
&& assert_permission(rule, asset.key.owner, caller, controllers)
})
.collect()
}
Expand Down

0 comments on commit b645468

Please sign in to comment.