feat: s3 tests classification (#1392)

Co-authored-by: houseme <housemecn@gmail.com>

crates/policy/src/policy/action.rs

@@ -22,10 +22,42 @@ use strum::{EnumString, IntoStaticStr};
 
 use super::{Error as IamError, Validator, utils::wildcard};
 
-#[derive(Serialize, Clone, Default, Debug)]
+/// A set of policy actions that serializes as a single string when containing one item,
+/// or as an array when containing multiple items (matching AWS S3 API format).
+#[derive(Clone, Default, Debug)]
 pub struct ActionSet(pub HashSet<Action>);
 
+impl Serialize for ActionSet {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeSeq;
+
+        if self.0.len() == 1 {
+            // Serialize single action as string (not array)
+            if let Some(action) = self.0.iter().next() {
+                let action_str: &str = action.into();
+                return serializer.serialize_str(action_str);
+            }
+        }
+
+        // Serialize multiple actions as array
+        let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
+        for action in &self.0 {
+            let action_str: &str = action.into();
+            seq.serialize_element(action_str)?;
+        }
+        seq.end()
+    }
+}
+
 impl ActionSet {
+    /// Returns true if the action set is empty.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
     pub fn is_match(&self, action: &Action) -> bool {
         for act in self.0.iter() {
             if act.is_match(action) {
@@ -150,6 +182,10 @@ impl Action {
 impl TryFrom<&str> for Action {
     type Error = Error;
     fn try_from(value: &str) -> std::result::Result<Self, Self::Error> {
+        // Support wildcard "*" which matches all S3 actions (AWS S3 standard)
+        if value == "*" {
+            return Ok(Self::S3Action(S3Action::AllActions));
+        }
         if value.starts_with(Self::S3_PREFIX) {
             Ok(Self::S3Action(
                 S3Action::try_from(value).map_err(|_| IamError::InvalidAction(value.into()))?,
@@ -559,3 +595,53 @@ pub enum KmsAction {
     #[strum(serialize = "kms:*")]
     AllActions,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashSet;
+
+    #[test]
+    fn test_action_wildcard_parsing() {
+        // Test that "*" parses to S3Action::AllActions
+        let action = Action::try_from("*").expect("Should parse wildcard");
+        assert!(matches!(action, Action::S3Action(S3Action::AllActions)));
+    }
+
+    #[test]
+    fn test_actionset_serialize_single_element() {
+        // Single element should serialize as string
+        let mut set = HashSet::new();
+        set.insert(Action::S3Action(S3Action::GetObjectAction));
+        let actionset = ActionSet(set);
+
+        let json = serde_json::to_string(&actionset).expect("Should serialize");
+        assert_eq!(json, "\"s3:GetObject\"");
+    }
+
+    #[test]
+    fn test_actionset_serialize_multiple_elements() {
+        // Multiple elements should serialize as array
+        let mut set = HashSet::new();
+        set.insert(Action::S3Action(S3Action::GetObjectAction));
+        set.insert(Action::S3Action(S3Action::PutObjectAction));
+        let actionset = ActionSet(set);
+
+        let json = serde_json::to_string(&actionset).expect("Should serialize");
+        let parsed: serde_json::Value = serde_json::from_str(&json).expect("Should parse");
+        assert!(parsed.is_array());
+        let arr = parsed.as_array().expect("Should be array");
+        assert_eq!(arr.len(), 2);
+    }
+
+    #[test]
+    fn test_actionset_wildcard_serialization() {
+        // Wildcard action should serialize correctly
+        let mut set = HashSet::new();
+        set.insert(Action::try_from("*").expect("Should parse wildcard"));
+        let actionset = ActionSet(set);
+
+        let json = serde_json::to_string(&actionset).expect("Should serialize");
+        assert_eq!(json, "\"s3:*\"");
+    }
+}

crates/policy/src/policy/id.rs

@@ -21,6 +21,13 @@ use super::Validator;
 #[derive(Serialize, Deserialize, Clone, Default, Debug)]
 pub struct ID(pub String);
 
+impl ID {
+    /// Returns true if the ID is empty.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+}
+
 impl Validator for ID {
     type Error = Error;
     /// if id is a valid utf string, then it is valid.

crates/policy/src/policy/policy.rs

@@ -177,9 +177,11 @@ pub struct BucketPolicyArgs<'a> {
     pub object: &'a str,
 }
 
+/// Bucket Policy with AWS S3-compatible JSON serialization.
+/// Empty optional fields are omitted from output to match AWS format.
 #[derive(Serialize, Deserialize, Clone, Default, Debug)]
 pub struct BucketPolicy {
-    #[serde(default, rename = "ID")]
+    #[serde(default, rename = "ID", skip_serializing_if = "ID::is_empty")]
     pub id: ID,
     #[serde(rename = "Version")]
     pub version: String,
@@ -950,4 +952,106 @@ mod test {
             );
         }
     }
+
+    #[test]
+    fn test_bucket_policy_serialize_omits_empty_fields() {
+        use crate::policy::action::{Action, ActionSet, S3Action};
+        use crate::policy::resource::{Resource, ResourceSet};
+        use crate::policy::{Effect, Functions, Principal};
+
+        // Create a BucketPolicy with empty optional fields
+        // Use JSON deserialization to create Principal (since aws field is private)
+        let principal: Principal = serde_json::from_str(r#"{"AWS": "*"}"#).expect("Should parse principal");
+
+        let mut policy = BucketPolicy {
+            id: ID::default(), // Empty ID
+            version: "2012-10-17".to_string(),
+            statements: vec![BPStatement {
+                sid: ID::default(), // Empty Sid
+                effect: Effect::Allow,
+                principal,
+                actions: ActionSet::default(),
+                not_actions: ActionSet::default(), // Empty NotAction
+                resources: ResourceSet::default(),
+                not_resources: ResourceSet::default(), // Empty NotResource
+                conditions: Functions::default(),      // Empty Condition
+            }],
+        };
+
+        // Set actions and resources (required fields)
+        policy.statements[0]
+            .actions
+            .0
+            .insert(Action::S3Action(S3Action::ListBucketAction));
+        policy.statements[0]
+            .resources
+            .0
+            .insert(Resource::try_from("arn:aws:s3:::test/*").unwrap());
+
+        let json = serde_json::to_string(&policy).expect("Should serialize");
+        let parsed: serde_json::Value = serde_json::from_str(&json).expect("Should parse");
+
+        // Verify empty fields are omitted
+        assert!(!parsed.as_object().unwrap().contains_key("ID"), "Empty ID should be omitted");
+
+        let statement = &parsed["Statement"][0];
+        assert!(!statement.as_object().unwrap().contains_key("Sid"), "Empty Sid should be omitted");
+        assert!(
+            !statement.as_object().unwrap().contains_key("NotAction"),
+            "Empty NotAction should be omitted"
+        );
+        assert!(
+            !statement.as_object().unwrap().contains_key("NotResource"),
+            "Empty NotResource should be omitted"
+        );
+        assert!(
+            !statement.as_object().unwrap().contains_key("Condition"),
+            "Empty Condition should be omitted"
+        );
+
+        // Verify required fields are present
+        assert_eq!(parsed["Version"], "2012-10-17");
+        assert_eq!(statement["Effect"], "Allow");
+        assert_eq!(statement["Principal"]["AWS"], "*");
+    }
+
+    #[test]
+    fn test_bucket_policy_serialize_single_action_as_string() {
+        use crate::policy::action::{Action, ActionSet, S3Action};
+        use crate::policy::resource::{Resource, ResourceSet};
+        use crate::policy::{Effect, Principal};
+
+        // Use JSON deserialization to create Principal (since aws field is private)
+        let principal: Principal = serde_json::from_str(r#"{"AWS": "*"}"#).expect("Should parse principal");
+
+        let mut policy = BucketPolicy {
+            version: "2012-10-17".to_string(),
+            statements: vec![BPStatement {
+                effect: Effect::Allow,
+                principal,
+                actions: ActionSet::default(),
+                resources: ResourceSet::default(),
+                ..Default::default()
+            }],
+            ..Default::default()
+        };
+
+        // Single action
+        policy.statements[0]
+            .actions
+            .0
+            .insert(Action::S3Action(S3Action::ListBucketAction));
+        policy.statements[0]
+            .resources
+            .0
+            .insert(Resource::try_from("arn:aws:s3:::test/*").unwrap());
+
+        let json = serde_json::to_string(&policy).expect("Should serialize");
+        let parsed: serde_json::Value = serde_json::from_str(&json).expect("Should parse");
+        let action = &parsed["Statement"][0]["Action"];
+
+        // Single action should be serialized as string
+        assert!(action.is_string(), "Single action should serialize as string");
+        assert_eq!(action.as_str().unwrap(), "s3:ListBucket");
+    }
 }

crates/policy/src/policy/principal.rs

@@ -17,13 +17,35 @@ use crate::error::Error;
 use serde::Serialize;
 use std::collections::HashSet;
 
-#[derive(Debug, Clone, Serialize, Default, PartialEq, Eq)]
-#[serde(rename_all = "PascalCase", default)]
+/// Principal that serializes AWS field as single string when containing only "*",
+/// or as an array otherwise (matching AWS S3 API format).
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
 pub struct Principal {
-    #[serde(rename = "AWS")]
     aws: HashSet<String>,
 }
 
+impl Serialize for Principal {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeMap;
+
+        let mut map = serializer.serialize_map(Some(1))?;
+
+        // If single element, serialize as string; otherwise as array
+        if self.aws.len() == 1 {
+            if let Some(val) = self.aws.iter().next() {
+                map.serialize_entry("AWS", val)?;
+            }
+        } else {
+            map.serialize_entry("AWS", &self.aws)?;
+        }
+
+        map.end()
+    }
+}
+
 #[derive(serde::Deserialize)]
 #[serde(untagged)]
 enum PrincipalFormat {
@@ -118,4 +140,30 @@ mod test {
         };
         assert!(result);
     }
+
+    #[test]
+    fn test_principal_serialize_single_element() {
+        // Single element should serialize as string (AWS format)
+        let principal = Principal {
+            aws: HashSet::from(["*".to_string()]),
+        };
+
+        let json = serde_json::to_string(&principal).expect("Should serialize");
+        assert_eq!(json, r#"{"AWS":"*"}"#);
+    }
+
+    #[test]
+    fn test_principal_serialize_multiple_elements() {
+        // Multiple elements should serialize as array
+        let principal = Principal {
+            aws: HashSet::from(["*".to_string(), "arn:aws:iam::123456789012:root".to_string()]),
+        };
+
+        let json = serde_json::to_string(&principal).expect("Should serialize");
+        let parsed: serde_json::Value = serde_json::from_str(&json).expect("Should parse");
+        let aws_value = parsed.get("AWS").expect("Should have AWS field");
+        assert!(aws_value.is_array());
+        let arr = aws_value.as_array().expect("Should be array");
+        assert_eq!(arr.len(), 2);
+    }
 }

crates/policy/src/policy/resource.rs

@@ -35,6 +35,11 @@ use super::{
 pub struct ResourceSet(pub HashSet<Resource>);
 
 impl ResourceSet {
+    /// Returns true if the resource set is empty.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
     pub async fn is_match(&self, resource: &str, conditions: &HashMap<String, Vec<String>>) -> bool {
         self.is_match_with_resolver(resource, conditions, None).await
     }

crates/policy/src/policy/statement.rs

@@ -179,10 +179,12 @@ impl PartialEq for Statement {
     }
 }
 
+/// Bucket Policy Statement with AWS S3-compatible JSON serialization.
+/// Empty optional fields are omitted from output to match AWS format.
 #[derive(Debug, Deserialize, Serialize, Default, Clone)]
 #[serde(rename_all = "PascalCase", default)]
 pub struct BPStatement {
-    #[serde(rename = "Sid", default)]
+    #[serde(rename = "Sid", default, skip_serializing_if = "ID::is_empty")]
     pub sid: ID,
     #[serde(rename = "Effect")]
     pub effect: Effect,
@@ -190,13 +192,13 @@ pub struct BPStatement {
     pub principal: Principal,
     #[serde(rename = "Action")]
     pub actions: ActionSet,
-    #[serde(rename = "NotAction", default)]
+    #[serde(rename = "NotAction", default, skip_serializing_if = "ActionSet::is_empty")]
     pub not_actions: ActionSet,
     #[serde(rename = "Resource", default)]
     pub resources: ResourceSet,
-    #[serde(rename = "NotResource", default)]
+    #[serde(rename = "NotResource", default, skip_serializing_if = "ResourceSet::is_empty")]
     pub not_resources: ResourceSet,
-    #[serde(rename = "Condition", default)]
+    #[serde(rename = "Condition", default, skip_serializing_if = "Functions::is_empty")]
     pub conditions: Functions,
 }