fix:allow NotResource-only policies in statement validation (#1364)

Co-authored-by: loverustfs <hello@rustfs.com>

crates/policy/src/policy.rs

@@ -51,6 +51,9 @@ pub enum Error {
     #[error("'Resource' is empty")]
     NonResource,
 
+    #[error("'Resource' and 'NotResource' cannot both be specified in the same statement")]
+    BothResourceAndNotResource,
+
     #[error("invalid key name: '{0}'")]
     InvalidKeyName(String),
 

crates/policy/src/policy/policy.rs

@@ -868,4 +868,86 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    fn test_statement_with_only_notresource_is_valid() {
+        // Test: A statement with only NotResource (and no Resource) is valid
+        let data = r#"
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["s3:GetObject"],
+      "NotResource": ["arn:aws:s3:::mybucket/private/*"]
+    }
+  ]
+}
+"#;
+
+        let result = Policy::parse_config(data.as_bytes());
+        assert!(result.is_ok(), "Statement with only NotResource should be valid");
+    }
+
+    #[test]
+    fn test_statement_with_both_resource_and_notresource_is_invalid() {
+        // Test: A statement with both Resource and NotResource returns BothResourceAndNotResource error
+        let data = r#"
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["s3:GetObject"],
+      "Resource": ["arn:aws:s3:::mybucket/public/*"],
+      "NotResource": ["arn:aws:s3:::mybucket/private/*"]
+    }
+  ]
+}
+"#;
+
+        let result = Policy::parse_config(data.as_bytes());
+        assert!(result.is_err(), "Statement with both Resource and NotResource should be invalid");
+
+        // Verify the specific error type
+        if let Err(e) = result {
+            let error_msg = format!("{}", e);
+            assert!(
+                error_msg.contains("Resource")
+                    && error_msg.contains("NotResource")
+                    && error_msg.contains("cannot both be specified"),
+                "Error should be BothResourceAndNotResource, got: {}",
+                error_msg
+            );
+        }
+    }
+
+    #[test]
+    fn test_statement_with_neither_resource_nor_notresource_is_invalid() {
+        // Test: A statement with neither Resource nor NotResource returns NonResource error
+        let data = r#"
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["s3:GetObject"]
+    }
+  ]
+}
+"#;
+
+        let result = Policy::parse_config(data.as_bytes());
+        assert!(result.is_err(), "Statement with neither Resource nor NotResource should be invalid");
+
+        // Verify the specific error type
+        if let Err(e) = result {
+            let error_msg = format!("{}", e);
+            assert!(
+                error_msg.contains("Resource") && error_msg.contains("empty"),
+                "Error should be NonResource, got: {}",
+                error_msg
+            );
+        }
+    }
 }

crates/policy/src/policy/statement.rs

@@ -107,10 +107,26 @@ impl Statement {
                 break 'c self.conditions.evaluate_with_resolver(args.conditions, Some(&resolver)).await;
             }
 
-            if !self
+            if self.resources.is_empty() && self.not_resources.is_empty() && !self.is_admin() && !self.is_sts() {
-                .resources
+                break 'c false;
-                .is_match_with_resolver(&resource, args.conditions, Some(&resolver))
+            }
-                .await
+
+            if !self.resources.is_empty()
+                && !self
+                    .resources
+                    .is_match_with_resolver(&resource, args.conditions, Some(&resolver))
+                    .await
+                && !self.is_admin()
+                && !self.is_sts()
+            {
+                break 'c false;
+            }
+
+            if !self.not_resources.is_empty()
+                && self
+                    .not_resources
+                    .is_match_with_resolver(&resource, args.conditions, Some(&resolver))
+                    .await
                 && !self.is_admin()
                 && !self.is_sts()
             {
@@ -135,13 +151,19 @@ impl Validator for Statement {
             return Err(IamError::NonAction.into());
         }
 
-        if self.resources.is_empty() {
+        // policy must contain either Resource or NotResource (but not both), and cannot have both empty.
+        if self.resources.is_empty() && self.not_resources.is_empty() {
             return Err(IamError::NonResource.into());
         }
 
+        if !self.resources.is_empty() && !self.not_resources.is_empty() {
+            return Err(IamError::BothResourceAndNotResource.into());
+        }
+
         self.actions.is_valid()?;
         self.not_actions.is_valid()?;
         self.resources.is_valid()?;
+        self.not_resources.is_valid()?;
 
         Ok(())
     }
@@ -228,13 +250,18 @@ impl Validator for BPStatement {
             return Err(IamError::NonAction.into());
         }
 
-        if self.resources.is_empty() {
+        if self.resources.is_empty() && self.not_resources.is_empty() {
             return Err(IamError::NonResource.into());
         }
 
+        if !self.resources.is_empty() && !self.not_resources.is_empty() {
+            return Err(IamError::BothResourceAndNotResource.into());
+        }
+
         self.actions.is_valid()?;
         self.not_actions.is_valid()?;
         self.resources.is_valid()?;
+        self.not_resources.is_valid()?;
 
         Ok(())
     }

crates/policy/tests/policy_is_allowed.rs

@@ -611,6 +611,52 @@ struct ArgsBuilder {
     } => false;
     "24"
 )]
+#[test_case(
+    Policy{
+        version: DEFAULT_VERSION.into(),
+        statements: vec![
+            Statement{
+                effect: Allow,
+                actions: ActionSet(vec![rustfs_policy::policy::action::Action::S3Action(GetObjectAction)].into_iter().collect()),
+                resources: ResourceSet::default(), // Empty Resource
+                not_resources: ResourceSet(vec!["arn:aws:s3:::mybucket/private/*".try_into().unwrap()].into_iter().collect()),
+                ..Default::default()
+            }
+        ],
+        ..Default::default()
+    },
+    ArgsBuilder{
+        account: "Q3AM3UQ867SPQQA43P2F".into(),
+        action: "s3:GetObject".into(),
+        bucket: "mybucket".into(),
+        object: "public/file.txt".into(),
+        ..Default::default()
+    } => true;
+    "notresource_allows_access_outside_blacklist"
+)]
+#[test_case(
+    Policy{
+        version: DEFAULT_VERSION.into(),
+        statements: vec![
+            Statement{
+                effect: Allow,
+                actions: ActionSet(vec![rustfs_policy::policy::action::Action::S3Action(GetObjectAction)].into_iter().collect()),
+                resources: ResourceSet::default(), // Empty Resource
+                not_resources: ResourceSet(vec!["arn:aws:s3:::mybucket/private/*".try_into().unwrap()].into_iter().collect()),
+                ..Default::default()
+            }
+        ],
+        ..Default::default()
+    },
+    ArgsBuilder{
+        account: "Q3AM3UQ867SPQQA43P2F".into(),
+        action: "s3:GetObject".into(),
+        bucket: "mybucket".into(),
+        object: "private/secret.txt".into(),
+        ..Default::default()
+    } => false;
+    "notresource_denies_access_in_blacklist"
+)]
 fn policy_is_allowed(policy: Policy, args: ArgsBuilder) -> bool {
     pollster::block_on(policy.is_allowed(&Args {
         account: &args.account,