diff --git a/.sqlx/query-bc760664c6a1f3f7324acf3d45deb9256373908ceeea0ed599fc3827733c9d5f.json b/.sqlx/query-bc760664c6a1f3f7324acf3d45deb9256373908ceeea0ed599fc3827733c9d5f.json deleted file mode 100644 index 591dad2ba..000000000 --- a/.sqlx/query-bc760664c6a1f3f7324acf3d45deb9256373908ceeea0ed599fc3827733c9d5f.json +++ /dev/null @@ -1,52 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT mf.id, mf.title, COALESCE(s.step_count, 0) AS \"step_count!: i64\", COALESCE(array_agg(g.name ORDER BY g.name) FILTER (WHERE g.name IS NOT NULL), '{}') AS \"group_names!: Vec\", lmf.position, lmf.is_default FROM location_mfa_flow lmf JOIN mfa_flow mf ON mf.id = lmf.flow_id LEFT JOIN ( SELECT flow_id, COUNT(*) AS step_count FROM mfa_flow_step GROUP BY flow_id ) s ON s.flow_id = mf.id LEFT JOIN location_mfa_flow_group lmfg ON lmfg.location_id = lmf.location_id AND lmfg.flow_id = lmf.flow_id LEFT JOIN \"group\" g ON g.id = lmfg.group_id WHERE lmf.location_id = $1 GROUP BY mf.id, mf.title, s.step_count, lmf.position, lmf.is_default ORDER BY lmf.position", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "title", - "type_info": "Text" - }, - { - "ordinal": 2, - "name": "step_count!: i64", - "type_info": "Int8" - }, - { - "ordinal": 3, - "name": "group_names!: Vec", - "type_info": "TextArray" - }, - { - "ordinal": 4, - "name": "position", - "type_info": "Int4" - }, - { - "ordinal": 5, - "name": "is_default", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [ - false, - false, - null, - null, - false, - false - ] - }, - "hash": "bc760664c6a1f3f7324acf3d45deb9256373908ceeea0ed599fc3827733c9d5f" -} diff --git a/.sqlx/query-c2ae1735ee44ce964580e17ac3c0fdb9f80fd38c47ca9b5c9ff10a0f50e7b00e.json b/.sqlx/query-c2ae1735ee44ce964580e17ac3c0fdb9f80fd38c47ca9b5c9ff10a0f50e7b00e.json new file mode 100644 index 000000000..6b5558b93 --- /dev/null +++ b/.sqlx/query-c2ae1735ee44ce964580e17ac3c0fdb9f80fd38c47ca9b5c9ff10a0f50e7b00e.json @@ -0,0 +1,58 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT mf.id, mf.title, COALESCE(s.step_count, 0) AS \"step_count!: i64\", COALESCE(array_agg(lmfg.group_id ORDER BY lmfg.group_id) FILTER (WHERE lmfg.group_id IS NOT NULL), '{}') AS \"group_ids!: Vec\", COALESCE(array_agg(g.name ORDER BY lmfg.group_id) FILTER (WHERE g.name IS NOT NULL), '{}') AS \"group_names!: Vec\", lmf.position, lmf.is_default FROM location_mfa_flow lmf JOIN mfa_flow mf ON mf.id = lmf.flow_id LEFT JOIN ( SELECT flow_id, COUNT(*) AS step_count FROM mfa_flow_step GROUP BY flow_id ) s ON s.flow_id = mf.id LEFT JOIN location_mfa_flow_group lmfg ON lmfg.location_id = lmf.location_id AND lmfg.flow_id = lmf.flow_id LEFT JOIN \"group\" g ON g.id = lmfg.group_id WHERE lmf.location_id = $1 GROUP BY mf.id, mf.title, s.step_count, lmf.position, lmf.is_default ORDER BY lmf.position", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "title", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "step_count!: i64", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "group_ids!: Vec", + "type_info": "Int8Array" + }, + { + "ordinal": 4, + "name": "group_names!: Vec", + "type_info": "TextArray" + }, + { + "ordinal": 5, + "name": "position", + "type_info": "Int4" + }, + { + "ordinal": 6, + "name": "is_default", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false, + false, + null, + null, + null, + false, + false + ] + }, + "hash": "c2ae1735ee44ce964580e17ac3c0fdb9f80fd38c47ca9b5c9ff10a0f50e7b00e" +} diff --git a/crates/defguard_common/src/db/models/mfa_flow.rs b/crates/defguard_common/src/db/models/mfa_flow.rs index d61c77b19..ab4d0bdcc 100644 --- a/crates/defguard_common/src/db/models/mfa_flow.rs +++ b/crates/defguard_common/src/db/models/mfa_flow.rs @@ -49,19 +49,20 @@ pub struct MfaFlowSnapshot { pub steps: Vec>, } -/// Assignment of an MFA flow to a location, enriched for API consumption. +/// MFA flow assignment with location metadata. #[derive(Clone, Debug, Serialize)] pub struct LocationMfaFlowItem { pub id: Id, pub title: String, pub step_count: i64, + pub group_ids: Vec, pub group_names: Vec, pub position: i32, pub is_default: bool, } /// Input for a single flow assignment to a location. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize, ToSchema)] pub struct LocationMfaFlowAssignment { pub flow_id: Id, pub is_default: bool, @@ -117,6 +118,19 @@ pub enum MfaFlowAssignmentError { Sqlx(#[from] sqlx::Error), } +/// License-related errors that can occur during MFA flow assignment. +#[derive(Debug, Error)] +pub enum MfaFlowAssignmentLicenseError { + #[error("MFA flow group assignments require an Enterprise license")] + GroupAssignmentNotAllowed, + #[error("Multi-step MFA flows require a Business license")] + MultipleStepsNotAllowed, + #[error("Multiple MFA flows can only be assigned with Business license")] + MultipleMfaFlowsNotAllowed, + #[error(transparent)] + Database(#[from] sqlx::Error), +} + /// Errors that can occur when updating an MFA flow. #[derive(Debug, Error)] pub enum MfaFlowUpdateError { @@ -516,6 +530,44 @@ impl MfaFlow { Ok(()) } + /// Validates whether the current license permits the requested MFA flow assignments. + pub async fn validate_mfa_flow_assignments_license( + conn: &mut PgConnection, + assignments: &[LocationMfaFlowAssignment], + has_enterprise_access: bool, + is_business_license_active: bool, + ) -> Result<(), MfaFlowAssignmentLicenseError> { + // Enterprise can make all assignments. + if has_enterprise_access { + return Ok(()); + } + + // Business and Free can't assign groups. + if assignments.iter().any(|a| !a.group_ids.is_empty()) { + return Err(MfaFlowAssignmentLicenseError::GroupAssignmentNotAllowed); + } + + // Business can assign multiple and multi-step flows. + if is_business_license_active { + return Ok(()); + } + + // Free can't assign multi-step flows. + if let Some(assignment) = assignments.first() { + let steps = MfaFlowStep::find_by_flow(&mut *conn, assignment.flow_id).await?; + if steps.len() > 1 { + return Err(MfaFlowAssignmentLicenseError::MultipleStepsNotAllowed); + } + } + + // Free can't assign multiple flows. + if assignments.len() > 1 { + return Err(MfaFlowAssignmentLicenseError::MultipleMfaFlowsNotAllowed); + } + + Ok(()) + } + /// Returns the enriched assignment list for a location, ordered by position. pub async fn for_location<'e, E: PgExecutor<'e>>( executor: E, @@ -525,7 +577,10 @@ impl MfaFlow { LocationMfaFlowItem, "SELECT mf.id, mf.title, \ COALESCE(s.step_count, 0) AS \"step_count!: i64\", \ - COALESCE(array_agg(g.name ORDER BY g.name) \ + COALESCE(array_agg(lmfg.group_id ORDER BY lmfg.group_id) \ + FILTER (WHERE lmfg.group_id IS NOT NULL), '{}') \ + AS \"group_ids!: Vec\", \ + COALESCE(array_agg(g.name ORDER BY lmfg.group_id) \ FILTER (WHERE g.name IS NOT NULL), '{}') \ AS \"group_names!: Vec\", \ lmf.position, lmf.is_default \ diff --git a/crates/defguard_common/src/db/models/wizard.rs b/crates/defguard_common/src/db/models/wizard.rs index 7c7a34bd3..f95eb8e73 100644 --- a/crates/defguard_common/src/db/models/wizard.rs +++ b/crates/defguard_common/src/db/models/wizard.rs @@ -122,15 +122,13 @@ impl Wizard { .fetch_one(executor) .await?; - let active_wizard; - - if has_auto_adopt_flags { - active_wizard = ActiveWizard::AutoAdoption; + let active_wizard = if has_auto_adopt_flags { + ActiveWizard::AutoAdoption } else if is_fresh_instance { - active_wizard = ActiveWizard::Initial; + ActiveWizard::Initial } else { - active_wizard = ActiveWizard::Migration; - } + ActiveWizard::Migration + }; wizard.active_wizard = active_wizard; diff --git a/crates/defguard_core/src/enterprise/handlers/device_posture.rs b/crates/defguard_core/src/enterprise/handlers/device_posture.rs index 3375a94e5..2fb1c05d6 100644 --- a/crates/defguard_core/src/enterprise/handlers/device_posture.rs +++ b/crates/defguard_core/src/enterprise/handlers/device_posture.rs @@ -1114,84 +1114,12 @@ pub async fn duplicate_device_posture( Ok(ApiResponse::json(response, StatusCode::CREATED)) } -/// Request body for assigning posture checks to a VPN location. -#[derive(Clone, Debug, Deserialize, Serialize, ToSchema)] -pub struct AssignPosturesData { - pub postures: Vec, -} - /// Request body for assigning VPN locations to a posture check. #[derive(Clone, Debug, Deserialize, Serialize, ToSchema)] pub struct AssignLocationsData { pub locations: Vec, } -/// Assign device posture check policies to a location -/// -/// Replaces the current assignment. -#[utoipa::path( - put, - path = "/api/v1/network/{id}/postures", - tag = "device posture", - params( - ("id" = i64, Path, description = "ID of the location.") - ), - request_body = AssignPosturesData, - responses( - (status = 200, description = "Device posture check policies assigned to the location.", body = [Id]), - (status = 400, description = "Posture checks cannot be assigned to a service location.", body = ApiErrorResponse, example = json!({"msg": "Posture checks cannot be assigned to service locations"})), - (status = 401, description = "Session is missing or invalid.", body = ApiErrorResponse, example = json!({"msg": "Session is required"})), - (status = 403, description = "Requires admin privileges and an active enterprise license.", body = ApiErrorResponse, example = json!({"msg": "requires privileged access"})), - (status = 404, description = "Location not found.", body = ApiErrorResponse, example = json!({"msg": "Location 1 not found"})), - (status = 500, description = "Unable to assign device posture check policies to the location.", body = ApiErrorResponse, example = json!({"msg": "Internal server error"})) - ), - security( - ("cookie" = []), - ("api_token" = []) - ) -)] -pub async fn set_postures_for_location( - _license: LicenseGated, - _admin: AdminRole, - session: SessionInfo, - context: ApiRequestContext, - Path(location_id): Path, - State(appstate): State, - Json(data): Json, -) -> ApiResult { - debug!( - "User {} assigning device posture checks {:?} to location {location_id}", - session.user.username, data.postures - ); - - let location = WireguardNetwork::find_by_id(&appstate.pool, location_id) - .await? - .ok_or_else(|| WebError::ObjectNotFound(format!("Location {location_id} not found")))?; - - let mut tx = appstate.pool.begin().await?; - let old_postures = DevicePostureLocation::find_by_location(&mut *tx, location_id).await?; - let result = - DevicePostureLocation::set_for_location(&mut tx, location_id, &data.postures).await?; - let gateway_commands = if same_id_set(&old_postures, &result) { - Vec::new() - } else { - build_location_peer_refresh_commands(&mut tx, [location_id]).await? - }; - tx.commit().await?; - - appstate.send_multiple_gateway_commands(gateway_commands); - - appstate.emit_event(ApiEvent { - context, - event: Box::new(ApiEventType::LocationPosturesAssigned { - location, - posture_ids: result.clone(), - }), - })?; - - Ok(ApiResponse::json(result, StatusCode::OK)) -} - /// Assign locations to a device posture check policy /// /// Replaces the current assignment. diff --git a/crates/defguard_core/src/handlers/mfa_flow.rs b/crates/defguard_core/src/handlers/mfa_flow.rs index 31f0faecc..84efdf2a5 100644 --- a/crates/defguard_core/src/handlers/mfa_flow.rs +++ b/crates/defguard_core/src/handlers/mfa_flow.rs @@ -10,9 +10,9 @@ use defguard_common::db::{ models::{ Settings, WireguardNetwork, mfa_flow::{ - LocationMfaFlowAssignment, LocationMfaFlowItem, MfaFlow, MfaFlowAssignmentError, - MfaFlowDeleteError, MfaFlowSnapshot, MfaFlowStep, MfaFlowUpdateError, - MfaFlowValidationField, MfaFlowWithStepCount, validate_flow_input, + LocationMfaFlowAssignment, MfaFlow, MfaFlowAssignmentError, MfaFlowDeleteError, + MfaFlowSnapshot, MfaFlowStep, MfaFlowUpdateError, MfaFlowValidationField, + MfaFlowWithStepCount, validate_flow_input, }, vpn_client_session::VpnClientMfaMethod, }, @@ -25,10 +25,7 @@ use utoipa::ToSchema; use crate::{ appstate::AppState, auth::{AdminRole, SessionInfo}, - enterprise::{ - db::models::openid_provider::OpenIdProvider, has_enterprise_access, - is_business_license_active, - }, + enterprise::{db::models::openid_provider::OpenIdProvider, is_business_license_active}, error::WebError, events::{ApiEvent, ApiEventType, ApiRequestContext}, handlers::{ApiErrorResponse, ApiResponse, ApiResult}, @@ -130,43 +127,21 @@ pub struct UpdateMfaFlowStep { pub methods: Vec, } -/// Request body for assigning flows to a location. -#[derive(Clone, Debug, Deserialize, Serialize, ToSchema)] -pub struct AssignMfaFlowsRequest { - pub assignments: Vec, -} - -/// A single entry in an assignment list. +/// A group scoped to a location MFA flow assignment. #[derive(Clone, Debug, Deserialize, Serialize, ToSchema)] -pub struct AssignMfaFlowEntry { - pub flow_id: Id, - pub is_default: bool, - #[serde(default)] - pub group_ids: Vec, +pub struct LocationMfaFlowGroupResponse { + pub id: Id, + pub name: String, } -/// Assignment item returned by `GET /location/{id}/mfa-flows`. +/// An MFA flow assignment rendered in the context of one location. #[derive(Clone, Debug, Deserialize, Serialize, ToSchema)] pub struct LocationMfaFlowResponse { pub id: Id, pub title: String, - pub step_count: i64, - pub group_names: Vec, - pub position: i32, + pub steps: Vec, pub is_default: bool, -} - -impl From for LocationMfaFlowResponse { - fn from(item: LocationMfaFlowItem) -> Self { - Self { - id: item.id, - title: item.title, - step_count: item.step_count, - group_names: item.group_names, - position: item.position, - is_default: item.is_default, - } - } + pub groups: Vec, } // Helpers @@ -177,7 +152,7 @@ impl From for LocationMfaFlowResponse { /// The status stays `403` rather than the `400` the impl spec tabulates: a licence refusal is not /// a malformed request, and the rest of the codebase answers licence gates with `403`. The /// top-level `error` discriminator distinguishes it from `validation_failed`. -fn license_error_response(field: String, code: &str) -> ApiResponse { +pub(crate) fn license_error_response(field: String, code: &str) -> ApiResponse { ApiResponse::new( json!({ "error": "license_required", @@ -261,36 +236,14 @@ fn check_method_prerequisites( } } -/// Check licence gates for flow assignment: group scoping requires Enterprise. -/// -/// Uses `has_enterprise_access(None)` (raw Enterprise tier) rather than -/// `has_enterprise_access(Some(LicenseFeature::MfaFlowGroupScoping))` because -/// adding a `LicenseFeature` variant would require coordination outside this -/// repo: the proto enum in the `proto` repo and license issuance must both -/// recognise the new variant. The `None` form gates strictly on the Enterprise -/// tier, which is the correct behaviour for this feature. -#[must_use] -fn check_assignment_license_gates(assignments: &[AssignMfaFlowEntry]) -> Option { - let scoped = assignments.iter().position(|a| !a.group_ids.is_empty()); - if let Some(index) = scoped - && !has_enterprise_access(None) - { - return Some(license_error_response( - format!("assignments[{index}].group_ids"), - "enterprise_license_required", - )); - } - None -} - /// Field path for the first assignment entry matching `predicate`, suffixed with `suffix`. /// /// Errors point at the row the admin submitted rather than at the list as a whole. When no entry /// matches, the path degrades to the bare `assignments` list, which is the best available anchor. fn assignment_field_path( - assignments: &[AssignMfaFlowEntry], + assignments: &[LocationMfaFlowAssignment], suffix: &str, - predicate: impl Fn(&AssignMfaFlowEntry) -> bool, + predicate: impl Fn(&LocationMfaFlowAssignment) -> bool, ) -> String { assignments.iter().position(predicate).map_or_else( || "assignments".to_owned(), @@ -299,12 +252,12 @@ fn assignment_field_path( } /// Field path for the assignment entry referencing `flow_id`. -fn assignment_field(assignments: &[AssignMfaFlowEntry], flow_id: Id) -> String { +fn assignment_field(assignments: &[LocationMfaFlowAssignment], flow_id: Id) -> String { assignment_field_path(assignments, "flow_id", |a| a.flow_id == flow_id) } /// Field path for the assignment entry referencing `group_id`. -fn group_field(assignments: &[AssignMfaFlowEntry], group_id: Id) -> String { +fn group_field(assignments: &[LocationMfaFlowAssignment], group_id: Id) -> String { assignment_field_path(assignments, "group_ids", |a| { a.group_ids.contains(&group_id) }) @@ -312,11 +265,47 @@ fn group_field(assignments: &[AssignMfaFlowEntry], group_id: Id) -> String { /// Field path for the assignment entry whose empty group set made it inert, pointing at the /// `group_ids` the admin must populate rather than at the flow as a whole. -fn non_default_group_field(assignments: &[AssignMfaFlowEntry], flow_id: Id) -> String { +fn non_default_group_field(assignments: &[LocationMfaFlowAssignment], flow_id: Id) -> String { assignment_field_path(assignments, "group_ids", |a| a.flow_id == flow_id) } /// Build a `400` response with structured `fields[]` errors. +pub(crate) fn assignment_error_response( + assignments: &[LocationMfaFlowAssignment], + error: MfaFlowAssignmentError, +) -> Result { + let (field, code) = match error { + MfaFlowAssignmentError::NoDefaultDesignated => { + ("mfa_flows".to_owned(), "no_default_designated") + } + MfaFlowAssignmentError::MultipleDefaultsDesignated => { + ("mfa_flows".to_owned(), "multiple_defaults_designated") + } + MfaFlowAssignmentError::DefaultHasGroups => { + ("mfa_flows".to_owned(), "default_must_have_no_groups") + } + MfaFlowAssignmentError::NonDefaultWithoutGroups(flow_id) => ( + non_default_group_field(assignments, flow_id), + "non_default_must_have_groups", + ), + MfaFlowAssignmentError::DuplicateFlow(flow_id) => { + (assignment_field(assignments, flow_id), "duplicate") + } + MfaFlowAssignmentError::UnknownFlow(flow_id) => { + (assignment_field(assignments, flow_id), "unknown_flow") + } + MfaFlowAssignmentError::UnknownGroup(group_id) => { + (group_field(assignments, group_id), "unknown_group") + } + MfaFlowAssignmentError::Sqlx(error) => return Err(WebError::from(error)), + }; + + Ok(validation_error_response(vec![MfaFlowValidationField { + field, + code: code.into(), + }])) +} + fn validation_error_response(errors: Vec) -> ApiResponse { let fields: Vec = errors .iter() @@ -706,7 +695,7 @@ pub async fn delete_mfa_flow( ("id" = i64, Path, description = "ID of the location.") ), responses( - (status = 200, description = "MFA flows assigned to the location.", body = [LocationMfaFlowResponse]), + (status = 200, description = "MFA flow assignments for the location.", body = [LocationMfaFlowResponse]), (status = 401, description = "Session is missing or invalid.", body = ApiErrorResponse), (status = 403, description = "Requires admin privileges.", body = ApiErrorResponse), (status = 500, description = "Unable to list assigned flows.", body = ApiErrorResponse) @@ -736,112 +725,24 @@ pub async fn get_location_mfa_flows( return Err(WebError::ObjectNotFound(format!("Location {id} not found"))); } - let items = MfaFlow::for_location(&appstate.pool, id).await?; - let response: Vec = items.into_iter().map(Into::into).collect(); - - Ok(ApiResponse::json(response, StatusCode::OK)) -} - -/// Assign MFA flows to a location (full replace) -#[utoipa::path( - put, - path = "/api/v1/location/{id}/mfa-flows", - tag = "mfa flow", - params( - ("id" = i64, Path, description = "ID of the location.") - ), - request_body = AssignMfaFlowsRequest, - responses( - (status = 200, description = "MFA flows assigned to the location.", body = [LocationMfaFlowResponse]), - (status = 400, description = "Invalid assignment: `no_default_designated`, `multiple_defaults_designated`, `default_must_have_no_groups`, or `non_default_must_have_groups`.", body = ApiErrorResponse, example = json!({"error": "validation_failed", "fields": [{"field": "mfa_flows", "code": "no_default_designated"}]})), - (status = 401, description = "Session is missing or invalid.", body = ApiErrorResponse), - (status = 403, description = "Requires admin privileges, or group scoping without an enterprise license (`enterprise_license_required`).", body = ApiErrorResponse, example = json!({"error": "license_required", "fields": [{"field": "assignments[0].group_ids", "code": "enterprise_license_required"}]})), - (status = 500, description = "Unable to assign flows.", body = ApiErrorResponse) - ), - security( - ("cookie" = []), - ("api_token" = []) - ) -)] -pub async fn set_location_mfa_flows( - _admin: AdminRole, - session: SessionInfo, - context: ApiRequestContext, - Path(location_id): Path, - State(appstate): State, - Json(data): Json, -) -> ApiResult { - debug!( - "User {} assigning MFA flows to location {location_id}", - session.user.username - ); - - // The location has to exist before we can replace its assignments, and its name is needed for - // the audit event. - let location = WireguardNetwork::find_by_id(&appstate.pool, location_id) - .await? - .ok_or_else(|| WebError::ObjectNotFound(format!("Location {location_id} not found")))?; - - let assignments: Vec = data - .assignments - .iter() - .map(|a| LocationMfaFlowAssignment { - flow_id: a.flow_id, - is_default: a.is_default, - group_ids: a.group_ids.clone(), - }) - .collect(); - - if let Some(resp) = check_assignment_license_gates(&data.assignments) { - return Ok(resp); - } - - let mut tx = appstate.pool.begin().await?; - if let Err(e) = MfaFlow::assign_to_location(&mut tx, location_id, &assignments).await { - let (field, code) = match e { - MfaFlowAssignmentError::NoDefaultDesignated => { - ("mfa_flows".to_owned(), "no_default_designated") - } - MfaFlowAssignmentError::MultipleDefaultsDesignated => { - ("mfa_flows".to_owned(), "multiple_defaults_designated") - } - MfaFlowAssignmentError::DefaultHasGroups => { - ("mfa_flows".to_owned(), "default_must_have_no_groups") - } - MfaFlowAssignmentError::NonDefaultWithoutGroups(flow_id) => ( - non_default_group_field(&data.assignments, flow_id), - "non_default_must_have_groups", - ), - MfaFlowAssignmentError::DuplicateFlow(flow_id) => { - (assignment_field(&data.assignments, flow_id), "duplicate") - } - MfaFlowAssignmentError::UnknownFlow(flow_id) => { - (assignment_field(&data.assignments, flow_id), "unknown_flow") - } - MfaFlowAssignmentError::UnknownGroup(group_id) => { - (group_field(&data.assignments, group_id), "unknown_group") - } - MfaFlowAssignmentError::Sqlx(e) => return Err(WebError::from(e)), - }; - - return Ok(validation_error_response(vec![MfaFlowValidationField { - field, - code: code.into(), - }])); + let assignments = MfaFlow::for_location(&appstate.pool, id).await?; + let mut response = Vec::with_capacity(assignments.len()); + for assignment in assignments { + let steps = MfaFlowStep::find_by_flow(&appstate.pool, assignment.id).await?; + let groups = assignment + .group_ids + .into_iter() + .zip(assignment.group_names) + .map(|(id, name)| LocationMfaFlowGroupResponse { id, name }) + .collect(); + response.push(LocationMfaFlowResponse { + id: assignment.id, + title: assignment.title, + steps: steps.into_iter().map(Into::into).collect(), + is_default: assignment.is_default, + groups, + }); } - tx.commit().await?; - - let items = MfaFlow::for_location(&appstate.pool, location_id).await?; - let response: Vec = items.into_iter().map(Into::into).collect(); - - appstate.emit_event(ApiEvent { - context, - event: Box::new(ApiEventType::LocationMfaFlowsAssigned { - location_id, - location_name: location.name, - assignments: LocationMfaFlowAssignment::snapshot(&assignments), - }), - })?; Ok(ApiResponse::json(response, StatusCode::OK)) } diff --git a/crates/defguard_core/src/handlers/wireguard.rs b/crates/defguard_core/src/handlers/wireguard.rs index 9d2d1c180..17fb0d6a1 100644 --- a/crates/defguard_core/src/handlers/wireguard.rs +++ b/crates/defguard_core/src/handlers/wireguard.rs @@ -11,7 +11,7 @@ use defguard_common::{ models::{ Device, DeviceConfig, DeviceType, User, WireguardNetwork, device::{AddDevice, DeviceInfo, ModifyDevice, WireguardNetworkDevice}, - mfa_flow::MfaFlow, + mfa_flow::{LocationMfaFlowAssignment, MfaFlow, MfaFlowAssignmentLicenseError}, wireguard::{MappedDevice, ServiceLocationMode}, }, }, @@ -36,13 +36,17 @@ use crate::{ }, firewall::try_get_location_firewall_config, handlers::CanManageDevices, - has_enterprise_access, + has_enterprise_access, is_business_license_active, license::{LicenseFeature, get_cached_license}, limits::{get_counts, update_counts}, }, events::{ApiEvent, ApiEventType, ApiRequestContext}, grpc::GatewayCommand, - handlers::{gateway::GatewayInfo, network_devices::DeviceWireGuardConfig}, + handlers::{ + gateway::GatewayInfo, + mfa_flow::{assignment_error_response, license_error_response}, + network_devices::DeviceWireGuardConfig, + }, location_management::{ allowed_peers::get_location_allowed_peers, handle_imported_devices, handle_mapped_devices, sync_location_allowed_devices, @@ -86,7 +90,8 @@ pub struct WireguardNetworkData { pub allowed_ips_from_acl: bool, pub mfa_enabled: bool, pub service_location_mode: ServiceLocationMode, - pub posture_checks: Option>, + pub posture_checks: Vec, + pub mfa_flows: Vec, } const MIN_PEER_DISCONNECT_THRESHOLD_WITH_MFA: i32 = 120; @@ -120,16 +125,11 @@ pub fn no_flows_assigned_response() -> ApiResponse { ) } -/// Rejects enabling MFA for a location while no MFA flow is assigned to it as its default. -/// -/// The check is per-location: an existing location (`Some(id)`) must carry a designated default -/// assignment, and a brand-new location (`None`, the create path) can never have one, so creating -/// with `mfa_enabled` is refused here too. The global `no_flows_exist` check runs first so a fresh -/// instance reports the more actionable "create a flow" error. Returns a structured `400` response -/// (not a `WebError`) so the body is parsed in one step. +/// Validate enabling MFA on an already-persisted location. /// -/// Shared by `create_network`, `modify_network` and the auto-adoption wizard, which sets -/// `mfa_enabled` on an already-persisted location, so the three entry points cannot drift. +/// Used by the auto-adoption wizard after it creates a location. The normal location create and +/// update handlers validate assignments inside their transactions. Returns a structured `400` +/// response (not a `WebError`) when no MFA flow or no default assignment exists. pub async fn validate_mfa_flows_exist<'e, E: sqlx::PgExecutor<'e> + Copy>( executor: E, mfa_enabled: bool, @@ -177,17 +177,6 @@ impl WireguardNetworkData { ))) } - /// Rejects enabling MFA for a location while no MFA flow is assigned to it as its default. - /// - /// Thin wrapper over [`validate_mfa_flows_exist`] for the create/modify request path. - pub(crate) async fn validate_mfa_flows_exist<'e, E: sqlx::PgExecutor<'e> + Copy>( - &self, - executor: E, - location_id: Option, - ) -> Result, WebError> { - validate_mfa_flows_exist(executor, self.mfa_enabled, location_id).await - } - /// Rejects service-location mode combined with location MFA: core cannot serve it and the /// client cannot represent it (`Location::is_service_location()` requires MFA disabled). pub(crate) fn validate_service_location_mfa(&self) -> Result<(), WebError> { @@ -243,6 +232,30 @@ pub struct ImportedNetworkData { pub devices: Vec, } +fn assignment_license_error_response( + error: MfaFlowAssignmentLicenseError, +) -> Result { + let code = match error { + MfaFlowAssignmentLicenseError::GroupAssignmentNotAllowed => "group_assignment_not_allowed", + MfaFlowAssignmentLicenseError::MultipleStepsNotAllowed => "multiple_steps_not_allowed", + MfaFlowAssignmentLicenseError::MultipleMfaFlowsNotAllowed => { + "multiple_mfa_flows_not_allowed" + } + MfaFlowAssignmentLicenseError::Database(error) => return Err(WebError::from(error)), + }; + Ok(license_error_response("mfa_flows".into(), code)) +} + +/// Prepare assignments for equality check. +fn normalize_mfa_flow_assignments( + mut assignments: Vec, +) -> Vec { + for assignment in &mut assignments { + assignment.group_ids.sort_unstable(); + } + assignments +} + /// Create a network #[utoipa::path( post, @@ -303,9 +316,6 @@ pub(crate) async fn create_network( data.validate_service_location_mfa()?; data.validate_keepalive_interval()?; data.validate_allowed_groups()?; - if let Some(resp) = data.validate_mfa_flows_exist(&appstate.pool, None).await? { - return Ok(resp); - } let allowed_ips = data.parse_allowed_ips(); let mut network = WireguardNetwork::new( @@ -337,24 +347,44 @@ pub(crate) async fn create_network( network.add_all_allowed_devices(&mut transaction).await?; info!("Assigning IPs for existing devices in network {network}"); - // assign posture checks - if let Some(ref posture_checks) = data.posture_checks { - debug!("Assigning posture checks {posture_checks:?} to {network}"); - if !has_enterprise_access(Some(LicenseFeature::DevicePosture)) && !posture_checks.is_empty() - { - error!( - "Cannot assign posture checks to new location {network}: Enterprise license required." - ); - return Ok(WebError::Forbidden( - "Cannot assign posture checks to new location: Enterprise license required.", - ) - .into()); - } - DevicePostureLocation::set_for_location(&mut transaction, network.id, posture_checks) - .await?; - info!("Assigned posture checks {posture_checks:?} to new location {network}"); + debug!( + "Assigning posture checks {:?} to {network}", + data.posture_checks + ); + if !has_enterprise_access(Some(LicenseFeature::DevicePosture)) + && !data.posture_checks.is_empty() + { + error!( + "Cannot assign posture checks to new location {network}: Enterprise license required." + ); + return Ok(WebError::Forbidden( + "Cannot assign posture checks to new location: Enterprise license required.", + ) + .into()); } + DevicePostureLocation::set_for_location(&mut transaction, network.id, &data.posture_checks) + .await?; + info!( + "Assigned posture checks {:?} to new location {network}", + data.posture_checks + ); + let mfa_assignments: Vec = data.mfa_flows.clone(); + if let Err(error) = MfaFlow::validate_mfa_flow_assignments_license( + &mut transaction, + &mfa_assignments, + has_enterprise_access(None), + is_business_license_active(), + ) + .await + { + return assignment_license_error_response(error); + } + if let Err(error) = + MfaFlow::assign_to_location(&mut transaction, network.id, &mfa_assignments).await + { + return assignment_error_response(&data.mfa_flows, error); + } transaction.commit().await?; appstate.send_gateway_command(GatewayCommand::NetworkCreated(network.id, network.clone())); @@ -364,6 +394,25 @@ pub(crate) async fn create_network( session.user.username ); + if !data.posture_checks.is_empty() { + appstate.emit_event(ApiEvent { + context: context.clone(), + event: Box::new(ApiEventType::LocationPosturesAssigned { + location: network.clone(), + posture_ids: data.posture_checks.clone(), + }), + })?; + } + if !mfa_assignments.is_empty() { + appstate.emit_event(ApiEvent { + context: context.clone(), + event: Box::new(ApiEventType::LocationMfaFlowsAssigned { + location_id: network.id, + location_name: network.name.clone(), + assignments: LocationMfaFlowAssignment::snapshot(&mfa_assignments), + }), + })?; + } appstate.emit_event(ApiEvent { context, event: Box::new(ApiEventType::VpnLocationAdded { @@ -416,7 +465,7 @@ pub(crate) async fn modify_network( session.user.username ); - // check if tries to modify service location without active enterprise + // check if tries to configure service location without active enterprise if data.service_location_mode != ServiceLocationMode::Disabled && !has_enterprise_access(Some(LicenseFeature::ServiceLocations)) { @@ -434,12 +483,6 @@ pub(crate) async fn modify_network( data.validate_service_location_mfa()?; data.validate_keepalive_interval()?; data.validate_allowed_groups()?; - if let Some(resp) = data - .validate_mfa_flows_exist(&appstate.pool, Some(network_id)) - .await? - { - return Ok(resp); - } let network = find_network(network_id, &appstate.pool).await?; // store network before mods @@ -470,6 +513,87 @@ pub(crate) async fn modify_network( .set_allowed_groups(&mut transaction, &data.allowed_groups) .await?; + // Don't error out on no license - otherwise users won't be able to update other location fields. + let postures_changed = if has_enterprise_access(Some(LicenseFeature::DevicePosture)) { + let mut current_postures = + DevicePostureLocation::find_by_location(&mut *transaction, network.id).await?; + let mut requested_postures = data.posture_checks.clone(); + + current_postures.sort_unstable(); + requested_postures.sort_unstable(); + + if current_postures != requested_postures { + DevicePostureLocation::set_for_location( + &mut transaction, + network.id, + &data.posture_checks, + ) + .await?; + } + current_postures != requested_postures + } else { + warn!( + location_id = network.id, + "Ignoring posture check assignments because the Enterprise license is inactive" + ); + false + }; + + // Only changed assignments can be written with a Business license; otherwise, preserve the + // existing assignments so a license downgrade does not block unrelated location updates. Flow + // order is significant, but group order is not, so normalize group IDs before comparing. + let current_mfa_assignments = normalize_mfa_flow_assignments( + MfaFlow::for_location(&mut *transaction, network.id) + .await? + .into_iter() + .map(|assignment| LocationMfaFlowAssignment { + flow_id: assignment.id, + is_default: assignment.is_default, + group_ids: assignment.group_ids, + }) + .collect(), + ); + let mfa_assignments = normalize_mfa_flow_assignments(data.mfa_flows.clone()); + let mfa_assignments_changed = current_mfa_assignments != mfa_assignments; + let mfa_assignments_updated = if mfa_assignments_changed { + match MfaFlow::validate_mfa_flow_assignments_license( + &mut transaction, + &mfa_assignments, + has_enterprise_access(None), + is_business_license_active(), + ) + .await + { + Ok(()) => true, + Err(MfaFlowAssignmentLicenseError::Database(error)) => return Err(error.into()), + Err(error) if is_business_license_active() => { + return assignment_license_error_response(error); + } + Err(error) => { + warn!( + location_id = network.id, + error = %error, + "Ignoring MFA flow assignments because of license limits" + ); + false + } + } + } else { + false + }; + + if mfa_assignments_updated { + if let Err(error) = + MfaFlow::assign_to_location(&mut transaction, network.id, &mfa_assignments).await + { + return assignment_error_response(&data.mfa_flows, error); + } + } else if let Some(response) = + validate_mfa_flows_exist(&appstate.pool, data.mfa_enabled, Some(network.id)).await? + { + return Ok(response); + } + let _events = sync_location_allowed_devices(&network, &mut transaction, None).await?; let peers = get_location_allowed_peers(&network, &mut transaction).await?; @@ -486,6 +610,25 @@ pub(crate) async fn modify_network( "User {} updated WireGuard network {network_id}", session.user.username, ); + if postures_changed { + appstate.emit_event(ApiEvent { + context: context.clone(), + event: Box::new(ApiEventType::LocationPosturesAssigned { + location: network.clone(), + posture_ids: data.posture_checks.clone(), + }), + })?; + } + if mfa_assignments_updated { + appstate.emit_event(ApiEvent { + context: context.clone(), + event: Box::new(ApiEventType::LocationMfaFlowsAssigned { + location_id: network.id, + location_name: network.name.clone(), + assignments: LocationMfaFlowAssignment::snapshot(&mfa_assignments), + }), + })?; + } appstate.emit_event(ApiEvent { context, event: Box::new(ApiEventType::VpnLocationModified { @@ -919,7 +1062,7 @@ pub(crate) struct AddDeviceResult { "pubkey": "pubkey", "dns": "8.8.8.8", "keepalive_interval": 5, - "mfa_enabled": false, + "mfa_enabled": false, "service_location_mode": "disabled" } ], diff --git a/crates/defguard_core/src/lib.rs b/crates/defguard_core/src/lib.rs index cf51c7ab9..c021a0c40 100644 --- a/crates/defguard_core/src/lib.rs +++ b/crates/defguard_core/src/lib.rs @@ -51,7 +51,7 @@ use handlers::{ group::{bulk_assign_to_groups, list_groups_info}, mfa_flow::{ create_mfa_flow, delete_mfa_flow, get_location_mfa_flows, get_method_availability, - get_mfa_flow, list_mfa_flows, set_location_mfa_flows, update_mfa_flow, + get_mfa_flow, list_mfa_flows, update_mfa_flow, }, network_devices::{ add_network_device, check_ip_availability, find_available_ips, get_network_device, @@ -122,7 +122,7 @@ use crate::{ device_posture::{ create_device_posture, delete_device_posture, duplicate_device_posture, get_device_posture, get_device_posture_versions, list_device_postures, - set_locations_for_posture, set_postures_for_location, update_device_posture, + set_locations_for_posture, update_device_posture, }, enterprise_settings::{get_enterprise_settings, patch_enterprise_settings}, openid_login::{auth_callback, get_auth_info}, @@ -584,10 +584,7 @@ pub fn build_webapp( "/mfa-flow/method-availability", get(get_method_availability), ) - .route( - "/location/{id}/mfa-flows", - get(get_location_mfa_flows).put(set_location_mfa_flows), - ), + .route("/location/{id}/mfa-flows", get(get_location_mfa_flows)), ); let api_router = api_router.nest( @@ -705,7 +702,6 @@ pub fn build_webapp( "/network/{location_id}/snat", get(list_snat_bindings).post(create_snat_binding), ) - .route("/network/{id}/postures", put(set_postures_for_location)) .route( "/network/{location_id}/snat/{user_id}", put(modify_snat_binding).delete(delete_snat_binding), diff --git a/crates/defguard_core/src/openapi.rs b/crates/defguard_core/src/openapi.rs index b0c76378f..bdf5d15c3 100644 --- a/crates/defguard_core/src/openapi.rs +++ b/crates/defguard_core/src/openapi.rs @@ -235,7 +235,6 @@ Errors are returned as a JSON object with a `msg` field and, for some of them, a device_posture::update_device_posture, device_posture::duplicate_device_posture, device_posture::set_locations_for_posture, - device_posture::set_postures_for_location, // SNAT snat::list_snat_bindings, snat::create_snat_binding, @@ -271,7 +270,6 @@ Errors are returned as a JSON object with a `msg` field and, for some of them, a mfa_flow::update_mfa_flow, mfa_flow::delete_mfa_flow, mfa_flow::get_location_mfa_flows, - mfa_flow::set_location_mfa_flows, mfa_flow::get_method_availability, // support mail::send_support_data, diff --git a/crates/defguard_core/tests/integration/api/common/mod.rs b/crates/defguard_core/tests/integration/api/common/mod.rs index 0651c1848..4a7931498 100644 --- a/crates/defguard_core/tests/integration/api/common/mod.rs +++ b/crates/defguard_core/tests/integration/api/common/mod.rs @@ -35,7 +35,7 @@ use defguard_core::{ handlers::{Auth, user::UserDetails}, }; use reqwest::{StatusCode, header::HeaderName}; -use serde_json::json; +use serde_json::{Value, json}; use sqlx::PgPool; use tokio::{ net::TcpListener, @@ -205,7 +205,88 @@ pub(crate) async fn exceed_enterprise_limits(client: &TestClient) { make_network(client, "network2").await; } -/// Create test network with a given name. +/// Save a complete MFA assignment list through the location update endpoint. +pub(crate) async fn update_location_mfa_flows( + client: &TestClient, + location_id: Id, + assignments: Value, +) -> TestResponse { + update_location_assignments(client, location_id, "mfa_flows", assignments).await +} + +/// Save a complete posture-check assignment list through the location update endpoint. +pub(crate) async fn update_location_posture_checks( + client: &TestClient, + location_id: Id, + posture_checks: Value, +) -> TestResponse { + update_location_assignments(client, location_id, "posture_checks", posture_checks).await +} + +async fn update_location_assignments( + client: &TestClient, + location_id: Id, + field: &str, + assignments: Value, +) -> TestResponse { + let response = client + .get(format!("/api/v1/network/{location_id}")) + .send() + .await; + if response.status() != StatusCode::OK { + return response; + } + let mut location: Value = response.json().await; + let data = location + .as_object_mut() + .expect("location must be an object"); + data.remove("id"); + data.remove("gateways"); + data.remove("has_devices"); + for array_field in ["address", "allowed_ips"] { + let value = data + .get_mut(array_field) + .and_then(Value::as_array_mut) + .expect("location field must be an array"); + let joined = value + .iter() + .map(|item| item.as_str().expect("location field item must be a string")) + .collect::>() + .join(","); + *data.get_mut(array_field).expect("field must exist") = Value::String(joined); + } + let response = client + .get(format!("/api/v1/location/{location_id}/mfa-flows")) + .send() + .await; + assert_eq!(response.status(), StatusCode::OK); + let flows: Vec = response.json().await; + let mfa_flows = flows + .into_iter() + .map(|flow| { + json!({ + "flow_id": flow["id"], + "is_default": flow["is_default"], + "group_ids": flow["groups"] + .as_array() + .expect("flow groups must be an array") + .iter() + .map(|group| group["id"].clone()) + .collect::>(), + }) + }) + .collect(); + data.insert("mfa_flows".into(), Value::Array(mfa_flows)); + data.insert(field.into(), assignments); + + client + .put(format!("/api/v1/network/{location_id}")) + .json(&location) + .send() + .await +} + +/// Create a test network with a given name. pub(crate) async fn make_network(client: &TestClient, name: &str) -> TestResponse { let response = client .post("/api/v1/network") @@ -226,7 +307,9 @@ pub(crate) async fn make_network(client: &TestClient, name: &str) -> TestRespons "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; diff --git a/crates/defguard_core/tests/integration/api/device_posture.rs b/crates/defguard_core/tests/integration/api/device_posture.rs index 7ddccc5f9..1d28db6e1 100644 --- a/crates/defguard_core/tests/integration/api/device_posture.rs +++ b/crates/defguard_core/tests/integration/api/device_posture.rs @@ -3,8 +3,8 @@ use defguard_core::{ enterprise::{ db::models::device_posture::{DevicePosture, DevicePostureSnapshot}, handlers::device_posture::{ - ApiDevicePosture, ApiOsRule, AssignLocationsData, AssignPosturesData, - DevicePostureVersionMetadata, EditDevicePosture, + ApiDevicePosture, ApiOsRule, AssignLocationsData, DevicePostureVersionMetadata, + EditDevicePosture, }, license::{get_cached_license, set_cached_license}, posture::version_list::{ @@ -16,6 +16,7 @@ use defguard_core::{ grpc::GatewayCommand, }; use reqwest::StatusCode; +use serde_json::json; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use super::{ @@ -34,7 +35,7 @@ fn make_edit(name: &str) -> EditDevicePosture { } } -use crate::api::common::{client::TestClient, make_network}; +use crate::api::common::{client::TestClient, make_network, update_location_posture_checks}; /// Set up a test client with enterprise license and admin session ready. /// All device posture tests that don't test license gating should use this. @@ -1021,25 +1022,23 @@ async fn test_device_posture_set_postures_for_location( client.drain_all_events(); // assign both postures to the location - let response = client - .put(format!("/api/v1/network/{location_id}/postures")) - .json(&AssignPosturesData { - postures: vec![p1.id, p2.id], - }) - .send() - .await; + let response = + update_location_posture_checks(&client, location_id, json!(vec![p1.id, p2.id])).await; assert_eq!(response.status(), StatusCode::OK); - let result: Vec = response.json().await; - assert_eq!(result.len(), 2); - assert!(result.contains(&p1.id)); - assert!(result.contains(&p2.id)); - let events = client.drain_all_events(); - assert_eq!(events.len(), 1); - assert!(matches!( - events[0].0, - ApiEventType::LocationPosturesAssigned { .. } - )); + assert_eq!(events.len(), 2); + assert!(events.iter().any(|event| matches!( + &event.0, + ApiEventType::LocationPosturesAssigned { + location, + posture_ids, + } if location.id == location_id && posture_ids == &[p1.id, p2.id] + ))); + assert!( + events + .iter() + .any(|event| matches!(event.0, ApiEventType::VpnLocationModified { .. })) + ); // GET on each posture shows the location for posture in [&p1, &p2] { @@ -1063,16 +1062,8 @@ async fn test_device_posture_set_postures_for_location( assert_eq!(posture_checks.len(), 2); // reassign with empty list — all postures removed - let response = client - .put(format!("/api/v1/network/{location_id}/postures")) - .json(&AssignPosturesData { - postures: Vec::new(), - }) - .send() - .await; + let response = update_location_posture_checks(&client, location_id, json!([])).await; assert_eq!(response.status(), StatusCode::OK); - let result: Vec = response.json().await; - assert!(result.is_empty()); client.drain_all_events(); // GET on postures now shows no locations @@ -1101,21 +1092,17 @@ async fn test_assigning_first_posture_refreshes_gateway_with_no_direct_peers( drain_gateway_events(&mut gateway_rx); client.drain_all_events(); - let response = client - .put(format!("/api/v1/network/{location_id}/postures")) - .json(&AssignPosturesData { - postures: vec![posture.id], - }) - .send() - .await; + let response = + update_location_posture_checks(&client, location_id, json!(vec![posture.id])).await; assert_eq!(response.status(), StatusCode::OK); expect_network_modified_peers(&mut gateway_rx, location_id, &[]); let events = client.drain_all_events(); - assert!(matches!( - events[0].0, - ApiEventType::LocationPosturesAssigned { .. } - )); + assert!( + events + .iter() + .any(|event| matches!(event.0, ApiEventType::VpnLocationModified { .. })) + ); } #[sqlx::test] @@ -1133,32 +1120,22 @@ async fn test_removing_last_posture_refreshes_gateway_with_direct_peers( drain_gateway_events(&mut gateway_rx); client.drain_all_events(); - let response = client - .put(format!("/api/v1/network/{location_id}/postures")) - .json(&AssignPosturesData { - postures: vec![posture.id], - }) - .send() - .await; + let response = + update_location_posture_checks(&client, location_id, json!(vec![posture.id])).await; assert_eq!(response.status(), StatusCode::OK); expect_network_modified_peers(&mut gateway_rx, location_id, &[]); client.drain_all_events(); - let response = client - .put(format!("/api/v1/network/{location_id}/postures")) - .json(&AssignPosturesData { - postures: Vec::new(), - }) - .send() - .await; + let response = update_location_posture_checks(&client, location_id, json!([])).await; assert_eq!(response.status(), StatusCode::OK); expect_network_modified_peers(&mut gateway_rx, location_id, &[device_pubkey]); let events = client.drain_all_events(); - assert!(matches!( - events[0].0, - ApiEventType::LocationPosturesAssigned { .. } - )); + assert!( + events + .iter() + .any(|event| matches!(event.0, ApiEventType::VpnLocationModified { .. })) + ); } #[sqlx::test] @@ -1222,13 +1199,8 @@ async fn test_deleting_assigned_posture_refreshes_gateway_with_direct_peers( drain_gateway_events(&mut gateway_rx); client.drain_all_events(); - let response = client - .put(format!("/api/v1/network/{location_id}/postures")) - .json(&AssignPosturesData { - postures: vec![posture.id], - }) - .send() - .await; + let response = + update_location_posture_checks(&client, location_id, json!(vec![posture.id])).await; assert_eq!(response.status(), StatusCode::OK); expect_network_modified_peers(&mut gateway_rx, location_id, &[]); client.drain_all_events(); @@ -1261,13 +1233,7 @@ async fn test_device_posture_assignment_not_found(_: PgPoolOptions, options: PgC assert_eq!(response.status(), StatusCode::NOT_FOUND); client.assert_event_queue_is_empty(); - let response = client - .put("/api/v1/network/999/postures") - .json(&AssignPosturesData { - postures: Vec::new(), - }) - .send() - .await; + let response = update_location_posture_checks(&client, 999, json!([])).await; assert_eq!(response.status(), StatusCode::NOT_FOUND); client.assert_event_queue_is_empty(); } @@ -1293,7 +1259,9 @@ async fn make_service_location(client: &TestClient, name: &str) -> i64 { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "prelogon" + "service_location_mode": "prelogon", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -1320,23 +1288,23 @@ async fn test_set_postures_for_service_location_allowed( client.drain_all_events(); // assigning posture checks to a service location is allowed - let response = client - .put(format!("/api/v1/network/{service_location_id}/postures")) - .json(&AssignPosturesData { - postures: vec![posture.id], - }) - .send() - .await; + let response = + update_location_posture_checks(&client, service_location_id, json!(vec![posture.id])).await; assert_eq!(response.status(), StatusCode::OK); - let result: Vec = response.json().await; - assert_eq!(result, vec![posture.id]); - let events = client.drain_all_events(); - assert_eq!(events.len(), 1); - assert!(matches!( - events[0].0, - ApiEventType::LocationPosturesAssigned { .. } - )); + assert_eq!(events.len(), 2); + assert!(events.iter().any(|event| matches!( + &event.0, + ApiEventType::LocationPosturesAssigned { + location, + posture_ids, + } if location.id == service_location_id && posture_ids == &[posture.id] + ))); + assert!( + events + .iter() + .any(|event| matches!(event.0, ApiEventType::VpnLocationModified { .. })) + ); // the assignment is visible on the posture let response = client @@ -1347,13 +1315,7 @@ async fn test_set_postures_for_service_location_allowed( assert_eq!(fetched.locations, vec![service_location_id]); // clearing (empty list) is still allowed on a service location - let response = client - .put(format!("/api/v1/network/{service_location_id}/postures")) - .json(&AssignPosturesData { - postures: Vec::new(), - }) - .send() - .await; + let response = update_location_posture_checks(&client, service_location_id, json!([])).await; assert_eq!(response.status(), StatusCode::OK); client.drain_all_events(); diff --git a/crates/defguard_core/tests/integration/api/enterprise_settings.rs b/crates/defguard_core/tests/integration/api/enterprise_settings.rs index e4eb978b2..9ae3d341d 100644 --- a/crates/defguard_core/tests/integration/api/enterprise_settings.rs +++ b/crates/defguard_core/tests/integration/api/enterprise_settings.rs @@ -100,7 +100,9 @@ async fn test_admin_devices_management_is_enforced(_: PgPoolOptions, options: Pg "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -217,7 +219,9 @@ async fn test_regular_user_device_management(_: PgPoolOptions, options: PgConnec "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -326,7 +330,9 @@ async fn dg25_12_test_enforce_client_activation_only(_: PgPoolOptions, options: "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -453,7 +459,9 @@ async fn dg25_13_test_disable_device_config(_: PgPoolOptions, options: PgConnect "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; diff --git a/crates/defguard_core/tests/integration/api/mfa_flow.rs b/crates/defguard_core/tests/integration/api/mfa_flow.rs index 7084f8baa..e2f495d9a 100644 --- a/crates/defguard_core/tests/integration/api/mfa_flow.rs +++ b/crates/defguard_core/tests/integration/api/mfa_flow.rs @@ -16,6 +16,7 @@ use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use super::common::{ authenticate_admin, configure_smtp, make_network, make_test_client, set_enterprise_license, + update_location_mfa_flows, }; /// Single-step flow without OIDC - should succeed without any license. @@ -280,48 +281,34 @@ async fn test_mfa_flow_group_scoping_requires_enterprise( .unwrap(); // Default assignment (empty group_ids) + scoped assignment (non-empty) - let assignment_body = json!({ - "assignments": [ - { - "flow_id": flow1_id, - "is_default": true, - "group_ids": [] - }, - { - "flow_id": flow2_id, - "is_default": false, - "group_ids": [admin_group_id] - } - ] - }); - - // Drain the create/location events so the refusal assertion below is exact. - let _ = client.drain_all_events(); + let assignment_body = json!([ + { + "flow_id": flow1_id, + "is_default": true, + "group_ids": [] + }, + { + "flow_id": flow2_id, + "is_default": false, + "group_ids": [admin_group_id] + } + ]); - // Business license → 403 (group scoping needs enterprise), and no audit event on refusal. - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&assignment_body) - .send() - .await; - assert_eq!(response.status(), StatusCode::FORBIDDEN); - assert!( - client.drain_all_events().is_empty(), - "refused request must not emit an audit event" - ); - - // Enterprise license → 200 set_enterprise_license(); - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&assignment_body) - .send() - .await; + let _ = client.drain_all_events(); + let response = update_location_mfa_flows(&client, location_id, assignment_body).await; assert_eq!(response.status(), StatusCode::OK); let events = client.drain_all_events(); - assert_eq!(events.len(), 1, "expected exactly 1 event after assign"); - let (event_type, _user_id, _username) = &events[0]; + assert_eq!( + events.len(), + 2, + "location save must emit assignment and modification events" + ); + let (event_type, _user_id, _username) = events + .iter() + .find(|event| matches!(event.0, ApiEventType::LocationMfaFlowsAssigned { .. })) + .expect("missing MFA assignment event"); assert_matches!( event_type, ApiEventType::LocationMfaFlowsAssigned { @@ -510,22 +497,24 @@ async fn test_location_mfa_flows_input_validation(_: PgPoolOptions, options: PgC let response = client.get("/api/v1/location/999999/mfa-flows").send().await; assert_eq!(response.status(), StatusCode::NOT_FOUND); - let response = client - .put("/api/v1/location/999999/mfa-flows") - .json(&json!({"assignments": [{"flow_id": flow_id, "is_default": true, "group_ids": []}]})) - .send() - .await; + let response = update_location_mfa_flows( + &client, + 999999, + json!([{"flow_id": flow_id, "is_default": true, "group_ids": []}]), + ) + .await; assert_eq!(response.status(), StatusCode::NOT_FOUND); // The same flow twice would violate the (location_id, flow_id) primary key. - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let response = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow_id, "is_default": true, "group_ids": []}, {"flow_id": flow_id, "is_default": false, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_eq!( response.json::().await["fields"][0]["code"], @@ -533,13 +522,14 @@ async fn test_location_mfa_flows_input_validation(_: PgPoolOptions, options: PgC ); // A nonexistent flow would violate the foreign key. - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let response = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": 999999, "is_default": true, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_eq!( response.json::().await["fields"][0]["code"], @@ -595,14 +585,15 @@ async fn test_location_mfa_flows_non_default_without_groups( // Clear the two create events before exercising the refusal path. let _ = client.drain_all_events(); - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let response = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow1_id, "is_default": false, "group_ids": []}, {"flow_id": flow2_id, "is_default": true, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); let body: serde_json::Value = response.json().await; assert_eq!(body["error"], "validation_failed"); @@ -642,18 +633,26 @@ async fn test_location_mfa_flows_clear_disabled_location( let _ = client.drain_all_events(); // Assign a default, then clear it. - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let response = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow_id, "is_default": true, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(response.status(), StatusCode::OK); let events = client.drain_all_events(); - assert_eq!(events.len(), 1, "expected exactly 1 event after assign"); - let (event_type, _user_id, _username) = &events[0]; + assert_eq!( + events.len(), + 2, + "location save must emit assignment and modification events" + ); + let (event_type, _user_id, _username) = events + .iter() + .find(|event| matches!(event.0, ApiEventType::LocationMfaFlowsAssigned { .. })) + .expect("missing MFA assignment event"); assert_matches!( event_type, ApiEventType::LocationMfaFlowsAssigned { @@ -668,16 +667,19 @@ async fn test_location_mfa_flows_clear_disabled_location( && assignments[0].group_ids.is_empty() ); - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": []})) - .send() - .await; + let response = update_location_mfa_flows(&client, location_id, json!([])).await; assert_eq!(response.status(), StatusCode::OK); let events = client.drain_all_events(); - assert_eq!(events.len(), 1, "expected exactly 1 event after clear"); - let (event_type, _user_id, _username) = &events[0]; + assert_eq!( + events.len(), + 2, + "location save must emit assignment and modification events" + ); + let (event_type, _user_id, _username) = events + .iter() + .find(|event| matches!(event.0, ApiEventType::LocationMfaFlowsAssigned { .. })) + .expect("missing MFA assignment event"); assert_matches!( event_type, ApiEventType::LocationMfaFlowsAssigned { @@ -859,7 +861,7 @@ async fn test_mfa_flow_update_preserves_backfilled_email( } /// The full `WireguardNetworkData` body used to toggle `mfa_enabled` on an existing location. -fn network_body(name: &str, mfa_enabled: bool) -> serde_json::Value { +fn network_body(name: &str, mfa_enabled: bool, flow_id: i64) -> serde_json::Value { json!({ "name": name, "address": "10.1.1.1/24", @@ -877,7 +879,9 @@ fn network_body(name: &str, mfa_enabled: bool) -> serde_json::Value { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": mfa_enabled, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [{"flow_id": flow_id, "is_default": true, "group_ids": []}] }) } @@ -910,26 +914,30 @@ async fn test_mfa_enabled_disable_preserves_assignments( .unwrap(); // Assign the flow as the location's default. - let resp = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let resp = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow_id, "is_default": true, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(resp.status(), StatusCode::OK); // Enable MFA, then disable it: the assignment list must survive untouched. let resp = client .put(format!("/api/v1/network/{location_id}")) - .json(&network_body("mfa-lifecycle", true)) + .json(&network_body("mfa-lifecycle", true, flow_id)) .send() .await; assert_eq!(resp.status(), StatusCode::OK); + let license = get_cached_license().clone(); + set_cached_license(None); + let body = network_body("mfa-lifecycle", false, flow_id); let resp = client .put(format!("/api/v1/network/{location_id}")) - .json(&network_body("mfa-lifecycle", false)) + .json(&body) .send() .await; assert_eq!(resp.status(), StatusCode::OK); @@ -947,13 +955,13 @@ async fn test_mfa_enabled_disable_preserves_assignments( "disabling MFA must preserve the assignment list" ); assert_eq!(assignments[0]["id"].as_i64(), Some(flow_id)); - assert_eq!(assignments[0]["position"].as_i64(), Some(0)); assert_eq!(assignments[0]["is_default"].as_bool(), Some(true)); // Re-enable: the same policy must be in force, resolving the same flow for a user. + set_cached_license(license); let resp = client .put(format!("/api/v1/network/{location_id}")) - .json(&network_body("mfa-lifecycle", true)) + .json(&network_body("mfa-lifecycle", true, flow_id)) .send() .await; assert_eq!(resp.status(), StatusCode::OK); @@ -998,19 +1006,20 @@ async fn test_mfa_flow_delete_location_requires_flow(_: PgPoolOptions, options: .as_i64() .unwrap(); - let resp = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let resp = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow_id, "is_default": true, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(resp.status(), StatusCode::OK); // Enable MFA so the location requires this flow. let resp = client .put(format!("/api/v1/network/{location_id}")) - .json(&network_body("delete-orphan", true)) + .json(&network_body("delete-orphan", true, flow_id)) .send() .await; assert_eq!(resp.status(), StatusCode::OK); @@ -1082,14 +1091,15 @@ async fn test_mfa_flow_delete_flow_is_default(_: PgPoolOptions, options: PgConne // flow1 is the default, flow2 is group-scoped; group scoping needs Enterprise. set_enterprise_license(); - let resp = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let resp = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow1_id, "is_default": true, "group_ids": []}, {"flow_id": flow2_id, "is_default": false, "group_ids": [admin_group_id]}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(resp.status(), StatusCode::OK); let _ = client.drain_all_events(); @@ -1249,13 +1259,14 @@ async fn test_location_mfa_flows_no_default_designated( let _ = client.drain_all_events(); - let resp = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let resp = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow_id, "is_default": false, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(resp.status(), StatusCode::BAD_REQUEST); let body: serde_json::Value = resp.json().await; assert_eq!(body["error"], "validation_failed"); diff --git a/crates/defguard_core/tests/integration/api/openid_login.rs b/crates/defguard_core/tests/integration/api/openid_login.rs index 058c695e7..5594aabb5 100644 --- a/crates/defguard_core/tests/integration/api/openid_login.rs +++ b/crates/defguard_core/tests/integration/api/openid_login.rs @@ -18,7 +18,9 @@ use serde::Deserialize; use serde_json::json; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; -use super::common::{exceed_enterprise_limits, make_client, make_network, setup_pool}; +use super::common::{ + exceed_enterprise_limits, make_client, make_network, setup_pool, update_location_mfa_flows, +}; use crate::api::PaginatedApiResponse; #[derive(Deserialize)] @@ -445,13 +447,12 @@ async fn test_delete_openid_provider_reports_affected_locations( .unwrap(); // Assign the OIDC flow as the location's default, so the location genuinely depends on it. - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({ - "assignments": [{ "flow_id": flow_id, "is_default": true, "group_ids": [] }] - })) - .send() - .await; + let response = update_location_mfa_flows( + &client, + location_id, + json!([{ "flow_id": flow_id, "is_default": true, "group_ids": [] }]), + ) + .await; assert_eq!(response.status(), StatusCode::OK); // Deletion proceeds and names the location whose flows are now unsatisfiable. diff --git a/crates/defguard_core/tests/integration/api/wireguard.rs b/crates/defguard_core/tests/integration/api/wireguard.rs index 61d1ae59a..c1b3077e3 100644 --- a/crates/defguard_core/tests/integration/api/wireguard.rs +++ b/crates/defguard_core/tests/integration/api/wireguard.rs @@ -25,17 +25,70 @@ use defguard_core::{ use ipnetwork::IpNetwork; use matches::assert_matches; use reqwest::StatusCode; -use serde_json::json; +use serde_json::{Value, json}; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use super::common::{ - authenticate_admin, client::TestClient, fetch_user_details, make_network, make_test_client, - setup_pool, + authenticate_admin, + client::{TestClient, TestResponse}, + fetch_user_details, make_network, make_test_client, setup_pool, update_location_mfa_flows, + update_location_posture_checks, }; const INVALID_MFA_PEER_DISCONNECT_THRESHOLD: i32 = 119; const MINIMUM_MFA_PEER_DISCONNECT_THRESHOLD: i32 = 120; +async fn create_mfa_flow(client: &TestClient, title: &str, steps: Value) -> Id { + let response = client + .post("/api/v1/mfa-flow") + .json(&json!({"title": title, "steps": steps})) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); + response.json::().await["id"].as_i64().unwrap() +} + +async fn create_network_with_mfa_flows( + client: &TestClient, + name: &str, + address: &str, + mfa_flows: Value, +) -> TestResponse { + client + .post("/api/v1/network") + .json(&json!({ + "name": name, + "address": address, + "port": 55555, + "endpoint": "192.168.4.14", + "allowed_ips": address, + "dns": "1.1.1.1", + "mtu": 1420, + "fwmark": 0, + "allowed_groups": ["admin"], + "allow_all_groups": false, + "keepalive_interval": 25, + "peer_disconnect_threshold": 300, + "acl_enabled": false, + "acl_default_allow": false, + "allowed_ips_from_acl": false, + "mfa_enabled": false, + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": mfa_flows, + })) + .send() + .await +} + +async fn assert_assignment_license_error(response: TestResponse, code: &str) { + assert_eq!(response.status(), StatusCode::FORBIDDEN); + let body: Value = response.json().await; + assert_eq!(body["error"], "license_required"); + assert_eq!(body["fields"][0]["field"], "mfa_flows"); + assert_eq!(body["fields"][0]["code"], code); +} + #[sqlx::test] async fn test_network(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; @@ -84,7 +137,8 @@ async fn test_network(_: PgPoolOptions, options: PgConnectOptions) { allowed_ips_from_acl: false, mfa_enabled: false, service_location_mode: ServiceLocationMode::Disabled, - posture_checks: None, + posture_checks: Vec::new(), + mfa_flows: Vec::new(), }; let response = client .put(format!("/api/v1/network/{}", network.id)) @@ -188,7 +242,9 @@ async fn test_create_network_blocked_when_location_count_exceeds_license_limit( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -197,6 +253,165 @@ async fn test_create_network_blocked_when_location_count_exceeds_license_limit( set_cached_license(license); } +#[sqlx::test] +async fn test_create_network_mfa_assignment_license_gates( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let (mut client, _) = make_test_client(pool.clone()).await; + authenticate_admin(&mut client).await; + let business_license = get_cached_license().clone(); + + let single_step_flow = + create_mfa_flow(&client, "Single-step flow", json!([{"methods": ["totp"]}])).await; + let second_single_step_flow = create_mfa_flow( + &client, + "Second single-step flow", + json!([{"methods": ["biometric"]}]), + ) + .await; + let multi_step_flow = create_mfa_flow( + &client, + "Multi-step flow", + json!([ + {"methods": ["totp"]}, + {"methods": ["biometric"]} + ]), + ) + .await; + let admin_group_id = Group::find_by_name(&pool, "admin") + .await + .unwrap() + .unwrap() + .id; + + set_cached_license(None); + + let response = create_network_with_mfa_flows( + &client, + "free-single-step", + "10.10.1.1/24", + json!([{ + "flow_id": single_step_flow, + "is_default": true, + "group_ids": [] + }]), + ) + .await; + assert_eq!(response.status(), StatusCode::CREATED); + + let response = create_network_with_mfa_flows( + &client, + "free-multi-step", + "10.10.2.1/24", + json!([{ + "flow_id": multi_step_flow, + "is_default": true, + "group_ids": [] + }]), + ) + .await; + assert_assignment_license_error(response, "multiple_steps_not_allowed").await; + + let response = create_network_with_mfa_flows( + &client, + "free-multiple-flows", + "10.10.3.1/24", + json!([ + { + "flow_id": single_step_flow, + "is_default": true, + "group_ids": [] + }, + { + "flow_id": second_single_step_flow, + "is_default": false, + "group_ids": [] + } + ]), + ) + .await; + assert_assignment_license_error(response, "multiple_mfa_flows_not_allowed").await; + + set_cached_license(business_license.clone()); + + let response = create_network_with_mfa_flows( + &client, + "business-multi-step", + "10.10.4.1/24", + json!([{ + "flow_id": multi_step_flow, + "is_default": true, + "group_ids": [] + }]), + ) + .await; + assert_eq!(response.status(), StatusCode::CREATED); + + let scoped_assignments = json!([ + { + "flow_id": single_step_flow, + "is_default": true, + "group_ids": [] + }, + { + "flow_id": second_single_step_flow, + "is_default": false, + "group_ids": [admin_group_id] + } + ]); + let response = create_network_with_mfa_flows( + &client, + "business-group-scoping", + "10.10.5.1/24", + scoped_assignments.clone(), + ) + .await; + assert_assignment_license_error(response, "group_assignment_not_allowed").await; + assert!( + WireguardNetwork::find_by_name(&pool, "business-group-scoping") + .await + .unwrap() + .is_none() + ); + + set_enterprise_license(); + + let response = create_network_with_mfa_flows( + &client, + "enterprise-group-scoping", + "10.10.6.1/24", + scoped_assignments.clone(), + ) + .await; + assert_eq!(response.status(), StatusCode::CREATED); + let location_id = response.json::().await["id"].as_i64().unwrap(); + + set_cached_license(business_license.clone()); + + let response = update_location_mfa_flows( + &client, + location_id, + json!([ + { + "flow_id": second_single_step_flow, + "is_default": true, + "group_ids": [] + }, + { + "flow_id": single_step_flow, + "is_default": false, + "group_ids": [admin_group_id] + } + ]), + ) + .await; + assert_assignment_license_error(response, "group_assignment_not_allowed").await; + + set_cached_license(business_license); +} + #[sqlx::test] async fn test_create_network_with_posture_checks_assigns_postures( _: PgPoolOptions, @@ -262,7 +477,8 @@ async fn test_create_network_with_posture_checks_assigns_postures( "allowed_ips_from_acl": false, "mfa_enabled": false, "service_location_mode": "disabled", - "posture_checks": posture_ids + "posture_checks": posture_ids, + "mfa_flows": [] })) .send() .await; @@ -324,7 +540,8 @@ async fn test_create_network_with_posture_checks_requires_enterprise_license( "allowed_ips_from_acl": false, "mfa_enabled": false, "service_location_mode": "disabled", - "posture_checks": [1] + "posture_checks": [1], + "mfa_flows": [] })) .send() .await; @@ -344,7 +561,6 @@ async fn test_create_network_with_posture_checks_requires_enterprise_license( } /// Build a location payload with overridable name, address and mode fields. -/// `posture_checks` is intentionally absent — add it explicitly where it matters. fn location_payload( name: &str, address: &str, @@ -368,7 +584,9 @@ fn location_payload( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": mfa_enabled, - "service_location_mode": service_location_mode + "service_location_mode": service_location_mode, + "posture_checks": [], + "mfa_flows": [] }) } @@ -395,11 +613,12 @@ async fn make_mfa_flow(client: &TestClient) -> i64 { /// Assign a flow as a location's default so the location can be MFA-enabled. async fn assign_default_mfa_flow(client: &TestClient, location_id: i64, flow_id: i64) { - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({ "assignments": [{ "flow_id": flow_id, "is_default": true, "group_ids": [] }] })) - .send() - .await; + let response = update_location_mfa_flows( + client, + location_id, + json!([{ "flow_id": flow_id, "is_default": true, "group_ids": [] }]), + ) + .await; assert_eq!(response.status(), StatusCode::OK); } @@ -430,7 +649,9 @@ async fn test_mfa_enabled_no_flows_structured_body(_: PgPoolOptions, options: Pg "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": true, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -438,8 +659,8 @@ async fn test_mfa_enabled_no_flows_structured_body(_: PgPoolOptions, options: Pg let body: serde_json::Value = response.json().await; assert_eq!(body["error"], "validation_failed"); - assert_eq!(body["fields"][0]["field"], "mfa_enabled"); - assert_eq!(body["fields"][0]["code"], "no_flows_exist"); + assert_eq!(body["fields"][0]["field"], "mfa_flows"); + assert_eq!(body["fields"][0]["code"], "no_default_designated"); assert!( body.get("msg").is_none(), "the refusal body must not be double-encoded via msg" @@ -476,21 +697,18 @@ async fn test_enable_mfa_after_clear_refused_without_flows( .as_i64() .unwrap(); - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": [ + let response = update_location_mfa_flows( + &client, + location_id, + json!([ {"flow_id": flow_id, "is_default": true, "group_ids": []}, - ]})) - .send() - .await; + ]), + ) + .await; assert_eq!(response.status(), StatusCode::OK); // Clearing is allowed on the MFA-disabled location. - let response = client - .put(format!("/api/v1/location/{location_id}/mfa-flows")) - .json(&json!({"assignments": []})) - .send() - .await; + let response = update_location_mfa_flows(&client, location_id, json!([])).await; assert_eq!(response.status(), StatusCode::OK); // Delete the now-unassigned flow so no flows exist globally. @@ -520,7 +738,9 @@ async fn test_enable_mfa_after_clear_refused_without_flows( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": true, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -562,7 +782,9 @@ async fn test_enable_mfa_without_assignment_refused(_: PgPoolOptions, options: P "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": true, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -854,16 +1076,14 @@ async fn test_modify_network_rejects_service_location_with_mfa( } #[sqlx::test] -async fn test_modify_network_without_posture_checks_keeps_assignments( - _: PgPoolOptions, - options: PgConnectOptions, -) { +async fn test_modify_network_replaces_posture_checks(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; let (mut client, _client_state) = make_test_client(pool).await; authenticate_admin(&mut client).await; set_enterprise_license(); let posture = make_posture_check(&client, "Posture").await; + let replacement_posture = make_posture_check(&client, "Replacement posture").await; let mut payload = location_payload("location", "10.1.1.1/24", false, "disabled"); payload["posture_checks"] = json!([posture]); @@ -875,11 +1095,27 @@ async fn test_modify_network_without_posture_checks_keeps_assignments( vec![posture] ); - // a payload with the field omitted must leave the assignment alone + // an explicit list replaces the current assignments with the location save + let mut payload = location_payload("renamed-location", "10.1.1.1/24", false, "disabled"); + payload["posture_checks"] = json!([replacement_posture]); + let response = client + .put(format!("/api/v1/network/{}", location.id)) + .json(&payload) + .send() + .await; + assert_eq!(response.status(), StatusCode::OK); + let modified: WireguardNetwork = response.json().await; + assert_eq!(modified.name, "renamed-location"); + assert_eq!( + fetch_location_postures(&client, location.id).await, + vec![replacement_posture] + ); + + // an empty list clears assignments let response = client .put(format!("/api/v1/network/{}", location.id)) .json(&location_payload( - "renamed-location", + "location", "10.1.1.1/24", false, "disabled", @@ -887,26 +1123,50 @@ async fn test_modify_network_without_posture_checks_keeps_assignments( .send() .await; assert_eq!(response.status(), StatusCode::OK); - let modified: WireguardNetwork = response.json().await; - assert_eq!(modified.name, "renamed-location"); - assert_eq!( - fetch_location_postures(&client, location.id).await, - vec![posture] + assert!( + fetch_location_postures(&client, location.id) + .await + .is_empty() ); +} + +#[sqlx::test] +async fn test_modify_network_preserves_posture_checks_without_enterprise_license( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let (mut client, _client_state) = make_test_client(pool).await; + authenticate_admin(&mut client).await; + set_enterprise_license(); - // an explicit `null` behaves the same way + let posture = make_posture_check(&client, "Posture").await; let mut payload = location_payload("location", "10.1.1.1/24", false, "disabled"); - payload["posture_checks"] = json!(null); + payload["posture_checks"] = json!([posture]); + let response = client.post("/api/v1/network").json(&payload).send().await; + assert_eq!(response.status(), StatusCode::CREATED); + let location: WireguardNetwork = response.json().await; + + let license = get_cached_license().clone(); + set_cached_license(None); let response = client .put(format!("/api/v1/network/{}", location.id)) - .json(&payload) + .json(&location_payload( + "renamed-location", + "10.1.1.1/24", + false, + "disabled", + )) .send() .await; assert_eq!(response.status(), StatusCode::OK); + let modified: WireguardNetwork = response.json().await; + assert_eq!(modified.name, "renamed-location"); assert_eq!( fetch_location_postures(&client, location.id).await, vec![posture] ); + set_cached_license(license); } #[sqlx::test] @@ -944,14 +1204,11 @@ async fn test_posture_checks_allowed_on_service_locations( assert_eq!(response.status(), StatusCode::CREATED); let location: WireguardNetwork = response.json().await; + let mut payload = location_payload("regular-location", "10.2.2.1/24", false, "alwayson"); + payload["posture_checks"] = json!([posture]); let response = client .put(format!("/api/v1/network/{}", location.id)) - .json(&location_payload( - "regular-location", - "10.2.2.1/24", - false, - "alwayson", - )) + .json(&payload) .send() .await; assert_eq!(response.status(), StatusCode::OK); @@ -965,7 +1222,7 @@ async fn test_posture_checks_allowed_on_service_locations( vec![posture] ); - // dedicated assignment path: posture checks can be assigned to an existing service location + // posture checks can be assigned to an existing service location through the location save let response = client .post("/api/v1/network") .json(&location_payload( @@ -984,14 +1241,12 @@ async fn test_posture_checks_allowed_on_service_locations( .is_empty() ); - let response = client - .put(format!( - "/api/v1/network/{}/postures", - service_location_without_postures.id - )) - .json(&json!({ "postures": [posture] })) - .send() - .await; + let response = update_location_posture_checks( + &client, + service_location_without_postures.id, + json!([posture]), + ) + .await; assert_eq!(response.status(), StatusCode::OK); assert_eq!( fetch_location_postures(&client, service_location_without_postures.id).await, @@ -1028,7 +1283,8 @@ async fn test_peer_disconnect_threshold_validation_create( allowed_ips_from_acl: false, mfa_enabled: false, service_location_mode: ServiceLocationMode::Disabled, - posture_checks: None, + posture_checks: Vec::new(), + mfa_flows: Vec::new(), }; let response = client @@ -1058,7 +1314,7 @@ async fn test_peer_disconnect_threshold_validation_create( .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); let body: serde_json::Value = response.json().await; - assert_eq!(body["fields"][0]["code"], "no_flows_assigned"); + assert_eq!(body["fields"][0]["code"], "no_default_designated"); } #[sqlx::test] @@ -1090,7 +1346,8 @@ async fn test_peer_disconnect_threshold_validation_modify( allowed_ips_from_acl: false, mfa_enabled: false, service_location_mode: ServiceLocationMode::Disabled, - posture_checks: None, + posture_checks: Vec::new(), + mfa_flows: Vec::new(), }; let response = client @@ -1103,6 +1360,13 @@ async fn test_peer_disconnect_threshold_validation_modify( // Give the location a default flow so the threshold checks below operate on a // MFA-enableable location. assign_default_mfa_flow(&client, 1, flow_id).await; + location_data.mfa_flows = vec![ + defguard_common::db::models::mfa_flow::LocationMfaFlowAssignment { + flow_id, + is_default: true, + group_ids: Vec::new(), + }, + ]; let response = client .put("/api/v1/network/1") @@ -1367,7 +1631,9 @@ async fn test_network_address_reassignment(_: PgPoolOptions, options: PgConnectO "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] }); let response = client .put(format!("/api/v1/network/{network_id}")) @@ -1696,7 +1962,9 @@ async fn test_network_size_validation(_: PgPoolOptions, options: PgConnectOption "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] }); let response = client .put(format!("/api/v1/network/{}", network_from_details.id)) @@ -1724,7 +1992,9 @@ async fn test_network_size_validation(_: PgPoolOptions, options: PgConnectOption "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] }); let response = client .put(format!("/api/v1/network/{}", network_from_details.id)) @@ -1855,7 +2125,9 @@ async fn test_user_device_configs_auth(_: PgPoolOptions, options: PgConnectOptio "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -1945,7 +2217,9 @@ async fn test_add_device_for_disabled_user(_: PgPoolOptions, options: PgConnectO "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2011,7 +2285,9 @@ async fn test_user_device_configs_excludes_mfa_locations( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await @@ -2039,7 +2315,9 @@ async fn test_user_device_configs_excludes_mfa_locations( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await @@ -2067,7 +2345,9 @@ async fn test_user_device_configs_excludes_mfa_locations( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": true, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [{"flow_id": flow_id, "is_default": true, "group_ids": []}] })) .send() .await; @@ -2132,7 +2412,9 @@ async fn test_location_allowed_ips_from_acl_flag(_: PgPoolOptions, options: PgCo "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2145,8 +2427,15 @@ async fn test_location_allowed_ips_from_acl_flag(_: PgPoolOptions, options: PgCo // Verify API event was emitted for location creation let events = client.drain_all_events(); - assert_eq!(events.len(), 1, "expected exactly 1 event after create"); - let (event_type, _user_id, _username) = &events[0]; + assert_eq!( + events.len(), + 1, + "location save must emit only a location event" + ); + let (event_type, _user_id, _username) = events + .iter() + .find(|event| matches!(event.0, ApiEventType::VpnLocationAdded { .. })) + .expect("missing location event"); assert_matches!( event_type, ApiEventType::VpnLocationAdded { location: event_location } @@ -2173,7 +2462,9 @@ async fn test_location_allowed_ips_from_acl_flag(_: PgPoolOptions, options: PgCo "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2185,8 +2476,15 @@ async fn test_location_allowed_ips_from_acl_flag(_: PgPoolOptions, options: PgCo ); let events = client.drain_all_events(); - assert_eq!(events.len(), 1, "expected exactly 1 event after edit off"); - let (event_type, _user_id, _username) = &events[0]; + assert_eq!( + events.len(), + 1, + "location save must emit only a location event" + ); + let (event_type, _user_id, _username) = events + .iter() + .find(|event| matches!(event.0, ApiEventType::VpnLocationModified { .. })) + .expect("missing location event"); assert_matches!( event_type, ApiEventType::VpnLocationModified { before: before_loc, after: after_loc } @@ -2216,7 +2514,9 @@ async fn test_location_allowed_ips_from_acl_flag(_: PgPoolOptions, options: PgCo "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2228,8 +2528,15 @@ async fn test_location_allowed_ips_from_acl_flag(_: PgPoolOptions, options: PgCo ); let events = client.drain_all_events(); - assert_eq!(events.len(), 1, "expected exactly 1 event after edit on"); - let (event_type, _user_id, _username) = &events[0]; + assert_eq!( + events.len(), + 1, + "location save must emit only a location event" + ); + let (event_type, _user_id, _username) = events + .iter() + .find(|event| matches!(event.0, ApiEventType::VpnLocationModified { .. })) + .expect("missing location event"); assert_matches!( event_type, ApiEventType::VpnLocationModified { before: before_loc, after: after_loc } @@ -2357,7 +2664,9 @@ async fn test_config_allowed_ips_from_acl_merged(_: PgPoolOptions, options: PgCo "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2426,7 +2735,9 @@ async fn test_config_allowed_ips_from_acl_no_match(_: PgPoolOptions, options: Pg "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2533,7 +2844,9 @@ async fn test_config_allowed_ips_from_acl_toggle_off(_: PgPoolOptions, options: "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2605,7 +2918,9 @@ async fn test_config_allowed_ips_from_acl_any_address_skipped( "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2685,7 +3000,9 @@ async fn test_config_allowed_ips_from_acl_no_license(_: PgPoolOptions, options: "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -2756,7 +3073,9 @@ async fn test_config_allowed_ips_from_acl_disabled(_: PgPoolOptions, options: Pg "acl_default_allow": false, "allowed_ips_from_acl": true, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; diff --git a/crates/defguard_core/tests/integration/api/wireguard_network_allowed_groups.rs b/crates/defguard_core/tests/integration/api/wireguard_network_allowed_groups.rs index 34e4fe976..c3bc2a4c6 100644 --- a/crates/defguard_core/tests/integration/api/wireguard_network_allowed_groups.rs +++ b/crates/defguard_core/tests/integration/api/wireguard_network_allowed_groups.rs @@ -174,7 +174,9 @@ async fn test_create_new_network(_: PgPoolOptions, options: PgConnectOptions) { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -227,7 +229,9 @@ async fn test_create_new_network_allow_all_groups(_: PgPoolOptions, options: PgC "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -288,7 +292,9 @@ async fn test_modify_network(_: PgPoolOptions, options: PgConnectOptions) { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -326,7 +332,9 @@ async fn test_modify_network(_: PgPoolOptions, options: PgConnectOptions) { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -363,7 +371,9 @@ async fn test_modify_network(_: PgPoolOptions, options: PgConnectOptions) { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -401,7 +411,9 @@ async fn test_modify_network(_: PgPoolOptions, options: PgConnectOptions) { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -454,7 +466,9 @@ async fn test_modify_network_enable_allow_all_groups(_: PgPoolOptions, options: "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -489,7 +503,9 @@ async fn test_modify_network_enable_allow_all_groups(_: PgPoolOptions, options: "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -760,7 +776,9 @@ async fn test_modify_user(_: PgPoolOptions, options: PgConnectOptions) { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -875,7 +893,9 @@ async fn test_modify_user_no_effect_when_allow_all_groups( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -991,7 +1011,9 @@ async fn test_delete_only_allowed_group_rejected(_: PgPoolOptions, options: PgCo "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -1064,7 +1086,9 @@ async fn test_delete_allowed_group_when_location_keeps_other_groups( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -1115,7 +1139,8 @@ async fn test_create_network_without_groups_rejected(_: PgPoolOptions, options: allowed_ips_from_acl: false, mfa_enabled: false, // mfa_enabled service_location_mode: ServiceLocationMode::Disabled, - posture_checks: None, + posture_checks: Vec::new(), + mfa_flows: Vec::new(), }; // allow_all_groups=false with no groups should be rejected @@ -1170,7 +1195,8 @@ async fn test_modify_network_without_groups_rejected(_: PgPoolOptions, options: allowed_ips_from_acl: false, mfa_enabled: false, // mfa_enabled service_location_mode: ServiceLocationMode::Disabled, - posture_checks: None, + posture_checks: Vec::new(), + mfa_flows: Vec::new(), }; let response = client .post("/api/v1/network") diff --git a/crates/defguard_core/tests/integration/api/wireguard_network_devices.rs b/crates/defguard_core/tests/integration/api/wireguard_network_devices.rs index 9125b0a48..59d6affaa 100644 --- a/crates/defguard_core/tests/integration/api/wireguard_network_devices.rs +++ b/crates/defguard_core/tests/integration/api/wireguard_network_devices.rs @@ -17,7 +17,7 @@ use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use super::common::{ client::{TestClient, TestResponse}, - make_test_client, setup_pool, + make_test_client, setup_pool, update_location_mfa_flows, }; async fn make_first_network(client: &TestClient) -> TestResponse { @@ -40,7 +40,9 @@ async fn make_first_network(client: &TestClient) -> TestResponse { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -68,7 +70,9 @@ async fn make_second_network(client: &TestClient) -> TestResponse { "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; @@ -336,7 +340,9 @@ async fn test_device_ip_validation(_: PgPoolOptions, options: PgConnectOptions) "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] }); let response = client.post("/api/v1/network").json(&location).send().await; assert_eq!(response.status(), StatusCode::CREATED); @@ -464,18 +470,21 @@ async fn test_network_device_config_skips_mfa_location( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": false, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [] })) .send() .await; assert_eq!(response.status(), StatusCode::CREATED); let location: WireguardNetwork = response.json().await; - let response = client - .put(format!("/api/v1/location/{}/mfa-flows", location.id)) - .json(&json!({ "assignments": [{ "flow_id": flow_id, "is_default": true, "group_ids": [] }] })) - .send() - .await; + let response = update_location_mfa_flows( + &client, + location.id, + json!([{ "flow_id": flow_id, "is_default": true, "group_ids": [] }]), + ) + .await; assert_eq!(response.status(), StatusCode::OK); let response = client @@ -497,7 +506,9 @@ async fn test_network_device_config_skips_mfa_location( "acl_default_allow": false, "allowed_ips_from_acl": false, "mfa_enabled": true, - "service_location_mode": "disabled" + "service_location_mode": "disabled", + "posture_checks": [], + "mfa_flows": [{"flow_id": flow_id, "is_default": true, "group_ids": []}] })) .send() .await; diff --git a/crates/defguard_setup/src/handlers/auto_wizard.rs b/crates/defguard_setup/src/handlers/auto_wizard.rs index 21bf0c4df..b083d90fc 100644 --- a/crates/defguard_setup/src/handlers/auto_wizard.rs +++ b/crates/defguard_setup/src/handlers/auto_wizard.rs @@ -23,7 +23,7 @@ use defguard_core::{ apply_internal_url_settings as apply_core_internal_url_settings, }, error::WebError, - handlers::{ApiResponse, ApiResult}, + handlers::{ApiResponse, ApiResult, wireguard::validate_mfa_flows_exist}, }; use reqwest::StatusCode; use serde::{Deserialize, Serialize}; @@ -334,12 +334,8 @@ pub async fn set_mfa_settings( // location, so "enabled with no policy" is unrepresentable. This is the same check // `create_network` and `modify_network` apply, shared rather than reimplemented so the three // entry points cannot drift. Validated before saving, so a refusal writes nothing. - if let Some(response) = defguard_core::handlers::wireguard::validate_mfa_flows_exist( - &pool, - mfa_settings.mfa_enabled, - Some(first_network_id), - ) - .await? + if let Some(response) = + validate_mfa_flows_exist(&pool, mfa_settings.mfa_enabled, Some(first_network_id)).await? { return Ok(response); } diff --git a/e2e/tests/gatewayAdoption.spec.ts b/e2e/tests/gatewayAdoption.spec.ts index b5f337c4f..4eb6b2fde 100644 --- a/e2e/tests/gatewayAdoption.spec.ts +++ b/e2e/tests/gatewayAdoption.spec.ts @@ -44,6 +44,8 @@ test.describe('Gateway Adoption', () => { acl_default_allow: false, allowed_ips_from_acl: false, mfa_enabled: false, + posture_checks: [], + mfa_flows: [], service_location_mode: 'disabled', }, }); diff --git a/web/messages/en/location.json b/web/messages/en/location.json index 14a0e6a97..871ee4b5a 100644 --- a/web/messages/en/location.json +++ b/web/messages/en/location.json @@ -97,7 +97,6 @@ "location_access_edit_groups": "Edit groups", "location_posture_checks_edit": "Edit posture check", "location_posture_checks_select": "Select posture check", - "location_posture_checks_update_failed": "Failed to update posture checks", "location_posture_checks_empty_state_before_link": "You don't have any posture checks yet. Create at least one in the", "location_posture_checks_empty_state_after_link": "section before assigning it to this location.", "location_access_select_allowed_groups": "Select allowed groups", diff --git a/web/src/pages/AddLocationPage/useAddLocationStore.tsx b/web/src/pages/AddLocationPage/useAddLocationStore.tsx index 3b7808cd6..bebaca882 100644 --- a/web/src/pages/AddLocationPage/useAddLocationStore.tsx +++ b/web/src/pages/AddLocationPage/useAddLocationStore.tsx @@ -39,6 +39,7 @@ const defaults: StoreValues = { mfa_enabled: false, service_location_mode: LocationServiceMode.Disabled, posture_checks: [], + mfa_flows: [], }; export const useAddLocationStore = create()( diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index 849696c6c..7a6d98cf6 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -3,14 +3,16 @@ import './style.scss'; import { useMutation, useQuery, useSuspenseQuery } from '@tanstack/react-query'; import { Link, useNavigate, useParams } from '@tanstack/react-router'; import { cloneDeep, omit } from 'lodash-es'; -import { useMemo } from 'react'; +import { useMemo, useState } from 'react'; import z from 'zod'; import { m } from '../../paraglide/messages'; import api from '../../shared/api/api'; import { type EditNetworkLocation, LicenseFeature, + type LocationMfaFlowResponse, LocationServiceMode, + type MfaFlowAssignment, type NetworkLocation, } from '../../shared/api/types'; import { EditPage } from '../../shared/components/EditPage/EditPage'; @@ -37,7 +39,11 @@ import { useAppForm } from '../../shared/form'; import { formChangeLogic } from '../../shared/formLogic'; import { openModal } from '../../shared/hooks/modalControls/modalsSubjects'; import { ModalName } from '../../shared/hooks/modalControls/modalTypes'; -import { getLicenseInfoQueryOptions, getLocationQueryOptions } from '../../shared/query'; +import { + getLicenseInfoQueryOptions, + getLocationMfaFlowsQueryOptions, + getLocationQueryOptions, +} from '../../shared/query'; import { canUseBusinessFeature, canUseEnterpriseFeature, @@ -53,6 +59,9 @@ export const EditLocationPage = () => { from: '/_authorized/_default/locations/$locationId/edit', }); const { data: location } = useSuspenseQuery(getLocationQueryOptions(Number(paramsId))); + const { data: mfaFlows } = useSuspenseQuery( + getLocationMfaFlowsQueryOptions(location.id), + ); return ( { title: m.location_edit_title({ name: location.name }), }} > - + ); }; @@ -238,6 +247,8 @@ const areEqualStringArrays = (left: string[], right: string[]) => const buildLocationSubmissionData = ( value: FormFields, location: NetworkLocation, + postureChecks: number[], + mfaFlows: MfaFlowAssignment[], ): EditNetworkLocation => { const normalizedValue = cloneDeep(value); @@ -252,6 +263,8 @@ const buildLocationSubmissionData = ( acl_enabled: normalizedValue.firewall !== LocationFirewall.Disabled, peer_disconnect_threshold: normalizedValue.peer_disconnect_threshold ?? location.peer_disconnect_threshold, + posture_checks: postureChecks, + mfa_flows: mfaFlows, }; }; @@ -344,7 +357,13 @@ const getDisconnectRelevantChangedFields = ( return Array.from(changedFields); }; -const EditLocationForm = ({ location }: { location: NetworkLocation }) => { +const EditLocationForm = ({ + location, + mfaFlows, +}: { + location: NetworkLocation; + mfaFlows: LocationMfaFlowResponse[]; +}) => { const navigate = useNavigate(); const { data: licenseInfo } = useQuery(getLicenseInfoQueryOptions); @@ -371,16 +390,22 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { }); const serviceLocationLocked = isPresent(canUseServiceLocations) && !canUseServiceLocations; + const [pendingPostureChecks, setPendingPostureChecks] = useState( + location.posture_checks ?? [], + ); const postureChecksSectionState = useMemo( () => getPostureChecksSectionState({ - assignedPostureChecksCount: location.posture_checks?.length ?? 0, + assignedPostureChecksCount: pendingPostureChecks.length, canUseEnterprise: canUseDevicePosture, postureChecksCount: postureChecks.length, }), - [canUseDevicePosture, location.posture_checks?.length, postureChecks.length], + [canUseDevicePosture, pendingPostureChecks.length, postureChecks.length], ); const firewallLocked = isPresent(canUseBusiness) && !canUseBusiness; + const hasPendingPostureCheckChanges = + pendingPostureChecks.length !== (location.posture_checks?.length ?? 0) || + pendingPostureChecks.some((id) => !location.posture_checks?.includes(id)); const postureCheckOptions = useMemo( () => @@ -464,26 +489,10 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { }, }); - const { mutateAsync: setLocationPosturesAsync, isPending: isUpdatingLocationPostures } = - useMutation({ - mutationFn: (data: { postures: number[] }) => - api.devicePosture.setLocationPostures(location.id, data), - meta: { - invalidate: [['device-posture'], ['network'], ['activity-log']], - }, - onError: () => { - Snackbar.error(m.location_posture_checks_update_failed()); - }, - }); - const handlePostureSelection = (values: (string | number)[]) => { - const next = values.filter((value): value is number => typeof value === 'number'); - confirmLocationPostureChange({ - current: location.posture_checks ?? [], - next, - options: postureCheckOptions, - actionPromise: () => setLocationPosturesAsync({ postures: next }), - }); + setPendingPostureChecks( + values.filter((value): value is number => typeof value === 'number'), + ); }; const openPostureChecksSelection = () => { @@ -499,7 +508,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { unknown >, searchPlaceholder: m.controls_search(), - selected: new Set(location.posture_checks), + selected: new Set(pendingPostureChecks), visibleItemsLimit: 4, onSubmit: handlePostureSelection, }); @@ -527,11 +536,22 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { [location], ); + const mfaFlowAssignments = mfaFlows.map((flow) => ({ + flow_id: flow.id, + is_default: flow.is_default, + group_ids: flow.groups.map((group) => group.id), + })); + // Reuses the same save request for direct submits and confirmed warning actions. const submitLocationChanges = async (value: FormFields) => { await editLocation({ id: location.id, - data: buildLocationSubmissionData(value, location), + data: buildLocationSubmissionData( + value, + location, + pendingPostureChecks, + mfaFlowAssignments, + ), }); }; @@ -554,9 +574,21 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { const changedFields = getDisconnectRelevantChangedFields( getDisconnectRelevantLocationData( - buildLocationSubmissionData(defaultValues, location), + buildLocationSubmissionData( + defaultValues, + location, + location.posture_checks ?? [], + mfaFlowAssignments, + ), + ), + getDisconnectRelevantLocationData( + buildLocationSubmissionData( + value, + location, + pendingPostureChecks, + mfaFlowAssignments, + ), ), - getDisconnectRelevantLocationData(buildLocationSubmissionData(value, location)), ); if (changedFields.length > 0) { @@ -576,6 +608,17 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { return; } + if ( + confirmLocationPostureChange({ + current: location.posture_checks ?? [], + next: pendingPostureChecks, + options: postureCheckOptions, + actionPromise: () => submitLocationChanges(value), + }) + ) { + return; + } + await submitLocationChanges(value); }, }); @@ -914,7 +957,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => {
{