fix: address PR review feedback

- Add authorization check for get_bucket_logging in access layer,
  ensuring AccessDenied is returned before NotImplemented for
  unauthorized callers
- Include Service principals in Principal::is_match so policies with
  Service-only principals can participate in statement evaluation
- Add missing NumericLessThanEquals and DateLessThan deserialization
  arms in Condition::from_deserializer
- Use recursive to_key_with_suffix() in IfExists serialization to
  correctly handle hypothetical nested IfExists conditions

Made-with: Cursor
This commit is contained in:
overtrue
2026-03-01 10:06:51 +08:00
parent df99f6eab2
commit e1d8d6ee68
3 changed files with 13 additions and 9 deletions

View File

@@ -78,11 +78,13 @@ impl Condition {
"NumericEquals" => Self::NumericEquals(d.next_value()?),
"NumericNotEquals" => Self::NumericNotEquals(d.next_value()?),
"NumericLessThan" => Self::NumericLessThan(d.next_value()?),
"NumericLessThanEquals" => Self::NumericLessThanEquals(d.next_value()?),
"NumericGreaterThan" => Self::NumericGreaterThan(d.next_value()?),
"NumericGreaterThanIfExists" => Self::NumericGreaterThanIfExists(d.next_value()?),
"NumericGreaterThanEquals" => Self::NumericGreaterThanEquals(d.next_value()?),
"DateEquals" => Self::DateEquals(d.next_value()?),
"DateNotEquals" => Self::DateNotEquals(d.next_value()?),
"DateLessThan" => Self::DateLessThan(d.next_value()?),
"DateLessThanEquals" => Self::DateLessThanEquals(d.next_value()?),
"DateGreaterThan" => Self::DateGreaterThan(d.next_value()?),
"DateGreaterThanEquals" => Self::DateGreaterThanEquals(d.next_value()?),
@@ -131,7 +133,7 @@ impl Condition {
pub fn to_key_with_suffix(&self) -> String {
match self {
Condition::IfExists(inner) => format!("{}IfExists", inner.to_key()),
Condition::IfExists(inner) => format!("{}IfExists", inner.to_key_with_suffix()),
_ => self.to_key().to_owned(),
}
}

View File

@@ -130,9 +130,11 @@ impl Principal {
return true;
}
}
// Service principals (e.g., logging.s3.amazonaws.com) allow internal
// AWS services. Treat them as non-matching for user requests — they
// only apply to service-initiated actions.
for pattern in self.service.iter() {
if wildcard::is_simple_match(pattern, principal) {
return true;
}
}
false
}
}

View File

@@ -935,11 +935,11 @@ impl S3Access for FS {
authorize_request(req, Action::S3Action(S3Action::GetBucketLocationAction)).await
}
/// Checks whether the GetBucketLogging request has accesses to the resources.
///
/// This method returns `Ok(())` by default.
async fn get_bucket_logging(&self, _req: &mut S3Request<GetBucketLoggingInput>) -> S3Result<()> {
Ok(())
async fn get_bucket_logging(&self, req: &mut S3Request<GetBucketLoggingInput>) -> S3Result<()> {
let req_info = ext_req_info_mut(&mut req.extensions)?;
req_info.bucket = Some(req.input.bucket.clone());
authorize_request(req, Action::S3Action(S3Action::GetBucketLoggingAction)).await
}
/// Checks whether the GetBucketMetricsConfiguration request has accesses to the resources.