From 62088fc2ead63a8f5766a2132a9eb44596b76a49 Mon Sep 17 00:00:00 2001 From: lxl66566 Date: Thu, 18 Jul 2024 08:44:36 +0800 Subject: [PATCH] refactor(client)!: AuthClient::role_grant_permission Signed-off-by: lxl66566 --- crates/xline-client/examples/auth_role.rs | 14 +-- crates/xline-client/src/clients/auth.rs | 34 ++++-- crates/xline-client/src/types/auth.rs | 104 +++++++----------- crates/xline-client/tests/it/auth.rs | 56 +++++----- crates/xline-test-utils/src/lib.rs | 10 +- .../xlinectl/src/command/role/grant_perm.rs | 63 ++++++----- 6 files changed, 134 insertions(+), 147 deletions(-) diff --git a/crates/xline-client/examples/auth_role.rs b/crates/xline-client/examples/auth_role.rs index 70e146f72..a23e686a8 100644 --- a/crates/xline-client/examples/auth_role.rs +++ b/crates/xline-client/examples/auth_role.rs @@ -1,8 +1,6 @@ use anyhow::Result; use xline_client::{ - types::auth::{ - AuthRoleGrantPermissionRequest, AuthRoleRevokePermissionRequest, Permission, PermissionType, - }, + types::auth::{AuthRoleRevokePermissionRequest, PermissionType}, Client, ClientOptions, }; @@ -21,16 +19,10 @@ async fn main() -> Result<()> { // grant permissions to roles client - .role_grant_permission(AuthRoleGrantPermissionRequest::new( - "role1", - Permission::new(PermissionType::Read, "key1"), - )) + .role_grant_permission("role1", PermissionType::Read, "key1", None) .await?; client - .role_grant_permission(AuthRoleGrantPermissionRequest::new( - "role2", - Permission::new(PermissionType::Readwrite, "key2"), - )) + .role_grant_permission("role2", PermissionType::Readwrite, "key2", None) .await?; // list all roles and their permissions diff --git a/crates/xline-client/src/clients/auth.rs b/crates/xline-client/src/clients/auth.rs index b4f845e8a..44038cc28 100644 --- a/crates/xline-client/src/clients/auth.rs +++ b/crates/xline-client/src/clients/auth.rs @@ -13,7 +13,10 @@ use xlineapi::{ use crate::{ error::{Result, XlineClientError}, - types::auth::{AuthRoleGrantPermissionRequest, AuthRoleRevokePermissionRequest}, + types::{ + auth::{AuthRoleRevokePermissionRequest, Permission, PermissionType}, + range_end::RangeOption, + }, AuthService, CurpClient, }; @@ -660,7 +663,7 @@ impl AuthClient { /// /// ```no_run /// use xline_client::{ - /// types::auth::{AuthRoleGrantPermissionRequest, Permission, PermissionType}, + /// types::auth::{Permission, PermissionType}, /// Client, ClientOptions, /// }; /// use anyhow::Result; @@ -676,10 +679,12 @@ impl AuthClient { /// // add the role and key /// /// client - /// .role_grant_permission(AuthRoleGrantPermissionRequest::new( + /// .role_grant_permission( /// "role", - /// Permission::new(PermissionType::Read, "key"), - /// )) + /// PermissionType::Read, + /// "key", + /// None + /// ) /// .await?; /// /// Ok(()) @@ -688,14 +693,19 @@ impl AuthClient { #[inline] pub async fn role_grant_permission( &self, - request: AuthRoleGrantPermissionRequest, + name: impl Into, + perm_type: PermissionType, + perm_key: impl Into>, + range_option: Option, ) -> Result { - if request.inner.perm.is_none() { - return Err(XlineClientError::InvalidArgs(String::from( - "Permission not given", - ))); - } - self.handle_req(request.inner, false).await + self.handle_req( + xlineapi::AuthRoleGrantPermissionRequest { + name: name.into(), + perm: Some(Permission::new(perm_type, perm_key.into(), range_option).into()), + }, + false, + ) + .await } /// Revokes role permission. diff --git a/crates/xline-client/src/types/auth.rs b/crates/xline-client/src/types/auth.rs index ca10dc170..87291ee57 100644 --- a/crates/xline-client/src/types/auth.rs +++ b/crates/xline-client/src/types/auth.rs @@ -8,35 +8,7 @@ pub use xlineapi::{ AuthenticateResponse, Type as PermissionType, }; -/// Request for `AuthRoleGrantPermission` -#[derive(Debug, PartialEq)] -pub struct AuthRoleGrantPermissionRequest { - /// Inner request - pub(crate) inner: xlineapi::AuthRoleGrantPermissionRequest, -} - -impl AuthRoleGrantPermissionRequest { - /// Creates a new `AuthRoleGrantPermissionRequest` - /// - /// `role` is the name of the role to grant permission, - /// `perm` is the permission name to grant. - #[inline] - pub fn new(role: impl Into, perm: Permission) -> Self { - Self { - inner: xlineapi::AuthRoleGrantPermissionRequest { - name: role.into(), - perm: Some(perm.into()), - }, - } - } -} - -impl From for xlineapi::AuthRoleGrantPermissionRequest { - #[inline] - fn from(req: AuthRoleGrantPermissionRequest) -> Self { - req.inner - } -} +use super::range_end::RangeOption; /// Request for `AuthRoleRevokePermission` #[derive(Debug, PartialEq)] @@ -107,6 +79,8 @@ impl From for xlineapi::AuthRoleRevokePermissio pub struct Permission { /// The inner Permission inner: xlineapi::Permission, + /// The range option + range_option: Option, } impl Permission { @@ -114,55 +88,57 @@ impl Permission { /// /// `perm_type` is the permission type, /// `key` is the key to grant with the permission. + /// `range_option` is the range option of how to get `range_end` from key. #[inline] #[must_use] - pub fn new(perm_type: PermissionType, key: impl Into>) -> Self { - Self { - inner: xlineapi::Permission { - perm_type: perm_type.into(), - key: key.into(), - ..Default::default() - }, - } + pub fn new( + perm_type: PermissionType, + key: impl Into>, + range_option: Option, + ) -> Self { + Self::from((perm_type, key.into(), range_option)) } +} - /// If set, Xline will return all keys with the matching prefix +impl From for xlineapi::Permission { #[inline] - #[must_use] - pub fn with_prefix(mut self) -> Self { - if self.inner.key.is_empty() { - self.inner.key = vec![0]; - self.inner.range_end = vec![0]; - } else { - self.inner.range_end = KeyRange::get_prefix(&self.inner.key); - } - self + fn from(mut perm: Permission) -> Self { + perm.inner.range_end = perm + .range_option + .unwrap_or_default() + .get_range_end(&mut perm.inner.key); + perm.inner } +} - /// If set, Xline will return all keys that are equal or greater than the given key +impl PartialEq for Permission { #[inline] - #[must_use] - pub fn with_from_key(mut self) -> Self { - if self.inner.key.is_empty() { - self.inner.key = vec![0]; - } - self.inner.range_end = vec![0]; - self + fn eq(&self, other: &Self) -> bool { + self.inner == other.inner && self.range_option == other.range_option } +} - /// `range_end` is the upper bound on the requested range \[key,` range_en`d). - /// If `range_end` is '\0', the range is all keys >= key. +impl Eq for Permission {} + +impl From<(PermissionType, Vec, Option)> for Permission { #[inline] - #[must_use] - pub fn with_range_end(mut self, range_end: impl Into>) -> Self { - self.inner.range_end = range_end.into(); - self + fn from( + (perm_type, key, range_option): (PermissionType, Vec, Option), + ) -> Self { + Permission { + inner: xlineapi::Permission { + perm_type: perm_type.into(), + key, + ..Default::default() + }, + range_option, + } } } -impl From for xlineapi::Permission { +impl From<(PermissionType, &str, Option)> for Permission { #[inline] - fn from(perm: Permission) -> Self { - perm.inner + fn from(value: (PermissionType, &str, Option)) -> Self { + Self::from((value.0, value.1.as_bytes().to_vec(), value.2)) } } diff --git a/crates/xline-client/tests/it/auth.rs b/crates/xline-client/tests/it/auth.rs index dad88d0b5..ecd77e5b0 100644 --- a/crates/xline-client/tests/it/auth.rs +++ b/crates/xline-client/tests/it/auth.rs @@ -1,8 +1,9 @@ //! The following tests are originally from `etcd-client` use xline_client::{ error::Result, - types::auth::{ - AuthRoleGrantPermissionRequest, AuthRoleRevokePermissionRequest, Permission, PermissionType, + types::{ + auth::{AuthRoleRevokePermissionRequest, Permission, PermissionType}, + range_end::RangeOption, }, }; @@ -42,38 +43,39 @@ async fn permission_operations_should_success_in_normal_path() -> Result<()> { let client = client.auth_client(); let role1 = "role1"; - let perm1 = Permission::new(PermissionType::Read, "123"); - let perm2 = Permission::new(PermissionType::Write, "abc").with_from_key(); - let perm3 = Permission::new(PermissionType::Readwrite, "hi").with_range_end("hjj"); - let perm4 = Permission::new(PermissionType::Write, "pp").with_prefix(); - let perm5 = Permission::new(PermissionType::Read, vec![0]).with_from_key(); + let perm1 = (PermissionType::Read, "123", None); + let perm2 = (PermissionType::Write, "abc", Some(RangeOption::FromKey)); + let perm3 = ( + PermissionType::Readwrite, + "hi", + Some(RangeOption::RangeEnd("hjj".into())), + ); + let perm4 = (PermissionType::Write, "pp", Some(RangeOption::Prefix)); + let perm5 = (PermissionType::Read, vec![0], Some(RangeOption::FromKey)); client.role_add(role1).await?; - client - .role_grant_permission(AuthRoleGrantPermissionRequest::new(role1, perm1.clone())) - .await?; - client - .role_grant_permission(AuthRoleGrantPermissionRequest::new(role1, perm2.clone())) - .await?; - client - .role_grant_permission(AuthRoleGrantPermissionRequest::new(role1, perm3.clone())) - .await?; - client - .role_grant_permission(AuthRoleGrantPermissionRequest::new(role1, perm4.clone())) - .await?; - client - .role_grant_permission(AuthRoleGrantPermissionRequest::new(role1, perm5.clone())) - .await?; + let (p1, p2, p3) = perm1.clone(); + client.role_grant_permission(role1, p1, p2, p3).await?; + let (p1, p2, p3) = perm2.clone(); + client.role_grant_permission(role1, p1, p2, p3).await?; + let (p1, p2, p3) = perm3.clone(); + client.role_grant_permission(role1, p1, p2, p3).await?; + let (p1, p2, p3) = perm4.clone(); + client.role_grant_permission(role1, p1, p2, p3).await?; + let (p1, p2, p3) = perm5.clone(); + client.role_grant_permission(role1, p1, p2, p3).await?; { + // get permissions for role1, and validate the result let resp = client.role_get(role1).await?; let permissions = resp.perm; - assert!(permissions.contains(&perm1.into())); - assert!(permissions.contains(&perm2.into())); - assert!(permissions.contains(&perm3.into())); - assert!(permissions.contains(&perm4.into())); - assert!(permissions.contains(&perm5.into())); + + assert!(permissions.contains(&Permission::from(perm1).into())); + assert!(permissions.contains(&Permission::from(perm2).into())); + assert!(permissions.contains(&Permission::from(perm3).into())); + assert!(permissions.contains(&Permission::from(perm4).into())); + assert!(permissions.contains(&Permission::from(perm5).into())); } // revoke all permission diff --git a/crates/xline-test-utils/src/lib.rs b/crates/xline-test-utils/src/lib.rs index 6a293036f..b3135bf24 100644 --- a/crates/xline-test-utils/src/lib.rs +++ b/crates/xline-test-utils/src/lib.rs @@ -14,7 +14,7 @@ use utils::config::{ LogConfig, MetricsConfig, StorageConfig, TlsConfig, TraceConfig, XlineServerConfig, }; use xline::server::XlineServer; -use xline_client::types::auth::{AuthRoleGrantPermissionRequest, Permission, PermissionType}; +use xline_client::types::{auth::PermissionType, range_end::RangeOption}; pub use xline_client::{clients, types, Client, ClientOptions}; /// Cluster @@ -350,10 +350,12 @@ pub async fn set_user( client.user_grant_role(name, role).await?; if !key.is_empty() { client - .role_grant_permission(AuthRoleGrantPermissionRequest::new( + .role_grant_permission( role, - Permission::new(PermissionType::Readwrite, key).with_range_end(range_end), - )) + PermissionType::Readwrite, + key, + Some(RangeOption::RangeEnd(range_end.to_vec())), + ) .await?; } Ok(()) diff --git a/crates/xlinectl/src/command/role/grant_perm.rs b/crates/xlinectl/src/command/role/grant_perm.rs index c4c0ac91d..d81b0f41d 100644 --- a/crates/xlinectl/src/command/role/grant_perm.rs +++ b/crates/xlinectl/src/command/role/grant_perm.rs @@ -1,13 +1,12 @@ use clap::{arg, ArgMatches, Command}; -use xline_client::{ - error::Result, - types::auth::{AuthRoleGrantPermissionRequest, Permission}, - Client, -}; +use xline_client::{error::Result, types::range_end::RangeOption, Client}; use xlineapi::Type; use crate::utils::printer::Printer; +/// Temp return type for `grant_perm` command, indicates `(name, PermissionType, key, RangeOption)` +type AuthRoleGrantPermissionRequest = (String, Type, Vec, Option); + /// Definition of `grant_perm` command pub(super) fn command() -> Command { Command::new("grant_perm") @@ -32,34 +31,36 @@ pub(super) fn build_request(matches: &ArgMatches) -> AuthRoleGrantPermissionRequ let prefix = matches.get_flag("prefix"); let from_key = matches.get_flag("from_key"); - let perm_type = match perm_type_local.as_str() { - "Read" => Type::Read, - "Write" => Type::Write, - "ReadWrite" => Type::Readwrite, + let perm_type = match perm_type_local.to_lowercase().as_str() { + "read" => Type::Read, + "write" => Type::Write, + "readwrite" => Type::Readwrite, _ => unreachable!("should be checked by clap"), }; - let mut perm = Permission::new(perm_type, key.as_bytes()); - - if let Some(range_end) = range_end { - perm = perm.with_range_end(range_end.as_bytes()); + let range_option = if prefix { + Some(RangeOption::Prefix) + } else if from_key { + Some(RangeOption::FromKey) + } else { + range_end.map(|inner| RangeOption::RangeEnd(inner.as_bytes().to_vec())) }; - if prefix { - perm = perm.with_prefix(); - } - - if from_key { - perm = perm.with_from_key(); - } - - AuthRoleGrantPermissionRequest::new(name, perm) + ( + name.to_owned(), + perm_type, + key.as_bytes().to_vec(), + range_option, + ) } /// Execute the command pub(super) async fn execute(client: &mut Client, matches: &ArgMatches) -> Result<()> { let req = build_request(matches); - let resp = client.auth_client().role_grant_permission(req).await?; + let resp = client + .auth_client() + .role_grant_permission(req.0, req.1, req.2, req.3) + .await?; resp.print(); Ok(()) @@ -77,16 +78,20 @@ mod tests { let test_cases = vec![ TestCase::new( vec!["grant_perm", "Admin", "Read", "key1", "key2"], - Some(AuthRoleGrantPermissionRequest::new( - "Admin", - Permission::new(Type::Read, "key1").with_range_end("key2"), + Some(( + "Admin".into(), + Type::Read, + "key1".into(), + Some(RangeOption::RangeEnd("key2".into())), )), ), TestCase::new( vec!["grant_perm", "Admin", "Write", "key3", "--from_key"], - Some(AuthRoleGrantPermissionRequest::new( - "Admin", - Permission::new(Type::Write, "key3").with_from_key(), + Some(( + "Admin".into(), + Type::Write, + "key3".into(), + Some(RangeOption::FromKey), )), ), ];