mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-27 15:37:02 +00:00
fix(auth): preserve IAMAuth clone and correct missing-key error (#2123)
This commit is contained in:
@@ -21,7 +21,6 @@ use matchit::Params;
|
|||||||
use rustfs_common::heal_channel::HealOpts;
|
use rustfs_common::heal_channel::HealOpts;
|
||||||
use rustfs_config::MAX_HEAL_REQUEST_SIZE;
|
use rustfs_config::MAX_HEAL_REQUEST_SIZE;
|
||||||
use rustfs_ecstore::bucket::utils::is_valid_object_prefix;
|
use rustfs_ecstore::bucket::utils::is_valid_object_prefix;
|
||||||
use rustfs_ecstore::error::StorageError;
|
|
||||||
use rustfs_ecstore::store_utils::is_reserved_or_invalid_bucket;
|
use rustfs_ecstore::store_utils::is_reserved_or_invalid_bucket;
|
||||||
use rustfs_utils::path::path_join;
|
use rustfs_utils::path::path_join;
|
||||||
use s3s::{Body, S3Request, S3Response, S3Result, s3_error};
|
use s3s::{Body, S3Request, S3Response, S3Result, s3_error};
|
||||||
@@ -141,8 +140,7 @@ impl Operation for HealHandler {
|
|||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
struct HealResp {
|
struct HealResp {
|
||||||
resp_bytes: Vec<u8>,
|
resp_bytes: Vec<u8>,
|
||||||
_api_err: Option<StorageError>,
|
api_err: Option<String>,
|
||||||
_err_body: String,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let heal_path = path_join(&[PathBuf::from(hip.bucket.clone()), PathBuf::from(hip.obj_prefix.clone())]);
|
let heal_path = path_join(&[PathBuf::from(hip.bucket.clone()), PathBuf::from(hip.obj_prefix.clone())]);
|
||||||
@@ -156,7 +154,6 @@ impl Operation for HealHandler {
|
|||||||
spawn(async move {
|
spawn(async move {
|
||||||
match rustfs_common::heal_channel::query_heal_status(heal_path_str, client_token).await {
|
match rustfs_common::heal_channel::query_heal_status(heal_path_str, client_token).await {
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
// TODO: Get actual response from channel
|
|
||||||
let _ = tx_clone
|
let _ = tx_clone
|
||||||
.send(HealResp {
|
.send(HealResp {
|
||||||
resp_bytes: vec![],
|
resp_bytes: vec![],
|
||||||
@@ -167,7 +164,7 @@ impl Operation for HealHandler {
|
|||||||
Err(e) => {
|
Err(e) => {
|
||||||
let _ = tx_clone
|
let _ = tx_clone
|
||||||
.send(HealResp {
|
.send(HealResp {
|
||||||
_api_err: Some(StorageError::other(e)),
|
api_err: Some(e),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
@@ -181,7 +178,6 @@ impl Operation for HealHandler {
|
|||||||
spawn(async move {
|
spawn(async move {
|
||||||
match rustfs_common::heal_channel::cancel_heal_task(heal_path_str).await {
|
match rustfs_common::heal_channel::cancel_heal_task(heal_path_str).await {
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
// TODO: Get actual response from channel
|
|
||||||
let _ = tx_clone
|
let _ = tx_clone
|
||||||
.send(HealResp {
|
.send(HealResp {
|
||||||
resp_bytes: vec![],
|
resp_bytes: vec![],
|
||||||
@@ -192,7 +188,7 @@ impl Operation for HealHandler {
|
|||||||
Err(e) => {
|
Err(e) => {
|
||||||
let _ = tx_clone
|
let _ = tx_clone
|
||||||
.send(HealResp {
|
.send(HealResp {
|
||||||
_api_err: Some(StorageError::other(e)),
|
api_err: Some(e),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
@@ -229,7 +225,7 @@ impl Operation for HealHandler {
|
|||||||
// Error - send error response
|
// Error - send error response
|
||||||
let _ = tx_clone
|
let _ = tx_clone
|
||||||
.send(HealResp {
|
.send(HealResp {
|
||||||
_api_err: Some(StorageError::other(e)),
|
api_err: Some(e),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
@@ -239,7 +235,12 @@ impl Operation for HealHandler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
match rx.recv().await {
|
match rx.recv().await {
|
||||||
Some(result) => Ok(S3Response::new((StatusCode::OK, Body::from(result.resp_bytes)))),
|
Some(result) => {
|
||||||
|
if let Some(api_err) = result.api_err {
|
||||||
|
return Err(s3_error!(InternalError, "{api_err}"));
|
||||||
|
}
|
||||||
|
Ok(S3Response::new((StatusCode::OK, Body::from(result.resp_bytes))))
|
||||||
|
}
|
||||||
None => Ok(S3Response::new((StatusCode::INTERNAL_SERVER_ERROR, Body::from(vec![])))),
|
None => Ok(S3Response::new((StatusCode::INTERNAL_SERVER_ERROR, Body::from(vec![])))),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -85,6 +85,7 @@ fn test_register_routes_cover_representative_admin_paths() {
|
|||||||
assert_route(&router, Method::GET, &admin_path("/v3/rebalance/status"));
|
assert_route(&router, Method::GET, &admin_path("/v3/rebalance/status"));
|
||||||
assert_route(&router, Method::POST, &admin_path("/v3/heal/test-bucket"));
|
assert_route(&router, Method::POST, &admin_path("/v3/heal/test-bucket"));
|
||||||
assert_route(&router, Method::POST, &admin_path("/v3/heal/test-bucket/prefix"));
|
assert_route(&router, Method::POST, &admin_path("/v3/heal/test-bucket/prefix"));
|
||||||
|
assert_route(&router, Method::POST, &admin_path("/v3/background-heal/status"));
|
||||||
|
|
||||||
assert_route(&router, Method::GET, &admin_path("/v3/tier"));
|
assert_route(&router, Method::GET, &admin_path("/v3/tier"));
|
||||||
assert_route(&router, Method::POST, &admin_path("/v3/tier/clear"));
|
assert_route(&router, Method::POST, &admin_path("/v3/tier/clear"));
|
||||||
|
|||||||
+33
-12
@@ -91,26 +91,32 @@ pub enum AuthType {
|
|||||||
StreamingUnsignedTrailer,
|
StreamingUnsignedTrailer,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
|
||||||
pub struct IAMAuth {
|
pub struct IAMAuth {
|
||||||
simple_auth: SimpleAuth,
|
simple_auth: SimpleAuth,
|
||||||
|
access_key: String,
|
||||||
|
secret_key: SecretKey,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Clone for IAMAuth {
|
impl Clone for IAMAuth {
|
||||||
fn clone(&self) -> Self {
|
fn clone(&self) -> Self {
|
||||||
// Since SimpleAuth doesn't implement Clone, we create a new one
|
|
||||||
// This is a simplified implementation - in a real scenario, you might need
|
|
||||||
// to store the credentials separately to properly clone
|
|
||||||
Self {
|
Self {
|
||||||
simple_auth: SimpleAuth::new(),
|
simple_auth: SimpleAuth::from_single(self.access_key.clone(), self.secret_key.clone()),
|
||||||
|
access_key: self.access_key.clone(),
|
||||||
|
secret_key: self.secret_key.clone(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl IAMAuth {
|
impl IAMAuth {
|
||||||
pub fn new(ak: impl Into<String>, sk: impl Into<SecretKey>) -> Self {
|
pub fn new(ak: impl Into<String>, sk: impl Into<SecretKey>) -> Self {
|
||||||
let simple_auth = SimpleAuth::from_single(ak, sk);
|
let access_key = ak.into();
|
||||||
Self { simple_auth }
|
let secret_key = sk.into();
|
||||||
|
let simple_auth = SimpleAuth::from_single(access_key.clone(), secret_key.clone());
|
||||||
|
Self {
|
||||||
|
simple_auth,
|
||||||
|
access_key,
|
||||||
|
secret_key,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -143,6 +149,10 @@ impl S3Auth for IAMAuth {
|
|||||||
return Ok(SecretKey::from(String::new()));
|
return Ok(SecretKey::from(String::new()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if access_key == self.access_key {
|
||||||
|
return Ok(self.secret_key.clone());
|
||||||
|
}
|
||||||
|
|
||||||
if let Ok(key) = self.simple_auth.get_secret_key(access_key).await {
|
if let Ok(key) = self.simple_auth.get_secret_key(access_key).await {
|
||||||
return Ok(key);
|
return Ok(key);
|
||||||
}
|
}
|
||||||
@@ -264,17 +274,19 @@ pub async fn check_key_valid(session_token: &str, access_key: &str) -> S3Result<
|
|||||||
.map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("check claims failed1 {e}")))?;
|
.map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("check claims failed1 {e}")))?;
|
||||||
|
|
||||||
if !ok {
|
if !ok {
|
||||||
if let Some(u) = u
|
let Some(u) = u else {
|
||||||
&& u.credentials.status == "off"
|
return Err(s3_error!(InvalidAccessKeyId, "check key failed"));
|
||||||
{
|
};
|
||||||
|
|
||||||
|
if u.credentials.status == "off" {
|
||||||
return Err(s3_error!(InvalidRequest, "ErrAccessKeyDisabled"));
|
return Err(s3_error!(InvalidRequest, "ErrAccessKeyDisabled"));
|
||||||
}
|
}
|
||||||
|
|
||||||
return Err(s3_error!(InvalidRequest, "ErrAccessKeyDisabled"));
|
return Err(s3_error!(InvalidRequest, "check key failed"));
|
||||||
}
|
}
|
||||||
|
|
||||||
let Some(u) = u else {
|
let Some(u) = u else {
|
||||||
return Err(s3_error!(InvalidRequest, "check key failed"));
|
return Err(s3_error!(InvalidAccessKeyId, "check key failed"));
|
||||||
};
|
};
|
||||||
|
|
||||||
cred = u.credentials;
|
cred = u.credentials;
|
||||||
@@ -899,6 +911,15 @@ mod tests {
|
|||||||
assert_eq!(size_of_val(&iam_auth), size_of::<IAMAuth>());
|
assert_eq!(size_of_val(&iam_auth), size_of::<IAMAuth>());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_iam_auth_clone_preserves_bootstrap_secret() {
|
||||||
|
let iam_auth = IAMAuth::new("test-ak", SecretKey::from("test-sk"));
|
||||||
|
let cloned = iam_auth.clone();
|
||||||
|
|
||||||
|
let secret = cloned.get_secret_key("test-ak").await;
|
||||||
|
assert!(secret.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_iam_auth_get_secret_key_empty_access_key() {
|
async fn test_iam_auth_get_secret_key_empty_access_key() {
|
||||||
let iam_auth = IAMAuth::new("test-ak", SecretKey::from("test-sk"));
|
let iam_auth = IAMAuth::new("test-ak", SecretKey::from("test-sk"));
|
||||||
|
|||||||
Reference in New Issue
Block a user