Atomic postures and mfa-flows assignments - #3590
Conversation
j-chmielewski
changed the title
880b20d to
c131ec5
Compare
| ); | ||
| let mfa_assignments = normalize_mfa_flow_assignments(data.mfa_flows.clone()); | ||
| let mfa_assignments_changed = current_mfa_assignments != mfa_assignments; | ||
| let mfa_assignments_updated = mfa_assignments_changed && is_business_license_active(); |
There was a problem hiding this comment.
If the license is not active this just makes the assignment fail silently. We should probably at least log a debug/warning message.
There was a problem hiding this comment.
Also this seems to block adding the default flow in the free tier.
| "User {} updated WireGuard network {network_id}", | ||
| session.user.username, | ||
| ); | ||
| if postures_changed { |
There was a problem hiding this comment.
The validate -> assign -> emit event flow is similar between create and modify endpoints. Maybe consider extracting a helper?
| } | ||
|
|
||
| /// Validates whether the current license permits the requested MFA flow assignments. | ||
| async fn validate_mfa_flow_assignments( |
There was a problem hiding this comment.
Not sure if this is the correct place for the validation logic. Consider putting it next to MfaFlow::assign_to_location.
| } | ||
|
|
||
| // Free can't assign multi-step flows. | ||
| if let Some(assignment) = assignments.first() { |
There was a problem hiding this comment.
Free should also have no more than one flow assigned. This is probably already the case due to earlier checks, but maybe add some explicit logic so a refactor does not break validation.
| ]), | ||
| ) | ||
| .await; | ||
| assert_assignment_license_error(response, "group_assignment_not_allowed").await; |
There was a problem hiding this comment.
To confirm the operation is atomic we should check that no assignment is created in the DB
2 participants
Related issue: #3563
LocationMfaFlowItemnow returnsgroup_idsalongsidegroup_names, so theclient can round-trip assignments without a name lookup.