Skip to content

Commit

Permalink
Move threshold and override overlap validation to matrix builder
Browse files Browse the repository at this point in the history
  • Loading branch information
@sergiupuhalschi-rdx
sergiupuhalschi-rdx committed Jan 22, 2025
1 parent 3104a9f commit fd67683
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 134 deletions.
4 changes: 4 additions & 0 deletions ...rofile/models/security-structures/src/roles_matrices_structures/matrices/builder/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ pub enum MatrixRolesInCombinationBasicViolation {
pub enum MatrixRolesInCombinationForeverInvalid {
#[error("Recovery and confirmation factors overlap. No factor may be used in both the recovery and confirmation roles")]
RecoveryAndConfirmationFactorsOverlap,
#[error("Primary role cannot have multiple devices")]
PrimaryCannotHaveMultipleDevices,
#[error("Threshold and override factors overlap. No factor may be used in both the threshold and override list kinds")]
ThresholdAndOverrideFactorsOverlap,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, thiserror::Error)]
Expand Down
58 changes: 52 additions & 6 deletions ...dels/security-structures/src/roles_matrices_structures/matrices/builder/matrix_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ impl MatrixBuilder {
) -> MatrixBuilderMutateResult {
self.primary_role
.validate_threshold_factors()
.into_matrix_err(RoleKind::Primary)
.into_matrix_err(RoleKind::Primary)?;
Self::validate_primary_list_cannot_have_multiple_devices(
self.primary_role.get_threshold_factors().iter().cloned(),
)
}

pub fn validate_recovery_role_in_isolation(
Expand Down Expand Up @@ -543,6 +546,52 @@ impl MatrixBuilder {
Ok(())
}

fn validate_primary_cannot_have_multiple_devices(
&self,
) -> MatrixBuilderMutateResult {
Self::validate_primary_list_cannot_have_multiple_devices(
self.primary_role.all_factors().into_iter().cloned(),
)
}

fn validate_no_factor_may_be_used_in_both_primary_threshold_and_override(
&self,
) -> MatrixBuilderMutateResult {
if self
.primary_role
.has_same_factor_in_both_threshold_and_override()
{
Err(MatrixBuilderValidation::CombinationViolation(
MatrixRolesInCombinationViolation::ForeverInvalid(
MatrixRolesInCombinationForeverInvalid::ThresholdAndOverrideFactorsOverlap,
),
))
} else {
Ok(())
}
}

fn validate_primary_list_cannot_have_multiple_devices(
factors: impl IntoIterator<Item = FactorSourceID>,
) -> MatrixBuilderMutateResult {
let device_count = factors
.into_iter()
.filter(|fsid| {
fsid.get_factor_source_kind() == FactorSourceKind::Device
})
.count();

if device_count > 1 {
Err(MatrixBuilderValidation::CombinationViolation(
MatrixRolesInCombinationViolation::ForeverInvalid(
MatrixRolesInCombinationForeverInvalid::PrimaryCannotHaveMultipleDevices,
),
))
} else {
Ok(())
}
}

/// Security Shield Rules
/// In addition to the factor/role rules above, the wallet must enforce certain rules for combinations of
/// factors across the three roles. The construction method described in the next section will automatically
Expand All @@ -557,13 +606,10 @@ impl MatrixBuilder {
/// 4. Number of days until auto confirm is greater than zero
fn validate_combination(&self) -> MatrixBuilderMutateResult {
self.validate_if_primary_has_single_it_must_not_be_used_by_any_other_role()?;
self.validate_primary_cannot_have_multiple_devices()?;
self.validate_no_factor_may_be_used_in_both_primary_threshold_and_override()?;
self.validate_no_factor_may_be_used_in_both_recovery_and_confirmation(
)?;

// N.B. the third 3:
// "3. No factor may be used in both the `Primary` threshold and `Primary` override"
// is already enforced by the RoleBuilder

self.validate_number_of_days_until_auto_confirm()?;
Ok(())
}
Expand Down
38 changes: 18 additions & 20 deletions ...ty-structures/src/roles_matrices_structures/matrices/builder/matrix_builder_unit_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,10 +527,9 @@ mod validation_for_addition_of_factor_source_for_each {
FactorSourceID::sample_device(),
ForeverInvalidReason::FactorSourceAlreadyPresent
),
FactorSourceInRoleBuilderValidationStatus::forever_invalid(
FactorSourceInRoleBuilderValidationStatus::ok(
RoleKind::Primary,
FactorSourceID::sample_device_other(),
ForeverInvalidReason::PrimaryCannotHaveMultipleDevices
),
]
);
Expand Down Expand Up @@ -579,10 +578,9 @@ mod validation_for_addition_of_factor_source_for_each {
FactorSourceID::sample_device(),
ForeverInvalidReason::FactorSourceAlreadyPresent
),
FactorSourceInRoleBuilderValidationStatus::forever_invalid(
FactorSourceInRoleBuilderValidationStatus::ok(
RoleKind::Primary,
FactorSourceID::sample_device_other(),
ForeverInvalidReason::PrimaryCannotHaveMultipleDevices
),
]
);
Expand Down Expand Up @@ -1119,7 +1117,7 @@ mod validation_of_addition_of_kind {
}

#[test]
fn device_is_err_for_second_3x_threshold() {
fn device_is_ok_for_second_3x_threshold() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
Expand All @@ -1132,11 +1130,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_2x_threshold_override() {
fn device_is_ok_for_second_2x_threshold_override() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
Expand All @@ -1149,11 +1147,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_threshold_override_threshold() {
fn device_is_ok_for_second_threshold_override_threshold() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
Expand All @@ -1166,11 +1164,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_threshold_override_2x() {
fn device_is_ok_for_second_threshold_override_2x() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
Expand All @@ -1183,11 +1181,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_3x_override() {
fn device_is_ok_for_second_3x_override() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
Expand All @@ -1200,11 +1198,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_2x_override_threshold() {
fn device_is_ok_for_second_2x_override_threshold() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
Expand All @@ -1217,11 +1215,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_override_threshold_2x() {
fn device_is_ok_for_second_override_threshold_2x() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
Expand All @@ -1234,11 +1232,11 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_threshold(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}

#[test]
fn device_is_err_for_second_override_threshold_override() {
fn device_is_ok_for_second_override_threshold_override() {
let mut sut = make();
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
Expand All @@ -1251,7 +1249,7 @@ mod validation_of_addition_of_kind {
let res = sut.validation_for_addition_of_factor_source_of_kind_to_primary_override(
FactorSourceKind::Device,
);
assert!(res.is_err());
assert!(res.is_ok());
}
}
}
Expand Down
9 changes: 2 additions & 7 deletions ...tructures/src/roles_matrices_structures/roles/builder/primary_roles_builder_unit_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ mod device_factor_source {
}

#[test]
fn two_different_is_err() {
fn two_different_is_ok() {
// Arrange
let mut sut = make();

Expand All @@ -980,12 +980,7 @@ mod device_factor_source {
let res = sut.add_factor_source_to_threshold(sample_other());

// Assert
assert!(matches!(
res,
MutRes::Err(Validation::ForeverInvalid(
ForeverInvalidReason::PrimaryCannotHaveMultipleDevices
))
));
assert!(matches!(res, Ok(())));
}
}

Expand Down
47 changes: 14 additions & 33 deletions ...e/models/security-structures/src/roles_matrices_structures/roles/builder/roles_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ pub enum ForeverInvalidReason {
#[error("Factor source already present")]
FactorSourceAlreadyPresent,

#[error("Primary role cannot have multiple devices")]
PrimaryCannotHaveMultipleDevices,

#[error("Primary role cannot have password in override list")]
PrimaryCannotHavePasswordInOverrideList,

Expand Down Expand Up @@ -426,15 +423,6 @@ impl<const ROLE: u8> RoleBuilder<ROLE> {
self.get_threshold_factors().contains(factor_source_id)
}

fn override_contains_factor_source_of_kind(
&self,
factor_source_kind: FactorSourceKind,
) -> bool {
self.get_override_factors()
.iter()
.any(|f| f.get_factor_source_kind() == factor_source_kind)
}

fn threshold_contains_factor_source_of_kind(
&self,
factor_source_kind: FactorSourceKind,
Expand Down Expand Up @@ -571,6 +559,18 @@ impl<const ROLE: u8> RoleBuilder<ROLE> {
self.validate_addition_of_password_and_minimum_factor_count()
}

pub(crate) fn has_same_factor_in_both_threshold_and_override(
&self,
) -> bool {
let threshold_set =
HashSet::<_>::from_iter(self.get_threshold_factors());
let override_set = HashSet::<_>::from_iter(self.get_override_factors());
let intersection = threshold_set
.intersection(&override_set)
.collect::<HashSet<_>>();
!intersection.is_empty()
}

fn validation_for_addition_of_factor_source_of_kind_to_override_for_non_primary_role(
&self,
factor_source_kind: FactorSourceKind,
Expand Down Expand Up @@ -660,14 +660,6 @@ impl<const ROLE: u8> RoleBuilder<ROLE> {
self._validation_add(factor_source_kind, factor_list_kind, mode)
}

fn contains_factor_source_of_kind(
&self,
factor_source_kind: FactorSourceKind,
) -> bool {
self.override_contains_factor_source_of_kind(factor_source_kind)
|| self.threshold_contains_factor_source_of_kind(factor_source_kind)
}

/// Removes a factor source from the list of `factor_list_kind`.
///
/// Lowers the threshold if the deleted factor source is in the `factor_list_kind` list
Expand Down Expand Up @@ -749,19 +741,8 @@ impl<const ROLE: u8> RoleBuilder<ROLE> {
PrimaryCannotContainTrustedContact,
);
}
FactorSourceKind::Device => match mode {
SecurityShieldBuilderMode::Strict => {
if self.contains_factor_source_of_kind(
FactorSourceKind::Device,
) {
return RoleBuilderMutateResult::forever_invalid(
PrimaryCannotHaveMultipleDevices,
);
}
}
SecurityShieldBuilderMode::Lenient => {}
},
FactorSourceKind::LedgerHQHardwareWallet
FactorSourceKind::Device
| FactorSourceKind::LedgerHQHardwareWallet
| FactorSourceKind::ArculusCard
| FactorSourceKind::OffDeviceMnemonic => {}
}
Expand Down
Loading

0 comments on commit fd67683

Please sign in to comment.