Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[ISSUE #1385]🚀Add UserInfo and AclInfo #1414

Merged
merged 1 commit into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rocketmq-remoting/src/protocol/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub mod get_consumer_listby_group_response_body;

pub mod consumer_connection;

pub mod acl_info;
pub mod check_client_request_body;
pub mod check_rocksdb_cqwrite_progress_response_body;
pub mod cluster_acl_version_info;
Expand All @@ -41,3 +42,4 @@ pub mod set_message_request_mode_request_body;
pub mod topic;
pub mod topic_info_wrapper;
pub mod unlock_batch_request_body;
pub mod user_info;
212 changes: 212 additions & 0 deletions rocketmq-remoting/src/protocol/body/acl_info.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
use cheetah_string::CheetahString;
use serde::Deserialize;
use serde::Serialize;

#[derive(Serialize, Deserialize, Debug, Default)]
#[serde(rename_all = "camelCase")]
pub struct PolicyEntryInfo {
pub resource: Option<CheetahString>,
pub actions: Option<CheetahString>,
pub source_ips: Option<Vec<CheetahString>>,
pub decision: Option<CheetahString>,
}
Comment on lines +21 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add documentation for PolicyEntryInfo struct and fields

Consider adding documentation comments to explain:

  • The purpose of PolicyEntryInfo
  • Valid values for the decision field (e.g., "allow", "deny")
  • Expected format for resource and actions
  • Constraints on source_ips if any
 #[derive(Serialize, Deserialize, Debug, Default)]
 #[serde(rename_all = "camelCase")]
+/// Represents an entry in an access control policy
+/// 
+/// # Fields
+/// * `resource` - The resource to which this policy applies
+/// * `actions` - Permitted or denied actions on the resource
+/// * `source_ips` - List of IP addresses this policy applies to
+/// * `decision` - Policy decision ("allow" or "deny")
 pub struct PolicyEntryInfo {
     pub resource: Option<CheetahString>,
     pub actions: Option<CheetahString>,
     pub source_ips: Option<Vec<CheetahString>>,
     pub decision: Option<CheetahString>,
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[derive(Serialize, Deserialize, Debug, Default)]
#[serde(rename_all = "camelCase")]
pub struct PolicyEntryInfo {
pub resource: Option<CheetahString>,
pub actions: Option<CheetahString>,
pub source_ips: Option<Vec<CheetahString>>,
pub decision: Option<CheetahString>,
}
#[derive(Serialize, Deserialize, Debug, Default)]
#[serde(rename_all = "camelCase")]
/// Represents an entry in an access control policy
///
/// # Fields
/// * `resource` - The resource to which this policy applies
/// * `actions` - Permitted or denied actions on the resource
/// * `source_ips` - List of IP addresses this policy applies to
/// * `decision` - Policy decision ("allow" or "deny")
pub struct PolicyEntryInfo {
pub resource: Option<CheetahString>,
pub actions: Option<CheetahString>,
pub source_ips: Option<Vec<CheetahString>>,
pub decision: Option<CheetahString>,
}


#[derive(Serialize, Deserialize, Debug, Default)]
#[serde(rename_all = "camelCase")]
pub struct PolicyInfo {
pub policy_type: Option<CheetahString>,
pub entries: Option<Vec<PolicyEntryInfo>>,
}

#[derive(Serialize, Deserialize, Debug, Default)]
#[serde(rename_all = "camelCase")]
pub struct AclInfo {
pub subject: Option<CheetahString>,
pub policies: Option<Vec<PolicyInfo>>,
}
Comment on lines +30 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add documentation for PolicyInfo and AclInfo structs

Similar to PolicyEntryInfo, these structs would benefit from documentation explaining their purpose and relationships.

Additionally, consider adding validation for the policy_type field values if there are specific allowed types.


#[cfg(test)]
mod tests {
use serde_json;

use super::*;

#[test]
fn policy_entry_info_default_values() {
let policy_entry = PolicyEntryInfo::default();
assert!(policy_entry.resource.is_none());
assert!(policy_entry.actions.is_none());
assert!(policy_entry.source_ips.is_none());
assert!(policy_entry.decision.is_none());
}

#[test]
fn policy_entry_info_with_values() {
let policy_entry = PolicyEntryInfo {
resource: Some(CheetahString::from("resource")),
actions: Some(CheetahString::from("actions")),
source_ips: Some(vec![CheetahString::from("192.168.1.1")]),
decision: Some(CheetahString::from("allow")),
};
assert_eq!(policy_entry.resource, Some(CheetahString::from("resource")));
assert_eq!(policy_entry.actions, Some(CheetahString::from("actions")));
assert_eq!(
policy_entry.source_ips,
Some(vec![CheetahString::from("192.168.1.1")])
);
assert_eq!(policy_entry.decision, Some(CheetahString::from("allow")));
}

#[test]
fn serialize_policy_entry_info() {
let policy_entry = PolicyEntryInfo {
resource: Some(CheetahString::from("resource")),
actions: Some(CheetahString::from("actions")),
source_ips: Some(vec![CheetahString::from("192.168.1.1")]),
decision: Some(CheetahString::from("allow")),
};
let serialized = serde_json::to_string(&policy_entry).unwrap();
assert!(serialized.contains("\"resource\":\"resource\""));
assert!(serialized.contains("\"actions\":\"actions\""));
assert!(serialized.contains("\"sourceIps\":[\"192.168.1.1\"]"));
assert!(serialized.contains("\"decision\":\"allow\""));
}

#[test]
fn deserialize_policy_entry_info() {
let json = r#"{
"resource": "resource",
"actions": "actions",
"sourceIps": ["192.168.1.1"],
"decision": "allow"
}"#;
let deserialized: PolicyEntryInfo = serde_json::from_str(json).unwrap();
assert_eq!(deserialized.resource, Some(CheetahString::from("resource")));
assert_eq!(deserialized.actions, Some(CheetahString::from("actions")));
assert_eq!(
deserialized.source_ips,
Some(vec![CheetahString::from("192.168.1.1")])
);
assert_eq!(deserialized.decision, Some(CheetahString::from("allow")));
}

#[test]
fn deserialize_policy_entry_info_missing_optional_fields() {
let json = r#"{}"#;
let deserialized: PolicyEntryInfo = serde_json::from_str(json).unwrap();
assert!(deserialized.resource.is_none());
assert!(deserialized.actions.is_none());
assert!(deserialized.source_ips.is_none());
assert!(deserialized.decision.is_none());
}

#[test]
fn policy_info_default_values() {
let policy_info = PolicyInfo::default();
assert!(policy_info.policy_type.is_none());
assert!(policy_info.entries.is_none());
}

#[test]
fn policy_info_with_values() {
let policy_info = PolicyInfo {
policy_type: Some(CheetahString::from("type")),
entries: Some(vec![PolicyEntryInfo::default()]),
};
assert_eq!(policy_info.policy_type, Some(CheetahString::from("type")));
assert_eq!(policy_info.entries.unwrap().len(), 1);
}

#[test]
fn serialize_policy_info() {
let policy_info = PolicyInfo {
policy_type: Some(CheetahString::from("type")),
entries: Some(vec![PolicyEntryInfo::default()]),
};
let serialized = serde_json::to_string(&policy_info).unwrap();
assert!(serialized.contains("\"policyType\":\"type\""));
assert!(serialized.contains("\"entries\":["));
}

#[test]
fn deserialize_policy_info() {
let json = r#"{
"policyType": "type",
"entries": [{}]
}"#;
let deserialized: PolicyInfo = serde_json::from_str(json).unwrap();
assert_eq!(deserialized.policy_type, Some(CheetahString::from("type")));
assert_eq!(deserialized.entries.unwrap().len(), 1);
}

#[test]
fn deserialize_policy_info_missing_optional_fields() {
let json = r#"{}"#;
let deserialized: PolicyInfo = serde_json::from_str(json).unwrap();
assert!(deserialized.policy_type.is_none());
assert!(deserialized.entries.is_none());
}

#[test]
fn acl_info_default_values() {
let acl_info = AclInfo::default();
assert!(acl_info.subject.is_none());
assert!(acl_info.policies.is_none());
}

#[test]
fn acl_info_with_values() {
let acl_info = AclInfo {
subject: Some(CheetahString::from("subject")),
policies: Some(vec![PolicyInfo::default()]),
};
assert_eq!(acl_info.subject, Some(CheetahString::from("subject")));
assert_eq!(acl_info.policies.unwrap().len(), 1);
}

#[test]
fn serialize_acl_info() {
let acl_info = AclInfo {
subject: Some(CheetahString::from("subject")),
policies: Some(vec![PolicyInfo::default()]),
};
let serialized = serde_json::to_string(&acl_info).unwrap();
assert!(serialized.contains("\"subject\":\"subject\""));
assert!(serialized.contains("\"policies\":["));
}

#[test]
fn deserialize_acl_info() {
let json = r#"{
"subject": "subject",
"policies": [{}]
}"#;
let deserialized: AclInfo = serde_json::from_str(json).unwrap();
assert_eq!(deserialized.subject, Some(CheetahString::from("subject")));
assert_eq!(deserialized.policies.unwrap().len(), 1);
}

#[test]
fn deserialize_acl_info_missing_optional_fields() {
let json = r#"{}"#;
let deserialized: AclInfo = serde_json::from_str(json).unwrap();
assert!(deserialized.subject.is_none());
assert!(deserialized.policies.is_none());
}
}
131 changes: 131 additions & 0 deletions rocketmq-remoting/src/protocol/body/user_info.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
use std::fmt::Display;

use cheetah_string::CheetahString;
use serde::Deserialize;
use serde::Serialize;

#[derive(Serialize, Deserialize, Debug, Default)]
#[serde(rename_all = "camelCase")]
pub struct UserInfo {
pub username: Option<CheetahString>,
pub password: Option<CheetahString>,
pub user_type: Option<CheetahString>,
pub user_status: Option<CheetahString>,
}

impl Display for UserInfo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"UserInfo [username={}, password={}, user_type={}, user_status={}]",
self.username.as_ref().unwrap_or(&CheetahString::new()),
self.password.as_ref().unwrap_or(&CheetahString::new()),
self.user_type.as_ref().unwrap_or(&CheetahString::new()),
self.user_status.as_ref().unwrap_or(&CheetahString::new())
)
}
}
Comment on lines +32 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Secure the Display implementation to protect sensitive data.

The current Display implementation shows the password in clear text, which could lead to accidental exposure in logs.

Consider masking the password:

 impl Display for UserInfo {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(
             f,
             "UserInfo [username={}, password={}, user_type={}, user_status={}]",
             self.username.as_ref().unwrap_or(&CheetahString::new()),
-            self.password.as_ref().unwrap_or(&CheetahString::new()),
+            if self.password.is_some() { "****" } else { "" },
             self.user_type.as_ref().unwrap_or(&CheetahString::new()),
             self.user_status.as_ref().unwrap_or(&CheetahString::new())
         )
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
impl Display for UserInfo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"UserInfo [username={}, password={}, user_type={}, user_status={}]",
self.username.as_ref().unwrap_or(&CheetahString::new()),
self.password.as_ref().unwrap_or(&CheetahString::new()),
self.user_type.as_ref().unwrap_or(&CheetahString::new()),
self.user_status.as_ref().unwrap_or(&CheetahString::new())
)
}
}
impl Display for UserInfo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"UserInfo [username={}, password={}, user_type={}, user_status={}]",
self.username.as_ref().unwrap_or(&CheetahString::new()),
if self.password.is_some() { "****" } else { "" },
self.user_type.as_ref().unwrap_or(&CheetahString::new()),
self.user_status.as_ref().unwrap_or(&CheetahString::new())
)
}
}


#[cfg(test)]
mod tests {
use serde_json;

use super::*;

#[test]
fn user_info_default_values() {
let user_info = UserInfo::default();
assert!(user_info.username.is_none());
assert!(user_info.password.is_none());
assert!(user_info.user_type.is_none());
assert!(user_info.user_status.is_none());
}

#[test]
fn user_info_with_values() {
let user_info = UserInfo {
username: Some(CheetahString::from("user")),
password: Some(CheetahString::from("pass")),
user_type: Some(CheetahString::from("admin")),
user_status: Some(CheetahString::from("active")),
};
assert_eq!(user_info.username, Some(CheetahString::from("user")));
assert_eq!(user_info.password, Some(CheetahString::from("pass")));
assert_eq!(user_info.user_type, Some(CheetahString::from("admin")));
assert_eq!(user_info.user_status, Some(CheetahString::from("active")));
}

#[test]
fn serialize_user_info() {
let user_info = UserInfo {
username: Some(CheetahString::from("user")),
password: Some(CheetahString::from("pass")),
user_type: Some(CheetahString::from("admin")),
user_status: Some(CheetahString::from("active")),
};
let serialized = serde_json::to_string(&user_info).unwrap();
assert!(serialized.contains("\"username\":\"user\""));
assert!(serialized.contains("\"password\":\"pass\""));
assert!(serialized.contains("\"userType\":\"admin\""));
assert!(serialized.contains("\"userStatus\":\"active\""));
}

#[test]
fn deserialize_user_info() {
let json = r#"{
"username": "user",
"password": "pass",
"userType": "admin",
"userStatus": "active"
}"#;
let deserialized: UserInfo = serde_json::from_str(json).unwrap();
assert_eq!(deserialized.username, Some(CheetahString::from("user")));
assert_eq!(deserialized.password, Some(CheetahString::from("pass")));
assert_eq!(deserialized.user_type, Some(CheetahString::from("admin")));
assert_eq!(
deserialized.user_status,
Some(CheetahString::from("active"))
);
}

#[test]
fn deserialize_user_info_missing_optional_fields() {
let json = r#"{}"#;
let deserialized: UserInfo = serde_json::from_str(json).unwrap();
assert!(deserialized.username.is_none());
assert!(deserialized.password.is_none());
assert!(deserialized.user_type.is_none());
assert!(deserialized.user_status.is_none());
}

#[test]
fn display_user_info() {
let user_info = UserInfo {
username: Some(CheetahString::from("user")),
password: Some(CheetahString::from("pass")),
user_type: Some(CheetahString::from("admin")),
user_status: Some(CheetahString::from("active")),
};
let display = format!("{}", user_info);
assert_eq!(
display,
"UserInfo [username=user, password=pass, user_type=admin, user_status=active]"
);
}
}
Loading