From f6ab35b498bddcb9cabf22b1dbb999fb3ecc6d53 Mon Sep 17 00:00:00 2001 From: Adrian Benavides Date: Wed, 19 Jun 2024 09:09:21 +0200 Subject: [PATCH] refactor(rust): `project-member` commands, and adds the `show` command --- .../src/authenticator/direct/client.rs | 34 ++-- .../direct/direct_authenticator.rs | 41 ++++ .../direct/direct_authenticator_worker.rs | 11 +- .../rust/ockam/ockam_command/src/lease/mod.rs | 4 +- .../ockam/ockam_command/src/message/send.rs | 2 +- .../ockam/ockam_command/src/project/enroll.rs | 2 +- .../ockam/ockam_command/src/project/ticket.rs | 2 +- .../ockam_command/src/project_member/add.rs | 112 +++++++---- .../src/project_member/delete.rs | 183 ++++++++++++------ .../ockam_command/src/project_member/list.rs | 68 +++---- .../src/project_member/list_ids.rs | 58 +++--- .../ockam_command/src/project_member/mod.rs | 114 ++++++++++- .../ockam_command/src/project_member/show.rs | 117 +++++++++++ .../static/delete/after_long_help.txt | 3 + .../static/delete/long_about.txt | 4 +- .../static/show/after_long_help.txt | 4 + .../project_member/static/show/long_about.txt | 1 + .../src/secure_channel/create.rs | 4 +- .../ockam/ockam_command/src/shared_args.rs | 6 +- .../ockam/ockam_command/src/space/create.rs | 2 +- .../ockam/ockam_command/src/terminal/tui.rs | 3 + .../tests/bats/local/authority.bats | 26 ++- .../rust/ockam/ockam_core/src/api.rs | 2 + .../identities/storage/attributes_entry.rs | 62 ++++-- .../src/secure_channels/secure_client.rs | 8 +- 25 files changed, 654 insertions(+), 219 deletions(-) create mode 100644 implementations/rust/ockam/ockam_command/src/project_member/show.rs create mode 100644 implementations/rust/ockam/ockam_command/src/project_member/static/show/after_long_help.txt create mode 100644 implementations/rust/ockam/ockam_command/src/project_member/static/show/long_about.txt diff --git a/implementations/rust/ockam/ockam_api/src/authenticator/direct/client.rs b/implementations/rust/ockam/ockam_api/src/authenticator/direct/client.rs index 214a2d8178e..28d71a189d0 100644 --- a/implementations/rust/ockam/ockam_api/src/authenticator/direct/client.rs +++ b/implementations/rust/ockam/ockam_api/src/authenticator/direct/client.rs @@ -20,9 +20,13 @@ pub trait Members { attributes: BTreeMap, ) -> miette::Result<()>; - async fn delete_member(&self, ctx: &Context, identifier: Identifier) -> miette::Result<()>; + async fn show_member( + &self, + ctx: &Context, + identifier: Identifier, + ) -> miette::Result; - async fn delete_all_members(&self, ctx: &Context, except: Identifier) -> miette::Result<()>; + async fn delete_member(&self, ctx: &Context, identifier: Identifier) -> miette::Result<()>; async fn list_member_ids(&self, ctx: &Context) -> miette::Result>; @@ -49,6 +53,20 @@ impl Members for AuthorityNodeClient { .into_diagnostic() } + async fn show_member( + &self, + ctx: &Context, + identifier: Identifier, + ) -> miette::Result { + let req = Request::get(format!("/{identifier}")); + self.get_secure_client() + .ask(ctx, DefaultAddress::DIRECT_AUTHENTICATOR, req) + .await + .into_diagnostic()? + .success() + .into_diagnostic() + } + async fn delete_member(&self, ctx: &Context, identifier: Identifier) -> miette::Result<()> { let req = Request::delete(format!("/{identifier}")); self.get_secure_client() @@ -59,18 +77,6 @@ impl Members for AuthorityNodeClient { .into_diagnostic() } - async fn delete_all_members(&self, ctx: &Context, except: Identifier) -> miette::Result<()> { - let member_ids = self.list_member_ids(ctx).await?; - for id in member_ids { - if id != except { - if let Err(e) = self.delete_member(ctx, id.clone()).await { - warn!("Failed to delete member {}: {}", id, e); - } - } - } - Ok(()) - } - async fn list_member_ids(&self, ctx: &Context) -> miette::Result> { let req = Request::get("/member_ids"); self.get_secure_client() diff --git a/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator.rs b/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator.rs index 4db0bf173d0..9027c8e125f 100644 --- a/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator.rs +++ b/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator.rs @@ -133,6 +133,47 @@ impl DirectAuthenticator { Ok(Either::Left(())) } + #[instrument(skip_all, fields(enroller = %enroller))] + pub async fn show_member( + &self, + enroller: &Identifier, + identifier: &Identifier, + ) -> Result> { + let check = EnrollerAccessControlChecks::check_identifier( + self.members.clone(), + self.identities_attributes.clone(), + enroller, + &self.account_authority, + ) + .await?; + + if !check.is_enroller { + warn!("Non-enroller {} is trying to retrieve a member", enroller); + return Ok(Either::Right(DirectAuthenticatorError( + "Non-enroller is trying to retrieve a member".to_string(), + ))); + } + + match self.members.get_member(identifier).await? { + Some(member) => { + let entry = AttributesEntry::new( + member.attributes().clone(), + member.added_at(), + None, + Some(member.added_by().clone()), + ); + Ok(Either::Left(entry)) + } + None => { + warn!("Member {} not found", identifier); + Ok(Either::Right(DirectAuthenticatorError(format!( + "Member {} not found", + identifier + )))) + } + } + } + #[instrument(skip_all, fields(enroller = %enroller))] pub async fn list_members( &self, diff --git a/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator_worker.rs b/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator_worker.rs index 0af9d0bdda1..22630b95634 100644 --- a/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator_worker.rs +++ b/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator_worker.rs @@ -36,8 +36,8 @@ impl DirectAuthenticatorWorker { #[ockam_core::worker] impl Worker for DirectAuthenticatorWorker { - type Context = Context; type Message = Vec; + type Context = Context; async fn handle_message(&mut self, c: &mut Context, m: Routed) -> Result<()> { let secure_channel_info = match IdentitySecureChannelLocalInfo::find_info(m.local_message()) @@ -97,6 +97,15 @@ impl Worker for DirectAuthenticatorWorker { Either::Right(error) => Response::forbidden(&req, &error.0).to_vec()?, } } + (Some(Method::Get), [id]) | (Some(Method::Get), ["members", id]) => { + let identifier = Identifier::try_from(id.to_string())?; + let res = self.authenticator.show_member(&from, &identifier).await?; + + match res { + Either::Left(body) => Response::ok().with_headers(&req).body(body).to_vec()?, + Either::Right(error) => Response::forbidden(&req, &error.0).to_vec()?, + } + } (Some(Method::Delete), [id]) | (Some(Method::Delete), ["members", id]) => { let identifier = Identifier::try_from(id.to_string())?; let res = self.authenticator.delete_member(&from, &identifier).await?; diff --git a/implementations/rust/ockam/ockam_command/src/lease/mod.rs b/implementations/rust/ockam/ockam_command/src/lease/mod.rs index 95db1a6019f..f8461adbd23 100644 --- a/implementations/rust/ockam/ockam_command/src/lease/mod.rs +++ b/implementations/rust/ockam/ockam_command/src/lease/mod.rs @@ -67,14 +67,14 @@ async fn create_project_client( let node = InMemoryNode::start_with_project_name_and_identity( ctx, &opts.state, - identity_opts.identity.clone(), + identity_opts.identity_name.clone(), trust_opts.project_name.clone(), ) .await?; let identity = opts .state - .get_identity_name_or_default(&identity_opts.identity) + .get_identity_name_or_default(&identity_opts.identity_name) .await?; let project = opts .state diff --git a/implementations/rust/ockam/ockam_command/src/message/send.rs b/implementations/rust/ockam/ockam_command/src/message/send.rs index 32b100e46c6..4cb8e6addc0 100644 --- a/implementations/rust/ockam/ockam_command/src/message/send.rs +++ b/implementations/rust/ockam/ockam_command/src/message/send.rs @@ -90,7 +90,7 @@ impl Command for SendCommand { } else { let identity_name = opts .state - .get_identity_name_or_default(&self.identity_opts.identity) + .get_identity_name_or_default(&self.identity_opts.identity_name) .await?; info!("starting an in memory node to send a message"); diff --git a/implementations/rust/ockam/ockam_command/src/project/enroll.rs b/implementations/rust/ockam/ockam_command/src/project/enroll.rs index 5a4e6d1f011..bb4b2f50f31 100644 --- a/implementations/rust/ockam/ockam_command/src/project/enroll.rs +++ b/implementations/rust/ockam/ockam_command/src/project/enroll.rs @@ -84,7 +84,7 @@ impl Command for EnrollCommand { let identity = opts .state - .get_named_identity_or_default(&self.identity_opts.identity) + .get_named_identity_or_default(&self.identity_opts.identity_name) .await?; let project = self.store_project(&opts).await?; diff --git a/implementations/rust/ockam/ockam_command/src/project/ticket.rs b/implementations/rust/ockam/ockam_command/src/project/ticket.rs index dad8e297312..00c9a6c7a91 100644 --- a/implementations/rust/ockam/ockam_command/src/project/ticket.rs +++ b/implementations/rust/ockam/ockam_command/src/project/ticket.rs @@ -100,7 +100,7 @@ impl Command for TicketCommand { let identity = opts .state - .get_identity_name_or_default(&self.identity_opts.identity) + .get_identity_name_or_default(&self.identity_opts.identity_name) .await?; let authority_node_client = node diff --git a/implementations/rust/ockam/ockam_command/src/project_member/add.rs b/implementations/rust/ockam/ockam_command/src/project_member/add.rs index de20d455ccd..048972a45de 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/add.rs +++ b/implementations/rust/ockam/ockam_command/src/project_member/add.rs @@ -1,26 +1,30 @@ use async_trait::async_trait; use clap::Args; use colorful::Colorful; +use serde::Serialize; +use std::collections::BTreeMap; +use std::fmt::Display; use ockam::identity::Identifier; use ockam::Context; use ockam_api::authenticator::direct::Members; -use ockam_api::fmt_ok; -use ockam_api::nodes::InMemoryNode; +use ockam_api::colors::color_primary; +use ockam_api::{fmt_log, fmt_ok}; use ockam_multiaddr::MultiAddr; -use crate::project_member::{create_authority_client, create_member_attributes, get_project}; +use crate::project_member::{authority_client, create_member_attributes}; use crate::shared_args::{IdentityOpts, RetryOpts}; use crate::{docs, Command, CommandGlobalOpts, Error}; const LONG_ABOUT: &str = include_str!("./static/add/long_about.txt"); const AFTER_LONG_HELP: &str = include_str!("./static/add/after_long_help.txt"); -/// This attribute in credential allows member to create a relay on the Project node, the name of the relay should be -/// equal to the value of that attribute. If the value is `*` then any name is allowed +/// This credential attribute allows a member to establish a relay on the Project node. +/// The relay's name should match this attribute's value. +/// If the attribute's value is "*", it implies that any name is acceptable for the relay. pub const OCKAM_RELAY_ATTRIBUTE: &str = "ockam-relay"; -/// Add members to a Project, as an authorized enroller directly +/// Add or update members of a Project, directly as an authorized enroller #[derive(Clone, Debug, Args)] #[command( long_about = docs::about(LONG_ABOUT), @@ -30,22 +34,29 @@ pub struct AddCommand { #[command(flatten)] identity_opts: IdentityOpts, - /// Which project add member to + /// The route to the Project to which a member should be added #[arg(long, short, value_name = "ROUTE_TO_PROJECT")] to: Option, + /// The Identifier of the member to add #[arg(value_name = "IDENTIFIER")] member: Identifier, - /// Attributes in `key=value` format to be attached to the member. You can specify this option multiple times for multiple attributes + /// Attributes in `key=value` format to be attached to the member. + /// You can specify this option multiple times for multiple attributes #[arg(short, long = "attribute", value_name = "ATTRIBUTE")] attributes: Vec, - /// Name of the relay that the identity will be allowed to create. This name is transformed into attributes to prevent collisions when creating relay names. For example: `--relay foo` is shorthand for `--attribute ockam-relay=foo` - #[arg(long = "relay", value_name = "ENROLLEE_ALLOWED_RELAY_NAME")] + /// Name of the relay that the identity will be allowed to create. + /// This name is transformed into an attribute, and it's used to prevent collisions with other relays names. + /// E.g. `--relay foo` is a shorthand for `--attribute ockam-relay=foo` + #[arg(long = "relay", value_name = "ALLOWED_RELAY_NAME")] allowed_relay_name: Option, - /// Add the enroller role. If you specify it, this flag is transformed into the attributes `--attribute ockam-role=enroller`. This role allows the Identity using the ticket to enroll other Identities into the Project, typically something that only admins can do + /// Set the enroller role for the member. + /// When this flag is set, it is transformed into the attribute `ockam-role=enroller`. + /// This role grants the Identity holding the ticket the ability to enroll other Identities + /// into the Project, which is a privilege usually reserved for administrators. #[arg(long = "enroller")] enroller: bool, @@ -62,36 +73,69 @@ impl Command for AddCommand { } async fn async_run(self, ctx: &Context, opts: CommandGlobalOpts) -> crate::Result<()> { - let project = get_project(&opts.state, &self.to).await?; + let (authority_node_client, project_name) = + authority_client(ctx, &opts, &self.identity_opts, &self.to).await?; - let node = InMemoryNode::start_with_project_name( - ctx, - &opts.state, - Some(project.name().to_string()), - ) - .await?; - - let authority_node_client = - create_authority_client(&node, &opts.state, &self.identity_opts, &project).await?; + let attributes = + create_member_attributes(&self.attributes, &self.allowed_relay_name, self.enroller)?; authority_node_client - .add_member( - ctx, - self.member.clone(), - create_member_attributes( - &self.attributes, - &self.allowed_relay_name, - self.enroller, - )?, - ) + .add_member(ctx, self.member.clone(), attributes.clone()) .await .map_err(Error::Retry)?; - opts.terminal.stdout().plain(fmt_ok!( - "Identifier {} is now a Project member. It can get a credential and access Project resources, like portals of other members", - self.member - )); + let output = AddMemberOutput { + project: project_name, + identifier: self.member.clone(), + attributes, + }; + + opts.terminal + .stdout() + .plain(output.to_string()) + .json_obj(&output)? + .write_line()?; + + Ok(()) + } +} + +#[derive(Serialize)] +struct AddMemberOutput { + project: String, + identifier: Identifier, + attributes: BTreeMap, +} +impl Display for AddMemberOutput { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + writeln!( + f, + "{}", + fmt_ok!( + "Identifier {} is now a member of the Project {}", + color_primary(self.identifier.to_string()), + color_primary(&self.project) + ) + )?; + if !self.attributes.is_empty() { + let attributes = self + .attributes + .iter() + .map(|(k, v)| format!("{}={}", k, v)) + .collect::>() + .join(", "); + writeln!( + f, + "{}", + fmt_log!("With attributes: {}", color_primary(attributes)) + )?; + } + writeln!( + f, + "{}", + fmt_log!("It can get a credential and access Project resources, like portals of other members") + )?; Ok(()) } } diff --git a/implementations/rust/ockam/ockam_command/src/project_member/delete.rs b/implementations/rust/ockam/ockam_command/src/project_member/delete.rs index b5108c5816f..71c39ba31bb 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/delete.rs +++ b/implementations/rust/ockam/ockam_command/src/project_member/delete.rs @@ -1,24 +1,26 @@ +use async_trait::async_trait; use clap::Args; use colorful::Colorful; use miette::miette; +use serde::Serialize; +use std::fmt::Display; +use tracing::warn; use ockam::identity::Identifier; use ockam::Context; use ockam_api::authenticator::direct::Members; use ockam_api::colors::color_primary; -use ockam_api::nodes::InMemoryNode; -use ockam_api::{fmt_err, fmt_ok}; +use ockam_api::{fmt_info, fmt_ok}; use ockam_multiaddr::MultiAddr; -use super::{create_authority_client, get_project}; +use super::authority_client; use crate::shared_args::IdentityOpts; -use crate::util::async_cmd; -use crate::{docs, CommandGlobalOpts}; +use crate::{docs, Command, CommandGlobalOpts}; const LONG_ABOUT: &str = include_str!("./static/delete/long_about.txt"); const AFTER_LONG_HELP: &str = include_str!("./static/delete/after_long_help.txt"); -/// Add members to a Project, as an authorized enroller, directly, or via an enrollment ticket +/// Remove members from a Project #[derive(Clone, Debug, Args)] #[command( long_about = docs::about(LONG_ABOUT), @@ -28,78 +30,147 @@ pub struct DeleteCommand { #[command(flatten)] identity_opts: IdentityOpts, - /// Which project's member to delete + /// The route of the Project that the member belongs to #[arg(long, short, value_name = "ROUTE_TO_PROJECT")] to: Option, + /// The Identifier of the member to delete #[arg(value_name = "IDENTIFIER")] member: Option, - /// Delete all members of the project except the default identity + /// Delete all members of the Project except the current default Identity #[arg(long, conflicts_with = "member")] all: bool, } -impl DeleteCommand { - pub fn run(self, opts: CommandGlobalOpts) -> miette::Result<()> { - async_cmd(&self.name(), opts.clone(), |ctx| async move { - self.async_run(&ctx, opts).await - }) - } +#[async_trait] +impl Command for DeleteCommand { + const NAME: &'static str = "project-member delete"; - pub fn name(&self) -> String { - "project-member delete".into() - } + async fn async_run(self, ctx: &Context, opts: CommandGlobalOpts) -> crate::Result<()> { + if self.member.is_none() && !self.all { + return Err(miette!( + "You need to specify either an identifier to delete or use the --all flag to delete all the members from a project." + ).into()); + } + + let (authority_node_client, project_name) = + authority_client(ctx, &opts, &self.identity_opts, &self.to).await?; - async fn async_run(&self, ctx: &Context, opts: CommandGlobalOpts) -> miette::Result<()> { let identity = opts .state - .get_named_identity_or_default(&self.identity_opts.identity) + .get_named_identity_or_default(&self.identity_opts.identity_name) .await?; - let project = get_project(&opts.state, &self.to).await?; - - let node = InMemoryNode::start_with_project_name( - ctx, - &opts.state, - Some(project.name().to_string()), - ) - .await?; - - let authority_node_client = - create_authority_client(&node, &opts.state, &self.identity_opts, &project).await?; - - match (&self.member, self.all) { - (Some(member), _) => { - authority_node_client - .delete_member(ctx, member.clone()) - .await?; - opts.terminal.stdout().plain(fmt_ok!( - "Identifier {} is no longer a member of the Project. It won't be able to get a credential and access Project resources, like portals of other members", - color_primary(member.to_string()) - )); + let mut output = DeleteMemberOutput { + project: project_name.clone(), + identifiers: vec![], + }; + + // Delete the passed member + if let Some(member) = &self.member { + authority_node_client + .delete_member(ctx, member.clone()) + .await?; + output.identifiers.push(member.clone()); + } + // Try to delete all members except the current default identity + else if self.all { + if !opts + .state + .is_identity_enrolled(&Some(identity.name())) + .await? + { + return Err(miette!( + "You need to use an enrolled identity to delete all the members from a Project." + ).into()); } - (None, true) => { - if !opts - .state - .is_identity_enrolled(&Some(identity.name())) - .await? + let self_identifier = identity.identifier(); + let member_identifiers = authority_node_client.list_member_ids(ctx).await?; + if !member_identifiers.is_empty() { + opts.terminal.write_line(fmt_info!( + "Found {} members in the Project {}", + member_identifiers.len(), + project_name + ))?; + } + + let members_to_delete = member_identifiers + .into_iter() + .filter(|id| id != &self_identifier) + .collect::>(); + + let pb = opts.terminal.progress_bar(); + for identifier in members_to_delete.into_iter() { + if let Some(pb) = &pb { + pb.set_message(format!("Trying to delete member {identifier}...")); + } + if let Err(e) = authority_node_client + .delete_member(ctx, identifier.clone()) + .await { - return Err(miette!(fmt_err!( - "You need to use an enrolled identity to delete all the members from a project." - ))); + warn!("Failed to delete member {}: {}", identifier, e); + } else { + output.identifiers.push(identifier.clone()); } - authority_node_client - .delete_all_members(ctx, identity.identifier()) - .await?; - opts.terminal.stdout().plain(fmt_ok!( - "All identifiers except {} are no longer members of the Project.", - color_primary(identity.identifier().to_string()) - )); } - _ => unreachable!(), + } else { + unreachable!("Either a member or the --all flag should be set"); } + opts.terminal + .stdout() + .plain(output.to_string()) + .json_obj(&output)? + .write_line()?; + + Ok(()) + } +} + +#[derive(Serialize)] +struct DeleteMemberOutput { + project: String, + identifiers: Vec, +} + +impl Display for DeleteMemberOutput { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self.identifiers[..] { + [] => { + writeln!( + f, + "{}", + fmt_ok!( + "There are no members that can be deleted from the Project {}", + self.project + ) + )?; + } + [identifier] => { + writeln!( + f, + "{}", + fmt_ok!( + "Identifier {} is no longer a member of the Project. \ + It won't be able to get a credential and access Project resources, \ + like portals of other members", + color_primary(identifier.to_string()) + ) + )?; + } + _ => { + writeln!( + f, + "{}", + fmt_ok!( + "{} members were deleted from the Project {}", + &self.identifiers.len(), + self.project + ) + )?; + } + } Ok(()) } } diff --git a/implementations/rust/ockam/ockam_command/src/project_member/list.rs b/implementations/rust/ockam/ockam_command/src/project_member/list.rs index 41fc6c7bec5..4dd79bff4d0 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/list.rs +++ b/implementations/rust/ockam/ockam_command/src/project_member/list.rs @@ -1,17 +1,16 @@ use async_trait::async_trait; use clap::Args; -use ockam::identity::{AttributesEntry, Identifier}; use ockam::Context; -use ockam_api::authenticator::direct::Members; -use ockam_api::nodes::InMemoryNode; +use ockam_api::authenticator::direct::{ + Members, OCKAM_ROLE_ATTRIBUTE_ENROLLER_VALUE, OCKAM_ROLE_ATTRIBUTE_KEY, +}; use ockam_multiaddr::MultiAddr; use crate::shared_args::IdentityOpts; use crate::{docs, Command, CommandGlobalOpts, Result}; -use ockam_api::output::Output; -use super::{create_authority_client, get_project}; +use super::{authority_client, MemberOutput}; const LONG_ABOUT: &str = include_str!("./static/list/long_about.txt"); @@ -24,9 +23,13 @@ pub struct ListCommand { #[command(flatten)] identity_opts: IdentityOpts, - /// Which project members to request + /// The route to the Project to list members from #[arg(long, short, value_name = "ROUTE_TO_PROJECT")] to: Option, + + /// Return only the enroller members + #[arg(long, visible_alias = "enroller")] + enrollers: bool, } #[async_trait] @@ -34,45 +37,32 @@ impl Command for ListCommand { const NAME: &'static str = "project-member list"; async fn async_run(self, ctx: &Context, opts: CommandGlobalOpts) -> Result<()> { - let project = get_project(&opts.state, &self.to).await?; - - let node = InMemoryNode::start_with_project_name( - ctx, - &opts.state, - Some(project.name().to_string()), - ) - .await?; - - let authority_node_client = - create_authority_client(&node, &opts.state, &self.identity_opts, &project).await?; + let (authority_node_client, _) = + authority_client(ctx, &opts, &self.identity_opts, &self.to).await?; let members = authority_node_client .list_members(ctx) .await? .into_iter() - .map(|i| MemberOutput(i.0, i.1)) - .collect(); - - print_members(&opts, members)?; + .filter(|(_, a)| { + !self.enrollers + || a.deserialized_key_value_attrs().contains(&format!( + "{}={}", + OCKAM_ROLE_ATTRIBUTE_KEY, OCKAM_ROLE_ATTRIBUTE_ENROLLER_VALUE + )) + }) + .map(|(i, a)| MemberOutput::new(i, a)) + .collect::>(); + + let plain = opts + .terminal + .build_list(&members, "No members found on the Authority node")?; + opts.terminal + .stdout() + .plain(plain) + .json_obj(&members)? + .write_line()?; Ok(()) } } - -struct MemberOutput(Identifier, AttributesEntry); - -impl Output for MemberOutput { - fn item(&self) -> ockam_api::Result { - Ok(format!("{}: {}", self.0, self.1)) - } -} - -fn print_members(opts: &CommandGlobalOpts, member_ids: Vec) -> miette::Result<()> { - let plain = opts - .terminal - .build_list(&member_ids, "No members found on that Authority node.")?; - - opts.terminal.clone().stdout().plain(plain).write_line()?; - - Ok(()) -} diff --git a/implementations/rust/ockam/ockam_command/src/project_member/list_ids.rs b/implementations/rust/ockam/ockam_command/src/project_member/list_ids.rs index d775d49e829..4a877971de2 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/list_ids.rs +++ b/implementations/rust/ockam/ockam_command/src/project_member/list_ids.rs @@ -1,20 +1,20 @@ use async_trait::async_trait; use clap::Args; +use serde::Serialize; use ockam::identity::Identifier; use ockam::Context; use ockam_api::authenticator::direct::Members; -use ockam_api::nodes::InMemoryNode; use ockam_multiaddr::MultiAddr; -use super::{create_authority_client, get_project}; +use super::authority_client; use crate::shared_args::IdentityOpts; use crate::{docs, Command, CommandGlobalOpts, Result}; use ockam_api::output::Output; const LONG_ABOUT: &str = include_str!("./static/list_ids/long_about.txt"); -/// Add members to a Project, as an authorized enroller, directly, or via an enrollment ticket +/// List members ID's of a Project #[derive(Clone, Debug, Args)] #[command( long_about = docs::about(LONG_ABOUT), @@ -23,7 +23,7 @@ pub struct ListIdsCommand { #[command(flatten)] identity_opts: IdentityOpts, - /// Which project members to request + /// The route to the Project to list members from #[arg(long, short, value_name = "ROUTE_TO_PROJECT")] to: Option, } @@ -33,48 +33,36 @@ impl Command for ListIdsCommand { const NAME: &'static str = "project-member list-ids"; async fn async_run(self, ctx: &Context, opts: CommandGlobalOpts) -> Result<()> { - let project = get_project(&opts.state, &self.to).await?; - - let node = InMemoryNode::start_with_project_name( - ctx, - &opts.state, - Some(project.name().to_string()), - ) - .await?; - - let authority_node_client = - create_authority_client(&node, &opts.state, &self.identity_opts, &project).await?; + let (authority_node_client, _) = + authority_client(ctx, &opts, &self.identity_opts, &self.to).await?; let member_ids = authority_node_client .list_member_ids(ctx) .await? .into_iter() - .map(IdentifierOutput) - .collect(); - - print_member_ids(&opts, member_ids)?; + .map(|identifier| ListIdsOutput { identifier }) + .collect::>(); + + let plain = opts + .terminal + .build_list(&member_ids, "No members found on the Authority node")?; + opts.terminal + .stdout() + .plain(plain) + .json_obj(&member_ids)? + .write_line()?; Ok(()) } } -struct IdentifierOutput(Identifier); +#[derive(Serialize)] +struct ListIdsOutput { + identifier: Identifier, +} -impl Output for IdentifierOutput { +impl Output for ListIdsOutput { fn item(&self) -> ockam_api::Result { - Ok(self.0.to_string()) + Ok(format!("{}", self.identifier)) } } - -fn print_member_ids( - opts: &CommandGlobalOpts, - member_ids: Vec, -) -> miette::Result<()> { - let plain = opts - .terminal - .build_list(&member_ids, "No members found on that Authority node.")?; - - opts.terminal.clone().stdout().plain(plain).write_line()?; - - Ok(()) -} diff --git a/implementations/rust/ockam/ockam_command/src/project_member/mod.rs b/implementations/rust/ockam/ockam_command/src/project_member/mod.rs index 798cbb8aaee..61f9cfa1dab 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/mod.rs +++ b/implementations/rust/ockam/ockam_command/src/project_member/mod.rs @@ -3,20 +3,28 @@ use std::collections::BTreeMap; use clap::Args; use clap::Subcommand; use miette::miette; +use serde::Serialize; +use std::fmt::Write; use add::{AddCommand, OCKAM_RELAY_ATTRIBUTE}; use delete::DeleteCommand; use list::ListCommand; use list_ids::ListIdsCommand; +use ockam::identity::{AttributesEntry, Identifier}; use ockam_api::authenticator::direct::{ OCKAM_ROLE_ATTRIBUTE_ENROLLER_VALUE, OCKAM_ROLE_ATTRIBUTE_KEY, }; use ockam_api::cloud::project::Project; use ockam_api::cloud::AuthorityNodeClient; -use ockam_api::nodes::NodeManager; +use ockam_api::colors::{color_primary, color_warn}; +use ockam_api::nodes::{InMemoryNode, NodeManager}; +use ockam_api::output::Output; +use ockam_api::terminal::fmt; use ockam_api::CliState; use ockam_multiaddr::{proto, MultiAddr, Protocol}; +use ockam_node::Context; +use crate::project_member::show::ShowCommand; use crate::shared_args::IdentityOpts; use crate::{docs, Command, CommandGlobalOpts}; @@ -24,6 +32,7 @@ mod add; mod delete; mod list; mod list_ids; +mod show; const LONG_ABOUT: &str = include_str!("./static/long_about.txt"); @@ -46,6 +55,7 @@ impl ProjectMemberCommand { ProjectMemberSubcommand::List(c) => c.run(opts), ProjectMemberSubcommand::ListIds(c) => c.run(opts), ProjectMemberSubcommand::Add(c) => c.run(opts), + ProjectMemberSubcommand::Show(c) => c.run(opts), ProjectMemberSubcommand::Delete(c) => c.run(opts), } } @@ -55,6 +65,7 @@ impl ProjectMemberCommand { ProjectMemberSubcommand::List(c) => c.name(), ProjectMemberSubcommand::ListIds(c) => c.name(), ProjectMemberSubcommand::Add(c) => c.name(), + ProjectMemberSubcommand::Show(c) => c.name(), ProjectMemberSubcommand::Delete(c) => c.name(), } } @@ -69,9 +80,27 @@ pub enum ProjectMemberSubcommand { #[command(display_order = 800)] Add(AddCommand), #[command(display_order = 800)] + Show(ShowCommand), + #[command(display_order = 800)] Delete(DeleteCommand), } +pub(super) async fn authority_client( + ctx: &Context, + opts: &CommandGlobalOpts, + identity_opts: &IdentityOpts, + project_route: &Option, +) -> crate::Result<(AuthorityNodeClient, String)> { + let project = get_project(&opts.state, project_route).await?; + let node = + InMemoryNode::start_with_project_name(ctx, &opts.state, Some(project.name().to_string())) + .await?; + Ok(( + create_authority_client(&node, &opts.state, identity_opts, &project).await?, + project.name().to_string(), + )) +} + /// Get the project authority from the first address protocol. /// /// If the first protocol is a `/project`, look up the project's config. @@ -112,7 +141,7 @@ pub(super) async fn create_authority_client( project: &Project, ) -> crate::Result { let identity = cli_state - .get_identity_name_or_default(&identity_opts.identity) + .get_identity_name_or_default(&identity_opts.identity_name) .await?; Ok(node @@ -128,8 +157,12 @@ pub(crate) fn create_member_attributes( let mut attributes = BTreeMap::new(); for attr in attrs { let mut parts = attr.splitn(2, '='); - let key = parts.next().ok_or(miette!("key expected"))?; - let value = parts.next().ok_or(miette!("value expected)"))?; + let key = parts + .next() + .ok_or(miette!("key expected in attribute {attr}"))?; + let value = parts + .next() + .ok_or(miette!("value expected in attribute {attr}"))?; attributes.insert(key.to_string(), value.to_string()); } if let Some(relay_name) = allowed_relay_name { @@ -143,3 +176,76 @@ pub(crate) fn create_member_attributes( } Ok(attributes) } + +#[derive(Serialize)] +struct MemberOutput { + identifier: Identifier, + attributes: AttributesEntry, +} + +impl MemberOutput { + fn new(identifier: Identifier, attributes: AttributesEntry) -> Self { + Self { + identifier, + attributes, + } + } + + fn to_string(&self, padding: &str) -> ockam_api::Result { + let mut f = String::new(); + writeln!( + f, + "{}{}", + padding, + color_primary(self.identifier.to_string()) + )?; + + if self.attributes.attrs().is_empty() { + writeln!(f, "{}Has no attributes", padding)?; + } else { + let attributes = self.attributes.deserialized_key_value_attrs(); + writeln!( + f, + "{}With attributes: {}", + padding, + color_primary(attributes.join(", ")) + )?; + writeln!( + f, + "{}{}Added at: {}", + padding, + fmt::INDENTATION, + color_warn(self.attributes.added_at().to_string()) + )?; + if let Some(expires_at) = self.attributes.expires_at() { + writeln!( + f, + "{}{}Expires at: {}", + padding, + fmt::INDENTATION, + color_warn(expires_at.to_string()) + )?; + } + if let Some(attested_by) = &self.attributes.attested_by() { + writeln!( + f, + "{}{}Attested by: {}", + padding, + fmt::INDENTATION, + color_primary(attested_by.to_string()) + )?; + } + } + Ok(f) + } +} + +impl Output for MemberOutput { + fn item(&self) -> ockam_api::Result { + self.to_string(fmt::PADDING) + } + + fn as_list_item(&self) -> ockam_api::Result { + self.to_string("") + } +} diff --git a/implementations/rust/ockam/ockam_command/src/project_member/show.rs b/implementations/rust/ockam/ockam_command/src/project_member/show.rs new file mode 100644 index 00000000000..9c527b72e5c --- /dev/null +++ b/implementations/rust/ockam/ockam_command/src/project_member/show.rs @@ -0,0 +1,117 @@ +use async_trait::async_trait; +use clap::Args; +use console::Term; +use miette::{miette, IntoDiagnostic}; +use std::str::FromStr; + +use ockam::identity::Identifier; +use ockam::Context; +use ockam_api::authenticator::direct::Members; +use ockam_api::cloud::AuthorityNodeClient; +use ockam_api::output::Output; +use ockam_api::terminal::{Terminal, TerminalStream}; +use ockam_core::AsyncTryClone; +use ockam_multiaddr::MultiAddr; + +use crate::project_member::{authority_client, MemberOutput}; +use crate::shared_args::IdentityOpts; +use crate::tui::{PluralTerm, ShowCommandTui}; +use crate::{docs, Command, CommandGlobalOpts}; + +const LONG_ABOUT: &str = include_str!("./static/show/long_about.txt"); +const AFTER_LONG_HELP: &str = include_str!("./static/show/after_long_help.txt"); + +/// Show the details of a member from a Project +#[derive(Clone, Debug, Args)] +#[command( +long_about = docs::about(LONG_ABOUT), +after_long_help = docs::after_help(AFTER_LONG_HELP), +hide = true, +)] +pub struct ShowCommand { + #[command(flatten)] + identity_opts: IdentityOpts, + + /// The route of the Project that the member belongs to + #[arg(long, short, value_name = "ROUTE_TO_PROJECT")] + to: Option, + + /// The Identifier of the member to show + #[arg(value_name = "IDENTIFIER")] + member: Option, +} + +#[async_trait] +impl Command for ShowCommand { + const NAME: &'static str = "project-member show"; + + async fn async_run(self, ctx: &Context, opts: CommandGlobalOpts) -> crate::Result<()> { + Ok(ShowTui::run(ctx.async_try_clone().await.into_diagnostic()?, opts, self).await?) + } +} + +pub struct ShowTui { + ctx: Context, + opts: CommandGlobalOpts, + member: Option, + client: AuthorityNodeClient, +} + +impl ShowTui { + pub async fn run( + ctx: Context, + opts: CommandGlobalOpts, + cmd: ShowCommand, + ) -> miette::Result<()> { + let (authority_node_client, _) = + authority_client(&ctx, &opts, &cmd.identity_opts, &cmd.to).await?; + let tui = Self { + ctx, + opts, + member: cmd.member, + client: authority_node_client, + }; + tui.show().await + } +} + +#[async_trait] +impl ShowCommandTui for ShowTui { + const ITEM_NAME: PluralTerm = PluralTerm::Member; + + fn cmd_arg_item_name(&self) -> Option { + self.member.as_ref().map(|m| m.to_string()) + } + + fn terminal(&self) -> Terminal> { + self.opts.terminal.clone() + } + + async fn get_arg_item_name_or_default(&self) -> miette::Result { + self.cmd_arg_item_name() + .ok_or(miette!("No member provided")) + } + + async fn list_items_names(&self) -> miette::Result> { + self.client.list_member_ids(&self.ctx).await.map(|ids| { + ids.into_iter() + .map(|id| id.to_string()) + .collect::>() + }) + } + + async fn show_single(&self, item_name: &str) -> miette::Result<()> { + let identifier = Identifier::from_str(item_name).into_diagnostic()?; + let attributes = self + .client + .show_member(&self.ctx, identifier.clone()) + .await?; + let member = MemberOutput::new(identifier, attributes); + self.terminal() + .stdout() + .plain(member.item()?) + .json_obj(&member)? + .write_line()?; + Ok(()) + } +} diff --git a/implementations/rust/ockam/ockam_command/src/project_member/static/delete/after_long_help.txt b/implementations/rust/ockam/ockam_command/src/project_member/static/delete/after_long_help.txt index f3452de8d19..b83b6a075bb 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/static/delete/after_long_help.txt +++ b/implementations/rust/ockam/ockam_command/src/project_member/static/delete/after_long_help.txt @@ -1,4 +1,7 @@ ```sh # Delete a member $ ockam project-member delete I6c20e814b56579306f55c64e8747e6c1b4a53d9a3f4ca83c252cc2fbfc72fa94 + +# Delete all members, expect the member associated with your own identifier +$ ockam project-member delete --all ``` diff --git a/implementations/rust/ockam/ockam_command/src/project_member/static/delete/long_about.txt b/implementations/rust/ockam/ockam_command/src/project_member/static/delete/long_about.txt index af4167a338c..48d5fd706bf 100644 --- a/implementations/rust/ockam/ockam_command/src/project_member/static/delete/long_about.txt +++ b/implementations/rust/ockam/ockam_command/src/project_member/static/delete/long_about.txt @@ -1 +1,3 @@ -This deletes a member from a given Project Membership Authority node. +This deletes the passed member from a given Project Membership Authority node. + +You can also use the `--all` flag to delete all members from a given Project Membership Authority node, except the member associated with your own identifier. diff --git a/implementations/rust/ockam/ockam_command/src/project_member/static/show/after_long_help.txt b/implementations/rust/ockam/ockam_command/src/project_member/static/show/after_long_help.txt new file mode 100644 index 00000000000..e1eaf2bd78b --- /dev/null +++ b/implementations/rust/ockam/ockam_command/src/project_member/static/show/after_long_help.txt @@ -0,0 +1,4 @@ +```sh +# Show a member +$ ockam project-member show I6c20e814b56579306f55c64e8747e6c1b4a53d9a3f4ca83c252cc2fbfc72fa94 +``` diff --git a/implementations/rust/ockam/ockam_command/src/project_member/static/show/long_about.txt b/implementations/rust/ockam/ockam_command/src/project_member/static/show/long_about.txt new file mode 100644 index 00000000000..1d57f755cfc --- /dev/null +++ b/implementations/rust/ockam/ockam_command/src/project_member/static/show/long_about.txt @@ -0,0 +1 @@ +This command shows the details of a member of a Project. diff --git a/implementations/rust/ockam/ockam_command/src/secure_channel/create.rs b/implementations/rust/ockam/ockam_command/src/secure_channel/create.rs index 46f1acb8223..2c690b5cac2 100644 --- a/implementations/rust/ockam/ockam_command/src/secure_channel/create.rs +++ b/implementations/rust/ockam/ockam_command/src/secure_channel/create.rs @@ -84,7 +84,7 @@ impl CreateCommand { .wrap_err(format!("Could not convert {} into route", &self.to))?; let identity_name = opts .state - .get_identity_name_or_default(&self.identity_opts.identity) + .get_identity_name_or_default(&self.identity_opts.identity_name) .await?; let projects_sc = get_projects_secure_channels_from_config_lookup( @@ -126,7 +126,7 @@ impl CreateCommand { let create_secure_channel = async { let identity_name = opts .state - .get_identity_name_or_default(&self.identity_opts.identity) + .get_identity_name_or_default(&self.identity_opts.identity_name) .await?; let payload = CreateSecureChannelRequest::new( &to, diff --git a/implementations/rust/ockam/ockam_command/src/shared_args.rs b/implementations/rust/ockam/ockam_command/src/shared_args.rs index a09ef5082d4..f121a80ee9f 100644 --- a/implementations/rust/ockam/ockam_command/src/shared_args.rs +++ b/implementations/rust/ockam/ockam_command/src/shared_args.rs @@ -6,9 +6,9 @@ use std::time::Duration; #[derive(Clone, Debug, Args)] pub struct IdentityOpts { - /// Run the command as the given Identity name - #[arg(global = true, value_name = "IDENTITY_NAME", long)] - pub identity: Option, + /// Run the command as the given Identity + #[arg(global = true, value_name = "IDENTITY_NAME", long = "identity")] + pub identity_name: Option, } #[derive(Clone, Debug, Args, Default, PartialEq)] diff --git a/implementations/rust/ockam/ockam_command/src/space/create.rs b/implementations/rust/ockam/ockam_command/src/space/create.rs index 7b82f8a1a68..35d4044d773 100644 --- a/implementations/rust/ockam/ockam_command/src/space/create.rs +++ b/implementations/rust/ockam/ockam_command/src/space/create.rs @@ -49,7 +49,7 @@ impl CreateCommand { async fn async_run(&self, ctx: &Context, opts: CommandGlobalOpts) -> miette::Result<()> { if !opts .state - .is_identity_enrolled(&self.identity_opts.identity) + .is_identity_enrolled(&self.identity_opts.identity_name) .await? { return Err(miette!( diff --git a/implementations/rust/ockam/ockam_command/src/terminal/tui.rs b/implementations/rust/ockam/ockam_command/src/terminal/tui.rs index 2893e873ade..a133cf44137 100644 --- a/implementations/rust/ockam/ockam_command/src/terminal/tui.rs +++ b/implementations/rust/ockam/ockam_command/src/terminal/tui.rs @@ -255,6 +255,7 @@ pub enum PluralTerm { Inlet, Outlet, Policy, + Member, } impl PluralTerm { @@ -269,6 +270,7 @@ impl PluralTerm { PluralTerm::Inlet => "inlet", PluralTerm::Outlet => "outlet", PluralTerm::Policy => "policy", + PluralTerm::Member => "member", } } @@ -283,6 +285,7 @@ impl PluralTerm { PluralTerm::Inlet => "inlets", PluralTerm::Outlet => "outlets", PluralTerm::Policy => "policies", + PluralTerm::Member => "members", } } } diff --git a/implementations/rust/ockam/ockam_command/tests/bats/local/authority.bats b/implementations/rust/ockam/ockam_command/tests/bats/local/authority.bats index d5d0c9e9378..694ad0d2baa 100644 --- a/implementations/rust/ockam/ockam_command/tests/bats/local/authority.bats +++ b/implementations/rust/ockam/ockam_command/tests/bats/local/authority.bats @@ -281,8 +281,8 @@ EOF run_success "$OCKAM" project-member list --identity enroller assert_output --partial "$enroller_identifier" - assert_output --partial "attrs: \"ockam-role=enroller\"" - assert_output --partial "attested_by: \"$authority_identifier\"" + assert_output --partial "\"ockam-role\":\"enroller\"" + assert_output --partial "\"attested_by\":\"$authority_identifier\"" run_success "$OCKAM" project-member add "$m_identifier" --identity enroller --attribute key=value --relay="*" @@ -291,14 +291,20 @@ EOF assert_output --partial "$m_identifier" run_success "$OCKAM" project-member list --identity enroller - assert_output --partial "$enroller_identifier" - assert_output --partial "attrs: \"ockam-role=enroller\"" - assert_output --partial "attested_by: \"$authority_identifier\"" - - assert_output --partial "$m_identifier" - assert_output --partial "key=value" - assert_output --partial "ockam-relay=*" - assert_output --partial "attested_by: \"$enroller_identifier\"" + assert_output --partial "\"identifier\":\"$enroller_identifier\"" + assert_output --partial "\"ockam-role\":\"enroller\"" + assert_output --partial "\"attested_by\":\"$authority_identifier\"" + + assert_output --partial "\"identifier\":\"$m_identifier\"" + assert_output --partial "\"key\":\"value\"" + assert_output --partial "\"ockam-relay\":\"*\"" + assert_output --partial "\"attested_by\":\"$enroller_identifier\"" + + run_success "$OCKAM" project-member show "$m_identifier" --identity enroller + assert_output --partial "\"identifier\":\"$m_identifier\"" + assert_output --partial "\"key\":\"value\"" + assert_output --partial "\"ockam-relay\":\"*\"" + assert_output --partial "\"attested_by\":\"$enroller_identifier\"" run_success "$OCKAM" project-member delete "$m_identifier" --identity enroller } diff --git a/implementations/rust/ockam/ockam_core/src/api.rs b/implementations/rust/ockam/ockam_core/src/api.rs index 1faaf677d13..191eb360a91 100644 --- a/implementations/rust/ockam/ockam_core/src/api.rs +++ b/implementations/rust/ockam/ockam_core/src/api.rs @@ -242,6 +242,7 @@ pub enum Status { #[n(401)] Unauthorized, #[n(403)] Forbidden, #[n(404)] NotFound, + #[n(408)] Timeout, #[n(409)] Conflict, #[n(405)] MethodNotAllowed, #[n(500)] InternalServerError, @@ -256,6 +257,7 @@ impl Display for Status { Status::Unauthorized => "401 Unauthorized", Status::Forbidden => "403 Forbidden", Status::NotFound => "404 NotFound", + Status::Timeout => "408 Timeout", Status::Conflict => "409 Conflict", Status::MethodNotAllowed => "405 MethodNotAllowed", Status::InternalServerError => "500 InternalServerError", diff --git a/implementations/rust/ockam/ockam_identity/src/identities/storage/attributes_entry.rs b/implementations/rust/ockam/ockam_identity/src/identities/storage/attributes_entry.rs index 4a655aaf04e..f28f15674c0 100644 --- a/implementations/rust/ockam/ockam_identity/src/identities/storage/attributes_entry.rs +++ b/implementations/rust/ockam/ockam_identity/src/identities/storage/attributes_entry.rs @@ -7,6 +7,7 @@ use ockam_core::compat::borrow::ToOwned; use ockam_core::compat::string::String; use ockam_core::compat::{collections::BTreeMap, vec::Vec}; use ockam_core::Result; +use serde::ser::SerializeMap; use serde::{Deserialize, Serialize}; /// An entry on the AuthenticatedIdentities table. @@ -14,22 +15,50 @@ use serde::{Deserialize, Serialize}; #[rustfmt::skip] #[cbor(map)] pub struct AttributesEntry { - #[n(1)] attrs: BTreeMap, Vec>, + #[n(1)] + #[serde(serialize_with = "serialize_attributes", deserialize_with = "deserialize_attributes")] + attributes: BTreeMap, Vec>, #[n(2)] added_at: TimestampInSeconds, #[n(3)] expires_at: Option, #[n(4)] attested_by: Option, } +fn serialize_attributes( + attrs: &BTreeMap, Vec>, + s: S, +) -> core::result::Result +where + S: serde::Serializer, +{ + let mut map = s.serialize_map(Some(attrs.len()))?; + for (key, value) in attrs { + map.serialize_entry( + &String::from_utf8_lossy(key), + &String::from_utf8_lossy(value), + )?; + } + map.end() +} + +fn deserialize_attributes<'de, D>( + d: D, +) -> core::result::Result, Vec>, D::Error> +where + D: serde::Deserializer<'de>, +{ + let map = >::deserialize(d)?; + let mut result = BTreeMap::new(); + for (key, value) in map { + result.insert(key.as_bytes().to_vec(), value.as_bytes().to_vec()); + } + Ok(result) +} + impl Display for AttributesEntry { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - let mut attributes = vec![]; - for (key, value) in self.attrs.clone() { - if let (Ok(k), Ok(v)) = (String::from_utf8(key), String::from_utf8(value)) { - attributes.push(format!("{k}={v}")) - } - } + let attributes = self.deserialized_key_value_attrs(); f.debug_struct("AttributesEntry") - .field("attrs", &attributes.join(",")) + .field("attributes", &attributes.join(",")) .field("added_at", &self.added_at) .field( "expires_at", @@ -55,7 +84,7 @@ impl AttributesEntry { attested_by: Option, ) -> Self { Self { - attrs, + attributes: attrs, added_at, expires_at, attested_by, @@ -64,7 +93,18 @@ impl AttributesEntry { /// The entry attributes pub fn attrs(&self) -> &BTreeMap, Vec> { - &self.attrs + &self.attributes + } + + /// The entry attributes as a list of key=value strings + pub fn deserialized_key_value_attrs(&self) -> Vec { + let mut attributes = vec![]; + for (key, value) in self.attributes.clone() { + if let (Ok(k), Ok(v)) = (String::from_utf8(key), String::from_utf8(value)) { + attributes.push(format!("{k}={v}")); + } + } + attributes } /// Expiration time for this entry @@ -93,7 +133,7 @@ impl AttributesEntry { ) -> Result { let attrs = BTreeMap::from([(attribute_name, attribute_value)]); Ok(Self { - attrs, + attributes: attrs, added_at: now()?, expires_at, attested_by, diff --git a/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs b/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs index af7a5efe7fd..01b9de24317 100644 --- a/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs +++ b/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs @@ -146,6 +146,7 @@ impl SecureClient { { Ok(bytes) => Response::parse_response_reply::(bytes.as_slice()), Err(err) => { + // TODO: we should return a Reply::Failed(timeout) error here error!("Error during SecureClient::ask to {} {}", api_service, err); Err(err) } @@ -166,15 +167,16 @@ impl SecureClient { let request_header = req.header().clone(); let bytes = self .request_with_timeout(ctx, api_service, req, self.request_timeout) + // TODO: we should return a Reply::Failed(timeout) error here .await?; let (response, decoder) = Response::parse_response_header(bytes.as_slice())?; - if !response.is_ok() { + if response.is_ok() { + Ok(Successful(())) + } else { Ok(Reply::Failed( Error::from_failed_request(&request_header, &response.parse_err_msg(decoder)), response.status(), )) - } else { - Ok(Successful(())) } }