From f5f6ea4a5c295ada47d769df945d53dc115e5fa8 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Sun, 4 Jan 2026 19:21:37 +0800 Subject: [PATCH] feat:policy Resources support string and array modes. (#1346) Co-authored-by: loverustfs --- crates/policy/src/policy/action.rs | 57 ++++++++++++++++++++++-- crates/policy/src/policy/policy.rs | 66 ++++++++++++++++++++++++++++ crates/policy/src/policy/resource.rs | 56 ++++++++++++++++++++++- rustfs/src/admin/handlers/sts.rs | 5 ++- 4 files changed, 178 insertions(+), 6 deletions(-) diff --git a/crates/policy/src/policy/action.rs b/crates/policy/src/policy/action.rs index cb47dec7..16f0e12b 100644 --- a/crates/policy/src/policy/action.rs +++ b/crates/policy/src/policy/action.rs @@ -13,13 +13,16 @@ // limitations under the License. use crate::error::{Error, Result}; -use serde::{Deserialize, Serialize}; -use std::{collections::HashSet, ops::Deref}; +use serde::{ + Deserialize, Deserializer, Serialize, + de::{self, Error as DeError, Visitor}, +}; +use std::{collections::HashSet, fmt, ops::Deref}; use strum::{EnumString, IntoStaticStr}; use super::{Error as IamError, Validator, utils::wildcard}; -#[derive(Serialize, Deserialize, Clone, Default, Debug)] +#[derive(Serialize, Clone, Default, Debug)] pub struct ActionSet(pub HashSet); impl ActionSet { @@ -61,6 +64,54 @@ impl PartialEq for ActionSet { } } +impl<'de> Deserialize<'de> for ActionSet { + fn deserialize(deserializer: D) -> std::result::Result + where + D: Deserializer<'de>, + { + struct ActionOrVecVisitor; + + impl<'de> Visitor<'de> for ActionOrVecVisitor { + type Value = ActionSet; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a string or an array of strings") + } + + fn visit_str(self, value: &str) -> std::result::Result + where + E: de::Error, + { + let action = Action::try_from(value).map_err(|e| E::custom(format!("invalid action: {}", e)))?; + let mut set = HashSet::new(); + set.insert(action); + Ok(ActionSet(set)) + } + + fn visit_seq(self, mut seq: A) -> std::result::Result + where + A: de::SeqAccess<'de>, + A::Error: DeError, + { + let mut set = HashSet::with_capacity(seq.size_hint().unwrap_or(0)); + while let Some(value) = seq.next_element::()? { + match Action::try_from(value.as_str()) { + Ok(action) => { + set.insert(action); + } + Err(e) => { + return Err(A::Error::custom(format!("invalid action: {}", e))); + } + } + } + Ok(ActionSet(set)) + } + } + + deserializer.deserialize_any(ActionOrVecVisitor) + } +} + #[derive(Serialize, Deserialize, Hash, PartialEq, Eq, Clone, Debug, Copy)] #[serde(try_from = "&str", untagged)] pub enum Action { diff --git a/crates/policy/src/policy/policy.rs b/crates/policy/src/policy/policy.rs index 6ddf6294..91dbb4c9 100644 --- a/crates/policy/src/policy/policy.rs +++ b/crates/policy/src/policy/policy.rs @@ -526,6 +526,72 @@ mod test { Ok(()) } + #[tokio::test] + async fn test_parse_policy_with_single_string_action_and_resource() -> Result<()> { + // Test policy with single string Action and Resource (AWS IAM allows both formats) + let data = r#" +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::test/analytics/customers/*" + } + ] +} +"#; + + let p = Policy::parse_config(data.as_bytes())?; + assert!(!p.statements.is_empty()); + assert!(!p.statements[0].actions.is_empty()); + assert!(!p.statements[0].resources.is_empty()); + + // Test with array format (should still work) + let data_array = r#" +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": ["s3:GetObject"], + "Resource": ["arn:aws:s3:::test/analytics/customers/*"] + } + ] +} +"#; + + let p2 = Policy::parse_config(data_array.as_bytes())?; + assert!(!p2.statements.is_empty()); + assert!(!p2.statements[0].actions.is_empty()); + assert!(!p2.statements[0].resources.is_empty()); + + // Verify that both formats produce equivalent results + assert_eq!( + p.statements.len(), + p2.statements.len(), + "Both policies should have the same number of statements" + ); + assert_eq!( + p.statements[0].actions, p2.statements[0].actions, + "ActionSet from string format should equal ActionSet from array format" + ); + assert_eq!( + p.statements[0].resources, p2.statements[0].resources, + "ResourceSet from string format should equal ResourceSet from array format" + ); + assert_eq!( + p.statements[0].effect, p2.statements[0].effect, + "Effect should be the same in both formats" + ); + + // Verify specific content + assert_eq!(p.statements[0].actions.len(), 1, "ActionSet should contain exactly one action"); + assert_eq!(p.statements[0].resources.len(), 1, "ResourceSet should contain exactly one resource"); + + Ok(()) + } + #[tokio::test] async fn test_aws_username_policy_variable() -> Result<()> { let data = r#" diff --git a/crates/policy/src/policy/resource.rs b/crates/policy/src/policy/resource.rs index 024c79a3..0d7ff9eb 100644 --- a/crates/policy/src/policy/resource.rs +++ b/crates/policy/src/policy/resource.rs @@ -13,9 +13,13 @@ // limitations under the License. use crate::error::{Error, Result}; -use serde::{Deserialize, Serialize}; +use serde::{ + Deserialize, Deserializer, Serialize, + de::{self, Error as DeError, Visitor}, +}; use std::{ collections::{HashMap, HashSet}, + fmt, hash::Hash, ops::Deref, }; @@ -27,7 +31,7 @@ use super::{ variables::PolicyVariableResolver, }; -#[derive(Serialize, Deserialize, Clone, Default, Debug)] +#[derive(Serialize, Clone, Default, Debug)] pub struct ResourceSet(pub HashSet); impl ResourceSet { @@ -86,6 +90,54 @@ impl PartialEq for ResourceSet { } } +impl<'de> Deserialize<'de> for ResourceSet { + fn deserialize(deserializer: D) -> std::result::Result + where + D: Deserializer<'de>, + { + struct ResourceOrVecVisitor; + + impl<'de> Visitor<'de> for ResourceOrVecVisitor { + type Value = ResourceSet; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a string or an array of strings") + } + + fn visit_str(self, value: &str) -> std::result::Result + where + E: de::Error, + { + let resource = Resource::try_from(value).map_err(|e| E::custom(format!("invalid resource: {}", e)))?; + let mut set = HashSet::new(); + set.insert(resource); + Ok(ResourceSet(set)) + } + + fn visit_seq(self, mut seq: A) -> std::result::Result + where + A: de::SeqAccess<'de>, + A::Error: DeError, + { + let mut set = HashSet::with_capacity(seq.size_hint().unwrap_or(0)); + while let Some(value) = seq.next_element::()? { + match Resource::try_from(value.as_str()) { + Ok(resource) => { + set.insert(resource); + } + Err(e) => { + return Err(A::Error::custom(format!("invalid resource: {}", e))); + } + } + } + Ok(ResourceSet(set)) + } + } + + deserializer.deserialize_any(ResourceOrVecVisitor) + } +} + #[derive(Hash, Eq, PartialEq, Clone, Debug)] pub enum Resource { S3(String), diff --git a/rustfs/src/admin/handlers/sts.rs b/rustfs/src/admin/handlers/sts.rs index 9770784d..550a90d9 100644 --- a/rustfs/src/admin/handlers/sts.rs +++ b/rustfs/src/admin/handlers/sts.rs @@ -167,7 +167,10 @@ impl Operation for AssumeRoleHandle { pub fn populate_session_policy(claims: &mut HashMap, policy: &str) -> S3Result<()> { if !policy.is_empty() { let session_policy = Policy::parse_config(policy.as_bytes()) - .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("parse policy err {e}")))?; + .map_err(|e| { + let error_msg = format!("Failed to parse session policy: {}. Please check that the policy is valid JSON format with standard brackets [] for arrays.", e); + S3Error::with_message(S3ErrorCode::InvalidRequest, error_msg) + })?; if session_policy.version.is_empty() { return Err(s3_error!(InvalidRequest, "invalid policy")); }