From cbca89bed14a21cb2a663ffae231dd01c08818e3 Mon Sep 17 00:00:00 2001 From: "ayush.jain@juspay.in" Date: Wed, 11 Mar 2026 16:18:14 +0530 Subject: [PATCH 1/4] feat(authz): ABAC for default_config, context and experiment --- .../src/api/context/handlers.rs | 140 +++++++++++- .../src/api/context/helpers.rs | 24 +++ .../src/api/context/operations.rs | 31 +++ .../src/api/default_config/handlers.rs | 16 +- .../src/api/experiments/handlers.rs | 66 +++++- .../src/api/experiments/helpers.rs | 26 +++ crates/frontend/src/api.rs | 200 +++++++++++------- crates/frontend/src/components/authz.rs | 16 +- .../service_utils/src/middlewares/auth_z.rs | 59 +++++- .../src/middlewares/auth_z/authorization.rs | 2 +- .../src/middlewares/auth_z/casbin.rs | 2 +- .../src/middlewares/auth_z/no_auth.rs | 2 +- crates/superposition_types/src/lib.rs | 4 +- 13 files changed, 477 insertions(+), 111 deletions(-) diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index 171fcac07..84e68b123 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -1,4 +1,4 @@ -use std::{cmp::min, collections::HashSet}; +use std::{cmp::min, collections::HashSet, ops::Deref}; use actix_web::{ Either, HttpResponse, Scope, delete, get, post, put, routes, @@ -17,22 +17,23 @@ use service_utils::{ helpers::{ WebhookData, execute_webhook_call, fetch_dimensions_info_map, parse_config_tags, }, + middlewares::auth_z::{Action as AuthZAction, AuthZ}, service::types::{ - AppHeader, AppState, CustomHeaders, DbConnection, WorkspaceContext, + AppHeader, AppState, CustomHeaders, DbConnection, SchemaName, WorkspaceContext, }, }; use superposition_core::helpers::{calculate_context_weight, hash}; use superposition_derives::{authorized, declare_resource}; use superposition_macros::{bad_argument, db_error, unexpected_error}; use superposition_types::{ - Contextual, InternalUserContext, ListResponse, Overridden, Overrides, - PaginatedResponse, Resource, SortBy, User, + Cac, Contextual, DBConnection, InternalUserContext, ListResponse, Overridden, + Overrides, PaginatedResponse, Resource, SortBy, User, api::{ DimensionMatchStrategy, context::{ BulkOperation, BulkOperationResponse, ContextAction, ContextBulkResponse, - ContextListFilters, ContextValidationRequest, MoveRequest, PutRequest, - SortOn, UpdateRequest, WeightRecomputeResponse, + ContextListFilters, ContextValidationRequest, Identifier, MoveRequest, + PutRequest, SortOn, UpdateRequest, WeightRecomputeResponse, }, webhook::Action, }, @@ -50,7 +51,7 @@ use superposition_types::{ use crate::{ api::context::{ - helpers::{query_description, validate_ctx}, + helpers::{changed_keys, query_description, validate_ctx}, operations, }, helpers::{add_config_version, put_config_in_redis, validate_change_reason}, @@ -72,6 +73,16 @@ pub fn endpoints() -> Scope { .service(validate_handler) } +async fn authorize_create( + auth_z: &AuthZ, + override_map: &Cac, +) -> superposition::Result<()> { + let keys = override_map.deref().keys().collect::>(); + auth_z + .authorize_action(&AuthZActionCreate::get(), &keys) + .await +} + #[allow(clippy::too_many_arguments)] #[authorized] #[put("")] @@ -84,6 +95,9 @@ async fn create_handler( user: User, internal_user: InternalUserContext, ) -> superposition::Result { + let req = req.into_inner(); + authorize_create(&_auth_z, &req.r#override).await?; + let tags = parse_config_tags(custom_headers.config_tags)?; let description = match req.description.clone() { Some(val) => val, @@ -117,7 +131,7 @@ async fn create_handler( let (put_response, version_id) = db_conn .transaction::<_, superposition::AppError, _>(|transaction_conn| { let put_response = operations::upsert( - req.into_inner(), + req, description, transaction_conn, true, @@ -179,6 +193,24 @@ async fn create_handler( Ok(http_resp.json(put_response)) } +async fn authorize_update( + auth_z: &AuthZ, + context: &Identifier, + new_overrides: &Cac, + schema_name: &SchemaName, + conn: &mut DBConnection, +) -> superposition::Result<()> { + let overrides = + operations::get_overrides_from_identifier(context, schema_name, conn)?; + + auth_z + .authorize_action( + &AuthZActionUpdate::get(), + &changed_keys(&overrides, &new_overrides.deref()), + ) + .await +} + #[authorized] #[routes] #[put("/overrides")] @@ -194,6 +226,15 @@ async fn update_handler( let tags = parse_config_tags(custom_headers.config_tags)?; let req_change_reason = req.change_reason.clone(); + authorize_update( + &_auth_z, + &req.context, + &req.override_, + &workspace_context.schema_name, + &mut db_conn, + ) + .await?; + validate_change_reason( &workspace_context, &req_change_reason, @@ -262,6 +303,21 @@ async fn update_handler( Ok(http_resp.json(override_resp)) } +async fn authorize_action( + auth_z: &AuthZ, + ctx_id: &String, + schema_name: &SchemaName, + conn: &mut DBConnection, +) -> superposition::Result<()> { + let overrides = operations::get_overrides_from_ctx_id(ctx_id, schema_name, conn)?; + auth_z + .authorize_action( + &AuthZActionMove::get(), + &overrides.keys().collect::>(), + ) + .await +} + #[allow(clippy::too_many_arguments)] #[authorized] #[put("/move/{ctx_id}")] @@ -275,6 +331,15 @@ async fn move_handler( user: User, internal_user: InternalUserContext, ) -> superposition::Result { + let ctx_id = path.into_inner(); + authorize_action( + &_auth_z, + &ctx_id, + &workspace_context.schema_name, + &mut db_conn, + ) + .await?; + let tags = parse_config_tags(custom_headers.config_tags)?; let description = match req.description.clone() { @@ -309,7 +374,7 @@ async fn move_handler( .transaction::<_, superposition::AppError, _>(|transaction_conn| { let move_response = operations::r#move( &workspace_context, - path.into_inner(), + ctx_id, req, description, transaction_conn, @@ -556,6 +621,21 @@ async fn list_handler( Ok(Json(paginated_response)) } +async fn authorize_delete( + auth_z: &AuthZ, + ctx_id: &String, + schema_name: &SchemaName, + conn: &mut DBConnection, +) -> superposition::Result<()> { + let overrides = operations::get_overrides_from_ctx_id(ctx_id, schema_name, conn)?; + auth_z + .authorize_action( + &AuthZActionDelete::get(), + &overrides.keys().collect::>(), + ) + .await +} + #[authorized] #[delete("/{ctx_id}")] async fn delete_handler( @@ -570,6 +650,14 @@ async fn delete_handler( contexts as contexts_table, id as context_id, }; let ctx_id = path.into_inner(); + authorize_delete( + &_auth_z, + &ctx_id, + &workspace_context.schema_name, + &mut db_conn, + ) + .await?; + let tags = parse_config_tags(custom_headers.config_tags)?; let (version_id, deleted_ctx) = db_conn .transaction::<_, superposition::AppError, _>(|transaction_conn| { @@ -632,6 +720,38 @@ async fn delete_handler( .finish()) } +async fn authorize_bulk( + auth_z: &AuthZ, + operations: &Vec, + schema_name: &SchemaName, + conn: &mut DBConnection, +) -> superposition::Result<()> { + for op in operations { + match op { + ContextAction::Put(put_req) => { + authorize_create(auth_z, &put_req.r#override).await?; + } + ContextAction::Replace(update_req) => { + authorize_update( + auth_z, + &update_req.context, + &update_req.override_, + schema_name, + conn, + ) + .await?; + } + ContextAction::Delete(ctx_id) => { + authorize_delete(auth_z, ctx_id, schema_name, conn).await?; + } + ContextAction::Move { id: ctx_id, .. } => { + authorize_action(auth_z, ctx_id, schema_name, conn).await?; + } + } + } + Ok(()) +} + #[allow(clippy::too_many_arguments)] #[authorized] #[put("/bulk-operations")] @@ -655,6 +775,8 @@ async fn bulk_operations_handler( bo.into_inner().operations } }; + authorize_bulk(&_auth_z, &ops, &workspace_context.schema_name, &mut conn).await?; + // Marking immutable. let is_v2 = is_v2; let mut all_change_reasons = Vec::new(); diff --git a/crates/context_aware_config/src/api/context/helpers.rs b/crates/context_aware_config/src/api/context/helpers.rs index c51325fcd..b2fde2a40 100644 --- a/crates/context_aware_config/src/api/context/helpers.rs +++ b/crates/context_aware_config/src/api/context/helpers.rs @@ -501,3 +501,27 @@ pub fn validate_ctx( )?; Ok(dimension_info_map) } + +pub fn changed_keys<'a>( + old_values: &'a Map, + new_values: &'a Map, +) -> Vec<&'a String> { + let keys = old_values + .keys() + .chain(new_values.keys()) + .collect::>(); + + keys.into_iter() + .filter(|key| { + let old_value = old_values + .get(*key) + .cloned() + .unwrap_or(Value::String("##NOT_PRESENT##".to_string())); + let new_value = new_values + .get(*key) + .cloned() + .unwrap_or(Value::String("##DELETED##".to_string())); + old_value != new_value + }) + .collect() +} diff --git a/crates/context_aware_config/src/api/context/operations.rs b/crates/context_aware_config/src/api/context/operations.rs index a631f1fa5..4016bfd32 100644 --- a/crates/context_aware_config/src/api/context/operations.rs +++ b/crates/context_aware_config/src/api/context/operations.rs @@ -1,3 +1,5 @@ +use std::ops::Deref; + use actix_web::web::Json; use chrono::Utc; use diesel::{ @@ -94,6 +96,35 @@ pub fn upsert( } } +pub fn get_overrides_from_ctx_id( + ctx_id: &str, + schema_name: &SchemaName, + conn: &mut PooledConnection>, +) -> result::Result { + let overrides = dsl::contexts + .filter(dsl::id.eq(ctx_id)) + .schema_name(&schema_name) + .select(dsl::override_) + .first::(conn)?; + + Ok(overrides) +} + +pub fn get_overrides_from_identifier( + identifier: &Identifier, + schema_name: &SchemaName, + conn: &mut PooledConnection>, +) -> result::Result { + let context_id = match identifier { + Identifier::Context(context) => { + &hash(&Value::Object((**context.deref()).clone())) + } + Identifier::Id(id) => id, + }; + + get_overrides_from_ctx_id(context_id, schema_name, conn) +} + pub fn update( workspace_context: &WorkspaceContext, req: UpdateRequest, diff --git a/crates/context_aware_config/src/api/default_config/handlers.rs b/crates/context_aware_config/src/api/default_config/handlers.rs index 2a631ab02..0038312c1 100644 --- a/crates/context_aware_config/src/api/default_config/handlers.rs +++ b/crates/context_aware_config/src/api/default_config/handlers.rs @@ -1,3 +1,5 @@ +use std::ops::Deref; + use actix_web::{ HttpResponse, Scope, delete, get, post, routes, web::{Data, Json, Path, Query}, @@ -75,8 +77,10 @@ async fn create_handler( db_conn: DbConnection, user: User, ) -> superposition::Result { - let DbConnection(mut conn) = db_conn; let req = request.into_inner(); + _auth_z.authorize(&[req.key.deref()]).await?; + + let DbConnection(mut conn) = db_conn; let key = req.key; let tags = parse_config_tags(custom_headers.config_tags)?; @@ -228,9 +232,12 @@ async fn update_handler( db_conn: DbConnection, user: User, ) -> superposition::Result { + let key = key.into_inner(); + _auth_z.authorize(&[key.deref()]).await?; + let DbConnection(mut conn) = db_conn; let req = request.into_inner(); - let key_str = key.into_inner().into(); + let key_str = key.into(); let tags = parse_config_tags(custom_headers.config_tags)?; let existing = fetch_default_key(&key_str, &mut conn, &workspace_context.schema_name) @@ -504,10 +511,13 @@ async fn delete_handler( db_conn: DbConnection, user: User, ) -> superposition::Result { + let key = path.into_inner(); + _auth_z.authorize(&[key.deref()]).await?; + let DbConnection(mut conn) = db_conn; let tags = parse_config_tags(custom_headers.config_tags)?; - let key: String = path.into_inner().into(); + let key: String = key.into(); let context_ids = get_key_usage_context_ids(&key, &mut conn, &workspace_context.schema_name) diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index b5111e1fc..7979c4fcb 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -28,12 +28,13 @@ use service_utils::{ WebhookData, construct_request_headers, execute_webhook_call, fetch_dimensions_info_map, generate_snowflake_id, request, }, + middlewares::auth_z::{Action as AuthZAction, AuthZ}, redis::{ EXPERIMENT_GROUPS_LIST_KEY_SUFFIX, EXPERIMENTS_LAST_MODIFIED_KEY_SUFFIX, EXPERIMENTS_LIST_KEY_SUFFIX, read_through_cache, }, service::types::{ - AppHeader, AppState, CustomHeaders, DbConnection, WorkspaceContext, + AppHeader, AppState, CustomHeaders, DbConnection, SchemaName, WorkspaceContext, }, }; use superposition_derives::{authorized, declare_resource}; @@ -86,7 +87,8 @@ use crate::api::{ }, experiments::{ helpers::{ - fetch_and_validate_change_reason_with_function, put_experiments_in_redis, + fetch_and_validate_change_reason_with_function, + get_control_overrides_from_exp_id, put_experiments_in_redis, validate_control_overrides, validate_delete_experiment_variants, }, types::StartedByChangeSet, @@ -131,6 +133,18 @@ fn add_config_version_to_header( } } +async fn authorize_create( + auth_z: AuthZ, + variants: &Vec, +) -> superposition::Result<()> { + let control_keys = variants + .iter() + .filter(|v| v.variant_type == VariantType::CONTROL) + .flat_map(|v| v.overrides.deref().keys()) + .collect::>(); + auth_z.authorize(&control_keys).await +} + #[authorized] #[post("")] async fn create_handler( @@ -142,7 +156,10 @@ async fn create_handler( user: User, ) -> superposition::Result { use superposition_types::database::schema::experiments::dsl::experiments; - let mut variants = req.variants.to_vec(); + let req = req.into_inner(); + authorize_create(_auth_z, &req.variants).await?; + + let mut variants = req.variants; let DbConnection(mut conn) = db_conn; let description = req.description.clone(); let change_reason = req.change_reason.clone(); @@ -158,7 +175,7 @@ async fn create_handler( // atleast 1 experimental variant check_variant_types(&variants)?; let unique_override_keys: Vec = - extract_override_keys(&variants[0].overrides.clone().into_inner()) + extract_override_keys(&variants[0].overrides.deref()) .into_iter() .collect(); @@ -178,7 +195,7 @@ async fn create_handler( // Checking if all the variants are overriding the mentioned keys let variant_overrides = variants .iter() - .map(|variant| variant.overrides.clone().into_inner()) + .map(|variant| variant.overrides.deref().clone()) .collect::>(); match req.experiment_type { @@ -409,6 +426,19 @@ async fn create_handler( Ok(http_resp.json(response)) } +async fn authorize_action( + auth_z: AuthZ, + exp_id: &i64, + schema_name: &SchemaName, + conn: &mut DBConnection, +) -> superposition::Result<()> { + let overrides = get_control_overrides_from_exp_id(exp_id, schema_name, conn)?; + + auth_z + .authorize(&overrides.deref().keys().collect::>()) + .await +} + #[allow(clippy::too_many_arguments)] #[authorized] #[patch("/{experiment_id}/conclude")] @@ -422,6 +452,8 @@ async fn conclude_handler( user: User, ) -> superposition::Result { let DbConnection(mut conn) = db_conn; + let exp_id = path.into_inner(); + authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -432,7 +464,7 @@ async fn conclude_handler( let (response, config_version_id) = conclude( &state, - path.into_inner(), + exp_id, custom_headers.config_tags, req.into_inner(), &mut conn, @@ -694,6 +726,8 @@ async fn discard_handler( user: User, ) -> superposition::Result { let DbConnection(mut conn) = db_conn; + let exp_id = path.into_inner(); + authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -704,7 +738,7 @@ async fn discard_handler( let (response, config_version_id) = discard( &state, - path.into_inner(), + exp_id, custom_headers.config_tags, req.into_inner(), &mut conn, @@ -1255,6 +1289,8 @@ async fn ramp_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let exp_id = params.into_inner(); + authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; + let change_reason = req.change_reason.clone(); fetch_and_validate_change_reason_with_function( @@ -1446,6 +1482,14 @@ async fn update_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let experiment_id = params.into_inner(); + authorize_action( + _auth_z, + &experiment_id, + &workspace_context.schema_name, + &mut conn, + ) + .await?; + let experiment_group_id = req.experiment_group_id.clone(); let description = req.description.clone(); let change_reason = req.change_reason.clone(); @@ -1785,6 +1829,8 @@ async fn pause_handler( user: User, ) -> superposition::Result { let DbConnection(mut conn) = db_conn; + let exp_id = path.into_inner(); + authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -1794,7 +1840,7 @@ async fn pause_handler( .await?; let response = pause( - path.into_inner(), + exp_id, req.into_inner(), &mut conn, &workspace_context, @@ -1878,6 +1924,8 @@ async fn resume_handler( user: User, ) -> superposition::Result { let DbConnection(mut conn) = db_conn; + let exp_id = path.into_inner(); + authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -1887,7 +1935,7 @@ async fn resume_handler( .await?; let response = resume( - path.into_inner(), + exp_id, req.into_inner(), &mut conn, &workspace_context, diff --git a/crates/experimentation_platform/src/api/experiments/helpers.rs b/crates/experimentation_platform/src/api/experiments/helpers.rs index 316be8e0e..c09cdc01d 100644 --- a/crates/experimentation_platform/src/api/experiments/helpers.rs +++ b/crates/experimentation_platform/src/api/experiments/helpers.rs @@ -38,6 +38,7 @@ use superposition_types::{ ChangeReason, experimentation::{ Experiment, ExperimentStatusType, GroupType, Variant, VariantType, + Variants, }, }, schema::experiments::dsl as experiments, @@ -850,3 +851,28 @@ pub async fn put_experiments_in_redis( log::debug!("Successfully updated experiments cache in Redis"); Ok(()) } + +pub fn get_control_overrides_from_exp_id( + exp_id: &i64, + schema_name: &SchemaName, + conn: &mut DBConnection, +) -> superposition::Result> { + use superposition_types::database::schema::experiments::dsl as experiments_dsl; + + let variants = experiments_dsl::experiments + .find(exp_id) + .schema_name(schema_name) + .select(experiments_dsl::variants) + .get_result::(conn)?; + + let control_overrides = variants + .into_inner() + .into_iter() + .find(|variant| variant.variant_type == VariantType::CONTROL) + .ok_or_else(|| { + log::error!("No control variant found for experiment id {}", exp_id); + bad_argument!("No control variant found for the given experiment id") + })?; + + Ok(control_overrides.overrides.into()) +} diff --git a/crates/frontend/src/api.rs b/crates/frontend/src/api.rs index 3c3cc60d4..29f1f1e2e 100644 --- a/crates/frontend/src/api.rs +++ b/crates/frontend/src/api.rs @@ -29,13 +29,7 @@ use crate::utils::{ pub mod casbin { use std::collections::HashMap; - use superposition_types::{ - Resource, - api::authz::{ - ResourceActionType, - casbin::{ActionGroupPolicyRequest, GroupingPolicyRequest, PolicyRequest}, - }, - }; + use superposition_types::{Resource, api::authz::ResourceActionType}; use super::*; @@ -70,91 +64,155 @@ pub mod casbin { } } - pub async fn list_policies(scope: AuthzScope) -> Result>, String> { - let (url, headers) = casbin_url_and_headers("policy", scope)?; + pub mod policy { + use superposition_types::api::authz::casbin::PolicyRequest; - let response = request(url, reqwest::Method::GET, None::<()>, headers).await?; - parse_json_response(response).await - } + use super::*; - pub async fn add_policy( - payload: PolicyRequest, - scope: AuthzScope, - ) -> Result<(), String> { - let (url, headers) = casbin_url_and_headers("policy", scope)?; + pub async fn list(scope: AuthzScope) -> Result>, String> { + let (url, headers) = casbin_url_and_headers("policy", scope)?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) - } + let response = + request(url, reqwest::Method::GET, None::<()>, headers).await?; + parse_json_response(response).await + } - pub async fn list_roles(scope: AuthzScope) -> Result>, String> { - let (url, headers) = casbin_url_and_headers("roles", scope)?; + pub async fn add( + payload: PolicyRequest, + scope: AuthzScope, + ) -> Result<(), String> { + let (url, headers) = casbin_url_and_headers("policy", scope)?; - let response = request(url, reqwest::Method::GET, None::<()>, headers).await?; - parse_json_response(response).await - } + request(url, reqwest::Method::POST, Some(payload), headers).await?; + Ok(()) + } - pub async fn add_role( - payload: GroupingPolicyRequest, - scope: AuthzScope, - ) -> Result<(), String> { - let (url, headers) = casbin_url_and_headers("roles", scope)?; + pub async fn delete( + payload: PolicyRequest, + scope: AuthzScope, + ) -> Result<(), String> { + let (url, headers) = casbin_url_and_headers("policy", scope)?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + Ok(()) + } } - pub async fn list_domain_action_groups( - scope: AuthzScope, - ) -> Result>, String> { - if matches!(scope, AuthzScope::Admin) { - return Err( - "domain action groups are only available for org/workspace scopes" - .to_string(), - ); + pub mod role { + use superposition_types::api::authz::casbin::GroupingPolicyRequest; + + use super::*; + + pub async fn list(scope: AuthzScope) -> Result>, String> { + let (url, headers) = casbin_url_and_headers("roles", scope)?; + + let response = + request(url, reqwest::Method::GET, None::<()>, headers).await?; + parse_json_response(response).await } - let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; - let response = request(url, reqwest::Method::GET, None::<()>, headers).await?; - parse_json_response(response).await - } + pub async fn add( + payload: GroupingPolicyRequest, + scope: AuthzScope, + ) -> Result<(), String> { + let (url, headers) = casbin_url_and_headers("roles", scope)?; - pub async fn add_domain_action_group( - payload: ActionGroupPolicyRequest, - scope: AuthzScope, - ) -> Result<(), String> { - if matches!(scope, AuthzScope::Admin) { - return Err( - "domain action groups are only available for org/workspace scopes" - .to_string(), - ); + request(url, reqwest::Method::POST, Some(payload), headers).await?; + Ok(()) } - let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + pub async fn delete( + payload: GroupingPolicyRequest, + scope: AuthzScope, + ) -> Result<(), String> { + let (url, headers) = casbin_url_and_headers("roles", scope)?; + + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + Ok(()) + } } - pub async fn list_action_groups() -> Result>, String> { - let host = use_host_server(); + pub mod action_group { + use superposition_types::api::authz::casbin::ActionGroupPolicyRequest; - let url = format!("{host}/authz/admin/casbin/action-groups"); - let headers = construct_request_headers(&[])?; + use super::*; - let response = request(url, reqwest::Method::GET, None::<()>, headers).await?; - parse_json_response(response).await - } + pub async fn list_domain(scope: AuthzScope) -> Result>, String> { + if matches!(scope, AuthzScope::Admin) { + return Err( + "domain action groups are only available for org/workspace scopes" + .to_string(), + ); + } + let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; - pub async fn add_action_group( - payload: ActionGroupPolicyRequest, - ) -> Result<(), String> { - let host = use_host_server(); + let response = + request(url, reqwest::Method::GET, None::<()>, headers).await?; + parse_json_response(response).await + } - let url = format!("{host}/authz/admin/casbin/action-groups"); - let headers = construct_request_headers(&[])?; + pub async fn add_domain( + payload: ActionGroupPolicyRequest, + scope: AuthzScope, + ) -> Result<(), String> { + if matches!(scope, AuthzScope::Admin) { + return Err( + "domain action groups are only available for org/workspace scopes" + .to_string(), + ); + } + let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; + + request(url, reqwest::Method::POST, Some(payload), headers).await?; + Ok(()) + } - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + pub async fn delete_domain( + payload: ActionGroupPolicyRequest, + scope: AuthzScope, + ) -> Result<(), String> { + if matches!(scope, AuthzScope::Admin) { + return Err( + "domain action groups are only available for org/workspace scopes" + .to_string(), + ); + } + let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; + + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + Ok(()) + } + + pub async fn list() -> Result>, String> { + let host = use_host_server(); + + let url = format!("{host}/authz/admin/casbin/action-groups"); + let headers = construct_request_headers(&[])?; + + let response = + request(url, reqwest::Method::GET, None::<()>, headers).await?; + parse_json_response(response).await + } + + pub async fn add(payload: ActionGroupPolicyRequest) -> Result<(), String> { + let host = use_host_server(); + + let url = format!("{host}/authz/admin/casbin/action-groups"); + let headers = construct_request_headers(&[])?; + + request(url, reqwest::Method::POST, Some(payload), headers).await?; + Ok(()) + } + + pub async fn delete(payload: ActionGroupPolicyRequest) -> Result<(), String> { + let host = use_host_server(); + + let url = format!("{host}/authz/admin/casbin/action-groups"); + let headers = construct_request_headers(&[])?; + + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + Ok(()) + } } pub async fn get_resource_action_map( diff --git a/crates/frontend/src/components/authz.rs b/crates/frontend/src/components/authz.rs index ccfef31d6..2dd7d9a88 100644 --- a/crates/frontend/src/components/authz.rs +++ b/crates/frontend/src/components/authz.rs @@ -185,7 +185,7 @@ fn PolicyViewer( let policies_resource = create_blocking_resource( move || (), move |_| async move { - casbin::list_policies(authz_scope.get_value()) + casbin::policy::list(authz_scope.get_value()) .await .map(|res| { res.into_iter() @@ -221,7 +221,7 @@ fn PolicyViewer( }; let attr = p_attr.get_untracked().trim().to_string(); - casbin::add_policy( + casbin::policy::add( PolicyRequest { sub, obj: p_obj.get_untracked(), @@ -361,7 +361,7 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { let roles_resource = create_blocking_resource( move || (), move |_| async move { - casbin::list_roles(authz_scope.get_value()) + casbin::role::list(authz_scope.get_value()) .await .map(|res| { res.into_iter() @@ -391,7 +391,7 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { let role = NonEmptyString::try_from(role) .map_err(|_| "Role is required".to_string())?; - casbin::add_role( + casbin::role::add( GroupingPolicyRequest { user, role }, authz_scope.get_value(), ) @@ -500,7 +500,7 @@ fn DomainResourceActionGroupViewer( let action_groups_resource = create_blocking_resource( move || (), move |_| async move { - casbin::list_domain_action_groups(authz_scope.get_value()) + casbin::action_group::list_domain(authz_scope.get_value()) .await .map(|res| { res.into_iter() @@ -532,7 +532,7 @@ fn DomainResourceActionGroupViewer( let action_group = NonEmptyString::try_from(action_group) .map_err(|_| "Action group is required (e.g. write)".to_string())?; - casbin::add_domain_action_group( + casbin::action_group::add_domain( ActionGroupPolicyRequest { resource: g2_resource.get_untracked(), action: NonEmptyString::try_from(action.get_name().to_string()) @@ -659,7 +659,7 @@ fn ResourceActionGroupViewer( let action_groups_resource = create_blocking_resource( move || (), move |_| async move { - casbin::list_action_groups().await.map(|res| { + casbin::action_group::list().await.map(|res| { res.into_iter() .enumerate() .map(|(idx, r)| { @@ -687,7 +687,7 @@ fn ResourceActionGroupViewer( let action_group = NonEmptyString::try_from(action_group) .map_err(|_| "Action group is required (e.g. write)".to_string())?; - casbin::add_action_group(ActionGroupPolicyRequest { + casbin::action_group::add(ActionGroupPolicyRequest { resource: g3_resource.get_untracked(), action: NonEmptyString::try_from(action.get_name().to_string()) .map_err(|_| "Invalid action".to_string())?, diff --git a/crates/service_utils/src/middlewares/auth_z.rs b/crates/service_utils/src/middlewares/auth_z.rs index 6d884cb37..626b41b8f 100644 --- a/crates/service_utils/src/middlewares/auth_z.rs +++ b/crates/service_utils/src/middlewares/auth_z.rs @@ -30,14 +30,59 @@ pub trait Action: Send + Sync + 'static { pub struct AuthZ { action: std::marker::PhantomData, + authz_handler: AuthZHandler, + domain: AuthZDomain, + user: User, + internal_user: bool, } impl AuthZ { - fn new() -> Self { + fn new( + authz_handler: AuthZHandler, + domain: AuthZDomain, + user: User, + internal_user: bool, + ) -> Self { Self { action: std::marker::PhantomData, + authz_handler, + domain, + user, + internal_user, } } + + pub async fn authorize_action( + &self, + action: &String, + attributes: &[&String], + ) -> superposition::Result<()> { + if self.internal_user { + return Ok(()); + } + + let resp = self + .authz_handler + .0 + .is_allowed( + &self.domain, + &self.user, + &A::resource(), + action, + Some(attributes), + ) + .await + .map_err(|e| unexpected_error!("Error checking authorization: {}", e))?; + + if !resp { + return Err(forbidden!("You are not authorized to perform this action.")); + } + Ok(()) + } + + pub async fn authorize(&self, attributes: &[&String]) -> superposition::Result<()> { + self.authorize_action(&A::get(), attributes).await + } } impl FromRequest for AuthZ { @@ -46,10 +91,6 @@ impl FromRequest for AuthZ { type Future = LocalBoxFuture<'static, Result>; fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - if req.extensions().get::().is_some() { - return Box::pin(async { Ok(AuthZ::new()) }); - } - let auth_z_handler = match req.extensions().get::() { Some(handler) => handler.clone(), None => { @@ -79,6 +120,12 @@ impl FromRequest for AuthZ { } }; + if req.extensions().get::().is_some() { + return Box::pin(async { + Ok(AuthZ::new(auth_z_handler, domain, user, true)) + }); + } + Box::pin(async move { let is_allowed = auth_z_handler .is_allowed(&domain, &user, &A::resource(), &A::get(), None) @@ -88,7 +135,7 @@ impl FromRequest for AuthZ { Err(e) => Err(unexpected_error!("Error checking authorization: {}", e)), Ok(is_allowed) => { if is_allowed { - Ok(AuthZ::new()) + Ok(AuthZ::new(auth_z_handler, domain, user, false)) } else { Err(forbidden!("You are not authorized to perform this action.")) } diff --git a/crates/service_utils/src/middlewares/auth_z/authorization.rs b/crates/service_utils/src/middlewares/auth_z/authorization.rs index 6340a4b1c..2ce364d7b 100644 --- a/crates/service_utils/src/middlewares/auth_z/authorization.rs +++ b/crates/service_utils/src/middlewares/auth_z/authorization.rs @@ -36,7 +36,7 @@ pub trait Authorizer: Sync + Send { user: &User, resource: &Resource, action: &str, - attributes: Option<&[&str]>, + attributes: Option<&[&String]>, ) -> LocalBoxFuture<'_, Result>; fn get_source_resource_action_map( diff --git a/crates/service_utils/src/middlewares/auth_z/casbin.rs b/crates/service_utils/src/middlewares/auth_z/casbin.rs index 51e15b5bf..0a44e8e91 100644 --- a/crates/service_utils/src/middlewares/auth_z/casbin.rs +++ b/crates/service_utils/src/middlewares/auth_z/casbin.rs @@ -218,7 +218,7 @@ impl Authorizer for CasbinPolicyEngine { user: &User, resource: &Resource, action: &str, - attributes: Option<&[&str]>, + attributes: Option<&[&String]>, ) -> LocalBoxFuture<'_, Result> { let sub = user.get_email(); let domain = domain.to_string(); diff --git a/crates/service_utils/src/middlewares/auth_z/no_auth.rs b/crates/service_utils/src/middlewares/auth_z/no_auth.rs index 2e1bcdf6c..9e30fcb32 100644 --- a/crates/service_utils/src/middlewares/auth_z/no_auth.rs +++ b/crates/service_utils/src/middlewares/auth_z/no_auth.rs @@ -14,7 +14,7 @@ impl Authorizer for NoAuth { _: &User, _: &Resource, _: &str, - _: Option<&[&str]>, + _: Option<&[&String]>, ) -> LocalBoxFuture<'_, Result> { Box::pin(async { Ok(true) }) } diff --git a/crates/superposition_types/src/lib.rs b/crates/superposition_types/src/lib.rs index 263098f3f..0403b1b4e 100644 --- a/crates/superposition_types/src/lib.rs +++ b/crates/superposition_types/src/lib.rs @@ -145,7 +145,7 @@ impl FromRequest for User { } } -#[derive(Clone, Debug, PartialEq, Copy, Serialize)] +#[derive(Clone, Debug, PartialEq, Copy, Serialize, Deref, DerefMut)] pub struct Cac(T); impl Cac { pub fn into_inner(self) -> T { @@ -153,7 +153,7 @@ impl Cac { } } -#[derive(Clone, Debug, PartialEq, Copy, Serialize)] +#[derive(Clone, Debug, PartialEq, Copy, Serialize, Deref, DerefMut)] pub struct Exp(T); impl Exp { pub fn into_inner(self) -> T { From 74eca574c54a1f94faf235bd7fe91042d7d63d88 Mon Sep 17 00:00:00 2001 From: "ayush.jain@juspay.in" Date: Wed, 11 Mar 2026 19:18:22 +0530 Subject: [PATCH 2/4] fix: some fixes --- .../src/api/context/handlers.rs | 6 +- .../src/api/context/helpers.rs | 10 +- .../src/api/context/operations.rs | 2 +- .../src/api/experiments/handlers.rs | 4 +- .../src/api/experiments/helpers.rs | 2 +- crates/frontend/src/api.rs | 81 ++- crates/frontend/src/components/authz.rs | 587 ++++++++++++++++-- crates/frontend/src/utils.rs | 2 +- .../service_utils/src/middlewares/auth_z.rs | 2 +- .../src/middlewares/auth_z/casbin/handlers.rs | 134 ++-- .../src/api/authz/casbin.rs | 10 +- crates/superposition_types/src/lib.rs | 4 +- 12 files changed, 698 insertions(+), 146 deletions(-) diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index 84e68b123..f73a13192 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -206,7 +206,7 @@ async fn authorize_update( auth_z .authorize_action( &AuthZActionUpdate::get(), - &changed_keys(&overrides, &new_overrides.deref()), + &changed_keys(&overrides, new_overrides.deref()), ) .await } @@ -305,7 +305,7 @@ async fn update_handler( async fn authorize_action( auth_z: &AuthZ, - ctx_id: &String, + ctx_id: &str, schema_name: &SchemaName, conn: &mut DBConnection, ) -> superposition::Result<()> { @@ -623,7 +623,7 @@ async fn list_handler( async fn authorize_delete( auth_z: &AuthZ, - ctx_id: &String, + ctx_id: &str, schema_name: &SchemaName, conn: &mut DBConnection, ) -> superposition::Result<()> { diff --git a/crates/context_aware_config/src/api/context/helpers.rs b/crates/context_aware_config/src/api/context/helpers.rs index b2fde2a40..3ef0e3b75 100644 --- a/crates/context_aware_config/src/api/context/helpers.rs +++ b/crates/context_aware_config/src/api/context/helpers.rs @@ -513,14 +513,8 @@ pub fn changed_keys<'a>( keys.into_iter() .filter(|key| { - let old_value = old_values - .get(*key) - .cloned() - .unwrap_or(Value::String("##NOT_PRESENT##".to_string())); - let new_value = new_values - .get(*key) - .cloned() - .unwrap_or(Value::String("##DELETED##".to_string())); + let old_value = old_values.get(*key).cloned(); + let new_value = new_values.get(*key).cloned(); old_value != new_value }) .collect() diff --git a/crates/context_aware_config/src/api/context/operations.rs b/crates/context_aware_config/src/api/context/operations.rs index 4016bfd32..d5c346397 100644 --- a/crates/context_aware_config/src/api/context/operations.rs +++ b/crates/context_aware_config/src/api/context/operations.rs @@ -103,7 +103,7 @@ pub fn get_overrides_from_ctx_id( ) -> result::Result { let overrides = dsl::contexts .filter(dsl::id.eq(ctx_id)) - .schema_name(&schema_name) + .schema_name(schema_name) .select(dsl::override_) .first::(conn)?; diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index 7979c4fcb..3fa69827d 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -135,7 +135,7 @@ fn add_config_version_to_header( async fn authorize_create( auth_z: AuthZ, - variants: &Vec, + variants: &[Variant], ) -> superposition::Result<()> { let control_keys = variants .iter() @@ -175,7 +175,7 @@ async fn create_handler( // atleast 1 experimental variant check_variant_types(&variants)?; let unique_override_keys: Vec = - extract_override_keys(&variants[0].overrides.deref()) + extract_override_keys(variants[0].overrides.deref()) .into_iter() .collect(); diff --git a/crates/experimentation_platform/src/api/experiments/helpers.rs b/crates/experimentation_platform/src/api/experiments/helpers.rs index c09cdc01d..0d3324c4e 100644 --- a/crates/experimentation_platform/src/api/experiments/helpers.rs +++ b/crates/experimentation_platform/src/api/experiments/helpers.rs @@ -874,5 +874,5 @@ pub fn get_control_overrides_from_exp_id( bad_argument!("No control variant found for the given experiment id") })?; - Ok(control_overrides.overrides.into()) + Ok(control_overrides.overrides) } diff --git a/crates/frontend/src/api.rs b/crates/frontend/src/api.rs index 29f1f1e2e..05126219d 100644 --- a/crates/frontend/src/api.rs +++ b/crates/frontend/src/api.rs @@ -27,19 +27,34 @@ use crate::utils::{ }; pub mod casbin { - use std::collections::HashMap; + use std::{collections::HashMap, fmt::Display}; - use superposition_types::{Resource, api::authz::ResourceActionType}; + use superposition_types::{ + Resource, + api::authz::{ResourceActionType, casbin::ActionResponse}, + }; use super::*; - #[derive(Clone, Debug, PartialEq, Eq)] + #[derive(Clone, PartialEq, Eq)] pub enum AuthzScope { Admin, Org(String), Workspace(String, String), } + impl Display for AuthzScope { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AuthzScope::Admin => write!(f, "*"), + AuthzScope::Org(org_id) => write!(f, "{org_id}_*"), + AuthzScope::Workspace(org_id, workspace) => { + write!(f, "{org_id}_{workspace}") + } + } + } + } + fn casbin_url_and_headers( endpoint: &str, scope: AuthzScope, @@ -80,21 +95,23 @@ pub mod casbin { pub async fn add( payload: PolicyRequest, scope: AuthzScope, - ) -> Result<(), String> { + ) -> Result { let (url, headers) = casbin_url_and_headers("policy", scope)?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::POST, Some(payload), headers).await?; + parse_json_response(response).await } pub async fn delete( payload: PolicyRequest, scope: AuthzScope, - ) -> Result<(), String> { + ) -> Result { let (url, headers) = casbin_url_and_headers("policy", scope)?; - request(url, reqwest::Method::DELETE, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + parse_json_response(response).await } } @@ -114,21 +131,23 @@ pub mod casbin { pub async fn add( payload: GroupingPolicyRequest, scope: AuthzScope, - ) -> Result<(), String> { + ) -> Result { let (url, headers) = casbin_url_and_headers("roles", scope)?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::POST, Some(payload), headers).await?; + parse_json_response(response).await } pub async fn delete( payload: GroupingPolicyRequest, scope: AuthzScope, - ) -> Result<(), String> { + ) -> Result { let (url, headers) = casbin_url_and_headers("roles", scope)?; - request(url, reqwest::Method::DELETE, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + parse_json_response(response).await } } @@ -154,7 +173,7 @@ pub mod casbin { pub async fn add_domain( payload: ActionGroupPolicyRequest, scope: AuthzScope, - ) -> Result<(), String> { + ) -> Result { if matches!(scope, AuthzScope::Admin) { return Err( "domain action groups are only available for org/workspace scopes" @@ -163,14 +182,15 @@ pub mod casbin { } let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::POST, Some(payload), headers).await?; + parse_json_response(response).await } pub async fn delete_domain( payload: ActionGroupPolicyRequest, scope: AuthzScope, - ) -> Result<(), String> { + ) -> Result { if matches!(scope, AuthzScope::Admin) { return Err( "domain action groups are only available for org/workspace scopes" @@ -179,8 +199,9 @@ pub mod casbin { } let (url, headers) = casbin_url_and_headers("domain-action-groups", scope)?; - request(url, reqwest::Method::DELETE, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + parse_json_response(response).await } pub async fn list() -> Result>, String> { @@ -194,24 +215,30 @@ pub mod casbin { parse_json_response(response).await } - pub async fn add(payload: ActionGroupPolicyRequest) -> Result<(), String> { + pub async fn add( + payload: ActionGroupPolicyRequest, + ) -> Result { let host = use_host_server(); let url = format!("{host}/authz/admin/casbin/action-groups"); let headers = construct_request_headers(&[])?; - request(url, reqwest::Method::POST, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::POST, Some(payload), headers).await?; + parse_json_response(response).await } - pub async fn delete(payload: ActionGroupPolicyRequest) -> Result<(), String> { + pub async fn delete( + payload: ActionGroupPolicyRequest, + ) -> Result { let host = use_host_server(); let url = format!("{host}/authz/admin/casbin/action-groups"); let headers = construct_request_headers(&[])?; - request(url, reqwest::Method::DELETE, Some(payload), headers).await?; - Ok(()) + let response = + request(url, reqwest::Method::DELETE, Some(payload), headers).await?; + parse_json_response(response).await } } diff --git a/crates/frontend/src/components/authz.rs b/crates/frontend/src/components/authz.rs index 2dd7d9a88..a5e54bd64 100644 --- a/crates/frontend/src/components/authz.rs +++ b/crates/frontend/src/components/authz.rs @@ -19,6 +19,7 @@ use crate::{ button::Button, dropdown::{Dropdown, DropdownBtnType, DropdownDirection, utils::DropdownOption}, form::label::Label, + modal::PortalModal, skeleton::{Skeleton, SkeletonVariant}, table::{ Table, @@ -113,10 +114,37 @@ fn ActionDropdown( } } -fn policy_columns() -> Vec { +fn policy_columns( + delete_click_handler: Callback>, + authz_scope: AuthzScope, +) -> Vec { + let actions_col_formatter = move |_: &str, row: &Map| { + let domain = row + .get("domain") + .and_then(|v| v.as_str()) + .unwrap_or_default() + .to_string(); + + if domain != authz_scope.to_string() { + ().into_view() + } else { + let row = row.clone(); + view! { + + + + } + .into_view() + } + }; + vec![ Column::new( - "subject".to_string(), + "sub".to_string(), false, default_formatter, ColumnSortable::No, @@ -132,7 +160,7 @@ fn policy_columns() -> Vec { |_| default_column_formatter("Domain"), ), Column::new( - "object".to_string(), + "obj".to_string(), false, default_formatter, ColumnSortable::No, @@ -140,7 +168,7 @@ fn policy_columns() -> Vec { |_| default_column_formatter("Resource"), ), Column::new( - "action".to_string(), + "act".to_string(), false, default_formatter, ColumnSortable::No, @@ -148,13 +176,14 @@ fn policy_columns() -> Vec { |_| default_column_formatter("Action"), ), Column::new( - "attribute".to_string(), + "attr".to_string(), false, default_formatter, ColumnSortable::No, Expandable::Disabled, |_| default_column_formatter("Attribute"), ), + Column::default_with_cell_formatter("actions".to_string(), actions_col_formatter), ] } @@ -180,7 +209,11 @@ fn PolicyViewer( let p_act = RwSignal::new(None as Option); let p_attr = RwSignal::new(String::from("*")); - let columns = StoredValue::new(policy_columns()); + let delete_item_rws = RwSignal::new(None as Option>); + let columns = StoredValue::new(policy_columns( + Callback::new(move |row| delete_item_rws.set(Some(row))), + authz_scope.get_value(), + )); let policies_resource = create_blocking_resource( move || (), @@ -198,11 +231,11 @@ fn PolicyViewer( let attr = r.get(4).cloned().unwrap_or_default(); Map::from_iter([ ("idx".to_string(), Value::String(idx.to_string())), - ("subject".to_string(), Value::String(sub)), + ("sub".to_string(), Value::String(sub)), ("domain".to_string(), Value::String(dom)), - ("object".to_string(), Value::String(obj)), - ("action".to_string(), Value::String(act)), - ("attribute".to_string(), Value::String(attr)), + ("obj".to_string(), Value::String(obj)), + ("act".to_string(), Value::String(act)), + ("attr".to_string(), Value::String(attr)), ]) }) .collect() @@ -210,18 +243,63 @@ fn PolicyViewer( }, ); - let add_policy_action = create_action(move |_| async move { + let on_delete = Callback::new(move |row: Map| { + spawn_local(async move { + let policy_request = match serde_json::from_value(Value::Object(row)) { + Ok(pr) => pr, + Err(e) => { + logging::error!("Failed to parse policy for deletion: {}", e); + enqueue_alert( + "Error converting to policy request".to_string(), + AlertType::Error, + 4000, + ); + return; + } + }; + + let resp = + casbin::policy::delete(policy_request, authz_scope.get_value()).await; + + match resp { + Ok(resp) => { + policies_resource.refetch(); + delete_item_rws.set(None); + enqueue_alert( + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, + 4000, + ); + } + Err(e) => { + enqueue_alert( + format!("Failed to delete policy: {}", e), + AlertType::Error, + 5000, + ); + } + } + }) + }); + + let add_policy_action = Action::new(move |_| async move { let sub = p_sub.get_untracked().trim().to_string(); let act = p_act.get_untracked(); let sub = NonEmptyString::try_from(sub) .map_err(|_| "Subject is required".to_string())?; - let Some(act) = act else { + let Some(act) = + act.and_then(|s| NonEmptyString::try_from(s.get_name().to_string()).ok()) + else { return Err("Action is required".to_string()); }; let attr = p_attr.get_untracked().trim().to_string(); - casbin::policy::add( + let resp = casbin::policy::add( PolicyRequest { sub, obj: p_obj.get_untracked(), @@ -237,7 +315,15 @@ fn PolicyViewer( policies_resource.refetch(); resource_action_map_resource.refetch(); - enqueue_alert("Policy added".to_string(), AlertType::Success, 4000); + enqueue_alert( + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, + 4000, + ); Ok(()) }); @@ -319,13 +405,78 @@ fn PolicyViewer( + {move || match delete_item_rws.get() { + Some(row) => { + view! { + +

"Are you sure you want to delete this item?"

+ +
+
+ + } + } + None => ().into_view(), + }} } } -fn role_policy_columns() -> Vec { +fn role_policy_columns( + delete_click_handler: Callback>, + authz_scope: AuthzScope, +) -> Vec { + let actions_col_formatter = move |_: &str, row: &Map| { + let domain = row + .get("domain") + .and_then(|v| v.as_str()) + .unwrap_or_default() + .to_string(); + + if domain != authz_scope.to_string() { + ().into_view() + } else { + let row = row.clone(); + view! { + + + + } + .into_view() + } + }; + vec![ Column::new( - "subject".to_string(), + "user".to_string(), false, default_formatter, ColumnSortable::No, @@ -348,6 +499,7 @@ fn role_policy_columns() -> Vec { Expandable::Disabled, |_| default_column_formatter("Domain"), ), + Column::default_with_cell_formatter("actions".to_string(), actions_col_formatter), ] } @@ -356,7 +508,11 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { let g_user = RwSignal::new(String::new()); let g_role = RwSignal::new(String::new()); - let columns = StoredValue::new(role_policy_columns()); + let delete_item_rws = RwSignal::new(None as Option>); + let columns = StoredValue::new(role_policy_columns( + Callback::new(move |row| delete_item_rws.set(Some(row))), + authz_scope.get_value(), + )); let roles_resource = create_blocking_resource( move || (), @@ -372,7 +528,7 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { let dom = r.get(2).cloned().unwrap_or_default(); Map::from_iter([ ("idx".to_string(), Value::String(idx.to_string())), - ("subject".to_string(), Value::String(sub)), + ("user".to_string(), Value::String(sub)), ("role".to_string(), Value::String(role)), ("domain".to_string(), Value::String(dom)), ]) @@ -382,7 +538,49 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { }, ); - let add_role_action = create_action({ + let on_delete = Callback::new(move |row: Map| { + spawn_local(async move { + let role_request = match serde_json::from_value(Value::Object(row)) { + Ok(pr) => pr, + Err(e) => { + logging::error!("Failed to parse role for deletion: {}", e); + enqueue_alert( + "Error converting to role request".to_string(), + AlertType::Error, + 4000, + ); + return; + } + }; + + let resp = casbin::role::delete(role_request, authz_scope.get_value()).await; + + match resp { + Ok(resp) => { + roles_resource.refetch(); + delete_item_rws.set(None); + enqueue_alert( + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, + 4000, + ); + } + Err(e) => { + enqueue_alert( + format!("Failed to delete role: {}", e), + AlertType::Error, + 5000, + ); + } + } + }) + }); + + let add_role_action = Action::new({ move |_| async move { let user = g_user.get_untracked().trim().to_string(); let role = g_role.get_untracked().trim().to_string(); @@ -391,14 +589,22 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { let role = NonEmptyString::try_from(role) .map_err(|_| "Role is required".to_string())?; - casbin::role::add( + let resp = casbin::role::add( GroupingPolicyRequest { user, role }, authz_scope.get_value(), ) .await?; roles_resource.refetch(); - enqueue_alert("Role assigned".to_string(), AlertType::Success, 4000); + enqueue_alert( + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, + 4000, + ); Ok(()) } }); @@ -451,18 +657,91 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { + {move || match delete_item_rws.get() { + Some(row) => { + view! { + +

"Are you sure you want to delete this item?"

+
+
+
+ + } + } + None => ().into_view(), + }} } } -fn domain_resource_action_group_columns() -> Vec { +fn domain_resource_action_group_columns( + delete_click_handler: Callback>, + authz_scope: AuthzScope, +) -> Vec { + let actions_col_formatter = move |_: &str, row: &Map| { + let domain = row + .get("domain") + .and_then(|v| v.as_str()) + .unwrap_or_default() + .to_string(); + + if domain != authz_scope.to_string() { + ().into_view() + } else { + let row = row.clone(); + view! { + + + + } + .into_view() + } + }; + vec![ Column::new( - "subject".to_string(), + "resource".to_string(), + false, + default_formatter, + ColumnSortable::No, + Expandable::Disabled, + |_| default_column_formatter("Resource"), + ), + Column::new( + "action".to_string(), false, default_formatter, ColumnSortable::No, Expandable::Disabled, - |_| default_column_formatter("Resource:Action"), + |_| default_column_formatter("Action"), ), Column::new( "domain".to_string(), @@ -473,13 +752,14 @@ fn domain_resource_action_group_columns() -> Vec { |_| default_column_formatter("Domain"), ), Column::new( - "group".to_string(), + "action_group".to_string(), false, default_formatter, ColumnSortable::No, Expandable::Disabled, |_| default_column_formatter("Action Group"), ), + Column::default_with_cell_formatter("actions".to_string(), actions_col_formatter), ] } @@ -495,7 +775,11 @@ fn DomainResourceActionGroupViewer( let g2_action = RwSignal::new(None as Option); let g2_action_group = RwSignal::new(String::new()); - let columns = StoredValue::new(domain_resource_action_group_columns()); + let delete_item_rws = RwSignal::new(None as Option>); + let columns = StoredValue::new(domain_resource_action_group_columns( + Callback::new(move |row| delete_item_rws.set(Some(row))), + authz_scope.get_value(), + )); let action_groups_resource = create_blocking_resource( move || (), @@ -506,14 +790,21 @@ fn DomainResourceActionGroupViewer( res.into_iter() .enumerate() .map(|(idx, r)| { - let res = r.first().cloned().unwrap_or_default(); + let (res, action) = r + .first() + .and_then(|s| s.split_once(":")) + .map(|(res, action)| { + (res.to_string(), action.to_string()) + }) + .unwrap_or_default(); let dom = r.get(1).cloned().unwrap_or_default(); let group = r.get(2).cloned().unwrap_or_default(); Map::from_iter([ ("idx".to_string(), Value::String(idx.to_string())), - ("subject".to_string(), Value::String(res)), + ("resource".to_string(), Value::String(res)), + ("action".to_string(), Value::String(action)), ("domain".to_string(), Value::String(dom)), - ("group".to_string(), Value::String(group)), + ("action_group".to_string(), Value::String(group)), ]) }) .collect() @@ -521,7 +812,53 @@ fn DomainResourceActionGroupViewer( }, ); - let add_action_group_action = create_action({ + let on_delete = Callback::new(move |row: Map| { + spawn_local(async move { + let action_group_request = match serde_json::from_value(Value::Object(row)) { + Ok(pr) => pr, + Err(e) => { + logging::error!("Failed to parse action group for deletion: {}", e); + enqueue_alert( + "Error converting to action group request".to_string(), + AlertType::Error, + 4000, + ); + return; + } + }; + + let resp = casbin::action_group::delete_domain( + action_group_request, + authz_scope.get_value(), + ) + .await; + + match resp { + Ok(resp) => { + action_groups_resource.refetch(); + delete_item_rws.set(None); + enqueue_alert( + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, + 4000, + ); + } + Err(e) => { + enqueue_alert( + format!("Failed to delete role: {}", e), + AlertType::Error, + 5000, + ); + } + } + }) + }); + + let add_action_group_action = Action::new({ move |_| async move { let action = g2_action.get_untracked(); let action_group = g2_action_group.get_untracked().trim().to_string(); @@ -532,7 +869,7 @@ fn DomainResourceActionGroupViewer( let action_group = NonEmptyString::try_from(action_group) .map_err(|_| "Action group is required (e.g. write)".to_string())?; - casbin::action_group::add_domain( + let resp = casbin::action_group::add_domain( ActionGroupPolicyRequest { resource: g2_resource.get_untracked(), action: NonEmptyString::try_from(action.get_name().to_string()) @@ -546,8 +883,12 @@ fn DomainResourceActionGroupViewer( action_groups_resource.refetch(); resource_action_map_resource.refetch(); enqueue_alert( - "Action group mapping added".to_string(), - AlertType::Success, + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, 4000, ); Ok(()) @@ -619,27 +960,90 @@ fn DomainResourceActionGroupViewer( + {move || match delete_item_rws.get() { + Some(row) => { + view! { + +

"Are you sure you want to delete this item?"

+
+
+
+ + } + } + None => ().into_view(), + }} } } -fn resource_action_group_columns() -> Vec { +fn resource_action_group_columns( + delete_click_handler: Callback>, +) -> Vec { + let actions_col_formatter = move |_: &str, row: &Map| { + let row = row.clone(); + view! { + + + + } + .into_view() + }; + vec![ Column::new( - "subject".to_string(), + "resource".to_string(), false, default_formatter, ColumnSortable::No, Expandable::Disabled, - |_| default_column_formatter("Resource:Action"), + |_| default_column_formatter("Resource"), ), Column::new( - "group".to_string(), + "action".to_string(), + false, + default_formatter, + ColumnSortable::No, + Expandable::Disabled, + |_| default_column_formatter("Action"), + ), + Column::new( + "action_group".to_string(), false, default_formatter, ColumnSortable::No, Expandable::Disabled, |_| default_column_formatter("Action Group"), ), + Column::default_with_cell_formatter("actions".to_string(), actions_col_formatter), ] } @@ -654,7 +1058,11 @@ fn ResourceActionGroupViewer( let g3_action = RwSignal::new(None as Option); let g3_action_group = RwSignal::new(String::new()); - let columns = StoredValue::new(resource_action_group_columns()); + let delete_item_rws = RwSignal::new(None as Option>); + let columns = + StoredValue::new(resource_action_group_columns(Callback::new(move |row| { + delete_item_rws.set(Some(row)) + }))); let action_groups_resource = create_blocking_resource( move || (), @@ -663,12 +1071,17 @@ fn ResourceActionGroupViewer( res.into_iter() .enumerate() .map(|(idx, r)| { - let res = r.first().cloned().unwrap_or_default(); + let (res, action) = r + .first() + .and_then(|s| s.split_once(":")) + .map(|(res, action)| (res.to_string(), action.to_string())) + .unwrap_or_default(); let group = r.get(1).cloned().unwrap_or_default(); Map::from_iter([ ("idx".to_string(), Value::String(idx.to_string())), - ("subject".to_string(), Value::String(res)), - ("group".to_string(), Value::String(group)), + ("resource".to_string(), Value::String(res)), + ("action".to_string(), Value::String(action)), + ("action_group".to_string(), Value::String(group)), ]) }) .collect() @@ -676,7 +1089,49 @@ fn ResourceActionGroupViewer( }, ); - let add_action_group_action = create_action({ + let on_delete = Callback::new(move |row: Map| { + spawn_local(async move { + let action_group_request = match serde_json::from_value(Value::Object(row)) { + Ok(pr) => pr, + Err(e) => { + logging::error!("Failed to parse action group for deletion: {}", e); + enqueue_alert( + "Error converting to action group request".to_string(), + AlertType::Error, + 4000, + ); + return; + } + }; + + let resp = casbin::action_group::delete(action_group_request).await; + + match resp { + Ok(resp) => { + action_groups_resource.refetch(); + delete_item_rws.set(None); + enqueue_alert( + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, + 4000, + ); + } + Err(e) => { + enqueue_alert( + format!("Failed to delete action group: {}", e), + AlertType::Error, + 5000, + ); + } + } + }) + }); + + let add_action_group_action = Action::new({ move |_| async move { let action = g3_action.get_untracked(); let action_group = g3_action_group.get_untracked().trim().to_string(); @@ -687,7 +1142,7 @@ fn ResourceActionGroupViewer( let action_group = NonEmptyString::try_from(action_group) .map_err(|_| "Action group is required (e.g. write)".to_string())?; - casbin::action_group::add(ActionGroupPolicyRequest { + let resp = casbin::action_group::add(ActionGroupPolicyRequest { resource: g3_resource.get_untracked(), action: NonEmptyString::try_from(action.get_name().to_string()) .map_err(|_| "Invalid action".to_string())?, @@ -698,8 +1153,12 @@ fn ResourceActionGroupViewer( action_groups_resource.refetch(); resource_action_map_resource.refetch(); enqueue_alert( - "Action group mapping added".to_string(), - AlertType::Success, + resp.message, + if resp.success { + AlertType::Success + } else { + AlertType::Error + }, 4000, ); Ok(()) @@ -768,6 +1227,44 @@ fn ResourceActionGroupViewer( + {move || match delete_item_rws.get() { + Some(row) => { + view! { + +

"Are you sure you want to delete this item?"

+
+
+
+ + } + } + None => ().into_view(), + }} } } diff --git a/crates/frontend/src/utils.rs b/crates/frontend/src/utils.rs index 26bbca1d6..69a5f52ed 100644 --- a/crates/frontend/src/utils.rs +++ b/crates/frontend/src/utils.rs @@ -242,7 +242,7 @@ where let mut request_builder = HTTP_CLIENT.request(method.clone(), url).headers(headers); request_builder = match (method, body) { - (reqwest::Method::GET | reqwest::Method::DELETE, _) => request_builder, + (reqwest::Method::GET, _) => request_builder, (_, Some(data)) => request_builder.json(&data), _ => request_builder, }; diff --git a/crates/service_utils/src/middlewares/auth_z.rs b/crates/service_utils/src/middlewares/auth_z.rs index 626b41b8f..e76870d25 100644 --- a/crates/service_utils/src/middlewares/auth_z.rs +++ b/crates/service_utils/src/middlewares/auth_z.rs @@ -54,7 +54,7 @@ impl AuthZ { pub async fn authorize_action( &self, - action: &String, + action: &str, attributes: &[&String], ) -> superposition::Result<()> { if self.internal_user { diff --git a/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs b/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs index 0ad14bd9f..1d7aecdc0 100644 --- a/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs +++ b/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs @@ -15,7 +15,10 @@ use superposition_types::{ Resource, api::authz::{ ResourceActionType, - casbin::{ActionGroupPolicyRequest, GroupingPolicyRequest, PolicyRequest}, + casbin::{ + ActionGroupPolicyRequest, ActionResponse, GroupingPolicyRequest, + PolicyRequest, + }, }, database::superposition_schema::superposition::{organisations, workspaces}, result as superposition, @@ -87,20 +90,21 @@ async fn add_policy_handler( data: Data, body: Json, domain: AuthZDomain, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let added = data .enforcer_mut(async |enforcer| { let action_map = data.get_resource_action_map(enforcer, domain.clone()); + let act = body.act.to_string(); action_map .get(&body.obj) - .and_then(|actions| actions.contains(&body.act).then_some(())) + .and_then(|actions| actions.iter().find(|a| a.get_name() == act)) .ok_or_else(|| { bad_argument!( "The action '{}' is not valid for resource '{}'", - body.act.get_name(), + act, body.obj ) })?; @@ -110,7 +114,7 @@ async fn add_policy_handler( body.sub.into_inner(), domain.to_string(), body.obj.to_string(), - body.act.get_name().to_string(), + body.act.to_string(), body.attr.map(|a| a.into_inner()).unwrap_or("*".to_string()), ]) .await @@ -119,11 +123,14 @@ async fn add_policy_handler( .await .map_err(|e| unexpected_error!(e))?; - if added { - Ok(HttpResponse::Ok().body("Policy added")) - } else { - Ok(HttpResponse::Ok().body("Policy already exists")) - } + Ok(Json(ActionResponse { + success: added, + message: if added { + "Policy added".to_string() + } else { + "Policy already exists".to_string() + }, + })) } #[authorized] @@ -132,7 +139,7 @@ async fn delete_policy_handler( data: Data, body: Json, domain: AuthZDomain, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let removed = data @@ -142,7 +149,7 @@ async fn delete_policy_handler( body.sub.into_inner(), domain.to_string(), body.obj.to_string(), - body.act.get_name().to_string(), + body.act.to_string(), body.attr.map(|a| a.into_inner()).unwrap_or("*".to_string()), ]) .await @@ -151,11 +158,14 @@ async fn delete_policy_handler( .await .map_err(|e| unexpected_error!(e))?; - if removed { - Ok(HttpResponse::Ok().body("Policy removed")) - } else { - Ok(HttpResponse::Ok().body("Policy does not exist")) - } + Ok(Json(ActionResponse { + success: removed, + message: if removed { + "Policy removed".to_string() + } else { + "Policy does not exist".to_string() + }, + })) } #[authorized] @@ -187,7 +197,7 @@ async fn add_roles_handler( data: Data, body: Json, domain: AuthZDomain, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let added = data @@ -205,11 +215,14 @@ async fn add_roles_handler( .await .map_err(|e| unexpected_error!(e))?; - if added { - Ok(HttpResponse::Ok().body("Grouping policy added")) - } else { - Ok(HttpResponse::Ok().body("Grouping policy already exists")) - } + Ok(Json(ActionResponse { + success: added, + message: if added { + "Grouping policy added".to_string() + } else { + "Grouping policy already exists".to_string() + }, + })) } #[authorized] @@ -218,7 +231,7 @@ async fn delete_roles_handler( data: Data, body: Json, domain: AuthZDomain, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let removed = data @@ -235,11 +248,14 @@ async fn delete_roles_handler( .await .map_err(|e| unexpected_error!(e))?; - if removed { - Ok(HttpResponse::Ok().body("Grouping policy removed")) - } else { - Ok(HttpResponse::Ok().body("Grouping policy does not exist")) - } + Ok(Json(ActionResponse { + success: removed, + message: if removed { + "Grouping policy removed".to_string() + } else { + "Grouping policy does not exist".to_string() + }, + })) } #[authorized] @@ -272,7 +288,7 @@ async fn add_domain_action_group_handler( data: Data, body: Json, domain: AuthZDomain, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let added = data @@ -292,11 +308,14 @@ async fn add_domain_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; - if added { - Ok(HttpResponse::Ok().body("Action-group policy added")) - } else { - Ok(HttpResponse::Ok().body("Action-group policy already exists")) - } + Ok(Json(ActionResponse { + success: added, + message: if added { + "Action-group policy added".to_string() + } else { + "Action-group policy already exists".to_string() + }, + })) } #[authorized] @@ -305,7 +324,7 @@ async fn delete_domain_action_group_handler( data: Data, body: Json, domain: AuthZDomain, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let removed = data @@ -325,11 +344,14 @@ async fn delete_domain_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; - if removed { - Ok(HttpResponse::Ok().body("Action-group policy removed")) - } else { - Ok(HttpResponse::Ok().body("Action-group policy does not exist")) - } + Ok(Json(ActionResponse { + success: removed, + message: if removed { + "Action-group policy removed".to_string() + } else { + "Action-group policy does not exist".to_string() + }, + })) } #[authorized] @@ -355,7 +377,7 @@ async fn list_domain_action_group_handler( async fn add_action_group_handler( data: Data, body: Json, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let added = data @@ -374,11 +396,14 @@ async fn add_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; - if added { - Ok(HttpResponse::Ok().body("Action-group policy added")) - } else { - Ok(HttpResponse::Ok().body("Action-group policy already exists")) - } + Ok(Json(ActionResponse { + success: added, + message: if added { + "Action-group policy added".to_string() + } else { + "Action-group policy already exists".to_string() + }, + })) } #[authorized] @@ -386,7 +411,7 @@ async fn add_action_group_handler( async fn delete_action_group_handler( data: Data, body: Json, -) -> superposition::Result { +) -> superposition::Result> { let data = data.try_get_casbin_policy_engine()?; let body = body.into_inner(); let removed = data @@ -405,11 +430,14 @@ async fn delete_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; - if removed { - Ok(HttpResponse::Ok().body("Action-group policy removed")) - } else { - Ok(HttpResponse::Ok().body("Action-group policy does not exist")) - } + Ok(Json(ActionResponse { + success: removed, + message: if removed { + "Action-group policy removed".to_string() + } else { + "Action-group policy does not exist".to_string() + }, + })) } #[authorized] diff --git a/crates/superposition_types/src/api/authz/casbin.rs b/crates/superposition_types/src/api/authz/casbin.rs index 781d62013..86827c920 100644 --- a/crates/superposition_types/src/api/authz/casbin.rs +++ b/crates/superposition_types/src/api/authz/casbin.rs @@ -1,12 +1,12 @@ use serde::{Deserialize, Serialize}; -use crate::{api::authz::ResourceActionType, database::models::NonEmptyString, Resource}; +use crate::{database::models::NonEmptyString, Resource}; #[derive(Deserialize, Serialize)] pub struct PolicyRequest { pub sub: NonEmptyString, pub obj: Resource, - pub act: ResourceActionType, + pub act: NonEmptyString, pub attr: Option, } @@ -22,3 +22,9 @@ pub struct ActionGroupPolicyRequest { pub action: NonEmptyString, pub action_group: NonEmptyString, } + +#[derive(Deserialize, Serialize)] +pub struct ActionResponse { + pub success: bool, + pub message: String, +} diff --git a/crates/superposition_types/src/lib.rs b/crates/superposition_types/src/lib.rs index 0403b1b4e..35c57be05 100644 --- a/crates/superposition_types/src/lib.rs +++ b/crates/superposition_types/src/lib.rs @@ -145,7 +145,7 @@ impl FromRequest for User { } } -#[derive(Clone, Debug, PartialEq, Copy, Serialize, Deref, DerefMut)] +#[derive(Clone, Debug, PartialEq, Copy, Serialize, Deref)] pub struct Cac(T); impl Cac { pub fn into_inner(self) -> T { @@ -153,7 +153,7 @@ impl Cac { } } -#[derive(Clone, Debug, PartialEq, Copy, Serialize, Deref, DerefMut)] +#[derive(Clone, Debug, PartialEq, Copy, Serialize, Deref)] pub struct Exp(T); impl Exp { pub fn into_inner(self) -> T { From aa843aceac79bf6f70fd2deea77c6293af187e1f Mon Sep 17 00:00:00 2001 From: "ayush.jain@juspay.in" Date: Mon, 16 Mar 2026 16:37:29 +0530 Subject: [PATCH 3/4] chore: renaming functions --- .../src/api/context/handlers.rs | 50 +++++++++---------- .../src/api/default_config/handlers.rs | 6 +-- .../src/api/experiments/handlers.rs | 36 +++++++------ .../service_utils/src/middlewares/auth_z.rs | 24 ++++----- 4 files changed, 58 insertions(+), 58 deletions(-) diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index f73a13192..c2007a59c 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -1,4 +1,4 @@ -use std::{cmp::min, collections::HashSet, ops::Deref}; +use std::{cmp::min, collections::HashSet}; use actix_web::{ Either, HttpResponse, Scope, delete, get, post, put, routes, @@ -26,8 +26,8 @@ use superposition_core::helpers::{calculate_context_weight, hash}; use superposition_derives::{authorized, declare_resource}; use superposition_macros::{bad_argument, db_error, unexpected_error}; use superposition_types::{ - Cac, Contextual, DBConnection, InternalUserContext, ListResponse, Overridden, - Overrides, PaginatedResponse, Resource, SortBy, User, + Contextual, DBConnection, InternalUserContext, ListResponse, Overridden, Overrides, + PaginatedResponse, Resource, SortBy, User, api::{ DimensionMatchStrategy, context::{ @@ -73,13 +73,13 @@ pub fn endpoints() -> Scope { .service(validate_handler) } -async fn authorize_create( +async fn create_authorized( auth_z: &AuthZ, - override_map: &Cac, + override_map: &Overrides, ) -> superposition::Result<()> { - let keys = override_map.deref().keys().collect::>(); + let keys = override_map.keys().collect::>(); auth_z - .authorize_action(&AuthZActionCreate::get(), &keys) + .action_authorized(&AuthZActionCreate::get(), &keys) .await } @@ -96,7 +96,7 @@ async fn create_handler( internal_user: InternalUserContext, ) -> superposition::Result { let req = req.into_inner(); - authorize_create(&_auth_z, &req.r#override).await?; + create_authorized(&_auth_z, &req.r#override).await?; let tags = parse_config_tags(custom_headers.config_tags)?; let description = match req.description.clone() { @@ -193,10 +193,10 @@ async fn create_handler( Ok(http_resp.json(put_response)) } -async fn authorize_update( +async fn update_authorized( auth_z: &AuthZ, context: &Identifier, - new_overrides: &Cac, + new_overrides: &Overrides, schema_name: &SchemaName, conn: &mut DBConnection, ) -> superposition::Result<()> { @@ -204,9 +204,9 @@ async fn authorize_update( operations::get_overrides_from_identifier(context, schema_name, conn)?; auth_z - .authorize_action( + .action_authorized( &AuthZActionUpdate::get(), - &changed_keys(&overrides, new_overrides.deref()), + &changed_keys(&overrides, new_overrides), ) .await } @@ -226,7 +226,7 @@ async fn update_handler( let tags = parse_config_tags(custom_headers.config_tags)?; let req_change_reason = req.change_reason.clone(); - authorize_update( + update_authorized( &_auth_z, &req.context, &req.override_, @@ -303,7 +303,7 @@ async fn update_handler( Ok(http_resp.json(override_resp)) } -async fn authorize_action( +async fn move_authorized( auth_z: &AuthZ, ctx_id: &str, schema_name: &SchemaName, @@ -311,7 +311,7 @@ async fn authorize_action( ) -> superposition::Result<()> { let overrides = operations::get_overrides_from_ctx_id(ctx_id, schema_name, conn)?; auth_z - .authorize_action( + .action_authorized( &AuthZActionMove::get(), &overrides.keys().collect::>(), ) @@ -332,7 +332,7 @@ async fn move_handler( internal_user: InternalUserContext, ) -> superposition::Result { let ctx_id = path.into_inner(); - authorize_action( + move_authorized( &_auth_z, &ctx_id, &workspace_context.schema_name, @@ -621,7 +621,7 @@ async fn list_handler( Ok(Json(paginated_response)) } -async fn authorize_delete( +async fn delete_authorized( auth_z: &AuthZ, ctx_id: &str, schema_name: &SchemaName, @@ -629,7 +629,7 @@ async fn authorize_delete( ) -> superposition::Result<()> { let overrides = operations::get_overrides_from_ctx_id(ctx_id, schema_name, conn)?; auth_z - .authorize_action( + .action_authorized( &AuthZActionDelete::get(), &overrides.keys().collect::>(), ) @@ -650,7 +650,7 @@ async fn delete_handler( contexts as contexts_table, id as context_id, }; let ctx_id = path.into_inner(); - authorize_delete( + delete_authorized( &_auth_z, &ctx_id, &workspace_context.schema_name, @@ -720,7 +720,7 @@ async fn delete_handler( .finish()) } -async fn authorize_bulk( +async fn bulk_authorized( auth_z: &AuthZ, operations: &Vec, schema_name: &SchemaName, @@ -729,10 +729,10 @@ async fn authorize_bulk( for op in operations { match op { ContextAction::Put(put_req) => { - authorize_create(auth_z, &put_req.r#override).await?; + create_authorized(auth_z, &put_req.r#override).await?; } ContextAction::Replace(update_req) => { - authorize_update( + update_authorized( auth_z, &update_req.context, &update_req.override_, @@ -742,10 +742,10 @@ async fn authorize_bulk( .await?; } ContextAction::Delete(ctx_id) => { - authorize_delete(auth_z, ctx_id, schema_name, conn).await?; + delete_authorized(auth_z, ctx_id, schema_name, conn).await?; } ContextAction::Move { id: ctx_id, .. } => { - authorize_action(auth_z, ctx_id, schema_name, conn).await?; + move_authorized(auth_z, ctx_id, schema_name, conn).await?; } } } @@ -775,7 +775,7 @@ async fn bulk_operations_handler( bo.into_inner().operations } }; - authorize_bulk(&_auth_z, &ops, &workspace_context.schema_name, &mut conn).await?; + bulk_authorized(&_auth_z, &ops, &workspace_context.schema_name, &mut conn).await?; // Marking immutable. let is_v2 = is_v2; diff --git a/crates/context_aware_config/src/api/default_config/handlers.rs b/crates/context_aware_config/src/api/default_config/handlers.rs index 0038312c1..ad918f328 100644 --- a/crates/context_aware_config/src/api/default_config/handlers.rs +++ b/crates/context_aware_config/src/api/default_config/handlers.rs @@ -78,7 +78,7 @@ async fn create_handler( user: User, ) -> superposition::Result { let req = request.into_inner(); - _auth_z.authorize(&[req.key.deref()]).await?; + _auth_z.authorized(&[req.key.deref()]).await?; let DbConnection(mut conn) = db_conn; let key = req.key; @@ -233,7 +233,7 @@ async fn update_handler( user: User, ) -> superposition::Result { let key = key.into_inner(); - _auth_z.authorize(&[key.deref()]).await?; + _auth_z.authorized(&[key.deref()]).await?; let DbConnection(mut conn) = db_conn; let req = request.into_inner(); @@ -512,7 +512,7 @@ async fn delete_handler( user: User, ) -> superposition::Result { let key = path.into_inner(); - _auth_z.authorize(&[key.deref()]).await?; + _auth_z.authorized(&[key.deref()]).await?; let DbConnection(mut conn) = db_conn; let tags = parse_config_tags(custom_headers.config_tags)?; diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index 3fa69827d..f7c94b5ed 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -133,16 +133,16 @@ fn add_config_version_to_header( } } -async fn authorize_create( +async fn create_authorized( auth_z: AuthZ, variants: &[Variant], ) -> superposition::Result<()> { let control_keys = variants .iter() .filter(|v| v.variant_type == VariantType::CONTROL) - .flat_map(|v| v.overrides.deref().keys()) + .flat_map(|v| v.overrides.keys()) .collect::>(); - auth_z.authorize(&control_keys).await + auth_z.authorized(&control_keys).await } #[authorized] @@ -157,7 +157,7 @@ async fn create_handler( ) -> superposition::Result { use superposition_types::database::schema::experiments::dsl::experiments; let req = req.into_inner(); - authorize_create(_auth_z, &req.variants).await?; + create_authorized(_auth_z, &req.variants).await?; let mut variants = req.variants; let DbConnection(mut conn) = db_conn; @@ -174,10 +174,9 @@ async fn create_handler( // Checking if experiment has exactly 1 control variant, and // atleast 1 experimental variant check_variant_types(&variants)?; - let unique_override_keys: Vec = - extract_override_keys(variants[0].overrides.deref()) - .into_iter() - .collect(); + let unique_override_keys: Vec = extract_override_keys(&variants[0].overrides) + .into_iter() + .collect(); let unique_ids_of_variants_from_req: HashSet<&str> = HashSet::from_iter(variants.iter().map(|v| v.id.as_str())); @@ -426,7 +425,7 @@ async fn create_handler( Ok(http_resp.json(response)) } -async fn authorize_action( +async fn action_authorized( auth_z: AuthZ, exp_id: &i64, schema_name: &SchemaName, @@ -435,7 +434,7 @@ async fn authorize_action( let overrides = get_control_overrides_from_exp_id(exp_id, schema_name, conn)?; auth_z - .authorize(&overrides.deref().keys().collect::>()) + .authorized(&overrides.keys().collect::>()) .await } @@ -453,7 +452,8 @@ async fn conclude_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let exp_id = path.into_inner(); - authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; + action_authorized(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn) + .await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -727,7 +727,8 @@ async fn discard_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let exp_id = path.into_inner(); - authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; + action_authorized(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn) + .await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -1289,7 +1290,8 @@ async fn ramp_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let exp_id = params.into_inner(); - authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; + action_authorized(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn) + .await?; let change_reason = req.change_reason.clone(); @@ -1482,7 +1484,7 @@ async fn update_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let experiment_id = params.into_inner(); - authorize_action( + action_authorized( _auth_z, &experiment_id, &workspace_context.schema_name, @@ -1830,7 +1832,8 @@ async fn pause_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let exp_id = path.into_inner(); - authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; + action_authorized(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn) + .await?; fetch_and_validate_change_reason_with_function( &workspace_context, @@ -1925,7 +1928,8 @@ async fn resume_handler( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let exp_id = path.into_inner(); - authorize_action(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn).await?; + action_authorized(_auth_z, &exp_id, &workspace_context.schema_name, &mut conn) + .await?; fetch_and_validate_change_reason_with_function( &workspace_context, diff --git a/crates/service_utils/src/middlewares/auth_z.rs b/crates/service_utils/src/middlewares/auth_z.rs index e76870d25..fc0d8566b 100644 --- a/crates/service_utils/src/middlewares/auth_z.rs +++ b/crates/service_utils/src/middlewares/auth_z.rs @@ -52,7 +52,7 @@ impl AuthZ { } } - pub async fn authorize_action( + pub async fn action_authorized( &self, action: &str, attributes: &[&String], @@ -80,8 +80,8 @@ impl AuthZ { Ok(()) } - pub async fn authorize(&self, attributes: &[&String]) -> superposition::Result<()> { - self.authorize_action(&A::get(), attributes).await + pub async fn authorized(&self, attributes: &[&String]) -> superposition::Result<()> { + self.action_authorized(&A::get(), attributes).await } } @@ -129,17 +129,13 @@ impl FromRequest for AuthZ { Box::pin(async move { let is_allowed = auth_z_handler .is_allowed(&domain, &user, &A::resource(), &A::get(), None) - .await; - - match is_allowed { - Err(e) => Err(unexpected_error!("Error checking authorization: {}", e)), - Ok(is_allowed) => { - if is_allowed { - Ok(AuthZ::new(auth_z_handler, domain, user, false)) - } else { - Err(forbidden!("You are not authorized to perform this action.")) - } - } + .await + .map_err(|e| unexpected_error!("Error checking authorization: {}", e))?; + + if is_allowed { + Ok(AuthZ::new(auth_z_handler, domain, user, false)) + } else { + Err(forbidden!("You are not authorized to perform this action.")) } }) } From 4e11f7a9225bd3020b21da68346724981c5ce1b2 Mon Sep 17 00:00:00 2001 From: "ayush.jain@juspay.in" Date: Mon, 16 Mar 2026 18:27:01 +0530 Subject: [PATCH 4/4] chore: addressing comments --- crates/experimentation_client/src/lib.rs | 1 - .../src/api/experiment_groups/helpers.rs | 22 +++-- .../src/api/experiments/handlers.rs | 16 ++-- .../src/api/experiments/helpers.rs | 1 - crates/frontend/src/components/authz.rs | 80 ++--------------- crates/frontend/src/pages/experiment.rs | 4 +- .../src/middlewares/auth_z/casbin/handlers.rs | 88 +++++++++---------- crates/superposition_core/src/experiment.rs | 1 - .../src/api/authz/casbin.rs | 1 - .../src/database/models/experimentation.rs | 9 ++ 10 files changed, 74 insertions(+), 149 deletions(-) diff --git a/crates/experimentation_client/src/lib.rs b/crates/experimentation_client/src/lib.rs index e67b2de56..9d05449a3 100644 --- a/crates/experimentation_client/src/lib.rs +++ b/crates/experimentation_client/src/lib.rs @@ -212,7 +212,6 @@ impl Client { .filter_map(|experiment| { let variants: Vec<_> = experiment .variants - .into_inner() .into_iter() .filter_map(|mut variant| { Variant::filter_keys_by_prefix(&variant, &prefix_list) diff --git a/crates/experimentation_platform/src/api/experiment_groups/helpers.rs b/crates/experimentation_platform/src/api/experiment_groups/helpers.rs index 2d51a59ea..d6d8e08a1 100644 --- a/crates/experimentation_platform/src/api/experiment_groups/helpers.rs +++ b/crates/experimentation_platform/src/api/experiment_groups/helpers.rs @@ -264,13 +264,13 @@ fn assign_additional_buckets( unassigned_buckets.len() )); } - let variants = experiment.variants.clone().into_inner(); - let variants_len = variants.len(); + + let variants_len = experiment.variants.len(); // Reverse the unassigned_buckets to fill from the front unassigned_buckets.reverse(); - for variant in variants { + for variant in experiment.variants.iter() { for _ in 0..additional_needed / variants_len { if let Some(bucket) = unassigned_buckets.pop() { *bucket = Some(Bucket { @@ -289,9 +289,8 @@ fn unassign_excess_buckets( experiment: &Experiment, excess_count: usize, ) { - let variants = experiment.variants.clone().into_inner(); - let variants_len = variants.len(); - for variant in variants { + let variants_len = experiment.variants.len(); + for variant in experiment.variants.iter() { for _ in 0..excess_count / variants_len { if let Some(bucket) = current_buckets .iter_mut() @@ -363,10 +362,10 @@ pub fn create_system_generated_experiment_group( let context_hash = hash(&Value::Object(context.clone().into())); let now = chrono::Utc::now(); - let variants = experiment.variants.clone().into_inner(); - let group_traffic_percentage = - TrafficPercentage::try_from(variants.len() as u8 * **exp_traffic_percentage) - .map_err(|e| unexpected_error!(e))?; + let group_traffic_percentage = TrafficPercentage::try_from( + experiment.variants.len() as u8 * **exp_traffic_percentage, + ) + .map_err(|e| unexpected_error!(e))?; let mut buckets = Buckets::default(); update_bucket_allocation(experiment, &mut buckets, exp_traffic_percentage)?; @@ -416,8 +415,7 @@ pub fn update_experiment_group_buckets( let new_traffic_percentage = match experiment_group.group_type { GroupType::SystemGenerated => TrafficPercentage::try_from( - experiment.variants.clone().into_inner().len() as i32 - * (**exp_traffic_percentage as i32), + experiment.variants.len() as i32 * (**exp_traffic_percentage as i32), ) .map_err(|e| unexpected_error!(e))?, GroupType::UserCreated => experiment_group.traffic_percentage, diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index f7c94b5ed..b06c23cf9 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -539,7 +539,7 @@ pub async fn conclude( let mut operations: Vec = vec![]; let mut is_valid_winner_variant = false; - for variant in experiment.variants.clone().into_inner() { + for variant in experiment.variants.clone().into_iter() { let context_id = variant.context_id.ok_or_else(|| { log::error!("context id not available for variant {:?}", variant.id); unexpected_error!("Something went wrong, failed to conclude experiment") @@ -803,7 +803,6 @@ pub async fn discard( let operations: Vec = experiment .variants .clone() - .into_inner() .into_iter() .map(|variant| { variant @@ -1007,7 +1006,6 @@ async fn get_applicable_variants_handler( .filter_map(|(_, experiment)| { experiment .variants - .into_inner() .into_iter() .find(|variant| applicable_variants.contains(&variant.id)) }) @@ -1323,15 +1321,14 @@ async fn ramp_handler( )); } - let experiment_variants = experiment.variants.clone().into_inner(); - match experiment.experiment_type { ExperimentType::Default => { // Validate control overrides against resolved config when auto-populate is enabled and experiment is in CREATED state if workspace_context.settings.auto_populate_control && experiment.status == ExperimentStatusType::CREATED { - let control_variant = experiment_variants + let control_variant = experiment + .variants .iter() .find(|v| v.variant_type == VariantType::CONTROL) .ok_or_else(|| { @@ -1368,7 +1365,7 @@ async fn ramp_handler( let old_traffic_percentage = experiment.traffic_percentage; let new_traffic_percentage = &req.traffic_percentage; - let variants_count = experiment.variants.clone().into_inner().len() as u8; + let variants_count = experiment.variants.len() as u8; new_traffic_percentage .check_max_allowed(variants_count) @@ -1526,10 +1523,9 @@ async fn update_handler( )); } - let experiment_variants: Vec = experiment.variants.clone().into_inner(); - let id_to_existing_variant: HashMap = HashMap::from_iter( - experiment_variants + experiment + .variants .iter() .map(|variant| (variant.id.to_string(), variant)) .collect::>(), diff --git a/crates/experimentation_platform/src/api/experiments/helpers.rs b/crates/experimentation_platform/src/api/experiments/helpers.rs index 0d3324c4e..b4b5b819a 100644 --- a/crates/experimentation_platform/src/api/experiments/helpers.rs +++ b/crates/experimentation_platform/src/api/experiments/helpers.rs @@ -866,7 +866,6 @@ pub fn get_control_overrides_from_exp_id( .get_result::(conn)?; let control_overrides = variants - .into_inner() .into_iter() .find(|variant| variant.variant_type == VariantType::CONTROL) .ok_or_else(|| { diff --git a/crates/frontend/src/components/authz.rs b/crates/frontend/src/components/authz.rs index a5e54bd64..27bb89511 100644 --- a/crates/frontend/src/components/authz.rs +++ b/crates/frontend/src/components/authz.rs @@ -265,15 +265,7 @@ fn PolicyViewer( Ok(resp) => { policies_resource.refetch(); delete_item_rws.set(None); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); } Err(e) => { enqueue_alert( @@ -315,15 +307,7 @@ fn PolicyViewer( policies_resource.refetch(); resource_action_map_resource.refetch(); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); Ok(()) }); @@ -559,15 +543,7 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { Ok(resp) => { roles_resource.refetch(); delete_item_rws.set(None); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); } Err(e) => { enqueue_alert( @@ -596,15 +572,7 @@ fn RolePolicyViewer(authz_scope: StoredValue) -> impl IntoView { .await?; roles_resource.refetch(); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); Ok(()) } }); @@ -837,15 +805,7 @@ fn DomainResourceActionGroupViewer( Ok(resp) => { action_groups_resource.refetch(); delete_item_rws.set(None); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); } Err(e) => { enqueue_alert( @@ -882,15 +842,7 @@ fn DomainResourceActionGroupViewer( action_groups_resource.refetch(); resource_action_map_resource.refetch(); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); Ok(()) } }); @@ -1110,15 +1062,7 @@ fn ResourceActionGroupViewer( Ok(resp) => { action_groups_resource.refetch(); delete_item_rws.set(None); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); } Err(e) => { enqueue_alert( @@ -1152,15 +1096,7 @@ fn ResourceActionGroupViewer( action_groups_resource.refetch(); resource_action_map_resource.refetch(); - enqueue_alert( - resp.message, - if resp.success { - AlertType::Success - } else { - AlertType::Error - }, - 4000, - ); + enqueue_alert(resp.message, AlertType::Success, 4000); Ok(()) } }); diff --git a/crates/frontend/src/pages/experiment.rs b/crates/frontend/src/pages/experiment.rs index 04e4a2bd5..e50a20f06 100644 --- a/crates/frontend/src/pages/experiment.rs +++ b/crates/frontend/src/pages/experiment.rs @@ -151,9 +151,7 @@ pub fn ExperimentPage() -> impl IntoView { context=Conditions::from_iter( experiment_ef.context.into_inner(), ) - variants=FromIterator::from_iter( - experiment_ef.variants.into_inner(), - ) + variants=FromIterator::from_iter(experiment_ef.variants) default_config dimensions experiment_form_type=ExperimentFormType::from( diff --git a/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs b/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs index 1d7aecdc0..ab73ea9ea 100644 --- a/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs +++ b/crates/service_utils/src/middlewares/auth_z/casbin/handlers.rs @@ -123,13 +123,12 @@ async fn add_policy_handler( .await .map_err(|e| unexpected_error!(e))?; + if !added { + bad_argument!("The specified policy already exists"); + } + Ok(Json(ActionResponse { - success: added, - message: if added { - "Policy added".to_string() - } else { - "Policy already exists".to_string() - }, + message: "Policy added".to_string(), })) } @@ -158,13 +157,12 @@ async fn delete_policy_handler( .await .map_err(|e| unexpected_error!(e))?; + if !removed { + bad_argument!("The specified policy does not exist"); + } + Ok(Json(ActionResponse { - success: removed, - message: if removed { - "Policy removed".to_string() - } else { - "Policy does not exist".to_string() - }, + message: "Policy removed".to_string(), })) } @@ -215,13 +213,12 @@ async fn add_roles_handler( .await .map_err(|e| unexpected_error!(e))?; + if !added { + bad_argument!("The specified grouping policy already exists"); + } + Ok(Json(ActionResponse { - success: added, - message: if added { - "Grouping policy added".to_string() - } else { - "Grouping policy already exists".to_string() - }, + message: "Grouping policy added".to_string(), })) } @@ -248,13 +245,12 @@ async fn delete_roles_handler( .await .map_err(|e| unexpected_error!(e))?; + if !removed { + bad_argument!("The specified grouping policy does not exist"); + } + Ok(Json(ActionResponse { - success: removed, - message: if removed { - "Grouping policy removed".to_string() - } else { - "Grouping policy does not exist".to_string() - }, + message: "Grouping policy removed".to_string(), })) } @@ -308,13 +304,12 @@ async fn add_domain_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; + if !added { + bad_argument!("The specified policy already exists"); + } + Ok(Json(ActionResponse { - success: added, - message: if added { - "Action-group policy added".to_string() - } else { - "Action-group policy already exists".to_string() - }, + message: "Action-group policy added".to_string(), })) } @@ -344,13 +339,12 @@ async fn delete_domain_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; + if !removed { + bad_argument!("The specified action-group policy does not exist"); + } + Ok(Json(ActionResponse { - success: removed, - message: if removed { - "Action-group policy removed".to_string() - } else { - "Action-group policy does not exist".to_string() - }, + message: "Action-group policy removed".to_string(), })) } @@ -396,13 +390,12 @@ async fn add_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; + if !added { + bad_argument!("The specified action-group policy already exists"); + } + Ok(Json(ActionResponse { - success: added, - message: if added { - "Action-group policy added".to_string() - } else { - "Action-group policy already exists".to_string() - }, + message: "Action-group policy added".to_string(), })) } @@ -430,13 +423,12 @@ async fn delete_action_group_handler( .await .map_err(|e| unexpected_error!(e))?; + if !removed { + bad_argument!("The specified action-group policy does not exist"); + } + Ok(Json(ActionResponse { - success: removed, - message: if removed { - "Action-group policy removed".to_string() - } else { - "Action-group policy does not exist".to_string() - }, + message: "Action-group policy removed".to_string(), })) } diff --git a/crates/superposition_core/src/experiment.rs b/crates/superposition_core/src/experiment.rs index 974163e37..66166c3c7 100644 --- a/crates/superposition_core/src/experiment.rs +++ b/crates/superposition_core/src/experiment.rs @@ -243,7 +243,6 @@ fn filter_experiments_by_prefix( .filter_map(|experiment| { let variants: Vec<_> = experiment .variants - .into_inner() .into_iter() .filter_map(|mut variant| { Variant::filter_keys_by_prefix(&variant, &prefix_list) diff --git a/crates/superposition_types/src/api/authz/casbin.rs b/crates/superposition_types/src/api/authz/casbin.rs index 86827c920..2c6dc05d2 100644 --- a/crates/superposition_types/src/api/authz/casbin.rs +++ b/crates/superposition_types/src/api/authz/casbin.rs @@ -25,6 +25,5 @@ pub struct ActionGroupPolicyRequest { #[derive(Deserialize, Serialize)] pub struct ActionResponse { - pub success: bool, pub message: String, } diff --git a/crates/superposition_types/src/database/models/experimentation.rs b/crates/superposition_types/src/database/models/experimentation.rs index b650a2861..944815ddf 100644 --- a/crates/superposition_types/src/database/models/experimentation.rs +++ b/crates/superposition_types/src/database/models/experimentation.rs @@ -272,6 +272,15 @@ impl Variants { } } +impl IntoIterator for Variants { + type Item = Variant; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + #[derive(Serialize, Deserialize, Clone)] #[cfg_attr( feature = "diesel_derives",