refactor(exec): General-purpose visibility settings

rs/execution_environment/src/canister_logs.rs

@@ -1,3 +1,4 @@
+use crate::canister_settings::VisibilitySettings;
 use ic_config::flag_status::FlagStatus;
 use ic_error_types::{ErrorCode, UserError};
 use ic_management_canister_types_private::{
@@ -42,22 +43,13 @@ pub(crate) fn check_log_visibility_permission(
     log_visibility: &LogVisibilityV2,
     controllers: &std::collections::BTreeSet<PrincipalId>,
 ) -> Result<(), UserError> {
-    let has_access = match log_visibility {
-        LogVisibilityV2::Public => true,
-        LogVisibilityV2::Controllers => controllers.contains(caller),
-        LogVisibilityV2::AllowedViewers(principals) => {
-            principals.get().contains(caller) || controllers.contains(caller)
-        }
-    };
-
-    if has_access {
-        Ok(())
-    } else {
-        Err(UserError::new(
+    if !VisibilitySettings::from(log_visibility).has_access(caller, controllers) {
+        return Err(UserError::new(
             ErrorCode::CanisterRejectedMessage,
             format!("Caller {caller} is not allowed to access canister logs"),
-        ))
+        ));
     }
+    Ok(())
 }
 
 fn filter_records(

rs/execution_environment/src/canister_settings.rs

@@ -1,11 +1,13 @@
 use ic_base_types::{EnvironmentVariables, NumBytes, NumSeconds};
 use ic_error_types::{ErrorCode, UserError};
-use ic_management_canister_types_private::{CanisterSettingsArgs, LogVisibilityV2};
+use ic_management_canister_types_private::{
+    BoundedAllowedViewers, CanisterSettingsArgs, LogVisibilityV2,
+};
 use ic_types::{
     ComputeAllocation, Cycles, InvalidComputeAllocationError, MemoryAllocation, PrincipalId,
 };
 use num_traits::cast::ToPrimitive;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::convert::TryFrom;
 
 #[cfg(test)]
@@ -483,3 +485,36 @@ impl ValidatedCanisterSettings {
         self.environment_variables.as_ref()
     }
 }
+
+#[derive(Clone, Eq, PartialEq, Debug)]
+pub(crate) enum VisibilitySettings<'a> {
+    Public,
+    Controllers,
+    AllowedViewers(&'a BoundedAllowedViewers),
+}
+
+impl<'a> From<&'a LogVisibilityV2> for VisibilitySettings<'a> {
+    fn from(v: &'a LogVisibilityV2) -> Self {
+        match v {
+            LogVisibilityV2::Public => Self::Public,
+            LogVisibilityV2::Controllers => Self::Controllers,
+            LogVisibilityV2::AllowedViewers(principals) => Self::AllowedViewers(principals),
+        }
+    }
+}
+
+impl VisibilitySettings<'_> {
+    pub(crate) fn has_access(
+        &self,
+        caller: &PrincipalId,
+        controllers: &BTreeSet<PrincipalId>,
+    ) -> bool {
+        match self {
+            Self::Public => true,
+            Self::Controllers => controllers.contains(caller),
+            Self::AllowedViewers(allowed) => {
+                allowed.get().contains(caller) || controllers.contains(caller)
+            }
+        }
+    }
+}

rs/execution_environment/src/canister_settings/tests.rs

@@ -86,3 +86,56 @@ fn test_environment_variables_hash_output() {
     let actual = EnvironmentVariables::new(env_vars).hash();
     assert_eq!(actual, expected);
 }
+
+mod visibility_settings {
+    use crate::canister_settings::VisibilitySettings;
+    use ic_management_canister_types_private::BoundedAllowedViewers;
+    use ic_types::PrincipalId;
+    use proptest::prelude::*;
+    use std::collections::BTreeSet;
+
+    proptest! {
+        #[test]
+        fn public_always_grants_access(
+            caller in arb_principal_id(),
+            controllers in arb_controllers(),
+        ) {
+            prop_assert!(VisibilitySettings::Public.has_access(&caller, &controllers));
+        }
+
+        #[test]
+        fn controllers_grants_access_only_to_controllers(
+            caller in arb_principal_id(),
+            controllers in arb_controllers(),
+        ) {
+            prop_assert_eq!(
+                VisibilitySettings::Controllers.has_access(&caller, &controllers),
+                controllers.contains(&caller),
+            );
+        }
+
+        #[test]
+        fn allowed_viewers_grants_access_to_viewers_and_controllers(
+            caller in arb_principal_id(),
+            controllers in arb_controllers(),
+            viewers in proptest::collection::vec(arb_principal_id(), 0..=10),
+        ) {
+            let allowed = BoundedAllowedViewers::new(viewers.clone());
+            let settings = VisibilitySettings::AllowedViewers(&allowed);
+            prop_assert_eq!(
+                settings.has_access(&caller, &controllers),
+                viewers.contains(&caller) || controllers.contains(&caller),
+            );
+        }
+    }
+
+    fn arb_principal_id() -> BoxedStrategy<PrincipalId> {
+        (0..u64::MAX)
+            .prop_map(PrincipalId::new_user_test_id)
+            .boxed()
+    }
+
+    fn arb_controllers() -> BoxedStrategy<BTreeSet<PrincipalId>> {
+        proptest::collection::btree_set(arb_principal_id(), 0..=10).boxed()
+    }
+}