gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-07-28 00:58:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: admin permission check (#1783)

Browse Source 
Signed-off-by: GatewayJ <835269233@qq.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
GatewayJ
2026-02-25 11:58:30 +08:00
committed by GitHub
parent 0d9e5f1e93
commit 62b51b5649
6 changed files with 140 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/policy/src/error.rs
+6 -14
View File
@@ -192,7 +192,6 @@ mod tests {
match policy_error {
Error::Io(inner_io) => {
assert_eq!(inner_io.kind(), ErrorKind::PermissionDenied);
assert!(inner_io.to_string().contains("permission denied"));
}
_ => panic!("Expected Io variant"),
}
@@ -205,7 +204,6 @@ mod tests {
match policy_error {
Error::Io(io_error) => {
assert!(io_error.to_string().contains(custom_error));
assert_eq!(io_error.kind(), ErrorKind::Other);
}
_ => panic!("Expected Io variant"),
@@ -218,12 +216,9 @@ mod tests {
let crypto_error = rustfs_crypto::Error::ErrUnexpectedHeader;
let policy_error: Error = crypto_error.into();
match policy_error {
Error::CryptoError(_) => {
// Verify the conversion worked
assert!(policy_error.to_string().contains("crypto"));
}
_ => panic!("Expected CryptoError variant"),
match &policy_error {
Error::CryptoError(_) => {}
_ => panic!("Expected CryptoError variant, got {:?}", policy_error),
}
}
@@ -249,12 +244,9 @@ mod tests {
let jwt_error = jwt_result.unwrap_err();
let policy_error: Error = jwt_error.into();
match policy_error {
Error::JWTError(_) => {
// Verify the conversion worked
assert!(policy_error.to_string().contains("jwt err"));
}
_ => panic!("Expected JWTError variant"),
match &policy_error {
Error::JWTError(_) => {}
_ => panic!("Expected JWTError variant, got {:?}", policy_error),
}
}
crates/policy/src/policy.rs
+3
View File
@@ -48,6 +48,9 @@ pub enum Error {
#[error("both 'Action' and 'NotAction' are empty")]
NonAction,
#[error("'Action' and 'NotAction' cannot both be specified in the same statement")]
BothActionAndNotAction,
#[error("'Resource' is empty")]
NonResource,
crates/policy/src/policy/policy.rs
+104 -20
View File
@@ -1015,17 +1015,105 @@ mod test {
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
);
}
assert!(
matches!(result.as_ref().unwrap_err(), Error::PolicyError(IamError::BothResourceAndNotResource)),
"Error should be BothResourceAndNotResource, got: {:?}",
result.unwrap_err()
);
}
#[test]
fn test_statement_with_only_notaction_is_valid() {
// IAM allows a statement with only NotAction (no Action). Deserialization should accept
// missing "Action" (default to empty) and validate when exactly one of Action/NotAction is set.
let data = r#"
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"NotAction": ["s3:DeleteBucket", "s3:DeleteObject"],
"Resource": ["arn:aws:s3:::mybucket/*"]
}
]
}
"#;
let result = Policy::parse_config(data.as_bytes());
assert!(result.is_ok(), "Statement with only NotAction should be valid, got: {:?}", result.err());
let policy = result.unwrap();
assert_eq!(policy.statements.len(), 1);
assert!(policy.statements[0].actions.is_empty());
assert!(!policy.statements[0].not_actions.is_empty());
// Round-trip: serialization must omit empty Action so re-parse does not violate both-Action-and-NotAction
let json = serde_json::to_string(&policy).expect("Should serialize");
let round_tripped = Policy::parse_config(json.as_bytes());
assert!(
round_tripped.is_ok(),
"NotAction-only statement must round-trip without gaining Action: {}",
round_tripped.unwrap_err()
);
let parsed: serde_json::Value = serde_json::from_str(&json).expect("JSON valid");
let stmt = &parsed["Statement"][0];
assert!(
!stmt
.get("Action")
.is_some_and(|v| v.as_array().map(|a| a.is_empty()).unwrap_or(false)),
"Serialized JSON must not contain empty Action for NotAction-only statement"
);
}
#[test]
fn test_statement_with_both_action_and_notaction_is_invalid() {
// Test: A statement with both Action and NotAction returns BothActionAndNotAction error
let data = r#"
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["s3:GetObject"],
"NotAction": ["s3:DeleteObject"],
"Resource": ["arn:aws:s3:::mybucket/*"]
}
]
}
"#;
let result = Policy::parse_config(data.as_bytes());
assert!(result.is_err(), "Statement with both Action and NotAction should be invalid");
assert!(
matches!(result.as_ref().unwrap_err(), Error::PolicyError(IamError::BothActionAndNotAction)),
"Error should be BothActionAndNotAction, got: {:?}",
result.unwrap_err()
);
}
#[test]
fn test_statement_with_neither_action_nor_notaction_is_invalid() {
// Statement with both Action and NotAction omitted (both default to empty) fails validation.
let data = r#"
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Resource": ["arn:aws:s3:::mybucket/*"]
}
]
}
"#;
let result = Policy::parse_config(data.as_bytes());
assert!(result.is_err(), "Statement with neither Action nor NotAction should be invalid");
assert!(
matches!(result.as_ref().unwrap_err(), Error::PolicyError(IamError::NonAction)),
"Error should be NonAction, got: {:?}",
result.unwrap_err()
);
}
#[test]
@@ -1046,15 +1134,11 @@ mod test {
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
);
}
assert!(
matches!(result.as_ref().unwrap_err(), Error::PolicyError(IamError::NonResource)),
"Error should be NonResource, got: {:?}",
result.unwrap_err()
);
}
#[test]
crates/policy/src/policy/statement.rs
+14 -6
View File
@@ -26,13 +26,13 @@ pub struct Statement {
pub sid: ID,
#[serde(rename = "Effect")]
pub effect: Effect,
#[serde(rename = "Action")]
#[serde(rename = "Action", default, skip_serializing_if = "ActionSet::is_empty")]
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)]
#[serde(rename = "Resource", default, skip_serializing_if = "ResourceSet::is_empty")]
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)]
pub conditions: Functions,
@@ -151,6 +151,10 @@ impl Validator for Statement {
return Err(IamError::NonAction.into());
}
if !self.actions.is_empty() && !self.not_actions.is_empty() {
return Err(IamError::BothActionAndNotAction.into());
}
// 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());
@@ -190,11 +194,11 @@ pub struct BPStatement {
pub effect: Effect,
#[serde(rename = "Principal")]
pub principal: Principal,
#[serde(rename = "Action")]
#[serde(rename = "Action", default, skip_serializing_if = "ActionSet::is_empty")]
pub actions: ActionSet,
#[serde(rename = "NotAction", default, skip_serializing_if = "ActionSet::is_empty")]
pub not_actions: ActionSet,
#[serde(rename = "Resource", default)]
#[serde(rename = "Resource", default, skip_serializing_if = "ResourceSet::is_empty")]
pub resources: ResourceSet,
#[serde(rename = "NotResource", default, skip_serializing_if = "ResourceSet::is_empty")]
pub not_resources: ResourceSet,
@@ -252,6 +256,10 @@ impl Validator for BPStatement {
return Err(IamError::NonAction.into());
}
if !self.actions.is_empty() && !self.not_actions.is_empty() {
return Err(IamError::BothActionAndNotAction.into());
}
if self.resources.is_empty() && self.not_resources.is_empty() {
return Err(IamError::NonResource.into());
}
rustfs/src/admin/auth.rs
+12 -14
View File
@@ -52,15 +52,14 @@ pub async fn validate_admin_request(
remote_addr,
};
for action in actions {
match check_admin_request_auth(iam_store.clone(), &ctx, action, "", "").await {
Ok(_) => return Ok(()),
Err(_) => {
continue;
}
for action in &actions {
if check_admin_request_auth(iam_store.clone(), &ctx, *action, "", "")
.await
.is_ok()
{
return Ok(());
}
}
Err(s3_error!(AccessDenied, "Access Denied"))
}
@@ -113,14 +112,13 @@ pub async fn validate_admin_request_with_bucket(
remote_addr,
};
for action in actions {
match check_admin_request_auth(iam_store.clone(), &ctx, action, bucket, "").await {
Ok(_) => return Ok(()),
Err(_) => {
continue;
}
for action in &actions {
if check_admin_request_auth(iam_store.clone(), &ctx, *action, bucket, "")
.await
.is_ok()
{
return Ok(());
}
}
Err(s3_error!(AccessDenied, "Access Denied"))
}
rustfs/src/admin/handlers/policies.rs
+1 -2
View File
@@ -196,9 +196,8 @@ impl Operation for AddCannedPolicy {
})?;
if policy.version.is_empty() {
return Err(s3_error!(InvalidRequest, "policy version is empty"));
return Err(s3_error!(InvalidArgument, "policy version is empty"));
}
let Ok(iam_store) = rustfs_iam::get() else { return Err(s3_error!(InternalError, "iam not init")) };
iam_store.set_policy(&query.name, policy).await.map_err(|e| {