diff --git a/services/api/src/context/mod.rs b/services/api/src/context/mod.rs index 549c023df..1e9a4699b 100644 --- a/services/api/src/context/mod.rs +++ b/services/api/src/context/mod.rs @@ -12,7 +12,7 @@ mod secret_key; pub use database::{Database, DbConnection}; pub use messenger::{Body, ButtonBody, Email, Message, Messenger, NewUserBody}; -pub use rbac::Rbac; +pub use rbac::{Rbac, RbacError}; pub use secret_key::{JwtError, SecretKey}; pub struct ApiContext { diff --git a/services/api/src/context/rbac.rs b/services/api/src/context/rbac.rs index 5aa3ca83e..cc5fbc323 100644 --- a/services/api/src/context/rbac.rs +++ b/services/api/src/context/rbac.rs @@ -1,7 +1,7 @@ use bencher_rbac::{Organization, Project}; use oso::{Oso, ToPolar}; -use crate::{model::user::auth::AuthUser, ApiError}; +use crate::model::user::auth::AuthUser; pub struct Rbac(pub Oso); @@ -11,21 +11,41 @@ impl From for Rbac { } } +#[derive(Debug, thiserror::Error)] +pub enum RbacError { + #[error("Failed to check permissions: {0}")] + IsAllowed(oso::OsoError), + #[error("Permission ({permission}) denied for user ({auth_user:?}) on organization ({organization:?})")] + IsAllowedOrganization { + auth_user: AuthUser, + permission: bencher_rbac::organization::Permission, + organization: Organization, + }, + #[error("Permission ({permission}) denied for user ({auth_user:?}) on project ({project:?})")] + IsAllowedProject { + auth_user: AuthUser, + permission: bencher_rbac::project::Permission, + project: Project, + }, +} + impl Rbac { pub fn is_allowed( &self, actor: Actor, action: Action, resource: Resource, - ) -> Result + ) -> Result where Actor: ToPolar, Action: ToPolar, Resource: ToPolar, { - self.0 - .is_allowed(actor, action, resource) - .map_err(ApiError::IsAllowed) + self.0.is_allowed(actor, action, resource).map_err(|e| { + #[cfg(feature = "sentry")] + sentry::capture_error(&e); + RbacError::IsAllowed(e) + }) } pub fn is_allowed_unwrap( @@ -49,11 +69,11 @@ impl Rbac { auth_user: &AuthUser, permission: bencher_rbac::organization::Permission, organization: impl Into, - ) -> Result<(), ApiError> { + ) -> Result<(), RbacError> { let organization = organization.into(); self.is_allowed_unwrap(auth_user, permission, organization.clone()) .then_some(()) - .ok_or_else(|| ApiError::IsAllowedOrganization { + .ok_or_else(|| RbacError::IsAllowedOrganization { auth_user: auth_user.clone(), permission, organization, @@ -65,11 +85,11 @@ impl Rbac { auth_user: &AuthUser, permission: bencher_rbac::project::Permission, project: impl Into, - ) -> Result<(), ApiError> { + ) -> Result<(), RbacError> { let project = project.into(); self.is_allowed_unwrap(auth_user, permission, project.clone()) .then_some(()) - .ok_or_else(|| ApiError::IsAllowedProject { + .ok_or_else(|| RbacError::IsAllowedProject { auth_user: auth_user.clone(), permission, project, diff --git a/services/api/src/endpoints/organization/allowed.rs b/services/api/src/endpoints/organization/allowed.rs index 33f91e808..eabbc9632 100644 --- a/services/api/src/endpoints/organization/allowed.rs +++ b/services/api/src/endpoints/organization/allowed.rs @@ -51,7 +51,13 @@ pub async fn org_allowed_get( let json = get_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } diff --git a/services/api/src/endpoints/organization/members.rs b/services/api/src/endpoints/organization/members.rs index 8a0e3a3e1..c985046bc 100644 --- a/services/api/src/endpoints/organization/members.rs +++ b/services/api/src/endpoints/organization/members.rs @@ -96,7 +96,13 @@ pub async fn org_members_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -182,7 +188,13 @@ pub async fn org_member_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -309,7 +321,13 @@ pub async fn org_member_get( let json = get_one_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -353,7 +371,13 @@ pub async fn org_member_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -405,7 +429,13 @@ pub async fn org_member_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/organization/organizations.rs b/services/api/src/endpoints/organization/organizations.rs index 83606bdbe..f996f10a1 100644 --- a/services/api/src/endpoints/organization/organizations.rs +++ b/services/api/src/endpoints/organization/organizations.rs @@ -84,7 +84,13 @@ pub async fn organizations_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -146,7 +152,13 @@ pub async fn organization_post( let json = post_inner(rqctx.context(), body.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -222,7 +234,13 @@ pub async fn organization_get( let json = get_one_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -265,7 +283,13 @@ pub async fn organization_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/organization/plan.rs b/services/api/src/endpoints/organization/plan.rs index cf8f268b9..f43259dfb 100644 --- a/services/api/src/endpoints/organization/plan.rs +++ b/services/api/src/endpoints/organization/plan.rs @@ -69,7 +69,13 @@ pub async fn org_plan_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -154,7 +160,13 @@ pub async fn org_plan_get( let json = get_one_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } diff --git a/services/api/src/endpoints/organization/projects.rs b/services/api/src/endpoints/organization/projects.rs index 82ac04320..74c38f680 100644 --- a/services/api/src/endpoints/organization/projects.rs +++ b/services/api/src/endpoints/organization/projects.rs @@ -96,7 +96,13 @@ pub async fn org_projects_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -162,7 +168,13 @@ pub async fn org_project_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/organization/usage.rs b/services/api/src/endpoints/organization/usage.rs index 963b97b66..a1e6fbc23 100644 --- a/services/api/src/endpoints/organization/usage.rs +++ b/services/api/src/endpoints/organization/usage.rs @@ -71,7 +71,13 @@ pub async fn org_usage_get( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } diff --git a/services/api/src/endpoints/project/allowed.rs b/services/api/src/endpoints/project/allowed.rs index 71ba62f46..0340d45d8 100644 --- a/services/api/src/endpoints/project/allowed.rs +++ b/services/api/src/endpoints/project/allowed.rs @@ -51,7 +51,13 @@ pub async fn proj_allowed_get( let json = get_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -64,7 +70,7 @@ async fn get_inner( let conn = &mut *context.conn().await; Ok(JsonAllowed { - allowed: QueryProject::is_allowed_resource_id( + allowed: QueryProject::is_allowed( conn, &context.rbac, &path_params.project, diff --git a/services/api/src/endpoints/project/benchmarks.rs b/services/api/src/endpoints/project/benchmarks.rs index 5dec634c0..9fb4c27bf 100644 --- a/services/api/src/endpoints/project/benchmarks.rs +++ b/services/api/src/endpoints/project/benchmarks.rs @@ -93,7 +93,13 @@ pub async fn proj_benchmarks_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -158,7 +164,13 @@ pub async fn proj_benchmark_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -171,15 +183,17 @@ async fn post_inner( ) -> Result { let conn = &mut *context.conn().await; - let insert_benchmark = InsertBenchmark::from_json(conn, &path_params.project, json_benchmark)?; // Verify that the user is allowed - QueryProject::is_allowed_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, - insert_benchmark.project_id, + &path_params.project, auth_user, Permission::Create, - )?; + )? + .id; + + let insert_benchmark = InsertBenchmark::from_json(conn, project_id, json_benchmark); diesel::insert_into(schema::benchmark::table) .values(&insert_benchmark) @@ -230,7 +244,13 @@ pub async fn proj_benchmark_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -279,7 +299,13 @@ pub async fn proj_benchmark_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -293,16 +319,17 @@ async fn patch_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Edit, - )?; + )? + .id; let query_benchmark = - QueryBenchmark::from_resource_id(conn, query_project.id, &path_params.benchmark)?; + QueryBenchmark::from_resource_id(conn, project_id, &path_params.benchmark)?; diesel::update(schema::benchmark::table.filter(schema::benchmark::id.eq(query_benchmark.id))) .set(&UpdateBenchmark::from(json_benchmark)) .execute(conn) @@ -325,7 +352,13 @@ pub async fn proj_benchmark_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -338,16 +371,17 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Delete, - )?; + )? + .id; let query_benchmark = - QueryBenchmark::from_resource_id(conn, query_project.id, &path_params.benchmark)?; + QueryBenchmark::from_resource_id(conn, project_id, &path_params.benchmark)?; diesel::delete(schema::benchmark::table.filter(schema::benchmark::id.eq(query_benchmark.id))) .execute(conn) .map_err(api_error!())?; diff --git a/services/api/src/endpoints/project/branches.rs b/services/api/src/endpoints/project/branches.rs index dbbf4a551..6cbf326fa 100644 --- a/services/api/src/endpoints/project/branches.rs +++ b/services/api/src/endpoints/project/branches.rs @@ -90,7 +90,13 @@ pub async fn proj_branches_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -155,7 +161,13 @@ pub async fn proj_branch_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -167,12 +179,22 @@ async fn post_inner( auth_user: &AuthUser, ) -> Result { let conn = &mut *context.conn().await; + + // Verify that the user is allowed + let query_project = QueryProject::is_allowed( + conn, + &context.rbac, + &path_params.project, + auth_user, + Permission::Create, + )?; + // Soft creation // If the new branch name already exists then return the existing branch name // instead of erroring due to the unique constraint // This is useful to help prevent race conditions in CI if let Some(true) = json_branch.soft { - if let Ok(branch) = schema::branch::table + if let Ok(branch) = QueryBranch::belonging_to(&query_project) .filter(schema::branch::name.eq(json_branch.name.as_ref())) .first::(conn) { @@ -180,15 +202,7 @@ async fn post_inner( } } let start_point = json_branch.start_point.take(); - let insert_branch = InsertBranch::from_json(conn, &path_params.project, json_branch)?; - // Verify that the user is allowed - QueryProject::is_allowed_id( - conn, - &context.rbac, - insert_branch.project_id, - auth_user, - Permission::Create, - )?; + let insert_branch = InsertBranch::from_json(conn, query_project.id, json_branch); diesel::insert_into(schema::branch::table) .values(&insert_branch) @@ -244,7 +258,13 @@ pub async fn proj_branch_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -293,7 +313,13 @@ pub async fn proj_branch_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -307,15 +333,16 @@ async fn patch_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Edit, - )?; + )? + .id; - let query_branch = QueryBranch::from_resource_id(conn, query_project.id, &path_params.branch)?; + let query_branch = QueryBranch::from_resource_id(conn, project_id, &path_params.branch)?; if query_branch.is_system() { return Err(ApiError::SystemBranch); } @@ -341,7 +368,13 @@ pub async fn proj_branch_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -354,15 +387,16 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Delete, - )?; + )? + .id; - let query_branch = QueryBranch::from_resource_id(conn, query_project.id, &path_params.branch)?; + let query_branch = QueryBranch::from_resource_id(conn, project_id, &path_params.branch)?; if query_branch.is_system() { return Err(ApiError::SystemBranch); } diff --git a/services/api/src/endpoints/project/metric_kinds.rs b/services/api/src/endpoints/project/metric_kinds.rs index fa28f5eb2..581bd14e6 100644 --- a/services/api/src/endpoints/project/metric_kinds.rs +++ b/services/api/src/endpoints/project/metric_kinds.rs @@ -90,7 +90,13 @@ pub async fn proj_metric_kinds_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -155,7 +161,13 @@ pub async fn proj_metric_kind_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -168,16 +180,17 @@ async fn post_inner( ) -> Result { let conn = &mut *context.conn().await; - let insert_metric_kind = - InsertMetricKind::from_json(conn, &path_params.project, json_metric_kind)?; // Verify that the user is allowed - QueryProject::is_allowed_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, - insert_metric_kind.project_id, + &path_params.project, auth_user, Permission::Create, - )?; + )? + .id; + + let insert_metric_kind = InsertMetricKind::from_json(conn, project_id, json_metric_kind); // This check is required because not all system metric kinds are created at project init // And default metric kinds may be deleted @@ -233,7 +246,13 @@ pub async fn proj_metric_kind_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -282,7 +301,13 @@ pub async fn proj_metric_kind_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -296,16 +321,17 @@ async fn patch_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Edit, - )?; + )? + .id; let query_metric_kind = - QueryMetricKind::from_resource_id(conn, query_project.id, &path_params.metric_kind)?; + QueryMetricKind::from_resource_id(conn, project_id, &path_params.metric_kind)?; if query_metric_kind.is_system() { return Err(ApiError::SystemMetricKind); } @@ -333,7 +359,13 @@ pub async fn proj_metric_kind_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -346,16 +378,17 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Delete, - )?; + )? + .id; let query_metric_kind = - QueryMetricKind::from_resource_id(conn, query_project.id, &path_params.metric_kind)?; + QueryMetricKind::from_resource_id(conn, project_id, &path_params.metric_kind)?; diesel::delete( schema::metric_kind::table.filter(schema::metric_kind::id.eq(query_metric_kind.id)), diff --git a/services/api/src/endpoints/project/perf/img.rs b/services/api/src/endpoints/project/perf/img.rs index 353a7ca21..7b09f3a76 100644 --- a/services/api/src/endpoints/project/perf/img.rs +++ b/services/api/src/endpoints/project/perf/img.rs @@ -56,7 +56,13 @@ pub async fn proj_perf_img_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; Response::builder() .status(StatusCode::OK) diff --git a/services/api/src/endpoints/project/perf/mod.rs b/services/api/src/endpoints/project/perf/mod.rs index 238ad66e0..10dddf4de 100644 --- a/services/api/src/endpoints/project/perf/mod.rs +++ b/services/api/src/endpoints/project/perf/mod.rs @@ -90,7 +90,13 @@ pub async fn proj_perf_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) diff --git a/services/api/src/endpoints/project/projects.rs b/services/api/src/endpoints/project/projects.rs index a62c3d360..4e4b35756 100644 --- a/services/api/src/endpoints/project/projects.rs +++ b/services/api/src/endpoints/project/projects.rs @@ -84,7 +84,13 @@ pub async fn projects_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -174,7 +180,13 @@ pub async fn project_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -215,7 +227,13 @@ pub async fn project_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -229,7 +247,7 @@ async fn patch_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let query_project = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, @@ -279,7 +297,13 @@ pub async fn project_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -292,15 +316,16 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, bencher_rbac::project::Permission::Delete, - )?; + )? + .id; - diesel::delete(schema::project::table.filter(schema::project::id.eq(query_project.id))) + diesel::delete(schema::project::table.filter(schema::project::id.eq(project_id))) .execute(conn) .map_err(api_error!())?; diff --git a/services/api/src/endpoints/project/reports.rs b/services/api/src/endpoints/project/reports.rs index 590a761c3..8a90f9b87 100644 --- a/services/api/src/endpoints/project/reports.rs +++ b/services/api/src/endpoints/project/reports.rs @@ -19,7 +19,9 @@ use crate::{ }, error::api_error, model::project::{ + branch::QueryBranch, report::{results::ReportResults, InsertReport, QueryReport}, + testbed::QueryTestbed, version::{InsertVersion, QueryVersion, VersionId}, QueryProject, }, @@ -28,7 +30,6 @@ use crate::{ util::{ cors::{get_cors, CorsResponse}, error::database_map, - same_project::SameProject, }, ApiError, }; @@ -87,7 +88,13 @@ pub async fn proj_reports_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -181,27 +188,19 @@ async fn post_inner( ) -> Result { let conn = &mut *context.conn().await; - // Verify that the branch and testbed are part of the same project - let SameProject { - project, - branch_id, - testbed_id, - } = SameProject::validate( - conn, - &path_params.project, - &json_report.branch, - &json_report.testbed, - )?; - let project_id = project.id; - // Verify that the user is allowed - QueryProject::is_allowed_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, - project_id, + &path_params.project, auth_user, Permission::Create, - )?; + )? + .id; + + // Verify that the branch and testbed are part of the same project + let branch_id = QueryBranch::from_resource_id(conn, project_id, &json_report.branch)?.id; + let testbed_id = QueryTestbed::from_resource_id(conn, project_id, &json_report.testbed)?.id; // Check to see if the project is public or private // If private, then validate that there is an active subscription or license @@ -396,7 +395,13 @@ pub async fn proj_report_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -450,7 +455,13 @@ pub async fn proj_report_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -463,7 +474,7 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let query_project = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, diff --git a/services/api/src/endpoints/project/testbeds.rs b/services/api/src/endpoints/project/testbeds.rs index 74586311b..8d9a85ca9 100644 --- a/services/api/src/endpoints/project/testbeds.rs +++ b/services/api/src/endpoints/project/testbeds.rs @@ -90,7 +90,13 @@ pub async fn proj_testbeds_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -155,7 +161,13 @@ pub async fn proj_testbed_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -168,15 +180,17 @@ async fn post_inner( ) -> Result { let conn = &mut *context.conn().await; - let insert_testbed = InsertTestbed::from_json(conn, &path_params.project, json_testbed)?; // Verify that the user is allowed - QueryProject::is_allowed_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, - insert_testbed.project_id, + &path_params.project, auth_user, Permission::Create, - )?; + )? + .id; + + let insert_testbed = InsertTestbed::from_json(conn, project_id, json_testbed); diesel::insert_into(schema::testbed::table) .values(&insert_testbed) @@ -227,7 +241,13 @@ pub async fn proj_testbed_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -276,7 +296,13 @@ pub async fn proj_testbed_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -290,16 +316,16 @@ async fn patch_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Edit, - )?; + )? + .id; - let query_testbed = - QueryTestbed::from_resource_id(conn, query_project.id, &path_params.testbed)?; + let query_testbed = QueryTestbed::from_resource_id(conn, project_id, &path_params.testbed)?; if query_testbed.is_system() { return Err(ApiError::SystemTestbed); } @@ -325,7 +351,13 @@ pub async fn proj_testbed_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -338,16 +370,16 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Delete, - )?; + )? + .id; - let query_testbed = - QueryTestbed::from_resource_id(conn, query_project.id, &path_params.testbed)?; + let query_testbed = QueryTestbed::from_resource_id(conn, project_id, &path_params.testbed)?; if query_testbed.is_system() { return Err(ApiError::SystemTestbed); } diff --git a/services/api/src/endpoints/project/thresholds/alerts.rs b/services/api/src/endpoints/project/thresholds/alerts.rs index e9ae1982f..8ee934bb9 100644 --- a/services/api/src/endpoints/project/thresholds/alerts.rs +++ b/services/api/src/endpoints/project/thresholds/alerts.rs @@ -83,7 +83,13 @@ pub async fn proj_alerts_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -201,7 +207,13 @@ pub async fn proj_alert_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -244,7 +256,13 @@ pub async fn proj_alert_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -258,15 +276,16 @@ async fn patch_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, auth_user, Permission::Edit, - )?; + )? + .id; - let query_alert = QueryAlert::from_uuid(conn, query_project.id, path_params.alert)?; + let query_alert = QueryAlert::from_uuid(conn, project_id, path_params.alert)?; diesel::update(schema::alert::table.filter(schema::alert::id.eq(query_alert.id))) .set(&UpdateAlert::from(json_alert)) .execute(conn) @@ -307,7 +326,13 @@ pub async fn proj_alert_stats_get( path_params.into_inner(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) diff --git a/services/api/src/endpoints/project/thresholds/mod.rs b/services/api/src/endpoints/project/thresholds/mod.rs index 460cfecef..d5f6545cd 100644 --- a/services/api/src/endpoints/project/thresholds/mod.rs +++ b/services/api/src/endpoints/project/thresholds/mod.rs @@ -17,7 +17,9 @@ use crate::{ }, error::api_error, model::project::{ + branch::QueryBranch, metric_kind::QueryMetricKind, + testbed::QueryTestbed, threshold::{statistic::InsertStatistic, InsertThreshold, QueryThreshold, UpdateThreshold}, QueryProject, }, @@ -26,7 +28,6 @@ use crate::{ util::{ cors::{get_cors, CorsResponse}, error::into_json, - same_project::SameProject, }, ApiError, }; @@ -88,7 +89,13 @@ pub async fn proj_thresholds_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -152,7 +159,13 @@ pub async fn proj_threshold_post( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -165,30 +178,21 @@ async fn post_inner( ) -> Result { let conn = &mut *context.conn().await; - // Verify that the branch and testbed are part of the same project - let SameProject { - project, - branch_id, - testbed_id, - } = SameProject::validate( - conn, - &path_params.project, - &json_threshold.branch, - &json_threshold.testbed, - )?; - let project_id = project.id; - - let metric_kind_id = - QueryMetricKind::from_resource_id(conn, project_id, &json_threshold.metric_kind)?.id; - // Verify that the user is allowed - QueryProject::is_allowed_id( + let project_id = QueryProject::is_allowed( conn, &context.rbac, - project_id, + &path_params.project, auth_user, Permission::Create, - )?; + )? + .id; + + // Verify that the branch, testbed, and metric kind are part of the same project + let branch_id = QueryBranch::from_resource_id(conn, project_id, &json_threshold.branch)?.id; + let testbed_id = QueryTestbed::from_resource_id(conn, project_id, &json_threshold.testbed)?.id; + let metric_kind_id = + QueryMetricKind::from_resource_id(conn, project_id, &json_threshold.metric_kind)?.id; // Create the new threshold let threshold_id = InsertThreshold::from_json( @@ -245,7 +249,13 @@ pub async fn proj_threshold_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) @@ -292,7 +302,13 @@ pub async fn proj_threshold_put( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -306,7 +322,7 @@ async fn put_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let query_project = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, @@ -354,7 +370,13 @@ pub async fn proj_threshold_delete( let json = delete_inner(rqctx.context(), path_params.into_inner(), &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -367,7 +389,7 @@ async fn delete_inner( let conn = &mut *context.conn().await; // Verify that the user is allowed - let query_project = QueryProject::is_allowed_resource_id( + let query_project = QueryProject::is_allowed( conn, &context.rbac, &path_params.project, diff --git a/services/api/src/endpoints/project/thresholds/statistics.rs b/services/api/src/endpoints/project/thresholds/statistics.rs index 32ae160ba..398400d2d 100644 --- a/services/api/src/endpoints/project/thresholds/statistics.rs +++ b/services/api/src/endpoints/project/thresholds/statistics.rs @@ -60,7 +60,13 @@ pub async fn proj_statistic_get( auth_user.as_ref(), ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) diff --git a/services/api/src/endpoints/system/auth/confirm.rs b/services/api/src/endpoints/system/auth/confirm.rs index 339446fc8..76c8c93d3 100644 --- a/services/api/src/endpoints/system/auth/confirm.rs +++ b/services/api/src/endpoints/system/auth/confirm.rs @@ -45,7 +45,13 @@ pub async fn auth_confirm_post( let json = post_inner(rqctx.context(), body.into_inner()) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; pub_response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/system/auth/login.rs b/services/api/src/endpoints/system/auth/login.rs index ff4ffadfa..3b303f875 100644 --- a/services/api/src/endpoints/system/auth/login.rs +++ b/services/api/src/endpoints/system/auth/login.rs @@ -50,7 +50,13 @@ pub async fn auth_login_post( let json = post_inner(&rqctx.log, rqctx.context(), body.into_inner()) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; pub_response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/system/auth/signup.rs b/services/api/src/endpoints/system/auth/signup.rs index c61423e2a..d2fa64c3f 100644 --- a/services/api/src/endpoints/system/auth/signup.rs +++ b/services/api/src/endpoints/system/auth/signup.rs @@ -56,7 +56,13 @@ pub async fn auth_signup_post( let json = post_inner(&rqctx.log, rqctx.context(), body.into_inner()) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; pub_response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/system/server/backup.rs b/services/api/src/endpoints/system/server/backup.rs index 0ebfc64f0..b2edb232e 100644 --- a/services/api/src/endpoints/system/server/backup.rs +++ b/services/api/src/endpoints/system/server/backup.rs @@ -56,7 +56,13 @@ pub async fn server_backup_post( let json_restart = body.into_inner(); let json = post_inner(context, json_restart, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/system/server/config.rs b/services/api/src/endpoints/system/server/config.rs index 57ee6ccdb..f5fcd866d 100644 --- a/services/api/src/endpoints/system/server/config.rs +++ b/services/api/src/endpoints/system/server/config.rs @@ -47,7 +47,13 @@ pub async fn server_config_get( let context = rqctx.context(); let json = get_one_inner(&rqctx.log, context, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -80,7 +86,13 @@ pub async fn server_config_put( let json_config = body.into_inner(); let json = put_inner(&rqctx.log, context, json_config, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/system/server/endpoint.rs b/services/api/src/endpoints/system/server/endpoint.rs index 1616459b2..b6d8e58e6 100644 --- a/services/api/src/endpoints/system/server/endpoint.rs +++ b/services/api/src/endpoints/system/server/endpoint.rs @@ -39,7 +39,13 @@ pub async fn server_endpoint_get( let endpoint = Endpoint::new(ENDPOINT_RESOURCE, Method::GetOne); let context = rqctx.context(); - let json = get_one_inner(context).await.map_err(|e| endpoint.err(e))?; + let json = get_one_inner(context).await.map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; if auth_user.is_some() { response_ok!(endpoint, json) diff --git a/services/api/src/endpoints/system/server/restart.rs b/services/api/src/endpoints/system/server/restart.rs index 8cb0728f8..eabda834d 100644 --- a/services/api/src/endpoints/system/server/restart.rs +++ b/services/api/src/endpoints/system/server/restart.rs @@ -49,7 +49,13 @@ pub async fn server_restart_post( let json_restart = body.into_inner(); let json = post_inner(&rqctx.log, context, json_restart, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/user/tokens.rs b/services/api/src/endpoints/user/tokens.rs index 3df6d68f1..529dae430 100644 --- a/services/api/src/endpoints/user/tokens.rs +++ b/services/api/src/endpoints/user/tokens.rs @@ -92,7 +92,13 @@ pub async fn user_tokens_get( endpoint, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -156,7 +162,13 @@ pub async fn user_token_post( let json_token = body.into_inner(); let json = post_inner(context, path_params.into_inner(), json_token, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } @@ -226,7 +238,13 @@ pub async fn user_token_get( let path_params = path_params.into_inner(); let json = get_one_inner(context, path_params, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } @@ -267,7 +285,13 @@ pub async fn user_token_patch( &auth_user, ) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_accepted!(endpoint, json) } diff --git a/services/api/src/endpoints/user/users.rs b/services/api/src/endpoints/user/users.rs index dfb290247..3388a2bb6 100644 --- a/services/api/src/endpoints/user/users.rs +++ b/services/api/src/endpoints/user/users.rs @@ -56,7 +56,13 @@ pub async fn user_get( let path_params = path_params.into_inner(); let json = get_one_inner(context, path_params, &auth_user) .await - .map_err(|e| endpoint.err(e))?; + .map_err(|e| { + if let ApiError::HttpError(e) = e { + e + } else { + endpoint.err(e).into() + } + })?; response_ok!(endpoint, json) } diff --git a/services/api/src/error.rs b/services/api/src/error.rs index 09b1320e9..c76f6db86 100644 --- a/services/api/src/error.rs +++ b/services/api/src/error.rs @@ -1,7 +1,6 @@ use bencher_json::urlencoded::UrlEncodedError; use bencher_json::ResourceId; use bencher_plot::PlotError; -use bencher_rbac::{Organization, Project}; use dropshot::HttpError; use http::StatusCode; use thiserror::Error; @@ -12,7 +11,7 @@ use crate::{ endpoints::Endpoint, model::{ project::{branch::BranchId, testbed::TestbedId, ProjectId}, - user::{auth::AuthUser, UserId}, + user::UserId, }, }; @@ -94,22 +93,10 @@ pub enum ApiError { email: String, email_user_id: UserId, }, - #[error("Failed to check permissions: {0}")] - IsAllowed(oso::OsoError), + #[error("Failed to handle JWT (JSON Web Token): {0}")] JsonWebToken(#[from] jsonwebtoken::errors::Error), - #[error("Permission denied for user ({auth_user:?}) permission ({permission}) on organization ({organization:?}")] - IsAllowedOrganization { - auth_user: AuthUser, - permission: bencher_rbac::organization::Permission, - organization: Organization, - }, - #[error("Permission denied for user ({auth_user:?}) permission ({permission}) on project ({project:?}")] - IsAllowedProject { - auth_user: AuthUser, - permission: bencher_rbac::project::Permission, - project: Project, - }, + #[error("The branch ({branch_id}) project ID ({branch_project_id}) do not match the project ID ({project_id}).")] BranchProject { project_id: ProjectId, @@ -230,6 +217,8 @@ pub enum ApiError { #[error("Failed to parse JWT (JSON Web Token): {0}")] Jwt(#[from] crate::context::JwtError), + #[error("Failed to authorize RBAC (role-based access control): {0}")] + Rbac(#[from] crate::context::RbacError), } impl From for HttpError { @@ -266,53 +255,85 @@ macro_rules! api_error { pub(crate) use api_error; -macro_rules! resource_not_found { +macro_rules! resource_not_found_err { ($resource:ident, $id:expr) => { |e| crate::error::resource_not_found_error(crate::error::BencherResource::$resource($id), e) }; } -pub(crate) use resource_not_found; +pub(crate) use resource_not_found_err; #[derive(Debug)] pub enum BencherResource { Project(Id), + MetricKind(Id), Branch(Id), Testbed(Id), + Benchmark(Id), } impl BencherResource { pub fn name(&self) -> &str { match self { Self::Project(_) => "Project", + Self::MetricKind(_) => "Metric Kind", Self::Branch(_) => "Branch", Self::Testbed(_) => "Testbed", + Self::Benchmark(_) => "Benchmark", } } pub fn id(&self) -> &Id { match self { - Self::Project(id) => id, - Self::Branch(id) => id, - Self::Testbed(id) => id, + Self::Project(id) + | Self::MetricKind(id) + | Self::Branch(id) + | Self::Testbed(id) + | Self::Benchmark(id) => id, } } } +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status + +pub fn bad_request_error(error: E) -> HttpError +where + E: std::fmt::Display, +{ + HttpError::for_client_error(None, StatusCode::BAD_REQUEST, error.to_string()) +} + +pub fn unauthorized_error(error: E) -> HttpError +where + E: std::fmt::Display, +{ + HttpError::for_client_error(None, StatusCode::UNAUTHORIZED, error.to_string()) +} + +pub fn forbidden_error(error: E) -> HttpError +where + E: std::fmt::Display, +{ + HttpError::for_client_error(None, StatusCode::FORBIDDEN, error.to_string()) +} + +pub fn not_found_error(error: E) -> HttpError +where + E: std::fmt::Display, +{ + HttpError::for_client_error(None, StatusCode::NOT_FOUND, error.to_string()) +} + pub fn resource_not_found_error(resource: BencherResource, error: E) -> HttpError where Id: std::fmt::Display, E: std::fmt::Display, { - HttpError::for_client_error( - None, - StatusCode::NOT_FOUND, - format!( - "{resource} not found ({id}): {error}", - resource = resource.name(), - id = resource.id() - ), - ) + not_found_error(format!( + "{resource} ({id}) not found: {error}", + resource = resource.name(), + id = resource.id(), + )) } pub fn issue_error(status_code: StatusCode, title: &str, body: &str, error: E) -> HttpError diff --git a/services/api/src/model/project/benchmark.rs b/services/api/src/model/project/benchmark.rs index e72602bd1..d4086c0b5 100644 --- a/services/api/src/model/project/benchmark.rs +++ b/services/api/src/model/project/benchmark.rs @@ -6,6 +6,7 @@ use bencher_json::{ }; use chrono::Utc; use diesel::{ExpressionMethods, Insertable, JoinOnDsl, QueryDsl, Queryable, RunQueryDsl}; +use dropshot::HttpError; use uuid::Uuid; use super::{ @@ -15,7 +16,7 @@ use super::{ }; use crate::{ context::DbConnection, - error::api_error, + error::{api_error, resource_not_found_err}, schema, schema::benchmark as benchmark_table, util::{ @@ -86,12 +87,12 @@ impl QueryBenchmark { conn: &mut DbConnection, project_id: ProjectId, benchmark: &ResourceId, - ) -> Result { + ) -> Result { schema::benchmark::table .filter(schema::benchmark::project_id.eq(project_id)) .filter(resource_id(benchmark)?) .first::(conn) - .map_err(api_error!()) + .map_err(resource_not_found_err!(Benchmark, benchmark.clone())) } pub fn get_or_create( @@ -210,10 +211,9 @@ pub struct InsertBenchmark { impl InsertBenchmark { pub fn from_json( conn: &mut DbConnection, - project: &ResourceId, + project_id: ProjectId, benchmark: JsonNewBenchmark, - ) -> Result { - let project_id = QueryProject::from_resource_id(conn, project)?.id; + ) -> Self { let JsonNewBenchmark { name, slug } = benchmark; let slug = unwrap_child_slug!( conn, @@ -224,14 +224,14 @@ impl InsertBenchmark { QueryBenchmark ); let timestamp = Utc::now().timestamp(); - Ok(Self { + Self { uuid: Uuid::new_v4().to_string(), project_id, name: name.into(), slug, created: timestamp, modified: timestamp, - }) + } } fn from_name(project_id: ProjectId, name: String) -> Self { diff --git a/services/api/src/model/project/branch.rs b/services/api/src/model/project/branch.rs index 1b6b4bd41..0094355a9 100644 --- a/services/api/src/model/project/branch.rs +++ b/services/api/src/model/project/branch.rs @@ -19,7 +19,7 @@ use super::{ }; use crate::{ context::DbConnection, - error::{api_error, resource_not_found}, + error::{api_error, resource_not_found_err}, model::project::threshold::{InsertThreshold, QueryThreshold}, schema, schema::branch as branch_table, @@ -83,7 +83,7 @@ impl QueryBranch { .filter(schema::branch::project_id.eq(project_id)) .filter(resource_id(branch)?) .first::(conn) - .map_err(resource_not_found!(Branch, branch.clone())) + .map_err(resource_not_found_err!(Branch, branch.clone())) } pub fn get_branch_version_json( @@ -157,19 +157,6 @@ pub struct InsertBranch { impl InsertBranch { pub fn from_json( - conn: &mut DbConnection, - project: &ResourceId, - branch: JsonNewBranch, - ) -> Result { - let project_id = QueryProject::from_resource_id(conn, project)?.id; - Ok(Self::from_json_inner(conn, project_id, branch)) - } - - pub fn main(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewBranch::main()) - } - - fn from_json_inner( conn: &mut DbConnection, project_id: ProjectId, branch: JsonNewBranch, @@ -187,6 +174,10 @@ impl InsertBranch { } } + pub fn main(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewBranch::main()) + } + pub fn start_point( &self, conn: &mut DbConnection, diff --git a/services/api/src/model/project/metric_kind.rs b/services/api/src/model/project/metric_kind.rs index c5569ef12..ac9409205 100644 --- a/services/api/src/model/project/metric_kind.rs +++ b/services/api/src/model/project/metric_kind.rs @@ -11,11 +11,12 @@ use bencher_json::{ }; use chrono::Utc; use diesel::{ExpressionMethods, Insertable, QueryDsl, Queryable, RunQueryDsl}; +use dropshot::HttpError; use uuid::Uuid; use crate::{ context::DbConnection, - error::api_error, + error::{api_error, resource_not_found_err}, model::project::QueryProject, schema, schema::metric_kind as metric_kind_table, @@ -65,12 +66,12 @@ impl QueryMetricKind { conn: &mut DbConnection, project_id: ProjectId, metric_kind: &ResourceId, - ) -> Result { + ) -> Result { schema::metric_kind::table .filter(schema::metric_kind::project_id.eq(project_id)) .filter(resource_id(metric_kind)?) - .first::(conn) - .map_err(api_error!()) + .first::(conn) + .map_err(resource_not_found_err!(MetricKind, metric_kind.clone())) } pub fn into_json(self, conn: &mut DbConnection) -> Result { @@ -102,8 +103,8 @@ impl QueryMetricKind { ) -> Result { let query_metric_kind = Self::from_resource_id(conn, project_id, metric_kind); - let api_error = match query_metric_kind { - Ok(query) => return Ok(query.id), + let http_error = match query_metric_kind { + Ok(metric_kind) => return Ok(metric_kind.id), Err(e) => e, }; @@ -119,7 +120,7 @@ impl QueryMetricKind { L2_ACCESSES_SLUG_STR => InsertMetricKind::l2_accesses(conn, project_id), RAM_ACCESSES_SLUG_STR => InsertMetricKind::ram_accesses(conn, project_id), ESTIMATED_CYCLES_SLUG_STR => InsertMetricKind::estimated_cycles(conn, project_id), - _ => return Err(api_error), + _ => return Err(http_error.into()), }; diesel::insert_into(schema::metric_kind::table) .values(&insert_metric_kind) @@ -148,43 +149,6 @@ pub struct InsertMetricKind { impl InsertMetricKind { pub fn from_json( - conn: &mut DbConnection, - project: &ResourceId, - metric_kind: JsonNewMetricKind, - ) -> Result { - let project_id = QueryProject::from_resource_id(conn, project)?.id; - Ok(Self::from_json_inner(conn, project_id, metric_kind)) - } - - pub fn latency(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::latency()) - } - - pub fn throughput(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::throughput()) - } - - pub fn instructions(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::instructions()) - } - - pub fn l1_accesses(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::l1_accesses()) - } - - pub fn l2_accesses(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::l2_accesses()) - } - - pub fn ram_accesses(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::ram_accesses()) - } - - pub fn estimated_cycles(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewMetricKind::estimated_cycles()) - } - - fn from_json_inner( conn: &mut DbConnection, project_id: ProjectId, metric_kind: JsonNewMetricKind, @@ -210,6 +174,34 @@ impl InsertMetricKind { } } + pub fn latency(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::latency()) + } + + pub fn throughput(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::throughput()) + } + + pub fn instructions(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::instructions()) + } + + pub fn l1_accesses(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::l1_accesses()) + } + + pub fn l2_accesses(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::l2_accesses()) + } + + pub fn ram_accesses(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::ram_accesses()) + } + + pub fn estimated_cycles(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewMetricKind::estimated_cycles()) + } + pub fn is_system(&self) -> bool { is_system(self.name.as_ref(), self.slug.as_ref()) } diff --git a/services/api/src/model/project/mod.rs b/services/api/src/model/project/mod.rs index 1889ff62a..c18cedf57 100644 --- a/services/api/src/model/project/mod.rs +++ b/services/api/src/model/project/mod.rs @@ -18,7 +18,7 @@ use uuid::Uuid; use crate::{ context::{DbConnection, Rbac}, - error::{api_error, resource_not_found}, + error::{api_error, forbidden_error, resource_not_found_err, unauthorized_error}, model::{organization::QueryOrganization, user::auth::AuthUser}, schema::{self, project as project_table}, util::{query::fn_get, resource_id::fn_resource_id, slug::unwrap_slug, to_date_time}, @@ -27,7 +27,7 @@ use crate::{ use self::visibility::Visibility; -use super::organization::OrganizationId; +use super::{organization::OrganizationId, user::auth::BEARER_TOKEN_FORMAT}; pub mod benchmark; pub mod branch; @@ -134,8 +134,8 @@ impl QueryProject { ) -> Result { schema::project::table .filter(resource_id(project)?) - .first::(conn) - .map_err(resource_not_found!(Project, project.clone())) + .first::(conn) + .map_err(resource_not_found_err!(Project, project.clone())) } pub fn get_uuid(conn: &mut DbConnection, id: ProjectId) -> Result { @@ -202,34 +202,16 @@ impl QueryProject { }) } - pub fn is_allowed_resource_id( + pub fn is_allowed( conn: &mut DbConnection, rbac: &Rbac, project: &ResourceId, auth_user: &AuthUser, permission: bencher_rbac::project::Permission, - ) -> Result { - let query_project = QueryProject::from_resource_id(conn, project)?; - - rbac.is_allowed_project(auth_user, permission, &query_project)?; - - Ok(query_project) - } - - pub fn is_allowed_id( - conn: &mut DbConnection, - rbac: &Rbac, - project_id: ProjectId, - auth_user: &AuthUser, - permission: bencher_rbac::project::Permission, - ) -> Result { - let query_project = schema::project::table - .filter(schema::project::id.eq(project_id)) - .first(conn) - .map_err(api_error!())?; - - rbac.is_allowed_project(auth_user, permission, &query_project)?; - + ) -> Result { + let query_project = Self::from_resource_id(conn, project)?; + rbac.is_allowed_project(auth_user, permission, &query_project) + .map_err(forbidden_error)?; Ok(query_project) } @@ -238,26 +220,26 @@ impl QueryProject { rbac: &Rbac, project: &ResourceId, auth_user: Option<&AuthUser>, - ) -> Result { - // Get the project - let project = QueryProject::from_resource_id(conn, project)?; - + ) -> Result { + let query_project = Self::from_resource_id(conn, project)?; // Check to see if the project is public // If so, anyone can access it - if project.visibility.is_public() { - Ok(project) + if query_project.visibility.is_public() { + Ok(query_project) } else if let Some(auth_user) = auth_user { // If there is an `AuthUser` then validate access // Verify that the user is allowed - QueryProject::is_allowed_id( - conn, - rbac, - project.id, + rbac.is_allowed_project( auth_user, bencher_rbac::project::Permission::View, + &query_project, ) + .map_err(forbidden_error)?; + Ok(query_project) } else { - Err(ApiError::PrivateProject(project.id)) + Err(unauthorized_error(format!( + "Project ({query_project:?}) is not public and requires authentication.\n{BEARER_TOKEN_FORMAT}", + ))) } } } diff --git a/services/api/src/model/project/testbed.rs b/services/api/src/model/project/testbed.rs index a7d0ee1de..f5ba63c8c 100644 --- a/services/api/src/model/project/testbed.rs +++ b/services/api/src/model/project/testbed.rs @@ -12,7 +12,7 @@ use uuid::Uuid; use super::{ProjectId, QueryProject}; use crate::{ context::DbConnection, - error::{api_error, resource_not_found}, + error::{api_error, resource_not_found_err}, schema, schema::testbed as testbed_table, util::{ @@ -75,7 +75,7 @@ impl QueryTestbed { .filter(schema::testbed::project_id.eq(project_id)) .filter(resource_id(testbed)?) .first::(conn) - .map_err(resource_not_found!(Testbed, testbed.clone())) + .map_err(resource_not_found_err!(Testbed, testbed.clone())) } pub fn into_json(self, conn: &mut DbConnection) -> Result { @@ -117,19 +117,6 @@ pub struct InsertTestbed { impl InsertTestbed { pub fn from_json( - conn: &mut DbConnection, - project: &ResourceId, - testbed: JsonNewTestbed, - ) -> Result { - let project_id = QueryProject::from_resource_id(conn, project)?.id; - Ok(Self::from_json_inner(conn, project_id, testbed)) - } - - pub fn localhost(conn: &mut DbConnection, project_id: ProjectId) -> Self { - Self::from_json_inner(conn, project_id, JsonNewTestbed::localhost()) - } - - fn from_json_inner( conn: &mut DbConnection, project_id: ProjectId, testbed: JsonNewTestbed, @@ -146,6 +133,10 @@ impl InsertTestbed { modified: timestamp, } } + + pub fn localhost(conn: &mut DbConnection, project_id: ProjectId) -> Self { + Self::from_json(conn, project_id, JsonNewTestbed::localhost()) + } } #[derive(Debug, Clone, AsChangeset)] diff --git a/services/api/src/model/user/auth.rs b/services/api/src/model/user/auth.rs index 6ba8cbdc9..eee7203c4 100644 --- a/services/api/src/model/user/auth.rs +++ b/services/api/src/model/user/auth.rs @@ -11,12 +11,15 @@ use oso::{PolarValue, ToPolar}; use crate::{ context::{ApiContext, DbConnection, Rbac}, + error::{bad_request_error, forbidden_error, not_found_error}, model::{organization::OrganizationId, project::ProjectId}, schema, }; use super::UserId; +pub const BEARER_TOKEN_FORMAT: &str = "Expected format is `Authorization: Bearer `.\nWhere `` is your Bencher API token."; + #[derive(Debug, Clone)] pub struct AuthUser { pub id: UserId, @@ -52,70 +55,46 @@ impl AuthUser { pub async fn new(rqctx: &RequestContext) -> Result { let request = &rqctx.request; - const EXPECTED: &str = "Expected format is `Authorization: Bearer `.\nWhere `` is your Bencher API token."; let headers = request .headers() .get("Authorization") .ok_or_else(|| { - HttpError::for_client_error( - None, - StatusCode::UNAUTHORIZED, - format!("Request is missing \"Authorization\" header.\n{EXPECTED}"), - ) + bad_request_error(format!( + "Request is missing \"Authorization\" header.\n{BEARER_TOKEN_FORMAT}" + )) })? .to_str() .map_err(|e| { - HttpError::for_client_error( - None, - StatusCode::UNAUTHORIZED, - format!("Request has an invalid \"Authorization\" header: {e}\n{EXPECTED}"), - ) + bad_request_error(format!( + "Request has an invalid \"Authorization\" header: {e}\n{BEARER_TOKEN_FORMAT}" + )) })?; let (_, token) = headers.split_once("Bearer ").ok_or_else(|| { - HttpError::for_client_error( - None, - StatusCode::UNAUTHORIZED, - format!("Request is missing \"Authorization\" Bearer.\n{EXPECTED}"), - ) + bad_request_error(format!( + "Request is missing \"Authorization\" Bearer.\n{BEARER_TOKEN_FORMAT}" + )) })?; let token = token.trim(); - let jwt: Jwt = token.parse().map_err(|e| { - HttpError::for_client_error( - None, - StatusCode::UNAUTHORIZED, - format!("Malformed JSON Web Token: {e}"), - ) - })?; + let jwt: Jwt = token + .parse() + .map_err(|e| bad_request_error(format!("Malformed JSON Web Token: {e}")))?; let context = rqctx.context(); let conn = &mut *context.conn().await; - let claims = context.secret_key.validate_client(&jwt).map_err(|e| { - HttpError::for_client_error( - None, - StatusCode::UNAUTHORIZED, - format!("Failed to validate JSON Web Token: {e}"), - ) - })?; + let claims = context + .secret_key + .validate_client(&jwt) + .map_err(|e| bad_request_error(format!("Failed to validate JSON Web Token: {e}")))?; let email = claims.email(); let (user_id, admin, locked) = schema::user::table .filter(schema::user::email.eq(email)) .select((schema::user::id, schema::user::admin, schema::user::locked)) .first::<(UserId, bool, bool)>(conn) - .map_err(|e| { - HttpError::for_client_error( - None, - StatusCode::NOT_FOUND, - format!("Failed to find user ({email}): {e}"), - ) - })?; + .map_err(|e| not_found_error(format!("Failed to find user ({email}): {e}")))?; if locked { - return Err(HttpError::for_client_error( - None, - StatusCode::UNAUTHORIZED, - format!("User account is locked ({email})"), - )); + return Err(forbidden_error(format!("User account is locked ({email})"))); } let (org_ids, org_roles) = Self::organization_roles(conn, user_id, email)?; diff --git a/services/api/src/util/mod.rs b/services/api/src/util/mod.rs index 87364c420..2b1c4dcaa 100644 --- a/services/api/src/util/mod.rs +++ b/services/api/src/util/mod.rs @@ -8,7 +8,6 @@ pub mod headers; pub mod logger; pub mod query; pub mod resource_id; -pub mod same_project; pub mod slug; pub mod typed_id; diff --git a/services/api/src/util/same_project.rs b/services/api/src/util/same_project.rs deleted file mode 100644 index b713b0feb..000000000 --- a/services/api/src/util/same_project.rs +++ /dev/null @@ -1,74 +0,0 @@ -use bencher_json::ResourceId; -use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl}; -use dropshot::HttpError; - -use crate::{ - context::DbConnection, - error::api_error, - model::project::{ - branch::{BranchId, QueryBranch}, - testbed::{QueryTestbed, TestbedId}, - ProjectId, QueryProject, - }, - schema, ApiError, -}; - -pub struct SameProject { - pub project: QueryProject, - pub branch_id: BranchId, - pub testbed_id: TestbedId, -} - -impl SameProject { - pub fn validate( - conn: &mut DbConnection, - project: &ResourceId, - branch: &ResourceId, - testbed: &ResourceId, - ) -> Result { - let project = QueryProject::from_resource_id(conn, project)?; - let branch_id = QueryBranch::from_resource_id(conn, project.id, branch)?.id; - let testbed_id = QueryTestbed::from_resource_id(conn, project.id, testbed)?.id; - - Ok(Self { - project, - branch_id, - testbed_id, - }) - } - - pub fn validate_ids( - conn: &mut DbConnection, - project_id: ProjectId, - branch_id: BranchId, - testbed_id: TestbedId, - ) -> Result<(), ApiError> { - let branch_project_id = schema::branch::table - .filter(schema::branch::id.eq(branch_id)) - .select(schema::branch::project_id) - .first::(conn) - .map_err(api_error!())?; - if project_id != branch_project_id { - return Err(ApiError::BranchProject { - project_id, - branch_id, - branch_project_id, - }); - } - - let testbed_project_id = schema::testbed::table - .filter(schema::testbed::id.eq(testbed_id)) - .select(schema::testbed::project_id) - .first::(conn) - .map_err(api_error!())?; - if project_id != testbed_project_id { - return Err(ApiError::TestbedProject { - project_id, - testbed_id, - testbed_project_id, - }); - } - - Ok(()) - } -}