mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-07 05:43:14 +00:00
fix(site-replication): translate policy mapping userType at MinIO wire boundary (#5751)
* test(site-replication): pin MinIO IAMUserType wire semantics for policy mappings Red tests for P0-4: MinIO peers send SRPolicyMapping.UserType using the madmin IAMUserType table (unknown=-1, regUser=0, stsUser=1, svcUser=2), while RustFS deserializes the field as u64 and decodes it with the internal RPC table (None=0, Svc=1, Sts=2, Reg=3). - userType -1 (MinIO group mappings) fails to deserialize, rejecting the whole IAM item: group mappings never sync from MinIO. - stsUser=1 decodes as Svc, landing federated STS mappings under the wrong prefix and silently dropping their effect. * fix(site-replication): translate policy mapping userType at MinIO wire boundary SRPolicyMapping.userType travels on the wire using MinIO's IAMUserType table (unknown=-1, regUser=0, stsUser=1, svcUser=2), but RustFS stored the field as u64 and reused the internal RPC encoding UserType::to_u64/from_u64 (None=0, Svc=1, Sts=2, Reg=3) at the site replication boundary. Consequences: MinIO group mappings (userType -1) failed to deserialize and the whole IAM item was rejected, and MinIO STS mappings (1) were stored as service-account mappings, silently dropping federated users' policies. - Widen SRPolicyMapping.user_type and SRCredInfo.iam_user_type to i64 so MinIO's -1 deserializes. - Add sr_wire_user_type / user_type_from_sr_wire in rustfs-iam as the dedicated SR wire codec: MinIO table on both directions, groups always encoded as 0, and wire value 3 kept forever as an alias for Reg so mappings from pre-fix RustFS peers still decode; unknown values fail closed. - Route the SR inbound (apply_iam_item) and outbound (mapped_policy_to_sr_mapping, policy-mapping change hooks) paths through the codec. The internal UserType::to_u64/from_u64 encoding is untouched: it is the intra-cluster node RPC contract and changing it would break rolling restarts. Outbound compatibility with old RustFS peers is preserved because UserType::None and Reg share the users prefix in get_mapped_policy_path, so wire 0 lands in the same location Reg=3 did.
This commit is contained in:
唐小鸭
+119
-1
@@ -139,6 +139,59 @@ impl UserType {
|
||||
}
|
||||
}
|
||||
|
||||
/// Encode a [`UserType`] as the site-replication wire value for
|
||||
/// `SRPolicyMapping.userType` / `SRCredInfo.iamUserType`.
|
||||
///
|
||||
/// The wire uses MinIO's `IAMUserType` table (cmd/iam.go):
|
||||
///
|
||||
/// | wire | MinIO meaning |
|
||||
/// |------|---------------|
|
||||
/// | -1 | unknown |
|
||||
/// | 0 | regUser |
|
||||
/// | 1 | stsUser |
|
||||
/// | 2 | svcUser |
|
||||
///
|
||||
/// This is deliberately distinct from the internal encoding
|
||||
/// [`UserType::to_u64`]/[`UserType::from_u64`] (None=0, Svc=1, Sts=2, Reg=3),
|
||||
/// which is used by intra-cluster node RPC and must never change (a rolling
|
||||
/// restart mixes old and new nodes on that RPC). Do not "unify" the two
|
||||
/// tables: internal values on the SR wire mislabel users on MinIO peers.
|
||||
///
|
||||
/// Group mappings always encode as 0: MinIO routes group mappings by the
|
||||
/// `isGroup` flag (userType is effectively ignored), and pre-fix RustFS peers
|
||||
/// sent 0 for groups, so 0 is the one value every peer generation accepts.
|
||||
pub fn sr_wire_user_type(user_type: UserType, is_group: bool) -> i64 {
|
||||
if is_group {
|
||||
return 0;
|
||||
}
|
||||
match user_type {
|
||||
UserType::Reg | UserType::None => 0,
|
||||
UserType::Sts => 1,
|
||||
UserType::Svc => 2,
|
||||
}
|
||||
}
|
||||
|
||||
/// Decode a site-replication wire `userType` value (see [`sr_wire_user_type`]
|
||||
/// for the table) into a [`UserType`].
|
||||
///
|
||||
/// - `-1` (MinIO unknown, sent for group mappings) maps to [`UserType::None`];
|
||||
/// `policy_db_set` routes group items by `is_group`, and for non-group items
|
||||
/// `None` shares the users prefix with `Reg`.
|
||||
/// - `3` is a permanent alias for [`UserType::Reg`]: pre-fix RustFS peers sent
|
||||
/// the internal encoding (`Reg.to_u64() == 3`) on the wire. Keep it forever
|
||||
/// for mixed-version site replication; do not remove.
|
||||
/// - Anything else is unknown and rejected (`None`), so callers fail closed.
|
||||
pub fn user_type_from_sr_wire(v: i64) -> Option<UserType> {
|
||||
match v {
|
||||
-1 => Some(UserType::None),
|
||||
0 => Some(UserType::Reg),
|
||||
1 => Some(UserType::Sts),
|
||||
2 => Some(UserType::Svc),
|
||||
3 => Some(UserType::Reg),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone)]
|
||||
pub struct MappedPolicy {
|
||||
pub version: i64,
|
||||
@@ -214,7 +267,72 @@ impl GroupInfo {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{GroupInfo, MappedPolicy};
|
||||
use super::{GroupInfo, MappedPolicy, UserType, sr_wire_user_type, user_type_from_sr_wire};
|
||||
|
||||
/// Site-replication inbound decode of `SRPolicyMapping.userType` must
|
||||
/// follow MinIO IAMUserType wire semantics (cmd/iam.go): stsUser = 1.
|
||||
/// The internal `UserType::from_u64` table maps 1 to Svc — reusing it at
|
||||
/// the SR boundary lands federated STS mappings under the wrong prefix
|
||||
/// and silently drops their effect.
|
||||
#[test]
|
||||
fn sr_inbound_decodes_minio_sts_wire_value_as_sts() {
|
||||
assert_eq!(user_type_from_sr_wire(1), Some(UserType::Sts));
|
||||
}
|
||||
|
||||
/// Wire-constant contract: literal MinIO IAMUserType values (cmd/iam.go).
|
||||
/// WARNING: these literals are the cross-vendor wire format. Never "tidy"
|
||||
/// them to match `UserType::to_u64`/`from_u64` — that internal table
|
||||
/// (None=0, Svc=1, Sts=2, Reg=3) belongs to intra-cluster node RPC only.
|
||||
#[test]
|
||||
fn sr_wire_decode_matches_minio_iam_user_type_table() {
|
||||
assert_eq!(user_type_from_sr_wire(-1), Some(UserType::None)); // MinIO unknown (group mappings)
|
||||
assert_eq!(user_type_from_sr_wire(0), Some(UserType::Reg)); // MinIO regUser
|
||||
assert_eq!(user_type_from_sr_wire(1), Some(UserType::Sts)); // MinIO stsUser
|
||||
assert_eq!(user_type_from_sr_wire(2), Some(UserType::Svc)); // MinIO svcUser
|
||||
// Permanent alias: pre-fix RustFS peers sent internal Reg=3 on the wire.
|
||||
assert_eq!(user_type_from_sr_wire(3), Some(UserType::Reg));
|
||||
// Unknown values fail closed.
|
||||
assert_eq!(user_type_from_sr_wire(4), None);
|
||||
assert_eq!(user_type_from_sr_wire(-2), None);
|
||||
}
|
||||
|
||||
/// Wire-constant contract for the outbound direction.
|
||||
#[test]
|
||||
fn sr_wire_encode_matches_minio_iam_user_type_table() {
|
||||
assert_eq!(sr_wire_user_type(UserType::Reg, false), 0); // MinIO regUser
|
||||
assert_eq!(sr_wire_user_type(UserType::Sts, false), 1); // MinIO stsUser
|
||||
assert_eq!(sr_wire_user_type(UserType::Svc, false), 2); // MinIO svcUser
|
||||
assert_eq!(sr_wire_user_type(UserType::None, false), 0);
|
||||
// Group mappings always go out as 0 — the value both MinIO (routes by
|
||||
// isGroup) and pre-fix RustFS peers accept.
|
||||
for ut in [UserType::Reg, UserType::Sts, UserType::Svc, UserType::None] {
|
||||
assert_eq!(sr_wire_user_type(ut, true), 0);
|
||||
}
|
||||
}
|
||||
|
||||
/// Mixed-version matrix: every value a peer generation can emit decodes to
|
||||
/// a `UserType` the receiver stores correctly.
|
||||
#[test]
|
||||
fn sr_wire_round_trip_covers_old_rustfs_and_minio_peers() {
|
||||
// Old RustFS outbound: user mappings as internal Reg=3, groups as 0.
|
||||
assert_eq!(user_type_from_sr_wire(3), Some(UserType::Reg));
|
||||
assert_eq!(user_type_from_sr_wire(0), Some(UserType::Reg));
|
||||
// New RustFS outbound decodes on its own kind (self round-trip).
|
||||
for (ut, is_group) in [
|
||||
(UserType::Reg, false),
|
||||
(UserType::Sts, false),
|
||||
(UserType::Svc, false),
|
||||
(UserType::None, true),
|
||||
] {
|
||||
assert!(user_type_from_sr_wire(sr_wire_user_type(ut, is_group)).is_some());
|
||||
}
|
||||
// Internal RPC encoding is untouched (rolling-restart contract).
|
||||
assert_eq!(UserType::None.to_u64(), 0);
|
||||
assert_eq!(UserType::Svc.to_u64(), 1);
|
||||
assert_eq!(UserType::Sts.to_u64(), 2);
|
||||
assert_eq!(UserType::Reg.to_u64(), 3);
|
||||
assert_eq!(UserType::from_u64(1), Some(UserType::Svc));
|
||||
}
|
||||
|
||||
/// uses RFC3339 for updatedAt. MappedPolicy must serialize as RFC3339.
|
||||
#[test]
|
||||
|
||||
@@ -171,8 +171,12 @@ impl fmt::Debug for PeerInfo {
|
||||
pub struct SRPolicyMapping {
|
||||
#[serde(rename = "userOrGroup", default)]
|
||||
pub user_or_group: String,
|
||||
/// MinIO IAMUserType wire value (cmd/iam.go): unknown = -1, regUser = 0,
|
||||
/// stsUser = 1, svcUser = 2. Signed because MinIO sends -1 for group
|
||||
/// mappings. This is NOT the RustFS-internal `UserType` encoding; translate
|
||||
/// at the boundary with `rustfs_iam::store::{sr_wire_user_type, user_type_from_sr_wire}`.
|
||||
#[serde(rename = "userType", default)]
|
||||
pub user_type: u64,
|
||||
pub user_type: i64,
|
||||
#[serde(rename = "isGroup", default)]
|
||||
pub is_group: bool,
|
||||
#[serde(default)]
|
||||
@@ -330,8 +334,10 @@ pub struct SRSvcAccChange {
|
||||
pub struct SRCredInfo {
|
||||
#[serde(rename = "accessKey", default)]
|
||||
pub access_key: String,
|
||||
/// MinIO IAMUserType wire value (same table as `SRPolicyMapping::user_type`);
|
||||
/// signed because MinIO's unknown is -1.
|
||||
#[serde(rename = "iamUserType", default)]
|
||||
pub iam_user_type: u64,
|
||||
pub iam_user_type: i64,
|
||||
#[serde(rename = "isDeleteReq", default)]
|
||||
pub is_delete_req: bool,
|
||||
#[serde(rename = "userIdentityJSON", default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -1399,7 +1405,7 @@ pub struct SiteNetPerfResult {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{PeerInfo, PeerSite, SRInfo, SRResyncOpStatus};
|
||||
use super::{PeerInfo, PeerSite, SRCredInfo, SRInfo, SRPolicyMapping, SRResyncOpStatus};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
const TEST_CA_CERT: &str = "-----BEGIN CERTIFICATE-----\ntest-ca\n-----END CERTIFICATE-----";
|
||||
@@ -1500,6 +1506,35 @@ mod tests {
|
||||
assert!(peer_debug.contains("has_custom_ca: true"));
|
||||
}
|
||||
|
||||
/// MinIO IAMUserType wire semantics (cmd/iam.go): unknown = -1,
|
||||
/// regUser = 0, stsUser = 1, svcUser = 2. MinIO group policy mappings
|
||||
/// arrive with `userType: -1`; the wire field must accept negatives.
|
||||
#[test]
|
||||
fn sr_policy_mapping_accepts_minio_negative_user_type() {
|
||||
let mapping: SRPolicyMapping = serde_json::from_value(json!({
|
||||
"userOrGroup": "devs",
|
||||
"userType": -1,
|
||||
"isGroup": true,
|
||||
"policy": "readwrite"
|
||||
}))
|
||||
.expect("MinIO group mapping with userType -1 must deserialize");
|
||||
assert_eq!(mapping.user_type, -1);
|
||||
assert!(mapping.is_group);
|
||||
assert_eq!(mapping.policy, "readwrite");
|
||||
}
|
||||
|
||||
/// Same IAMUserType family as SRPolicyMapping: MinIO may send -1 (unknown).
|
||||
#[test]
|
||||
fn sr_cred_info_accepts_minio_negative_iam_user_type() {
|
||||
let cred: SRCredInfo = serde_json::from_value(json!({
|
||||
"accessKey": "replicated-user",
|
||||
"iamUserType": -1
|
||||
}))
|
||||
.expect("SRCredInfo with iamUserType -1 must deserialize");
|
||||
assert_eq!(cred.iam_user_type, -1);
|
||||
assert_eq!(cred.access_key, "replicated-user");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resync_status_legacy_json_defaults_new_lifecycle_fields() {
|
||||
let legacy_json = json!({
|
||||
|
||||
@@ -571,7 +571,7 @@ impl Operation for SetPolicyForUserOrGroup {
|
||||
r#type: "policy-mapping".to_string(),
|
||||
policy_mapping: Some(SRPolicyMapping {
|
||||
user_or_group: query.user_or_group.clone(),
|
||||
user_type: rustfs_iam::store::UserType::Reg.to_u64(),
|
||||
user_type: rustfs_iam::store::sr_wire_user_type(rustfs_iam::store::UserType::Reg, query.is_group),
|
||||
is_group: query.is_group,
|
||||
policy: query.policy_name.clone(),
|
||||
updated_at: Some(updated_at),
|
||||
@@ -1060,7 +1060,7 @@ pub(crate) async fn handle_builtin_policy_association(
|
||||
r#type: "policy-mapping".to_string(),
|
||||
policy_mapping: Some(SRPolicyMapping {
|
||||
user_or_group: target_name.clone(),
|
||||
user_type: rustfs_iam::store::UserType::Reg.to_u64(),
|
||||
user_type: rustfs_iam::store::sr_wire_user_type(rustfs_iam::store::UserType::Reg, is_group),
|
||||
is_group,
|
||||
policy: updated_policies.join(","),
|
||||
updated_at: Some(updated_at),
|
||||
|
||||
@@ -63,7 +63,7 @@ use rustfs_config::{
|
||||
};
|
||||
use rustfs_iam::error::is_err_no_such_service_account;
|
||||
use rustfs_iam::federation::OIDC_VIRTUAL_PARENT_CLAIM;
|
||||
use rustfs_iam::store::{MappedPolicy, UserType};
|
||||
use rustfs_iam::store::{MappedPolicy, UserType, sr_wire_user_type, user_type_from_sr_wire};
|
||||
use rustfs_iam::sys::{
|
||||
NewServiceAccountOpts, SITE_REPLICATOR_SERVICE_ACCOUNT, UpdateServiceAccountOpts, get_claims_from_token_with_secret,
|
||||
};
|
||||
@@ -4303,7 +4303,7 @@ fn local_idp_settings() -> IDPSettings {
|
||||
fn mapped_policy_to_sr_mapping(name: String, is_group: bool, user_type: UserType, mapping: MappedPolicy) -> SRPolicyMapping {
|
||||
SRPolicyMapping {
|
||||
user_or_group: name,
|
||||
user_type: user_type.to_u64(),
|
||||
user_type: sr_wire_user_type(user_type, is_group),
|
||||
is_group,
|
||||
policy: mapping.policies,
|
||||
updated_at: Some(mapping.update_at),
|
||||
@@ -7824,7 +7824,8 @@ async fn apply_iam_item(item: SRIAMItem) -> S3Result<()> {
|
||||
let Some(mapping) = item.policy_mapping else {
|
||||
return Err(s3_error!(InvalidRequest, "policyMapping is required"));
|
||||
};
|
||||
let user_type = UserType::from_u64(mapping.user_type).ok_or_else(|| s3_error!(InvalidRequest, "invalid userType"))?;
|
||||
let user_type =
|
||||
user_type_from_sr_wire(mapping.user_type).ok_or_else(|| s3_error!(InvalidRequest, "invalid userType"))?;
|
||||
iam_sys
|
||||
.policy_db_set(&mapping.user_or_group, user_type, mapping.is_group, &mapping.policy)
|
||||
.await
|
||||
@@ -11910,7 +11911,7 @@ mod tests {
|
||||
"alice".to_string(),
|
||||
SRPolicyMapping {
|
||||
user_or_group: "alice".to_string(),
|
||||
user_type: UserType::Reg.to_u64(),
|
||||
user_type: sr_wire_user_type(UserType::Reg, false),
|
||||
policy: "readwrite".to_string(),
|
||||
updated_at: Some(OffsetDateTime::UNIX_EPOCH),
|
||||
..Default::default()
|
||||
|
||||
Reference in New Issue
Block a user