diff --git a/src/bin/scheduler/setup/steps/rcc.rs b/src/bin/scheduler/setup/steps/rcc.rs index 4f5ec4f6..ee3f4d5e 100644 --- a/src/bin/scheduler/setup/steps/rcc.rs +++ b/src/bin/scheduler/setup/steps/rcc.rs @@ -24,36 +24,27 @@ use tokio_util::sync::CancellationToken; #[cfg(windows)] struct StepFilePermissions { target: Utf8PathBuf, - session: Session, + sid: String, icacls_permissions: String, } #[cfg(windows)] impl SetupStep for StepFilePermissions { fn setup(&self) -> Result<(), api::Error> { - if let Session::User(user_session) = &self.session { - log::info!( - "Granting user `{user}` {permissions} access to {target}.", - user = &user_session.user_name, - permissions = &self.icacls_permissions, - target = &self.target, - ); - run_icacls_command([ - self.target.as_str(), - "/grant", - &format!("{}:{}", &user_session.user_name, self.icacls_permissions), - ]) - .map_err(|err| { - api::Error::new( - format!( - "Adjusting permissions of {} for user `{}` failed", - self.target, &user_session.user_name - ), - err, - ) - })?; - } - Ok(()) + run_icacls_command([ + self.target.as_str(), + "/grant", + &format!("{}:{}", &self.sid, self.icacls_permissions), + ]) + .map_err(|err| { + api::Error::new( + format!( + "Adjusting permissions of {} for SID `{}` failed", + self.target, &self.sid + ), + err, + ) + }) } } @@ -253,20 +244,11 @@ pub fn gather_rcc_binary_permissions( config: &GlobalConfig, plans: Vec, ) -> Vec { - let (rcc_plans, system_plans): (Vec, Vec) = - partition_into_rcc_and_system_plans(plans); - let mut steps: Vec = vec![skip(system_plans)]; - for (session, plans_in_session) in plans_by_sessions(rcc_plans) { - steps.push(( - Box::new(StepFilePermissions { - target: config.rcc_config.binary_path.clone(), - session, - icacls_permissions: "(RX)".to_string(), - }), - plans_in_session, - )); - } - steps + gather_file_permissions_for_users_group( + config.rcc_config.binary_path.clone(), + "(RX)".into(), + plans, + ) } #[cfg(windows)] @@ -274,25 +256,14 @@ pub fn gather_rcc_profile_permissions( config: &GlobalConfig, plans: Vec, ) -> Vec { - let (rcc_plans, system_plans): (Vec, Vec) = - partition_into_rcc_and_system_plans(plans); - let mut steps: Vec = vec![skip(system_plans)]; match &config.rcc_config.profile_config { - RCCProfileConfig::Default => steps.push(skip(rcc_plans)), - RCCProfileConfig::Custom(custom_profile) => { - for (session, plans_in_session) in plans_by_sessions(rcc_plans) { - steps.push(( - Box::new(StepFilePermissions { - target: custom_profile.path.clone(), - session, - icacls_permissions: "(R)".to_string(), - }), - plans_in_session, - )); - } - } + RCCProfileConfig::Default => vec![skip(plans)], + RCCProfileConfig::Custom(custom_profile) => gather_file_permissions_for_users_group( + custom_profile.path.clone(), + "(R)".into(), + plans, + ), } - steps } pub fn gather_disable_rcc_telemetry(config: &GlobalConfig, plans: Vec) -> Vec { @@ -445,3 +416,33 @@ pub fn gather_disable_rcc_shared_holotree( } steps } + +#[cfg(windows)] +fn gather_file_permissions_for_users_group( + target: Utf8PathBuf, + icacls_permissions: String, + plans: Vec, +) -> Vec { + let (rcc_plans, system_plans): (Vec, Vec) = + partition_into_rcc_and_system_plans(plans); + let mut rcc_plans_in_current_session = vec![]; + let mut rcc_plans_in_other_session = vec![]; + for plan in rcc_plans { + match plan.session { + Session::Current(_) => rcc_plans_in_current_session.push(plan), + _ => rcc_plans_in_other_session.push(plan), + } + } + vec![ + skip(system_plans), + skip(rcc_plans_in_current_session), + ( + Box::new(StepFilePermissions { + target, + sid: "*S-1-5-32-545".to_string(), // Users (https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-security-identifiers) + icacls_permissions, + }), + rcc_plans_in_other_session, + ), + ] +} diff --git a/tests/test_scheduler.rs b/tests/test_scheduler.rs index 0813feab..33a72bfa 100644 --- a/tests/test_scheduler.rs +++ b/tests/test_scheduler.rs @@ -97,12 +97,7 @@ async fn test_scheduler() -> AnyhowResult<()> { ¤t_user_name, ) .await?; - assert_rcc( - &config.rcc_config, - #[cfg(windows)] - ¤t_user_name, - ) - .await?; + assert_rcc(&config.rcc_config).await?; #[cfg(windows)] assert_tasks().await?; assert_sequentiality( @@ -547,6 +542,14 @@ async fn assert_permissions(path: impl AsRef, permissions: &str) -> Anyho Ok(()) } +#[cfg(windows)] +async fn dacl_exists_for_sid(path: &Utf8Path, sid: &str) -> AnyhowResult { + Ok(run_icacls([path.as_str(), "/findsid", sid]) + .await? + .status + .success()) +} + fn assert_results_directory(results_directory: &Utf8Path) { assert!(results_directory.is_dir()); assert_eq!( @@ -580,12 +583,9 @@ async fn assert_managed_directory( Ok(()) } -async fn assert_rcc( - rcc_config: &RCCConfig, - #[cfg(windows)] headed_user_name: &str, -) -> AnyhowResult<()> { +async fn assert_rcc(rcc_config: &RCCConfig) -> AnyhowResult<()> { #[cfg(windows)] - assert_rcc_files_permissions(rcc_config, headed_user_name).await?; + assert_rcc_files_permissions(rcc_config).await?; assert_rcc_configuration( rcc_config, rcc_config @@ -601,19 +601,13 @@ async fn assert_rcc( } #[cfg(windows)] -async fn assert_rcc_files_permissions( - rcc_config: &RCCConfig, - headed_user_name: &str, -) -> AnyhowResult<()> { - assert_permissions(&rcc_config.binary_path, &format!("{headed_user_name}:(RX)")).await?; +async fn assert_rcc_files_permissions(rcc_config: &RCCConfig) -> AnyhowResult<()> { + assert!(dacl_exists_for_sid(&rcc_config.binary_path, "*S-1-5-32-544").await?); let RCCProfileConfig::Custom(custom_rcc_profile_config) = &rcc_config.profile_config else { return Ok(()); }; - assert_permissions( - &custom_rcc_profile_config.path, - &format!("{headed_user_name}:(R)"), - ) - .await + assert!(dacl_exists_for_sid(&custom_rcc_profile_config.path, "*S-1-5-32-544").await?); + Ok(()) } async fn assert_rcc_configuration(rcc_config: &RCCConfig, robocorp_home: &str) -> AnyhowResult<()> { @@ -700,12 +694,7 @@ async fn assert_robocorp_home( .len(), 3 // Administrator group + empty line + success message (suppressing the latter with /q does not seem to work) ); - assert!( - run_icacls([robocorp_home_base.as_str(), "/findsid", "*S-1-5-32-544"]) - .await? - .status - .success() - ); + assert!(dacl_exists_for_sid(robocorp_home_base, "*S-1-5-32-544").await?); assert!( get_permissions(robocorp_home_base.join(format!("user_{headed_user_name}"))) .await?