From 755405576744ae2fa0052cacee9dcd8057498d87 Mon Sep 17 00:00:00 2001 From: mbecker20 Date: Fri, 29 Mar 2024 21:55:10 -0700 Subject: [PATCH] api sanitize update tags api with name->id replacement, tag auto create --- bin/core/src/api/write/tag.rs | 28 ++++++++---- bin/core/src/helpers/mod.rs | 6 +-- bin/core/src/helpers/resource.rs | 78 +++++++++++++++++++------------- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/bin/core/src/api/write/tag.rs b/bin/core/src/api/write/tag.rs index b74f2ddf9..ca7bc4b1c 100644 --- a/bin/core/src/api/write/tag.rs +++ b/bin/core/src/api/write/tag.rs @@ -1,3 +1,5 @@ +use std::str::FromStr; + use anyhow::{anyhow, Context}; use async_trait::async_trait; use monitor_client::{ @@ -14,7 +16,7 @@ use monitor_client::{ }; use mungos::{ by_id::{delete_one_by_id, update_one_by_id}, - mongodb::bson::doc, + mongodb::bson::{doc, oid::ObjectId}, }; use resolver_api::Resolve; @@ -31,11 +33,16 @@ impl Resolve for State { CreateTag { name }: CreateTag, user: User, ) -> anyhow::Result { + if ObjectId::from_str(&name).is_ok() { + return Err(anyhow!("tag name cannot be ObjectId")); + } + let mut tag = Tag { id: Default::default(), name, owner: user.id.clone(), }; + tag.id = db_client() .await .tags @@ -46,6 +53,7 @@ impl Resolve for State { .as_object_id() .context("inserted_id is not ObjectId")? .to_string(); + Ok(tag) } } @@ -57,6 +65,10 @@ impl Resolve for State { RenameTag { id, name }: RenameTag, user: User, ) -> anyhow::Result { + if ObjectId::from_str(&name).is_ok() { + return Err(anyhow!("tag name cannot be ObjectId")); + } + get_tag_check_owner(&id, &user).await?; update_one_by_id( @@ -113,7 +125,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Build::update_tags_on_resource(&id, tags).await?; + Build::update_tags_on_resource(&id, tags, user).await?; } ResourceTarget::Builder(id) => { Builder::get_resource_check_permissions( @@ -122,7 +134,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Builder::update_tags_on_resource(&id, tags).await? + Builder::update_tags_on_resource(&id, tags, user).await? } ResourceTarget::Deployment(id) => { Deployment::get_resource_check_permissions( @@ -131,7 +143,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Deployment::update_tags_on_resource(&id, tags).await? + Deployment::update_tags_on_resource(&id, tags, user).await? } ResourceTarget::Server(id) => { Server::get_resource_check_permissions( @@ -140,7 +152,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Server::update_tags_on_resource(&id, tags).await? + Server::update_tags_on_resource(&id, tags, user).await? } ResourceTarget::Repo(id) => { Repo::get_resource_check_permissions( @@ -149,7 +161,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Repo::update_tags_on_resource(&id, tags).await? + Repo::update_tags_on_resource(&id, tags, user).await? } ResourceTarget::Alerter(id) => { Alerter::get_resource_check_permissions( @@ -158,7 +170,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Alerter::update_tags_on_resource(&id, tags).await? + Alerter::update_tags_on_resource(&id, tags, user).await? } ResourceTarget::Procedure(id) => { Procedure::get_resource_check_permissions( @@ -167,7 +179,7 @@ impl Resolve for State { PermissionLevel::Write, ) .await?; - Procedure::update_tags_on_resource(&id, tags).await? + Procedure::update_tags_on_resource(&id, tags, user).await? } }; Ok(UpdateTagsOnResourceResponse {}) diff --git a/bin/core/src/helpers/mod.rs b/bin/core/src/helpers/mod.rs index 1ad81248f..ac85e3abf 100644 --- a/bin/core/src/helpers/mod.rs +++ b/bin/core/src/helpers/mod.rs @@ -128,10 +128,10 @@ pub async fn get_tag_check_owner( user: &User, ) -> anyhow::Result { let tag = get_tag(id_or_name).await?; - if !user.admin && tag.owner != user.id { - return Err(anyhow!("user must be tag owner or admin")); + if user.admin || tag.owner == user.id { + return Ok(tag); } - Ok(tag) + Err(anyhow!("user must be tag owner or admin")) } // UPDATE diff --git a/bin/core/src/helpers/resource.rs b/bin/core/src/helpers/resource.rs index 014fe4969..d1a68ddb2 100644 --- a/bin/core/src/helpers/resource.rs +++ b/bin/core/src/helpers/resource.rs @@ -1,34 +1,37 @@ use std::str::FromStr; use anyhow::{anyhow, Context}; -use futures::{future::join_all, FutureExt}; -use monitor_client::entities::{ - alerter::{ - Alerter, AlerterConfig, AlerterInfo, AlerterListItem, - AlerterListItemInfo, +use futures::future::join_all; +use monitor_client::{ + api::write::CreateTag, + entities::{ + alerter::{ + Alerter, AlerterConfig, AlerterInfo, AlerterListItem, + AlerterListItemInfo, + }, + build::{ + Build, BuildConfig, BuildInfo, BuildListItem, BuildListItemInfo, + }, + builder::{ + Builder, BuilderConfig, BuilderListItem, BuilderListItemInfo, + }, + deployment::{ + Deployment, DeploymentConfig, DeploymentImage, + DeploymentListItem, DeploymentListItemInfo, + }, + permission::PermissionLevel, + procedure::{ + Procedure, ProcedureConfig, ProcedureListItem, + ProcedureListItemInfo, + }, + repo::{Repo, RepoConfig, RepoInfo, RepoListItem}, + resource::Resource, + server::{ + Server, ServerConfig, ServerListItem, ServerListItemInfo, + }, + update::{ResourceTarget, ResourceTargetVariant}, + user::User, }, - build::{ - Build, BuildConfig, BuildInfo, BuildListItem, BuildListItemInfo, - }, - builder::{ - Builder, BuilderConfig, BuilderListItem, BuilderListItemInfo, - }, - deployment::{ - Deployment, DeploymentConfig, DeploymentImage, - DeploymentListItem, DeploymentListItemInfo, - }, - permission::PermissionLevel, - procedure::{ - Procedure, ProcedureConfig, ProcedureListItem, - ProcedureListItemInfo, - }, - repo::{Repo, RepoConfig, RepoInfo, RepoListItem}, - resource::Resource, - server::{ - Server, ServerConfig, ServerListItem, ServerListItemInfo, - }, - update::{ResourceTarget, ResourceTargetVariant}, - user::User, }; use mungos::{ find::find_collect, @@ -37,9 +40,10 @@ use mungos::{ Collection, }, }; +use resolver_api::Resolve; use serde::{de::DeserializeOwned, Serialize}; -use crate::db::db_client; +use crate::{db::db_client, state::State}; use super::{ cache::{deployment_status_cache, server_status_cache}, @@ -195,10 +199,22 @@ pub trait StateResource { async fn update_tags_on_resource( id_or_name: &str, tags: Vec, + user: User, ) -> anyhow::Result<()> { - let futures = tags - .iter() - .map(|tag| get_tag(tag).map(|tag| tag.map(|tag| tag.id))); + let futures = tags.iter().map(|tag| async { + match get_tag(tag).await { + Ok(tag) => Ok(tag.id), + Err(_) => State + .resolve( + CreateTag { + name: tag.to_string(), + }, + user.clone(), + ) + .await + .map(|tag| tag.id), + } + }); let tags = join_all(futures) .await .into_iter()