Skip to content

Commit

Permalink
forbid using create group/entity methods for updating existing object
Browse files Browse the repository at this point in the history
  • Loading branch information
stormshield-gt committed Mar 12, 2024
1 parent 1dae5e2 commit 84a8ed2
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 111 deletions.
48 changes: 0 additions & 48 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,54 +301,6 @@ where
.ok_or(ClientError::ResponseDataEmptyError)
}

/// Executes an [Endpoint] and if it returns an empty HTTP response, it returns `None`, otherwise it returns the result.
///
/// This function is mostly useful for endpoints that can both *update* and *create*.
/// The *create* operation will return a result and the *update* one not.
///
/// If there is a result, a few operations are performed on it:
///
/// * Any potential API error responses from the execution are searched for and,
/// if found, converted to a [ClientError::APIError]
/// * All other errors are mapped from [rustify::errors::ClientError] to
/// [ClientError::RestClientError]
/// * An empty content body from the execution is rejected and a
/// [ClientError::ResponseEmptyError] is returned instead
/// * The enclosing [EndpointResult] is stripped off and any warnings found in
/// the result are logged
/// * An empty `data` field in the [EndpointResult] is rejected and a
/// [ClientError::ResponseDataEmptyError] is returned instead
/// * The value from the enclosed `data` field is returned along with any
/// propagated errors.
pub async fn exec_with_result_or_empty<E>(
client: &impl Client,
endpoint: E,
) -> Result<Option<E::Response>, ClientError>
where
E: Endpoint,
{
info!(
"Executing {} and expecting maybe a response",
endpoint.path()
);
endpoint
.with_middleware(client.middle())
.exec(client.http())
.await
.map(|res| {
if res.response.body().is_empty() {
Ok(None)
} else {
res.wrap::<EndpointResult<_>>()
.map_err(ClientError::from)
.map(strip)?
.map(Some)
.ok_or(ClientError::ResponseDataEmptyError)
}
})
.map_err(parse_err)?
}

/// Executes the given endpoint but requests that the Vault server to return a
/// token wrapped response.
///
Expand Down
9 changes: 6 additions & 3 deletions src/api/identity/entity/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@ use super::responses::{
///
/// This endpoint creates or updates an Entity.
///
/// Note that it's not possible to set the ID to update an existing entity, [`identity::entity::update_by_id`]
/// is the function to call for that use case.
///
/// * Path: identity/entity
/// * Method: POST
/// * Reference: https://developer.hashicorp.com/vault/api-docs/secret/identity/entity#create-an-entity
///
/// [`identity::entity::update_by_id`]: crate::identity::entity::update_by_id
#[derive(Builder, Debug, Default, Endpoint)]
#[endpoint(
path = "identity/entity",
Expand All @@ -25,8 +30,6 @@ use super::responses::{
pub struct CreateEntityRequest {
/// Name of the entity.
pub name: Option<String>,
/// ID of the entity. If set, updates the corresponding existing entity.
pub id: Option<String>,
/// Metadata to be associated with the entity.
pub metadata: Option<HashMap<String, String>>,
/// Policies to be tied to the entity.
Expand Down Expand Up @@ -107,7 +110,7 @@ pub struct DeleteEntityByIdRequest {

/// ## Batch delete entities
///
/// TThis endpoint deletes all entities provided.
/// This endpoint deletes all entities provided.
///
/// * Path: identity/entity/batch-delete
/// * Method: POST
Expand Down
10 changes: 8 additions & 2 deletions src/api/identity/entity/responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ pub struct ReadEntityByNameResponse {
pub creation_time: String,
pub metadata: Option<HashMap<String, String>>,
pub aliases: Vec<Alias>,
// TODO other fields
pub direct_group_ids: Vec<String>,
pub group_ids: Vec<String>,
pub inherited_group_ids: Vec<String>,
pub merged_entity_ids: Option<Vec<String>>,
pub namespace_id: String,
}

#[derive(Deserialize, Debug, Serialize)]
Expand All @@ -50,7 +54,9 @@ pub struct Alias {
pub local: bool,
pub mount_type: String,
pub mount_path: String,
// TODO other fields
pub custom_metadata: Option<HashMap<String, String>>,
pub metadata: Option<HashMap<String, String>>,
pub merged_from_canonical_ids: Option<Vec<String>>,
}

/// Response from executing
Expand Down
9 changes: 5 additions & 4 deletions src/api/identity/group/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ use std::{collections::HashMap, fmt::Debug};
///
/// This endpoint creates or updates a group.
///
/// Note that it's not possible to set the ID or the name to update an existing group, [`identity::group::update_by_id`]
/// is the function to call for that use case.
///
/// * Path: identity/group
/// * Method: POST
/// * Reference: https://developer.hashicorp.com/vault/api-docs/secret/identity/group#create-a-group
///
/// [`identity::group::update_by_id`]: crate::identity::group::update_by_id
#[derive(Builder, Debug, Default, Endpoint, Serialize)]
#[endpoint(
path = "identity/group",
Expand All @@ -22,10 +27,6 @@ use std::{collections::HashMap, fmt::Debug};
)]
#[builder(setter(into, strip_option), default)]
pub struct CreateGroupRequest {
/// Name of the group. If set (and ID is not set), updates the corresponding existing group.
pub name: Option<String>,
/// ID of the group. If set, updates the corresponding existing group.
pub id: Option<String>,
/// Type of the group, internal or external. Defaults to internal.
#[serde(rename = "type")]
pub group_type: Option<String>,
Expand Down
10 changes: 7 additions & 3 deletions src/api/identity/group/responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
#[derive(Deserialize, Debug, Serialize)]
pub struct CreateGroupResponse {
pub id: String,
pub name: Option<String>,
pub name: String,
}

/// Response from executing
Expand All @@ -21,10 +21,12 @@ pub struct ReadGroupByIdResponse {
pub last_update_time: String,
pub member_entity_ids: Vec<String>,
pub member_group_ids: Option<Vec<String>>,
pub parent_group_ids: Option<Vec<String>>,
pub metadata: Option<HashMap<String, String>>,
pub modify_index: u64,
pub namespace_id: String,
pub name: String,
pub policies: Vec<String>,
pub policies: Option<Vec<String>>,
#[serde(rename = "type")]
pub group_type: String,
}
Expand All @@ -47,10 +49,12 @@ pub struct ReadGroupByNameResponse {
pub last_update_time: String,
pub member_entity_ids: Vec<String>,
pub member_group_ids: Option<Vec<String>>,
pub parent_group_ids: Option<Vec<String>>,
pub metadata: Option<HashMap<String, String>>,
pub modify_index: u64,
pub namespace_id: String,
pub name: String,
pub policies: Vec<String>,
pub policies: Option<Vec<String>>,
#[serde(rename = "type")]
pub group_type: String,
}
Expand Down
4 changes: 4 additions & 0 deletions src/identity.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
//! Identiy secrets engine
//!
//! The Identity secrets engine is the identity management solution for Vault.
pub mod entity;
pub mod entity_alias;
pub mod group;
Expand Down
7 changes: 3 additions & 4 deletions src/identity/entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,17 @@ use crate::{
error::ClientError,
};

/// Create or update an entity. If the entity already exists, it would update it, and
/// maybe return `None` in that case.
/// Create an entity.
///
/// See [CreateEntityRequest]
#[instrument(skip(client, opts), err)]
pub async fn create(
client: &impl Client,
opts: Option<&mut CreateEntityRequestBuilder>,
) -> Result<Option<CreateEntityResponse>, ClientError> {
) -> Result<CreateEntityResponse, ClientError> {
let mut t = CreateEntityRequest::builder();
let endpoint = opts.unwrap_or(&mut t).build().unwrap();
api::exec_with_result_or_empty(client, endpoint).await
api::exec_with_result(client, endpoint).await
}

/// Reads entity by `id`.
Expand Down
6 changes: 3 additions & 3 deletions src/identity/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@ use crate::{
error::ClientError,
};

/// Creates or update a group.
/// Creates a group.
///
/// See [CreateGroupRequest]
#[instrument(skip(client, opts), err)]
pub async fn create(
client: &impl Client,
opts: Option<&mut CreateGroupRequestBuilder>,
) -> Result<Option<CreateGroupResponse>, ClientError> {
) -> Result<CreateGroupResponse, ClientError> {
let mut t = CreateGroupRequest::builder();
let endpoint = opts.unwrap_or(&mut t).build().unwrap();
api::exec_with_result_or_empty(client, endpoint).await
api::exec_with_result(client, endpoint).await
}

/// Reads group by `id`.
Expand Down
73 changes: 29 additions & 44 deletions tests/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ fn test_entity_and_entity_alias() {
let client = server.client();

let entity_id = test_create_entity(&client).await;
let alias_id = test_create_entity_alias(&client, &entity_id).await;
create_anonymous_entity(&client).await;
test_list_entity_by_id(&client, &entity_id).await;
test_read_entity_by_id(&client, &entity_id).await;
Expand All @@ -57,7 +58,6 @@ fn test_entity_and_entity_alias() {
test_batch_delete_entity(&client).await;
test_merge_entity(&client).await;

let alias_id = test_create_entity_alias(&client, &entity_id).await;
test_read_entity_alias_id(&client, &alias_id).await;
test_update_entity_alias_by_id(&client, &alias_id).await;
test_list_entity_alias_by_id(&client, &alias_id, &entity_id).await;
Expand Down Expand Up @@ -92,7 +92,7 @@ fn test_group_and_group_alias() {
}

async fn test_create_entity(client: &VaultClient) -> String {
identity::entity::create(
let entity = identity::entity::create(
client,
Some(
&mut CreateEntityRequestBuilder::default()
Expand All @@ -102,38 +102,13 @@ async fn test_create_entity(client: &VaultClient) -> String {
)
.await
.unwrap();
let entity = identity::entity::read_by_name(client, ENTITY_NAME)
.await
.unwrap();

assert!(!entity.disabled);

// This really update the entity but the Vault server return a response with an empty body.
identity::entity::create(
client,
Some(
&mut CreateEntityRequestBuilder::default()
.disabled(true)
.name(ENTITY_NAME)
.id(&entity.id),
),
)
.await
.unwrap();

let entity = identity::entity::read_by_name(client, ENTITY_NAME)
.await
.unwrap();
assert!(entity.disabled);
entity.id
}

async fn create_anonymous_entity(client: &VaultClient) {
// Without specifying anything Vault will create an entity for us and make sure it got an unique id.
let entity = identity::entity::create(client, None)
.await
.unwrap()
.unwrap();
let entity = identity::entity::create(client, None).await.unwrap();
assert!(!entity.id.is_empty());
assert!(entity.alias.is_none());
identity::entity::delete_by_id(client, &entity.id)
Expand Down Expand Up @@ -408,39 +383,49 @@ async fn test_list_entity_alias_by_id(client: &VaultClient, alias_id: &str, expe
}

async fn test_create_group(client: &VaultClient) -> String {
identity::group::create(
let group = identity::group::create(
client,
Some(
&mut CreateGroupRequestBuilder::default()
.policies(vec![POLICY.to_string()])
.name(GROUP_NAME),
&mut CreateGroupRequestBuilder::default().policies(vec![POLICY.to_string()]), // .name(GROUP_NAME),
),
)
.await
.unwrap();
let group = identity::group::read_by_name(client, GROUP_NAME)
identity::group::read_by_id(client, &group.id)
.await
.unwrap();
identity::group::read_by_name(client, &group.name)
.await
.unwrap();
identity::group::delete_by_id(client, &group.id)
.await
.unwrap();

assert!(group.metadata.is_none());
let metadata = HashMap::from([(String::from("company"), String::from("example-company"))]);
// We create a group without policy to see if we can also parse the response.
let group = identity::group::create(client, Some(&mut CreateGroupRequestBuilder::default()))
.await
.unwrap();
identity::group::read_by_id(client, &group.id)
.await
.unwrap();
identity::group::read_by_name(client, &group.name)
.await
.unwrap();
identity::group::delete_by_id(client, &group.id)
.await
.unwrap();

// This one it's called in "updating mode".
identity::group::create(
identity::group::create_or_update_by_name(
client,
Some(
&mut CreateGroupRequestBuilder::default()
.metadata(metadata.clone())
.name(GROUP_NAME)
.id(&group.id),
),
GROUP_NAME,
Some(&mut CreateGroupByNameRequestBuilder::default().policies(vec![POLICY.to_string()])),
)
.await
.unwrap();
let group = identity::group::read_by_name(client, GROUP_NAME)
.await
.unwrap();
assert_eq!(group.metadata, Some(metadata));

group.id
}

Expand Down

0 comments on commit 84a8ed2

Please sign in to comment.